charles/agent-hooks

Fork 0

refactor(architect): remove host-mode infrastructure and local-FS file paths #1142

Merged

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

2026-05-13 22:34:25 +00:00

code-lead commented

2026-05-13 22:07:59 +00:00

Collaborator

Closes #1133

Summary

Retires the host-mode escape hatch that #1131 / #1132 left in place: the
HOST_MODE_TYPES allowlist, the hostModeConfigDir helper, the local-FS
attach-picker / spec-editor fallbacks, and the git remote get-url
subprocess that ran on every /architect/config request. After this change
there is no non-containerised code path and no surface that touches
process.cwd() from the architect stack.

Removals

webhook-config.ts — HOST_MODE_TYPES / isHostModeType deleted;
container.enabled: false now fails loadWebhookConfig outright with
a clear migration hint.
agent-runner.ts — hostModeConfigDir deleted; buildAgentEnv no
longer accepts the opt and always wires CLAUDE_CONFIG_DIR to the
in-container bind target. The host_mode branch in runAgentTask
(dead since architect dispatch moved to ArchitectAgentDispatch) is
gone, and the unused HostContainerLifecycle adapter + tests are
deleted.
domain/agent/architect.ts — getSelfRepo, resetSelfRepoCache,
architectFilesRoot, resolveAllowlistedFile, FILE_ALLOWLIST_DIRS,
FILE_ALLOWLIST_EXACT deleted.
http/handlers/architect.ts — node:fs/promises import removed; every
file / spec handler routes through createForgeAdapterForRepo with the
architect token. _architectForgeFactory is the new module-level seam
tests inject a fake ForgePort through.
main.ts — selfRepo is resolved declaratively from
CLAUDE_HOOKS_SELF_REPO env var → cfg.repos[0] and passed via the
request context (replaces the git remote get-url subprocess).

Tests + docs

Deleted-symbol tests dropped; handleArchitectFilesList /
handleArchitectFileContent / handleArchitectSpecSave now drive
through the mock forge registry.
webhook-config / config-agent-types / onboarding fixtures
flipped to container.enabled: true, lifecycle: "hot" for the
architect type.
docs/architect.md updated to reflect the containerised model — all
references to container.enabled: false and cwd: process.cwd() are
gone.

Test plan

just qa — 3333 tests pass, typecheck + Biome lint/format clean,
sql-layer-check, paraglide-check, i18n-string-check,
flow-schema-check all green.
Production: /architect/config returns self_repo derived from
cfg.repos[0] without forking git.
Production: spec save / file content / file list all route through
the forge adapter; local-FS attach picker no longer enumerates the
host worktree.

Closes #1133 ## Summary Retires the host-mode escape hatch that #1131 / #1132 left in place: the `HOST_MODE_TYPES` allowlist, the `hostModeConfigDir` helper, the local-FS attach-picker / spec-editor fallbacks, and the `git remote get-url` subprocess that ran on every `/architect/config` request. After this change there is no non-containerised code path and no surface that touches `process.cwd()` from the architect stack. ### Removals - `webhook-config.ts` — `HOST_MODE_TYPES` / `isHostModeType` deleted; `container.enabled: false` now fails `loadWebhookConfig` outright with a clear migration hint. - `agent-runner.ts` — `hostModeConfigDir` deleted; `buildAgentEnv` no longer accepts the opt and always wires `CLAUDE_CONFIG_DIR` to the in-container bind target. The `host_mode` branch in `runAgentTask` (dead since architect dispatch moved to `ArchitectAgentDispatch`) is gone, and the unused `HostContainerLifecycle` adapter + tests are deleted. - `domain/agent/architect.ts` — `getSelfRepo`, `resetSelfRepoCache`, `architectFilesRoot`, `resolveAllowlistedFile`, `FILE_ALLOWLIST_DIRS`, `FILE_ALLOWLIST_EXACT` deleted. - `http/handlers/architect.ts` — `node:fs/promises` import removed; every file / spec handler routes through `createForgeAdapterForRepo` with the architect token. `_architectForgeFactory` is the new module-level seam tests inject a fake `ForgePort` through. - `main.ts` — `selfRepo` is resolved declaratively from `CLAUDE_HOOKS_SELF_REPO` env var → `cfg.repos[0]` and passed via the request context (replaces the `git remote get-url` subprocess). ### Tests + docs - Deleted-symbol tests dropped; `handleArchitectFilesList` / `handleArchitectFileContent` / `handleArchitectSpecSave` now drive through the mock forge registry. - `webhook-config` / `config-agent-types` / `onboarding` fixtures flipped to `container.enabled: true, lifecycle: "hot"` for the architect type. - `docs/architect.md` updated to reflect the containerised model — all references to `container.enabled: false` and `cwd: process.cwd()` are gone. ## Test plan - [x] `just qa` — 3333 tests pass, typecheck + Biome lint/format clean, `sql-layer-check`, `paraglide-check`, `i18n-string-check`, `flow-schema-check` all green. - [ ] Production: `/architect/config` returns `self_repo` derived from `cfg.repos[0]` without forking `git`. - [ ] Production: spec save / file content / file list all route through the forge adapter; local-FS attach picker no longer enumerates the host worktree.

