Harden and simplify delegation, Autopilot, and recovery - #51
Merged
Merged
Conversation
The macOS Seatbelt backend decided which host directories a Producer could write by sniffing basename(executable) and requiredEnv names — four producer-specific functions living inside src/platform/sandbox/. An adapter the sandbox failed to recognize silently ran with no state access, and every new lane had to edit the sandbox. ProducerInvocation.inheritedStateWritablePaths is now the declaration: each adapter states its own auth/config/state paths and the sandbox grants exactly those when no temporary home is in effect. The four OS-confined CLI probes also collapse into probeOsConfinedCli (resolve -> --version -> optional surface check -> confinement backend -> auth), with Pythinker's --help inspection supplied as a hook. Seatbelt tests now prove the seam (grants exactly the declared paths, ignores them under a temp home, never derives paths from identity or env names); each adapter test asserts its own declaration. The win32-separator HOME test moved with the join into the adapters, which never run on win32.
claude-implementer runs `claude -p --output-format json` as an untrusted Producer, so the architect can delegate implementation to Opus or Sonnet (producerOverrides.model) with an optional --effort override, under the same invariants as every other lane: fresh context, isolated worktree, frozen candidate, independent verification. Isolation is enforced by argv, each flag confirmed live against claude 2.1.250: --strict-mcp-config (no MCP servers, so no nested delegate tool), --tools without Agent (no nested subagents, no web), --setting-sources "" (no user/project/local settings, hooks, or CLAUDE.md discovery — the Producer sees only the rendered spec), --no-session-persistence, and --disable-slash-commands. Auth needs USER plus the real HOME (a temp HOME reports "Not logged in"), so the lane is inherited-config-only and declares ~/.claude and ~/.claude.json as its writable state. The result envelope can report is_error with exit 0, so normalizeEvents keys on both. darwin/arm64 only via the macos-seatbelt backend; a confined smoke run created a worktree file and got EPERM outside it. Opt-in real-CLI smoke test behind CLAUDE_ARCHITECT_CLAUDE_SMOKE=1. Design: docs/superpowers/specs/2026-08-27-claude-producer-adapter-design.md
The delegate skill now states that the architect session — whatever model it runs, Fable included — may dispatch Opus or Sonnet subagents through the host Agent tool for non-writing roles: scout, spec drafter, candidate reviewer, and advisor. A new read-only candidate-reviewer agent (opus; Read/Grep/Glob + reviewCandidate) reviews one frozen candidate without Producer context and returns two verdicts plus a recommendation; it never decides or integrates. An Opus/Sonnet implementer is the claude-implementer lane, never a bare subagent: the skill says so explicitly so the roster and the subagent roles cannot be confused.
…mpt rendering, probing, launch, and cache
…ership, durable write, and status emission
…broke The slice lifecycle now lives in SliceRunner: plan wave, create worktree, launch Producer, freeze, verify, review, compose, release anchor. Run-scoped facts travel as a RunContext value rather than a shared closure, and pipeline-runtime.ts drops from 2464 to 1545 lines. Finishing the extraction surfaced five defects in the seams between the new runner and the pipeline, none of which any test could reach while the tree did not compile: - runSliceReview was called without the run's borrowed checkout lease, so the review worktree blocked on the lock the same process already held. The parameter is now required-but-nullable, so omitting it is a type error. - Temporary slice refs were cleaned up inside the runner, before the final review round that resolves them. The runner hands them back; the pipeline's finally block remains the single place they are deleted. - The slice ref namespace was written as refs/claude-architect/runs/ while recovery swept refs/claude-architect/slices/ from its own copy of the literal. Orphaned refs would have accumulated with nothing failing. One declaration now lives in src/git/ref-namespace.ts. - Unparseable structured output was reclassified from invalid-output to producer-failure, losing the distinction between a malformed report and a crashed process, and the reported log ref pointed at the repair attempt rather than the output that failed validation. - The pipeline gate clearance was built twice with independent timestamps, so the archived record and the returned result disagreed on clearedAt. Neither reader could detect it alone. Two implementations that could diverge are now one. withManagedWorktree lives beside WorktreeManager and is borrowed by the pipeline, the slice runner, and candidate verification, which also gains the creation serialization it lacked; the queue is keyed per repository so unrelated repositories do not block each other. The superseded runSlicePhase/SlicePhaseDeps loop is deleted, and the suite that exercised it now drives the real runner. Also lands RunDecision, typed gate clearance, named verification modes, the RecoveryDependencies cleanup, and the documentation and skill work recorded under [Unreleased].
…ound reads runPipelineWithLease is 345 lines, down from 1048. Increments, review rounds, candidate promotion, the halted-slice path, salvage and archive, and the final gate are named functions over one explicit PipelineRunState value; each phase returns continue or a terminal PipelineResult instead of writing into a shared closure. RunContext gains a default sliceIndex so post-wave status lines no longer need a pipeline-local emitter. ArtifactStore is descriptor-driven: one ArtifactDescriptor per archived kind names the file, the read validator, the write-side redaction and validation, and the write mode. Every typed façade is one line over readArtifact and writeArtifact, reads share readEvidence's traversal and identity guards, and writes sit on PlatformSafety.writeAtomic. artifact-store-bytes.test.ts pins hashes recorded from the hand-written façades, so the rewrite is proven byte-identical. The store is bound to its run once. Read façades take no run id; the tool, review-snapshot, run-decision, and advisor-stage store interfaces follow, and prune reads each candidate run through a store bound to that run rather than through the caller's. tests/README.md maps every test file to the interface it crosses and records a verdict for each site that reaches past one. All fifteen call through to the real implementation and only observe or inject a fault, so nothing moved.
Apply the principal review: close trust gaps, cut the Autopilot shipping half, and remove duplicated or dead machinery. - Protocol 2.0.0 -> 3.0.0: autopilotStart drops pullRequest; Autopilot spec/state v2 and Final Branch Report v2 (status ready-for-human-review). Older versions are refused with explicit diagnostics. - Autopilot ends at a final-reviewed local branch handed to the delivery gate; promotions carry the user's Git identity; one state machine owns resume and cleanup; the remote is read only at create; it refuses to run under the human decision authority. - Managed worktrees live under <checkout>/.worktrees/claude-architect/. - Opt-in Jev screen can only withdraw autonomous acceptance. - Integration refuses acceptances without an artifact hash. - Split recovery-manager.ts (3,880 lines) into one module per concern; declarations moved verbatim. - Shared error, directory-flush, identity, and Git helpers replace copies. - CI pins every action to a commit and the Claude Code CLI to a version. - Tests: per-file isolated state dir, serialized worktree cleanups, and explicit e2e budgets remove cross-file flakiness.
…hardening # Conflicts: # README.md # assets/banner.svg
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: PyModel/claude-architect/.coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (195)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applies the principal review: closes trust gaps, cuts Autopilot's shipping half, and removes duplicated or dead machinery.
Breaking (protocol 2.0.0 → 3.0.0)
autopilotStartdropspullRequest; Autopilot spec/state v2; Final Branch Report v2 (ready-for-human-review). Older versions are refused with explicit diagnostics.Changes
humandecision authority.<checkout>/.worktrees/claude-architect/.CLAUDE_ARCHITECT_JEV=on) can only withdraw autonomous acceptance.recovery-manager.ts(3,880 lines) is split into one module per concern; declarations moved verbatim.Verification
npx tsc --noEmitclean.validate-release.shexit 0 on a committed copy.claude plugin validate .passed.