Project config plane: design pass + phased fix (#298)#301
Project config plane: design pass + phased fix (#298)#301polymath-orchestrator wants to merge 1 commit into
Conversation
Closes the four decisions issue #298 left open (AC2, AC3, AC5, AC8) with evidence from four parallel read-only investigations against the code and the live daemon. Key outcomes: - AC3: full-replace + ETag/If-Match. Merge/PATCH is rejected — ProjectConfig has zero pointer fields, so a typed merge cannot clear a scalar, and merge would break ops/project-config.mjs apply, our wipe-recovery path. - AC5: `codex debug models` exists, so the adapter catalog is achievable for codex and codex-fugu; claude-code has no enumeration path and falls back to a config-driven list. The Go switch disappears either way. - AC2: the Spawnable() gate as scoped passes all four live projects — no migration. Gating reviewers/prime/worktree-mode would brick 4/4. - AC8: defer the workspace-default flip. Committing the ignore rule blinds AO's dirty-guard, converting a loud leak into silent deletion of agent work. Also records four corrections to the ticket's own claims, including that `codex debug models` exists (the ticket assumed it did not) and that the teardown leak is a correct prediction rather than an observed failure. Refs #298 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TZTkC2D2ZgxAEhdzY8Ds57
|
Closing as an orphan. This PR is docs-only — a single commit adding PR #302 supersedes it. #302 is the live implementation for #298 — 31 files, +1911/-143, 8 test files — and it carries its own design doc ( If anything in this branch's design doc is better than #302's, lift it into #302 rather than reviving this PR. Do not merge both — that would land two competing design docs for the same ticket. Branch |
Design pass for #298 — the consolidated project-config plane ticket.
This PR currently contains the design doc only. Implementation phases land on top of it (see the delivery table in the doc). Opening it early and in draft so the design is reviewable and the work has a durable home — the two prior sessions on this ticket died before producing any artifact.
What the design closes
The ticket left four acceptance criteria carrying unresolved decisions. All four are now settled with evidence from four parallel read-only investigations against the code and the live daemon:
If-Match. Merge/PATCH is rejected:ProjectConfighas zero pointer fields, so a typed merge could never clear a scalar (autonomousMergeon but never off); merge would breakops/project-config.mjs apply, our documented wipe-recovery path; and merge doesn't even fix the reported reviewer-truncation symptom, which is frontend-only.Spawnable()) — the gate as scoped passes all four live projects, so no migration and nothing bricked. Three plausible-looking gates would brick 4/4 (gatingreviewers,prime.agent, orworkspace=worktree), so the operator's standard reviewer is a default to inject, never a gate.Corrections to the ticket
Four of the ticket's claims are wrong in ways that change the work:
codex debug modelsexists. The ticket asserts "there is no non-interactivecodex models list— which is precisely why AO hardcoded a Go switch", and hedges toward acodex app-serverJSON-RPC design. In factcodex debug modelsrenders the catalog as JSON, exit 0, in 0.27s. The app-server design is unnecessary.ErrMissingHarness), not the "deadlock" the ticket describes. The real deadlock surface ispermissions, and it is harness-dependent — codex's empty-permissions default already bypasses; only claude-family harnesses hang.backend/internal/adapters/workspace/gitworktree/…, notbackend/internal/gitworktree/….🔴 The finding that changes the plan
AC8 step 1, applied alone, is actively dangerous. Committing
.claude/worktrees/to the tracked.gitignoreblinds AO's dirty-guard:git worktree remove's clean check does not consider ignored files, soisDirty()reports clean,ErrWorkspaceDirtynever fires,Destroyreturns success, and the agent's nested worktree — including uncommitted work — is silently deleted. Lab-proven.It is harmless today only because in-place mode makes
Destroya no-op. So the ticket's "obvious safe first step" arms a landmine that step 3 would detonate. Hence: defer the flip.Relatedly, the teardown fix is not
--force. The no-forceDestroyis a deliberate, documented safety feature that converts a dirty worktree intoErrWorkspaceDirtyso agent work is preserved.Bug found while investigating (ships in the same PR, per rule 8)
A second lost-update path the ticket doesn't have:
observe/scm/observer.go:643-646refreshesRepoOriginURLvia a full-rowUpsertProject, rewriting theconfigcolumn from whatever it read. A config PUT landing in that window is silently reverted, and an HTTP ETag does not protect against it.Good news that shrinks the work
The operator's mandated "model per-harness, harness per-role" shape is already fully implemented in the backend —
AgentConfig.ModelByHarnessexists, is validated for cross-provider mismatches server-side, and already wins over the scalarModelat spawn.ConfiguredModelPinsalready enumerates every (harness, model) pair. The operator's table is that model made editable. AC12 is reached by deleting the scalar, not by building a new concept.Similarly, the reviewer path is manual-UI-only (no lifecycle hook, scheduler, or CLI triggers it), and the review tables have no CHECK constraint on harness — so widening the reviewer vocabulary needs no migration and carries no production traffic.
Refs #298
🤖 Generated with Claude Code
https://claude.ai/code/session_01TZTkC2D2ZgxAEhdzY8Ds57
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.