charles was assigned by code-lead

2026-05-13 22:07:59 +00:00

code-lead added 1 commit

2026-05-13 22:07:59 +00:00

refactor(architect): remove host-mode infrastructure and local-FS file paths (#1133 )

qa / dockerfile (pull_request) Successful in 12s

Details

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

Details

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

Details

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

Details

qa / qa-1 (pull_request) Successful in 5m54s

Details

qa / qa (pull_request) Successful in 0s

Details

750f22b504

Retires the host-mode escape hatch that #1131 / #1132 left in place: the
`HOST_MODE_TYPES` allowlist, the `hostModeConfigDir` helper, the local-FS
attach-picker / spec-editor fallbacks, and the `git remote get-url`
subprocess that ran on every `/architect/config` request.

- `webhook-config.ts` — `HOST_MODE_TYPES` / `isHostModeType` deleted;
  `container.enabled: false` now fails `loadWebhookConfig` outright with
  a clear migration hint.
- `agent-runner.ts` — `hostModeConfigDir` deleted; `buildAgentEnv` no
  longer accepts the opt and always wires `CLAUDE_CONFIG_DIR` to the
  in-container bind target. The `host_mode` branch in `runAgentTask`
  (dead since architect dispatch moved to `ArchitectAgentDispatch`) is
  gone, and the unused `HostContainerLifecycle` adapter + tests are
  deleted.
- `domain/agent/architect.ts` — `getSelfRepo`, `resetSelfRepoCache`,
  `architectFilesRoot`, `resolveAllowlistedFile`,
  `FILE_ALLOWLIST_DIRS`, `FILE_ALLOWLIST_EXACT` deleted.
- `http/handlers/architect.ts` — `node:fs/promises` import removed;
  every file / spec handler routes through `createForgeAdapterForRepo`
  with the architect token. `_architectForgeFactory` is the new
  module-level seam tests inject a fake `ForgePort` through.
- `main.ts` — `selfRepo` is resolved declaratively from
  `CLAUDE_HOOKS_SELF_REPO` env var → `cfg.repos[0]` and passed via the
  request context (replaces the `git remote get-url` subprocess).
- Tests: deleted-symbol tests dropped, `handleArchitectFilesList` /
  `handleArchitectFileContent` / `handleArchitectSpecSave` now drive
  through the mock forge registry; webhook-config / config-agent-types /
  onboarding fixtures flipped to `container.enabled: true,
  lifecycle: "hot"` for the architect type.
- `docs/architect.md` updated to reflect the containerised model.

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

code-lead requested review from reviewer

2026-05-13 22:16:31 +00:00

reviewer approved these changes

2026-05-13 22:19:52 +00:00

reviewer left a comment

Review — APPROVED ✅

CI: green (3333 tests pass, typecheck + Biome + all schema checks clean).

Overview

Complete removal of the host-mode escape hatch introduced in M18-4 / #165 and kept alive through #1131 / #1132. Every agent — including the architect singleton — now dispatches exclusively through a Docker container. The changes are systematic and consistent across all 23 affected files.

What's landed

Area	Change
`webhook-config.ts`	`HOST_MODE_TYPES` + `isHostModeType` deleted; `container.enabled=false` now hard-fails with migration hint
`agent-runner.ts`	`hostModeConfigDir` deleted; `buildAgentEnv` always sets `CONTAINER_CLAUDE_CONFIG_DIR`; host-mode branch in `runAgentTask` gone
`domain/agent/architect.ts`	`getSelfRepo` / `resetSelfRepoCache` (shelled out to `git remote get-url`), `architectFilesRoot`, `resolveAllowlistedFile`, `FILE_ALLOWLIST_DIRS/EXACT` all deleted
`http/handlers/architect.ts`	`node:fs/promises` import gone; all three file/spec handlers route through `_architectForgeFactory`; `selfRepo` comes from `ctx.selfRepo`
`host-container-lifecycle.{ts,test.ts}`	Deleted (147 + 103 lines)
`main.ts`	`selfRepo` resolved declaratively from `CLAUDE_HOOKS_SELF_REPO` → `cfg.repos[0]`; no subprocess
`sections.tsx`	`tier1NotAnthropicOnHostMode` now keys on `role === "host"` (previously `container.enabled === false` — always-false after this PR)
Fixtures / tests	All architect fixtures flipped to `container.enabled: true, lifecycle: "hot"`
`docs/architect.md`	Accurately reflects containerised model end-to-end

Code quality observations (non-blocking)

1. handleArchitectBreakdownPreview bypasses the test seam (minor)

The three file handlers (handleArchitectFilesList, handleArchitectFileContent, handleArchitectSpecSave) all route through _architectForgeFactory.create, making them mockable in tests. But handleArchitectBreakdownPreview still calls createForgeAdapterForRepo directly:

const file = await createForgeAdapterForRepo(repo, token).readFile(parseRepo(repo), specPath);

This leaves the forge read in the breakdown preview untestable via the mock seam. Not a behaviour bug, and the inline-content path is already well-tested. Worth aligning in a follow-up if handleArchitectBreakdownPreview ever needs forge-path tests.

2. runArchitectTurn passes containerMode: false but the comment says "container mode" (cosmetic)

// The architect chat path always relays into the persistent architect
// container via DockerExecClaudeAgent, so we build the env in
// container mode — buildAgentEnv already wires CLAUDE_CONFIG_DIR ...
const env = buildAgentEnv(agent, request.forgejo_token, false, null, undefined);

buildAgentEnv now always returns CONTAINER_CLAUDE_CONFIG_DIR regardless of containerMode, so false works. But the comment says "container mode" while the argument is false. This is accurate for the shim vars (which are only set when containerMode=true, and the architect's DockerExecClaudeAgent sets them itself), but it's confusing to read. A brief note that false intentionally skips the duplicate shim-var injection would clarify the intent.

3. has_tracking_issue / modified_at fields are now consistently absent (non-regression)

The old local-FS path for handleArchitectFilesList computed has_tracking_issue (regex scan) and modified_at (from stat.mtime). These were never present on the remote-repo path. Removing the self-repo branch makes the response shape uniform across all callers. Worth confirming no front-end code depends on has_tracking_issue — a quick grep of apps/web for the field before merging wouldn't hurt.

Security check

Path traversal protection in the new handleArchitectFileContent is sound:

const normalized = normalize(path);
const isSpec = normalized.startsWith("specs/") && normalized.endsWith(".md") && !normalized.includes("..");

normalize("specs/../../etc/passwd.md") → "../../etc/passwd.md", which fails startsWith("specs/") before the .. guard even fires. Belt-and-braces. ✅

Summary

Clean refactoring, no dead code remaining, tests updated throughout, docs accurate. The three non-blocking notes above are housekeeping items — none affect correctness or security. Approving.

## Review — APPROVED ✅ CI: **green** (3333 tests pass, typecheck + Biome + all schema checks clean). --- ### Overview Complete removal of the host-mode escape hatch introduced in M18-4 / #165 and kept alive through #1131 / #1132. Every agent — including the architect singleton — now dispatches exclusively through a Docker container. The changes are systematic and consistent across all 23 affected files. --- ### What's landed | Area | Change | |---|---| | `webhook-config.ts` | `HOST_MODE_TYPES` + `isHostModeType` deleted; `container.enabled=false` now hard-fails with migration hint | | `agent-runner.ts` | `hostModeConfigDir` deleted; `buildAgentEnv` always sets `CONTAINER_CLAUDE_CONFIG_DIR`; host-mode branch in `runAgentTask` gone | | `domain/agent/architect.ts` | `getSelfRepo` / `resetSelfRepoCache` (shelled out to `git remote get-url`), `architectFilesRoot`, `resolveAllowlistedFile`, `FILE_ALLOWLIST_DIRS/EXACT` all deleted | | `http/handlers/architect.ts` | `node:fs/promises` import gone; all three file/spec handlers route through `_architectForgeFactory`; `selfRepo` comes from `ctx.selfRepo` | | `host-container-lifecycle.{ts,test.ts}` | Deleted (147 + 103 lines) | | `main.ts` | `selfRepo` resolved declaratively from `CLAUDE_HOOKS_SELF_REPO` → `cfg.repos[0]`; no subprocess | | `sections.tsx` | `tier1NotAnthropicOnHostMode` now keys on `role === "host"` (previously `container.enabled === false` — always-false after this PR) | | Fixtures / tests | All architect fixtures flipped to `container.enabled: true, lifecycle: "hot"` | | `docs/architect.md` | Accurately reflects containerised model end-to-end | --- ### Code quality observations (non-blocking) **1. `handleArchitectBreakdownPreview` bypasses the test seam (minor)** The three file handlers (`handleArchitectFilesList`, `handleArchitectFileContent`, `handleArchitectSpecSave`) all route through `_architectForgeFactory.create`, making them mockable in tests. But `handleArchitectBreakdownPreview` still calls `createForgeAdapterForRepo` directly: ```ts const file = await createForgeAdapterForRepo(repo, token).readFile(parseRepo(repo), specPath); ``` This leaves the forge read in the breakdown preview untestable via the mock seam. Not a behaviour bug, and the inline-content path is already well-tested. Worth aligning in a follow-up if `handleArchitectBreakdownPreview` ever needs forge-path tests. **2. `runArchitectTurn` passes `containerMode: false` but the comment says "container mode" (cosmetic)** ```ts // The architect chat path always relays into the persistent architect // container via DockerExecClaudeAgent, so we build the env in // container mode — buildAgentEnv already wires CLAUDE_CONFIG_DIR ... const env = buildAgentEnv(agent, request.forgejo_token, false, null, undefined); ``` `buildAgentEnv` now always returns `CONTAINER_CLAUDE_CONFIG_DIR` regardless of `containerMode`, so `false` works. But the comment says "container mode" while the argument is `false`. This is accurate for the shim vars (which are only set when `containerMode=true`, and the architect's `DockerExecClaudeAgent` sets them itself), but it's confusing to read. A brief note that `false` intentionally skips the duplicate shim-var injection would clarify the intent. **3. `has_tracking_issue` / `modified_at` fields are now consistently absent (non-regression)** The old local-FS path for `handleArchitectFilesList` computed `has_tracking_issue` (regex scan) and `modified_at` (from `stat.mtime`). These were never present on the remote-repo path. Removing the self-repo branch makes the response shape uniform across all callers. Worth confirming no front-end code depends on `has_tracking_issue` — a quick grep of `apps/web` for the field before merging wouldn't hurt. --- ### Security check Path traversal protection in the new `handleArchitectFileContent` is sound: ```ts const normalized = normalize(path); const isSpec = normalized.startsWith("specs/") && normalized.endsWith(".md") && !normalized.includes(".."); ``` `normalize("specs/../../etc/passwd.md")` → `"../../etc/passwd.md"`, which fails `startsWith("specs/")` before the `..` guard even fires. Belt-and-braces. ✅ --- ### Summary Clean refactoring, no dead code remaining, tests updated throughout, docs accurate. The three non-blocking notes above are housekeeping items — none affect correctness or security. Approving.

charles merged commit 14f3d16383 into main

2026-05-13 22:34:25 +00:00

charles deleted branch code-lead/1133

2026-05-13 22:34:26 +00:00

charles referenced this pull request from a commit

2026-05-13 22:34:27 +00:00

refactor(architect): remove host-mode infrastructure and local-FS file paths (#1142)