refactor(architect): drop host-mode bind-mount collapse — uniform fleet mounts (AR-5) #1206

Merged

charles merged 1 commit from code-lead/1200 into main

2026-05-15 15:06:57 +00:00

code-lead commented

2026-05-15 14:51:25 +00:00

Collaborator

Summary

AR-5 finishes the host-mode retirement: every architect-only branch in
the container-reconcile path is gone. The architect container now
ships the same bind set as every other type (credentials dir + projects
sub-bind + cursor config dir + named state volume). The architect
process never writes to the projects or cursor binds, so the extra
mounts are inert — simpler reconcile code wins over a per-type collapse.

After AR-3 (#1198) and AR-4 (#1199), the only remaining architect-only
code path was the dockerRun bind-mount collapse + the companion drift
skip in matchesDesired. Both are deleted in this PR.

Acceptance criteria (issue #1200)

HOST_MODE_TYPES — already absent in code; only one historical M18-spec mention remained (annotated as retired).
HostContainerLifecycle — never existed (only DockerContainerLifecycle is present); only DockerContainerLifecycle.
getArchitectWorker host filter — already removed by AR-4; stale "#1200 will simplify" comment cleaned up.
dockerRun host_mode branch — deleted. Architect gets the standard --volume set.
matchesDesired host_mode branch — deleted. Drift detection is uniform.
specs/architect-dockerize.md — updated to describe uniform mounts; "Decoupling host_mode" + "Out of scope" sections collapsed.
Grep gate (HOST_MODE_TYPES, HostContainerLifecycle, host_mode, getArchitectWorker.*host) — zero hits.

Tests

New container-reconcile.test.ts case asserts architect and dev emit the same --volume argv set (state-volume name normalised).
The 'unchanged' when present + matches fixture now includes the projects bind, since matchesDesired no longer skips it for architect.
All 56 container-reconcile tests pass + 22 dispatch/registry tests + just qa clean (3362 tests, 0 fail).

Test plan

just qa clean
bun test src/infrastructure/container/container-reconcile.test.ts — 56/56 pass
bun test src/domain/dispatch/registry.test.ts — 22/22 pass
Grep gate verified — no live hits for HOST_MODE_TYPES / HostContainerLifecycle / host_mode / getArchitectWorker.*host

Closes #1200. Part of #1203 (editable-architect tracker).

🤖 Generated with Claude Code

## Summary AR-5 finishes the host-mode retirement: every architect-only branch in the container-reconcile path is gone. The architect container now ships the same bind set as every other type (credentials dir + projects sub-bind + cursor config dir + named state volume). The architect process never writes to the projects or cursor binds, so the extra mounts are inert — simpler reconcile code wins over a per-type collapse. After AR-3 (#1198) and AR-4 (#1199), the only remaining architect-only code path was the `dockerRun` bind-mount collapse + the companion drift skip in `matchesDesired`. Both are deleted in this PR. ## Acceptance criteria (issue #1200) - `HOST_MODE_TYPES` — already absent in code; only one historical M18-spec mention remained (annotated as retired). - `HostContainerLifecycle` — never existed (only `DockerContainerLifecycle` is present); only `DockerContainerLifecycle`. - `getArchitectWorker` host filter — already removed by AR-4; stale "#1200 will simplify" comment cleaned up. - `dockerRun` host_mode branch — **deleted**. Architect gets the standard `--volume` set. - `matchesDesired` host_mode branch — **deleted**. Drift detection is uniform. - `specs/architect-dockerize.md` — updated to describe uniform mounts; "Decoupling host_mode" + "Out of scope" sections collapsed. - Grep gate (`HOST_MODE_TYPES`, `HostContainerLifecycle`, `host_mode`, `getArchitectWorker.*host`) — zero hits. ## Tests - New `container-reconcile.test.ts` case asserts architect and dev emit the same `--volume` argv set (state-volume name normalised). - The `'unchanged' when present + matches` fixture now includes the projects bind, since `matchesDesired` no longer skips it for architect. - All 56 container-reconcile tests pass + 22 dispatch/registry tests + `just qa` clean (3362 tests, 0 fail). ## Test plan - [x] `just qa` clean - [x] `bun test src/infrastructure/container/container-reconcile.test.ts` — 56/56 pass - [x] `bun test src/domain/dispatch/registry.test.ts` — 22/22 pass - [x] Grep gate verified — no live hits for `HOST_MODE_TYPES` / `HostContainerLifecycle` / `host_mode` / `getArchitectWorker.*host` Closes #1200. Part of #1203 (editable-architect tracker). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-05-15 14:51:25 +00:00

refactor(architect): drop host-mode bind-mount collapse — uniform fleet mounts (AR-5)

qa / i18n-string-check (pull_request) Successful in 13s

Details

qa / sql-layer-check (pull_request) Successful in 12s

Details

qa / dockerfile (pull_request) Successful in 16s

Details

qa / db-schema (pull_request) Successful in 19s

Details

qa / qa-1 (pull_request) Successful in 3m32s

Details

qa / qa (pull_request) Successful in 0s

Details

7497423629

The architect container now ships the same bind set as every other
type (credentials dir, projects sub-bind, cursor config dir, named
state volume). The architect process never writes to the projects or
cursor binds, so the extra mounts are inert.

Deletions:
- `dockerRun`'s `minimalMount = agent.type === "architect"` branch and
  the `if (!minimalMount)` pre-create gate.
- `matchesDesired`'s `agent.type !== "architect"` skip on the projects
  sub-bind drift check.
- Stale "#1200 will simplify this further" comment in
  `getArchitectWorker`.

Tests:
- New `container-reconcile.test.ts` case asserts architect and dev
  emit the same `--volume` argv set (state-volume name normalised).
- The `'unchanged' when present + matches` fixture now includes the
  projects bind, since `matchesDesired` no longer skips it for
  architect.

Docs:
- `docs/architect.md` + `specs/architect-dockerize.md` updated to
  describe the uniform-mount reality.
- `specs/m18-ui-rewrite-and-architect.md` host-mode line annotated as
  retired (M18 milestone spec is a historical record).

Closes #1200.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer

2026-05-15 14:57:27 +00:00

reviewer approved these changes

2026-05-15 14:59:54 +00:00

reviewer left a comment

Review — AR-5: drop host-mode bind-mount collapse

CI: ✅ green. LGTM — approving.

What this does

Removes the minimalMount branch from dockerRun and the agent.type !== "architect" gate from matchesDesired, making the architect container provision identical to every other fleet agent (credentials dir + projects sub-bind + cursor config dir + named state volume). The architect process never writes to the extra mounts, so they're inert — the payoff is a completely uniform reconcile path. All AC items from #1200 verified in the diff.

Functional correctness ✅

dockerRun: minimalMount variable and both conditioned blocks (if (!minimalMount) mkdir + spread) are cleanly deleted. Unconditional mkdir projects + both --volume lines now follow the shared path. No residual branches.
matchesDesired: The if (agent.type !== "architect") gate around the projects check is removed. The cursor bind has never been checked in matchesDesired (for any type) — intentional pre-existing design, not a regression. The 'unchanged' inspect fixture correctly includes projects but not cursor, consistent with what the function actually validates.
On-deploy behaviour: An existing architect container provisioned under the old shape (credentials + state only) will fail the projects check on first reconcile → one-time recreate → standard bind set. Correct.

Tests ✅

The new "uniform bind set" test is well-constructed:

Runs architect and dev through reconcileOne independently, extracts --volume args with the new collectVolumes helper, normalises the instance-scoped state volume name, then sorts and compares — exactly the right approach.
Explicit assertions that projects and cursor binds are present catch any future re-introduction of the collapse.
collectVolumes is a clean extraction, reusable by other tests.

Minor observations (non-blocking)

specs/architect-dockerize.md "Out of scope" section — the first bullet now says "The chat relay … Tracked as AR-3 (#1198)", but AR-3 shipped in #1198. The bullet reads as a future-tense tracker item when it's actually historical. Consider removing it or retitling the section "Historical / done" since only the cursor item is genuinely still out of scope.
specs/m18-ui-rewrite-and-architect.md line 174 — the - [ ] checkbox for the retired host-mode flag is left unchecked. The appended note explains it's retired, but a checked [x] (or struck-through text) would better signal it was superseded rather than still outstanding.
'unchanged' test comment — after the AR-5 update the comment doesn't explain why the cursor bind is absent from the dockerInspectJson fixture. A one-liner (// cursor bind not checked by matchesDesired) would help the next reader avoid confusion.

None of these affect correctness. Ship it.

## Review — AR-5: drop host-mode bind-mount collapse CI: ✅ green. LGTM — approving. ### What this does Removes the `minimalMount` branch from `dockerRun` and the `agent.type !== "architect"` gate from `matchesDesired`, making the architect container provision identical to every other fleet agent (credentials dir + projects sub-bind + cursor config dir + named state volume). The architect process never writes to the extra mounts, so they're inert — the payoff is a completely uniform reconcile path. All AC items from #1200 verified in the diff. ### Functional correctness ✅ - **`dockerRun`**: `minimalMount` variable and both conditioned blocks (`if (!minimalMount)` mkdir + spread) are cleanly deleted. Unconditional `mkdir projects` + both `--volume` lines now follow the shared path. No residual branches. - **`matchesDesired`**: The `if (agent.type !== "architect")` gate around the `projects` check is removed. The cursor bind has never been checked in `matchesDesired` (for any type) — intentional pre-existing design, not a regression. The `'unchanged'` inspect fixture correctly includes projects but not cursor, consistent with what the function actually validates. - **On-deploy behaviour**: An existing architect container provisioned under the old shape (credentials + state only) will fail the projects check on first reconcile → one-time recreate → standard bind set. Correct. ### Tests ✅ The new "uniform bind set" test is well-constructed: - Runs architect and dev through `reconcileOne` independently, extracts `--volume` args with the new `collectVolumes` helper, normalises the instance-scoped state volume name, then sorts and compares — exactly the right approach. - Explicit assertions that `projects` and `cursor` binds are present catch any future re-introduction of the collapse. - `collectVolumes` is a clean extraction, reusable by other tests. ### Minor observations (non-blocking) 1. **`specs/architect-dockerize.md` "Out of scope" section** — the first bullet now says *"The chat relay … Tracked as AR-3 (#1198)"*, but AR-3 shipped in #1198. The bullet reads as a future-tense tracker item when it's actually historical. Consider removing it or retitling the section "Historical / done" since only the cursor item is genuinely still out of scope. 2. **`specs/m18-ui-rewrite-and-architect.md` line 174** — the `- [ ]` checkbox for the retired host-mode flag is left unchecked. The appended note explains it's retired, but a checked `[x]` (or struck-through text) would better signal it was superseded rather than still outstanding. 3. **`'unchanged'` test comment** — after the AR-5 update the comment doesn't explain why the cursor bind is absent from the `dockerInspectJson` fixture. A one-liner (`// cursor bind not checked by matchesDesired`) would help the next reader avoid confusion. None of these affect correctness. Ship it.