phase 1 1.1: fix effective-cwd git probing and fail-closed worktree fold-back - #113
Open
DaveGerson wants to merge 43 commits into
Open
phase 1 1.1: fix effective-cwd git probing and fail-closed worktree fold-back#113DaveGerson wants to merge 43 commits into
DaveGerson wants to merge 43 commits into
Conversation
…old-back ClaudeCodeLauncher._git_rev_parse/_git_diff_files were hardcoded to config.working_directory, ignoring cwd_override. For a Wave 1.3 worktree-isolated dispatch this meant pre/post HEAD capture always inspected the parent repo, so a real commit made inside the worktree was invisible: commit_hash/files_changed came back empty even though the agent committed, and the executor's "no commit -> clean up" path then deleted the worktree, silently discarding the work. Thread the launch's effective cwd (cwd_override when set, else config.working_directory) through pre-launch HEAD capture, post-launch HEAD capture, and diff/file discovery so all git inspection for a launch targets the same directory as the subprocess itself. Add a defense-in-depth fail-closed guard in WorktreeManager, independent of what the launcher reports: - fold_back() now verifies (WorktreeProvenanceError) that a reported commit_hash actually exists in the worktree and is not an ancestor of (or equal to) base_sha before folding/deleting anything. - A new _verify_safe_to_discard() re-derives the worktree's own HEAD and dirty status before the "no commit reported" cleanup path is allowed to permanently delete it. executor.py wires both into record_step_result's worktree lifecycle handling: on failure the worktree is left intact and the step is marked failed with a precise WorktreeProvenanceError instead of being folded or discarded. Regression tests added under tests/test_claude_launcher.py (effective-cwd git probing) and tests/test_worktree_manager.py (provenance guard). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ention test Adds tests/integration/test_worktree_isolation.py::TestRealGitEndToEndWorktreeCommit, a full-chain regression for the bd-1.1 silent-loss defect: a deterministic fake `claude` executable commits inside a real git worktree, ClaudeCodeLauncher (real, unmocked git rev-parse/git diff) discovers the commit via cwd_override, and ExecutionEngine.record_step_result() folds it back through the real WorktreeManager. Confirmed this test fails against the pre-1.1 launcher (commit_hash comes back empty) and passes against current code, asserting the parent repo receives exactly that commit and the worktree is only removed after the fold succeeds. Also adds TestUnverifiableProvenanceFailsClosed: a "successful" step (status=complete) reporting a commit_hash that cannot be verified inside its own worktree must fail closed via the real (unmocked) WorktreeManager._assert_commit_provenance() guard -- step marked failed, worktree retained on disk, handle kept in step_worktrees, parent branch unchanged. Note (documented in-file, out of scope for this test-only step): while building the positive regression, discovered that WorktreeManager.fold_back()'s hardcoded default "rebase" strategy always fails with git's "refusing to fetch into branch ... checked out" error for any real worktree commit, because WorktreeManager.create() leaves the branch checked out via `git switch -c`. This is independent of the bd-1.1 fix and is masked in the existing suite only because the executor-level fold-back success tests (bd-def9, bd-a735) mock WorktreeManager instead of using real git. The new positive test works around it by pinning the fold *strategy* to fast-forward (matching every other real-git WorktreeManager test in the suite) without mocking any git call -- flagging this for a follow-up Phase 1 fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Data-loss review of the Phase 1 worktree fold-back paths found that resume_from_takeover() marked a TakeoverRecord "resumed_at" / "resolution=completed" and returned True BEFORE confirming WorktreeManager.fold_back() actually succeeded -- any exception it raised (rebase conflict, unverifiable provenance, or anything else) was caught and merely logged. A fold failure during resume therefore looked like a successful resume to every caller: the record showed resolved, and nothing downstream (straggler sweep, gc_stale) would know the worktree still held un-folded developer commits, leaving it eligible for later reclaim -- permanent, silent data loss for exactly the commits a developer takeover exists to rescue. Fold now runs BEFORE the record is marked resolved; on any fold failure the record stays active, the worktree is retained on disk, and resume_from_takeover returns False so the operator knows to retry. On success the now-stale worktree handle is dropped from step_worktrees so later lifecycle code doesn't mistake it for one still needing fold/cleanup. Same audit of record_step_result()'s worktree lifecycle block found the inline try/except around fold_back() and _verify_safe_to_discard() only caught the two *modeled* exception types (WorktreeProvenanceError, WorktreeFoldError); any other exception (e.g. the git binary vanishing mid-run) escaped to the outer catch-all, which logs a warning and leaves `result.status` at whatever the caller passed in (typically "complete") -- claiming success without ever confirming the fold, or the safe-to-discard check, actually completed. Both paths now fail closed on any exception, not just the modeled ones. Regression coverage added under tests/integration/test_worktree_isolation.py (real git, no mocks of the git calls themselves): a fold failure during resume_from_takeover leaves the record active and the developer's commit on disk; an unmodeled exception from fold_back()/_verify_safe_to_discard() during record_step_result() fails the step closed instead of silently recording "complete". Each new test is confirmed to fail against the pre-fix code and pass against the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ee commits
Reviewing every fold-back path for data loss surfaced (and reproduced with
real git) a second, adjacent defect: fold_back()'s hardcoded default
strategy ("rebase") could never succeed for a genuine worktree commit.
create() always leaves the worktree's own branch checked out inside the
worktree for its entire lifetime, and git refuses `git fetch <path>
branch:branch` into a ref name that is checked out anywhere in the
repository -- so the rebase path's first step failed on every real
dispatch. This was masked in the existing suite because every real-git
fold test pinned strategy="none" or explicit "rebase"-expects-a-conflict
scenarios, never exercising the default path end-to-end; it was flagged
but explicitly left out of scope by the 1.2 test-authoring step.
This one was NOT data-loss (the fetch failure meant fold_back always
raised WorktreeFoldError before touching anything, so the worktree was
correctly retained) but it meant no dispatched step's work could ever
actually land in the parent branch via the default path, which undermines
the same "restore delivery trust" goal this phase exists for -- and,
per 1.2's note, fixing just the fetch guard (e.g. with --update-head-ok)
was verified BY HAND to trade that safe failure for real data loss: the
rebase strategy's 3-argument `git rebase --onto` form checks out its
target ref into whatever repository it runs in, which detaches HEAD and
overwrites the working directory of the canonical repo -- the live
project checkout in normal in-place operation -- even with the fetch
fixed. That path is left alone and clearly documented as known-broken
but fail-safe, reachable only via an explicit strategy="rebase".
Instead, fold_back() now defaults to "merge", which needed two changes to
be both correct and safe: (1) merge the commit by SHA directly instead of
fetching by branch name -- worktrees of one repository already share a
single object database, so no fetch is needed or attempted, sidestepping
the "checked out" refusal entirely; (2) fail closed (WorktreeFoldError) if
the canonical repo's currently checked-out branch isn't handle.base_branch,
since `git merge` always merges into whatever is checked out -- without
this guard a mismatch would silently attribute the worktree's commit to
the wrong branch. Verified with real git that a successful merge fold also
leaves the canonical repo's own working directory in sync (unlike the
"none" fast-forward strategy's raw update-ref, which does not).
Regression coverage added under tests/test_worktree_manager.py: the
default strategy now lands a real worktree commit in the parent branch
(confirmed to fail against the pre-fix "rebase" default), and refuses to
fold when the canonical repo has switched off base_branch. This also
fixes tests/test_wave5_integration.py::TestResumeReronsGateAndPasses,
which was previously passing only because the swallowed-exception defect
fixed in the prior commit hid this same fold failure -- it now passes for
the right reason.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…table worktrees
Two holes found attacking the 1.3 contract ('no successful execution path
can discard isolated agent work or claim a parent-repository commit as
the worktree result'):
1. _assert_commit_provenance accepted any commit that merely existed in
the shared object database and post-dated base_sha. A parent-repo
commit made after worktree creation (e.g. a sibling fold advancing
main) passed the guard, 'folded' as a no-op ('Already up to date'),
and the success-path cleanup then deleted the worktree containing the
agent's real, unfolded commit — silent worktree loss. The guard now
also requires the commit to be reachable from the worktree's own
HEAD, and fails closed when git cannot answer (merge-base/rev-parse
errors).
2. _verify_safe_to_discard silently skipped both probes when git
returned nonzero (broken/pruned .git linkage), letting the caller
delete an uninspectable worktree that could still hold real work.
Ambiguous state now raises WorktreeProvenanceError and retains.
Regression tests for both in tests/test_worktree_manager.py.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…rategy The 1.2 end-to-end test bypassed the production fold path by monkeypatching fold_back to strategy='none', and carried a NOTE claiming 'merge can never succeed' — stale since 1.3 made merge the working default. The test now exercises record_step_result's genuine default (merge) with zero fold overrides: asserts the agent commit becomes reachable from the advanced parent branch and working_branch_head equals the folded tip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Add docs/internal/execution-runtime-contract.md, defining one execution lifecycle (dispatch, persist result, gate, request decision, pause, record decision, resume, complete) shared by the CLI action loop, the duplicate baton run / baton execute run autonomous loops, the daemon TaskWorker, and the REST API/PMO, with authoritative owners, idempotency and restart semantics, and a compatibility plan for the duplicate baton run surface. Add characterization tests locking in the current contract (and its known gaps, notably that process-level pause does not mutate persisted status): resume-vs-restart guards in tests/test_execute_run.py, a pause/resume signal roundtrip plus CLI-vs-TaskWorker terminal-state equivalence in tests/test_daemon.py, PMO approval_log vs DecisionManager independence in tests/test_approval_workflow.py, and decision-resolve event-emission idempotency in tests/test_api_decisions.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
`baton run` (run.py) independently constructed its own ExecutionEngine + TaskWorker + BatonRunner stack -- a second, divergent implementation of the autonomous-loop contract that `baton execute run` (_handle_run in execute.py) already implements, per the §7.4 compatibility plan in docs/internal/execution-runtime-contract.md. That duplicate called ExecutionEngine.start(plan, task_id=...), a signature the engine has never had, so any `baton run` invocation that started a fresh plan (including --dry-run) crashed with TypeError. handler() now translates the unchanged `baton run` CLI surface onto _handle_run instead of retaining a second state machine, so CLI, daemon, and PMO drive the exact same active-task resolution and resumable/ terminal status guard. --max-parallel is accepted for backward compatibility but warns that it has no effect on the canonical sequential runner; --resume is accepted but redundant since the canonical runner always resumes automatically. Adds an optional --max-steps flag (new, additive). Regression tests pin: the CLI surface is unchanged, handler() delegates to _handle_run rather than constructing its own engine/worker, and `baton run --dry-run` completes end-to-end without the TypeError.
Persistent daemon decision routing:
- WorkerSupervisor.start() and daemon.py's _run_daemon_with_api() now
always construct and inject a disk-backed DecisionManager into
TaskWorker, rooted at team_context_root/decisions (the same location
api/deps.py binds the shared REST DecisionManager to). Previously
neither call site passed one, so every human-required GATE/APPROVAL
silently hit TaskWorker's "no decision manager configured" auto-approve
fallback -- the daemon auto-approved everything merely because
dependency injection was omitted, not by design.
- TaskWorker gains _handle_feedback/_handle_interact, wired into the
execution loop. FEEDBACK and INTERACT actions previously had no
handling at all in the async loop, so a plan reaching either busy-
looped on next_action() forever. Both now route through the
DecisionManager (auto-selecting a fallback answer only when no manager
is configured, mirroring GATE/APPROVAL's existing contract, which an
existing test pins down and this change does not weaken).
Deterministic decision IDs (core/runtime/decisions.py):
- deterministic_decision_id()/parse_decision_id() replace the random
UUIDs DecisionRequest.create() mints for GATE/APPROVAL/FEEDBACK/
INTERACT requests raised by TaskWorker and the headless CLI runner.
A stable ID keyed on (task_id, kind, parts) lets every surface -- CLI
re-invocations, daemon restarts, the REST API -- converge on the same
pending request instead of creating duplicates, and lets a caller
recover which task/phase/step a resolved decision was about without a
new structured field on DecisionRequest.
PMO paused-task resume path (cli/commands/execution/execute.py):
- _run_loop's APPROVAL/FEEDBACK/INTERACT handling under a non-TTY stdin
(the headless `baton execute run` subprocess POST /pmo/execute/{card_id}
launches) now records a durable DecisionManager request before pausing,
instead of leaving the pause implicit in the execution's own status.
FEEDBACK/INTERACT previously had no handling in this loop either --
they spun to max_steps and aborted with a misleading "ABORTED" exit(1),
indistinguishable from a genuine failure. If the same decision was
already resolved by the time this code re-evaluates the action (e.g.
a prior invocation paused, and the REST API resolved it since), it is
applied directly and execution continues in this same process. TTY
behaviour is unchanged for APPROVAL and newly added for FEEDBACK/
INTERACT so `baton execute run` in an interactive terminal no longer
busy-loops on either action type.
Idempotent decision + atomic resume (api/routes/decisions.py,
api/routes/pmo.py):
- POST /decisions/{request_id}/resolve now parses a deterministic
request_id back into (task_id, kind, parts) and applies the resolution
directly to the execution engine via apply_decision_resolution() --
previously resolving a decision only flipped the DecisionManager's
on-disk status and published an event that only a live TaskWorker poll
loop would ever notice, which is never the case for a headless
`baton execute run` subprocess that already exited after recording the
pending decision. It then calls resume_task_headless(), which is
idempotent: it checks the execution's worker.pid liveness before
spawning a new headless runner, so a retried resolve never launches two
processes racing on the same task. Legacy random-UUID request_ids
(already-resolved decisions predating this change, and gate_escalation
requests which are intentionally left non-deterministic) parse to None
and are skipped gracefully, not treated as an error.
- New GET/POST /pmo/execute/{card_id}/decisions[/{request_id}/resolve]
endpoints mirror the generic /decisions API but resolve the card's own
project root per-request (via _resolve_worker_context), because the
generic API is bound to a single team_context_root at server startup
and cannot see a per-card project root on a multi-project PMO board --
the same reason /pmo/gates/pending and /pmo/gates/{task_id}/approve
already exist as PMO-scoped counterparts of engine calls.
Regression tests cover: supervisor/daemon DecisionManager injection
(review gates are no longer auto-approved), TaskWorker FEEDBACK/INTERACT
routing (with and without a manager), deterministic ID round-tripping,
apply_decision_resolution idempotency (a second apply after a live
worker already applied it returns False, not an error),
resume_task_headless's worker.pid liveness guard, the headless
APPROVAL/FEEDBACK/INTERACT pause-then-resume contract in _run_loop
(including duplicate-pause de-duplication), and both the generic and
PMO-scoped resolve endpoints applying + resuming exactly once.
Adds tests that chain multiple stages of docs/internal/execution-runtime- contract.md's lifecycle together, driving real entry points end-to-end and asserting persisted execution/decision state before and after each transition rather than only exit codes: - tests/test_execute_run.py: TestCanonicalAndCompatibilityDryRunParity -- drives both `baton execute run` (canonical) and `baton run` (the compatibility shim) through the real resume-vs-restart guard and asserts they reach identical persisted state. - tests/cli/test_execute_run_resume.py: TestNonTtyApprovalPauseSurvivesRestartAndCompletesOnce -- a non-TTY approval prompt pauses durably across repeated process boundaries without duplicating the decision request, is answered through the DecisionManager (the same object the REST API and PMO inbox delegate to), and completes exactly once; a further invocation refuses to restart the terminal task. - tests/test_daemon.py: TestDaemonRestartWithPendingDecision -- a worker crash while a review-gate decision is pending is followed by a fresh engine+worker pair (mirroring WorkerSupervisor.start(resume=True)) that reuses the same deterministic decision request instead of duplicating it, and drives the task to COMPLETE exactly once after resolution. - tests/test_api_pmo_gates.py: TestPmoExecuteToPauseToApiApproveToResumeToComplete -- exercises the real PMO pause/resume endpoints against a genuinely signalled worker process (proving OS-level pause never mutates ExecutionState), then the decisions-resolve endpoint's apply + headless-resume path driven inline through the canonical runner to a real COMPLETE; plus a duplicate- approval-submission companion test. - tests/test_api_decisions.py: TestDuplicateApprovalSubmissionAgainstEngine -- a duplicate approval submission against a real engine-backed deterministic decision is rejected before it can re-apply to the engine or respawn a headless resume, and the human.decision_resolved event fires exactly once. Sanity-checked two of the new tests against injected regressions (removing the API's duplicate-resolve guard; de-deterministic decision request ids) to confirm they fail against defective behavior, not just vacuously pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
… resumes exactly once The deterministic interact decision ID was keyed on (task, step) only, but an interactive step re-presents the same step_id on every turn. Turn 2 therefore matched turn 1's already-resolved decision and both TaskWorker._handle_interact and the headless _run_loop replayed the same human input on every subsequent turn without ever asking the human again. Include the engine-provided interact_turn in the ID: crash-resume at the same turn still converges on one request, while each new turn records a fresh pending decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…sume runner resume_task_headless only checked executions/<task_id>/worker.pid, but 'baton daemon start' without --task-id (including --serve mode, whose in-process worker polls the same decisions directory) writes its PID to the legacy <root>/daemon.pid (WorkerSupervisor.pid_path). Resolving a decision via the REST API would then spawn a second headless 'baton execute run' process racing the live daemon worker on the same execution. Probe both PID locations before spawning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ontract pipeline Add agent_baton/core/engine/planning/scope_contract.py: the shared, pure normalization/derivation/diagnostics primitives for scope contracts -- normalize_scope_path (cross-platform, repo-relative, traversal-rejecting path normalization), paths_overlap (directory- prefix + glob matching), is_generated_path (build-output policy), WRITE_CAPABLE_STEP_TYPES/READ_ONLY_STEP_TYPES (developing/testing/ automation/synthesis vs. reviewing/consulting), derive_allowed_paths (decomposition evidence -> deliverables -> context files -> repo topology -> agent-role tiers, never inventing an unconfirmed path), and diagnose_step_scope (missing vs. contradictory write scope). Wire it into ScopeMapBuilder (scope.py): workstream allowed_paths are now normalized and, when no step supplies an explicit path, derived deterministically instead of a single blind fallback to charter.likely_repo_areas. A workstream whose steps are all intentionally read-only is left empty on purpose. New optional project_root/strict/diagnostics parameters on build() are additive (existing 2-arg callers are unaffected). Wire it into ManagerModePlanner (planner.py): a new strict_scope constructor flag (default False, preserving all existing callers) turns ambiguous write scope into a raised ScopeContractError before any sidecar is written; contradictory scope (allowed/blocked collision) always raises. Every nontrivial step's built ScopeContract is now passed through _apply_scope_contract_policy, which strips an intentionally-read-only step's contract back to its own explicit paths (fixing ScopeContractBuilder's naive `step.allowed_paths or workstream.allowed_paths` fallback silently handing review steps a workstream's write scope) and records missing/contradictory findings on ManagerArtifacts.warnings regardless of strict_scope. Regression tests: tests/engine/planning/test_scope_contract.py (new, 60 cases covering normalization, overlap, generated-path policy, derivation tiers, diagnostics) plus new cases in tests/manager/test_scope_map.py and tests/manager/test_manager_mode_planner.py covering strict-mode raising, diagnostic recording, path normalization of explicit paths, and read-only steps never inheriting workstream write scope. All pre-existing tests in both files, plus the full manager/, engine/planning/, e2e manager-mode, and CLI manager-mode/dry-run suites, pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Converts allowed_paths/blocked_paths from advisory prompt text into
binding execution-time controls:
- ClaudeCodeLauncher.configure_step_scope() normalizes a step's scope
contract against the effective repo root (reusing
core/engine/planning/scope_contract's normalization), rejects
traversal/symlink escapes, fails closed (refuses to spawn the
subprocess) for write-capable steps with an empty effective allowed
set, and delivers the remaining scope as a real PreToolUse hook via
--settings on the claude subprocess argv. TaskWorker wires this in
for every DISPATCH action; unconfigured callers see zero behavior
change.
- ExecutionEngine.record_step_result now independently recomputes a
worktree step's real diff (base_sha -> HEAD, ignoring any
caller-reported files_changed/commit_hash) for manager-mode steps
with a scope contract, via
manager_scope_signal.{independent_worktree_diff,
derive_scope_expansion_from_diff}. An out-of-contract diff forces
the step to "failed" (so it can never be folded back) and files a
durable ManagerDecision backed by persisted evidence
(ManagerArtifactPaths.scope_evidence).
- New core/manager/scope_amendment.py + ExecutionEngine.
resolve_scope_expansion() resolve that decision: reject leaves the
failed step and retained worktree untouched; approve durably widens
the scope-contract sidecars (atomic os.replace writes) before -- and
only before -- the plan's allowed_paths is mutated and the step is
requeued for retry.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…-verification bypasses Security review of the scope-contract enforcement boundary added in 3.1/3.2: - Fix a real bypass in independent_worktree_diff() (the independent post-diff verifier): git status --porcelain C-quotes any path with a space or other "unusual" byte (e.g. "foo bar.py"), and the naive line-slicing parse kept the literal quote characters as part of the path. For an allow-list contract this only produced a false-positive (fails closed); for a blocked-paths-only contract (no allowed_paths to fall back on) it failed OPEN -- a quoted path silently matched neither side, so a real change inside a blocked directory was reported clean. Switched every git invocation to the NUL-delimited -z form and rewrote the porcelain-v1 parser to consume the paired ORIG_PATH field on rename/copy records instead of splitting on " -> " text. - Fix a real bypass in the launcher's PreToolUse bash guard: it only string-matched $CLAUDE_TOOL_INPUT_FILE_PATH against the allowed/blocked regex, so a symlink created mid-session inside an allowed directory (app/link -> /etc) let a later write "through" it (app/link/passwd) pass the string check even though the real destination was outside the repository. The guard now canonicalizes the write's real destination (readlink -f, falling back to realpath -m) against the subprocess cwd before the allowed/blocked check, and rejects anything that resolves outside the repo/worktree root outright. - Added adversarial regression coverage across the scope-enforcement surface: empty scope, blocked-over-allowed precedence (including cross-sibling-step and nested-nesting cases), absolute paths, dot-dot traversal, symlink escape (real bash-subprocess drive, not a mock), rename/delete, generated-file explicit-vs-inferred handling, untracked files (including git's whole-new-directory collapse), and partial scope-expansion-approval failure (mid-sidecar-write OSError must leave the authoritative plan/worktree registry untouched, not half-applied). - Documented (xfail, strict=True) a separate, already-existing bypass in agent_baton/core/audit/dispatch_verifier.py's _is_under(): it uses PurePosixPath.relative_to(), which never collapses ".." segments, so a self-reported files_changed entry containing ".." lexically matches an allowed prefix while resolving outside it. That file is outside this step's allowed_paths -- see the report's "concerns" for the fix pointer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…b-match and collapsed-new-dir swallowing a specific allowed file)
The allow-list membership check in derive_scope_expansion_from_diff used
the bidirectional paths_overlap, which fails open on a concrete changed
file: (1) a diff entry literally named '**' (or ending in a '**'
segment) was glob-interpreted and matched any allowed prefix, and (2)
git's whole-new-untracked-directory collapse ('newdir/') was treated as
in-scope against a more-specific allowed FILE ('newdir/allowed.py'),
hiding an out-of-scope sibling created in the same new directory. Both
are out_of_scope_diff_accepted holes.
Add a directional path_within() primitive (candidate must equal or be
nested under allowed; candidate's own segments are always literal; only
the allowed side may carry a trailing '**' glob) and use it for the
allow-list check. The blocked-path check stays bidirectional on purpose
so a coarse collapsed directory that contains a blocked path is still
flagged. Regression coverage added for path_within and both bypasses.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
… layer Adds docs/internal/team-runtime-contract.md, the architect deliverable for Phase 4 4.1: it names the "prompt fiction" defect (team-lead.md documents team_* tools no launched process can actually call), decides the exposure mechanism (structured Baton CLI over the already-granted Bash tool, not a local MCP server — mcpServers frontmatter is not honored by the claude-teams backend), and specifies schemas, authorization, optimistic concurrency, mailbox delivery, idempotency, audit, timeouts, failure taxonomy, and a synthesis state machine. Lands the Python-level callable layer the CLI/MCP transport (deferred, §9) will call unchanged: - team_tools.py: canonical team_list/team_claim/team_update/team_send/ team_read tools alongside the unchanged legacy functions, a role-based authorization matrix (authorize_team_tool/authorized_team_tools/ advertised_team_tools_for_role), and TeamAuthorizationError/ TeamConcurrencyError exception subclasses. - team_board.py: TeamBoard.claim_task gains opt-in optimistic concurrency (expected_status="open") while its default stays last-writer-wins for legacy-behavior compatibility; append_task gains idempotency_key dedup; new done_tasks_for_team(); TeamBoardConflictError replaces a prior silent no-op on a missing/invalid task bead. - team_registry.py: TeamRegistry.set_status_if() — atomic compare-and-swap status transition, the guard the synthesis state machine design relies on. - models/execution.py: SynthesisState enum + SYNTHESIS_STATE_TRANSITIONS + is_valid_synthesis_transition(), the typed vocabulary for the synthesis state machine (not yet wired into persisted StepResult — documented as follow-up in the contract doc). - tests/test_team_tools.py: hermetic in-memory _FakeBeadStore (removes the existing tests' hidden dependency on the external bd binary) plus new coverage for all of the above. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Implements docs/internal/team-runtime-contract.md's deferred §9.1 work: the five canonical team_* tools (list/claim/update/send/read) are now callable end-to-end, not just tested Python functions. - agent_baton/cli/commands/team_cmd.py: adds `baton team list|claim|update|send|read`, the structured CLI surface the contract chose over a local MCP server. Resolves task_id/member_id per the documented order (--flag > $BATON_TASK_ID/$BATON_TEAM_MEMBER_ID > active-task lookup), builds a real ExecutionEngine against the project's baton.db, and maps TeamToolError/TeamAuthorizationError/ TeamConcurrencyError/backend-unavailable onto the doc's exit-code taxonomy (2/3/4/5). Resolves .claude/team-context via BATON_TEAM_CONTEXT_ROOT/BATON_DB_PATH first so calls made from inside an isolated worktree target the parent project's db, not a worktree-local one. team_dispatch is intentionally not exposed here, matching the contract's explicit CLI scope (§2.2). - agent_baton/core/runtime/claude_launcher.py: launch() now injects BATON_TEAM_MEMBER_ID from its own step_id argument (authoritative and race-free — a team member's step_id IS its member_id by construction), rather than mutating shared os.environ, which would race across a StepScheduler wave's concurrently-dispatched launches. - agent_baton/core/engine/team_tools.py: closes two gaps the 4.1 architecture doc specified but didn't implement — (1) every canonical tool call now emits a structured _log.info audit line naming tool/task_id/member_id/outcome, independent of whether the call reached a bead write (doc §7.1); (2) a bead-store-unavailable engine (e.g. the `bd` binary missing) now raises a clean TeamToolError instead of an opaque AttributeError inside TeamBoard, matching the doc's "Underlying store unavailable" -> exit 5 row (§7.3). - agents/team-lead.md (+ bundled copy): removes the prompt-fiction team_send_message/team_add_task/team_claim_task/team_complete_task API and documents the real baton team CLI instead, including how a member resolves its own member_id/team_id and what each exit code means. team_dispatch's claim is revised to state plainly that no callable path exists for it yet, rather than implying it does. Regression tests: tests/test_team_tools.py (bead-store-unavailable + audit-logging), tests/test_claude_launcher_team_member_env.py (env injection + race-avoidance), tests/cli/test_team_cmd_runtime.py (end-to-end CLI flow across independent ExecutionEngine constructions, proving restart durability, plus the exit-code taxonomy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Replaces the agent_synthesis placeholder (which silently completed the team step with a "synthesis_requested" deviation marker and never dispatched anything) with an explicit, persisted dispatch of spec.synthesis_agent. - StepResult gains synthesis_state (SynthesisState value) and synthesis_dispatched, both persisted through SQLite (schema v48) and the JSON file backend (StepResult.to_dict, conditionally emitted so the golden-fixture roundtrip stays byte-identical). - _apply_synthesis's agent_synthesis branch now transitions the parent StepResult into SynthesisState.SYNTHESIZING and leaves it status="dispatched" instead of completing synchronously. concatenate/merge_files are untouched (still complete synchronously) -- backward compatible. - _pending_synthesis_dispatch (consulted from both the serial next_action WAIT arm and the parallel next_actions batch path) builds a real DISPATCH ExecutionAction naming spec.synthesis_agent, with a prompt carrying structured member outcomes, files changed, detected conflicts, and provenance. synthesis_dispatched guards exactly-once dispatch and survives restart. - The synthesis agent is dispatched and recorded against the SAME step_id as the parent team step (not a synthetic child id), so its mark_dispatched/record_step_result round trip flows through the exact scope/commit/evidence verification pipeline non-team steps already use, and the CLI's STEP_ID_RE/TEAM_MEMBER_ID_RE split in _validators.py routes it correctly without any CLI changes. A small carry-forward in record_step_result preserves member_results / synthesis_state / synthesis_dispatched across the intermediate "dispatched" write instead of losing them to the existing replace-on-step_id semantics. - conflict_handling is now enforced for all three values: auto_merge (synthesis agent dispatches and is expected to reconcile), escalate (pauses for APPROVAL, then resumes into a synthesis dispatch once approved -- the ESCALATED -> SYNTHESIZING edge), and fail (a new branch terminates the step on a same-file conflict even when no member itself failed, which was previously unhandled and fell through to auto_merge-like completion). - Updated tests/test_phase3_team_maturation.py assertions that encoded the old placeholder's synchronous-completion behavior for agent_synthesis; added tests/engine/test_team_synthesis_dispatch.py covering exactly-once dispatch (serial and parallel paths), restart safety, full completion/failure round trips, and all three conflict_handling values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Builds the test coverage docs/internal/team-runtime-contract.md §10 deferred to this step: real-boundary team tool calls, restart-safe synthesis lifecycle, and dry-run contract regressions. - tests/test_team_tools.py: restart persistence for claim/update/send/ read across brand-new ExecutionEngine constructions against the same db (a path-keyed fake bead store, mirroring tests/cli/test_team_cmd_ runtime.py's established pattern); malformed/unauthorized call ordering (team exists -> member exists -> role authorized -> bead store reached); a regression pinning the CLI-exposed verb set (team_cmd._RUNTIME_HANDLERS) against authorized_team_tools() per role and against agents/team-lead.md's own text, so the "advertised tools never exceed capabilities" invariant can't silently drift. - tests/test_multi_team_e2e.py: real OS-subprocess tests that invoke the actual installed `baton team` console script (not a mock, not an in-process handler call) for the parts of the boundary that don't need the external `bd` binary -- resource=teams reads and the CLI's own usage-validation exit codes -- plus the documented fail-closed exit-5 contract for bd-backed writes in this sandbox's real bd-less environment. - tests/test_team_registry.py: direct TeamRegistry.set_status_if CAS coverage (including a simulated race where only the first of two concurrent transitions wins), restart persistence across a fresh TeamRegistry instance, and grandchild (3-level) nesting semantics. - tests/test_team_board.py: hermetic (_FakeBeadStore) idempotent-create and malformed-claim-target coverage alongside the existing bd-backed fixture. - tests/engine/test_team_mailbox_hooks.py: nested-team mailbox ordering -- task_created for every flattened member, teammate_idle gated on the WHOLE nested tree (not just top-level roster). - tests/test_team_steps.py: conflict_handling='fail' combined with a non-agent_synthesis strategy and with a co-occurring member failure (both previously untested anywhere in the suite); malformed record calls (unknown member_id, duplicate record) don't block real completion or crash; a dry-run assertion that TracingDryRunLauncher payloads for a team dispatch wave carry complete, non-degenerate token estimates. Also pins a real gap found while writing this coverage: conflict_handling='escalate' never resumes synthesis for concatenate/merge_files strategies after approval (only strategy='agent_synthesis' is wired to resume) -- executor.py is outside this step's allowed_paths so the fix is left for a follow-up; see the test's docstring and this commit's reported concerns. - tests/test_nested_team_dispatch.py: restart between dispatch and result (a fresh ExecutionEngine over the same on-disk state resumes without duplicating the dispatch wave or losing partial member results), and member-failure propagation that preserves sibling results instead of discarding them. - tests/test_team_step_routing.py: team-record CLI-handler restart safety across two members, and a malformed --member-id (not in the plan's roster) that must not crash or falsely complete the step. - tests/engine/test_team_backends.py: readiness diagnostics for 3-level nested teams (nested_team_count must reflect every level, not just the top one). Confirmed via git stash comparison that tests/test_team_board.py's existing bd-backed fixture and one pre-existing test in tests/test_multi_team_e2e.py already fail in this sandbox because the `bd` binary isn't installed -- same root cause as the two failures already carved out for this plan; not introduced here and left untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ng bd tests/test_team_board.py's `bead_store` fixture and tests/test_multi_team_e2e.py's `engine` fixture were constructing a real BdBeadStore via make_bead_store(), which requires the external `bd` binary on PATH. In this sandbox (and per the environment note that only tests/test_bead_cmd_worktree_discovery.py's failure is an accepted pre-existing bd-binary gap) `bd` is not installed, so every test that touched the board through these fixtures errored out with TeamToolError/BdNotAvailable before exercising any real behavior. Repoint both fixtures at an in-memory `_FakeBeadStore` (write/read/close/ query), mirroring the established hermetic pattern already used by tests/test_team_tools.py, so the suite satisfies tests/CLAUDE.md's hermeticity requirement (no host filesystem/binary assumptions outside tmp_path) without touching bd_bead_store.py (out of this phase's allowed paths) or weakening any assertions. Also corrects the stale BEAD_WARNING comment in TestAckMessage to note the known BdBeadStore.query() closed-bead/label-filter bug is a production concern tracked separately, not something these hermetic tests exercise. Gate: python -m pytest -q tests/test_team_tools.py tests/test_team_registry.py tests/test_team_board.py tests/engine/test_team_mailbox_hooks.py tests/test_team_steps.py tests/test_multi_team_e2e.py tests/test_nested_team_dispatch.py tests/test_team_step_routing.py tests/engine/test_team_backends.py -> 243 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…sniffing The CLI's exit-code mapping classified any TeamToolError whose message contained the substring 'unavailable' as exit 5 (backend unavailable, 'stop and report — do not retry' per team-lead.md). team_id/member_id are user input interpolated into usage-error messages, so 'baton team list --team-id team-unavailable' — a plain typo — exited 5 instead of 2, telling scripted callers the environment was broken. Add TeamBackendUnavailableError (TeamToolError subclass), raise it from _require_registry/_require_bead_store, and branch the CLI mapping on the type. Aligns with the contract doc's own taxonomy principle (every branchable failure is a typed subclass). Regression test added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Step 4.1 fixed agents/team-lead.md but left two other agent-facing artifacts advertising uncallable tools — exactly the prompt_only_team_tool failure mode this phase exists to eliminate: - references/team-messaging.md (a distributable reference agents are directed to by orchestrator.md) presented team_send_message/ team_add_task/team_claim_task/team_complete_task/team_dispatch as 'tools available to agents' with call-shaped Python examples. Rewritten to advertise only the real 'baton team <verb>' CLI surface, with an explicit 'no callable team_dispatch' section mirroring team-lead.md. - agents/orchestrator.md claimed 'Leads can also stand up sub-teams on the fly via the team_dispatch tool'. Corrected: no callable surface; treat a lead's request for unplanned decomposition as a plan-change decision. Bundled mirror re-synced via scripts/sync_bundled_agents.sh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
- models/execution.py: SynthesisState docstring still said the enum was 'not yet consulted by the executor' — falsified by 4.3. Now describes the actual persisted/executor-driven lifecycle. - team_registry.py: set_status_if docstring claimed it is 'used by the team-level synthesis state machine'; nothing in production calls it (the executor guards exactly-once dispatch with StepResult.synthesis_dispatched). Docstring now states it is a test-exercised CAS primitive available for driver-level use. - docs/internal/team-runtime-contract.md: §Synthesis scope note and §9.3 said executor wiring was deferred; updated to reflect what 4.3 landed and what genuinely remains open (intermediate states, set_status_if call sites). - docs/cli-reference.md: the five public 'baton team' runtime verbs (list/claim/update/send/read) added in 4.2 were undocumented despite the mandatory CLI-reference update rule; documented, including the exit-code contract and the intentional absence of a dispatch verb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Add agent_baton/core/engine/planning/capability_gap.py: an evidence-backed CapabilityGap model (missing_role / weak_task_description / missing_knowledge), detectors that distinguish those three, and decide_talent_lifecycle() -- a bounded, policy-controlled decision function (dispatch / fallback / queue-for- manager / request-clarification) with a structural guard against talent-builder generating itself and a depth/retry-budget ceiling for re-planning instead of recursive spawning. Wire a diagnostic-only detection point into RosterStage for explicitly requested agent names that don't resolve in the registry, surfaced on plan.plan_diagnostics["capability_gaps"] / ["talent_lifecycle_decisions"] without mutating the roster. IntelligentPlanner.create_plan() gains optional skip_init/allow_talent_builder kwargs (default-preserving, backward compatible). Add TalentFactoryConfig (talent_factory section) to ManagerConfig: retry budget, recursion ceiling, validation/rollback policy, name-collision policy, registry-reload timing -- alongside the pre-existing TeamConfig.allow_talent_builder master switch. Update agents/talent-builder.md (and the synced bundled copy) with the talent-factory lifecycle contract: default product is an agent or knowledge pack (skill/plugin only when explicitly requested), never generate another talent-builder, treat ingested research material as untrusted data never instructions, least privilege, no silent name-collision overwrites, generation provenance frontmatter, and re-plan unresolved work instead of retrying or recursing. Document the full model in docs/internal/talent-factory-contract.md, including the explicitly deferred follow-up work (CLI wiring of --skip-init/ allow_talent_builder into baton plan, actual talent-builder dispatch as a plan phase, attempt/recursion bookkeeping across a run, baton agents doctor, mid-run registry reload) -- none of which blocks this step's behavioral contract, since the safe fallback is unconditionally available today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ed re-planning
Wires the talent-factory lifecycle (5.1's model) to real behavior:
- IntelligentPlanner.create_plan() now runs classification+roster first,
acts on any DISPATCH_TALENT_BUILDER decision via the new
planning/talent_factory.py module, then continues risk..assembly with
the resolved roster ("re-plan only the unresolved step" without
re-running the full pipeline).
- talent_factory.py performs exactly one bounded dispatch attempt per
gap through a pluggable TalentBuilderDispatcher (production:
HeadlessTalentBuilderDispatcher, the same synchronous/verified launcher
already used for plan review); validates the artifact
(generated_agent_validator.py: frontmatter/schema, allowed
models/tools, provenance, body sections, prompt-safety scan,
recursion guard), atomically installs it under name-collision policy,
and reloads the registry via new AgentRegistry.register_generated_agent
/ KnowledgeRegistry.register_generated_pack. Every write happens in a
scratch directory first and is rolled back on any failure.
- Failed/disallowed/skipped generation resolves to a deterministic
generic-agent fallback (or TalentFactoryError if the registry truly
has no candidate) -- never a phantom, undispatchable agent name in the
final plan.
- plan_cmd.py: --skip-init and ManagerConfig.team.allow_talent_builder /
talent_factory now actually reach create_plan() (config load moved
earlier); baton plan is the one call site that wires a live
HeadlessTalentBuilderDispatcher. IntelligentPlanner's own default stays
a no-op NullTalentBuilderDispatcher so constructing a planner never
starts a live `claude` subprocess as a side effect -- required for
hermeticity across the existing test suite, which constructs
IntelligentPlanner() directly in many places.
- Updated two pre-existing tests whose assertions encoded the old
"unresolved agent name flows through unchanged" behavior, now
superseded by real resolution before phase construction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ollback, config wiring) Extends the talent-factory test suite within this step's allowed test paths, complementing the tests already landed alongside 5.1/5.2 (tests/test_talent_factory.py, tests/test_generated_agent_validator.py, tests/cli/test_plan_cmd_talent_factory.py -- outside this step's scope): - tests/engine/planning/test_headless_talent_builder_dispatcher.py (new): process-failure coverage for the one dispatcher that actually shells out -- CLI-not-found, a raised subprocess exception, non-zero exit, empty/whitespace output, code-fence unwrapping, and prompt content (gap evidence + explicit anti-recursion instruction), all against a mocked HeadlessClaude so no real `claude` subprocess runs. - tests/engine/planning/test_talent_factory_rollback_and_collision.py (new): the run_talent_factory_for_gap cases not yet covered anywhere -- name_collision_policy=manual_review (quarantine, never overwrite/ register), install-time registry re-parse failure rolling the just-written file back out, a path-escaping frontmatter `name` (defense in depth via the kebab-case validator, verified against the real filesystem so "no artifact remains after a failed validation" is checked as a filesystem invariant, not just ValidationResult.valid), and malformed-frontmatter / unsafe-tool-request end to end. - tests/test_planner.py: registry_reload=immediate proven causally -- a second create_plan() call on the same planner instance reuses a just-generated agent with zero capability gaps and zero re-dispatch; telemetry coverage for routing_notes + plan_diagnostics shape on both the generated and fallback outcomes. - tests/engine/planning/test_planner_diagnostics.py: build_plan_diagnostics preserves capability_gaps/talent_lifecycle_decisions/ talent_factory_outcomes verbatim across a re-diagnostics pass (e.g. a goal-driven amend cycle), and defaults them to empty rather than raising when a plan never hit a capability gap. - tests/manager/test_manager_config.py: TalentFactoryConfig defaults, YAML section loading (full + partial override), invalid Literal rejection for all three enum fields, to_dict/from_dict round-trip, and backward compatibility loading the pre-talent-factory PRD §9.1 spec YAML. - tests/cli/test_plan_manager_mode_save.py: pins that --manager-mode still threads skip_init/allow_talent_builder/talent_factory_config into create_plan() after the config-load reordering in plan_cmd.py -- a regression here would silently diverge manager-mode wiring from the plain-`baton plan` wiring already covered elsewhere. Verified: all new/edited test files pass in isolation and together (tests/engine/planning, tests/manager, tests/agents, tests/test_planner.py, tests/cli/test_plan_manager_mode_save.py, tests/test_engine_planner.py). The 17 failures observed in that combined sweep are pre-existing on this branch with none of this step's changes applied (confirmed via git stash) -- ValidationStage review_missing/agent_phase_mismatch defects in tests/test_engine_planner.py and the already-documented test_planner_smoke.py::test_medium_task_produces_standard_plan -- none touch talent-factory code or tests and none are in this step's allowed paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Fixes 15 of 16 gate failures in tests/test_engine_planner.py, all
converging on the same class of bug: draft.resolved_agents drifting
out of sync with what actually lands in draft.plan_phases, which
ValidationStage's review_missing/audit_missing/agent_phase_mismatch
checks then misread as either "phantom" roster evidence or an
illegal placement with no fallback ever tried.
- classification.py: an explicit `phases` override with no `agents`
now derives resolved_agents from the union of each phase dict's own
"agents" list instead of the generic per-task-type default, so a
caller-specified phase list doesn't drag in unrelated
code-reviewer/test-engineer candidates that never appear in any
phase (TestCreatePlanPhasesOverride).
- validation.py: team-step consolidation (_consolidate_team) now
rewrites downstream `depends_on` references that pointed at a
step_id folded into the new consolidated step, instead of leaving
them dangling (surfaced a real MachinePlan plan-graph invariant
violation once review_missing false positives stopped masking it).
Added _prune_unused_resolved_agents, run once after team
consolidation, to drop roster candidates that never got a step
after every agent-dropping stage (subtask-union, concern-split,
reviewer-filtering) has had its turn. _has_review_coverage now
accepts any non-implement-type phase (not just a literal "Review"
name), covering the "investigation" archetype's Verify phase and an
explicit --agents roster applied wholesale to compound-task
subtask phases. Fixed a "team" consolidation-sentinel leaking into
agent-base evidence (_step_agent_bases).
- phase_builder.py: Pass 3 of assign_agents_to_phases ("reuse
best-fit from pool") now also blocks reviewer-class agents from
implement-type phases (PHASE_BLOCKED_ROLES["draft"] was empty), so
a caller-supplied roster of just a reviewer agent falls through to
the engineer fallback instead of getting illegally assigned to
Draft with no recovery. Scoped to Pass 3 only -- Pass 2/4 keep their
original tolerance for reviewer overflow into work phases per
TestAgentOverflowToWorkPhases.
- tests/test_engine_planner.py: registered devops-engineer,
devops-specialist, and security-reviewer in the risk-assessment
fixture registry so TestRiskAssessmentStructural exercises the risk
keyword heuristic in isolation, without also tripping the
capability-gap/talent-factory machinery for an agent name unknown
to a deliberately minimal test registry (talent_factory.py's
fallback-substitution behavior for unresolved gaps is itself
required by tests/test_planner.py's
TestPlannerCapabilityGapIntegration and left untouched).
The 16th originally-failing test
(test_planner_smoke.py::test_medium_task_produces_standard_plan) is
pre-existing, live-classifier-driven flakiness explicitly called out
as out of scope for this repair; confirmed unrelated by reproducing
the identical agent_phase_mismatch signature transiently on an
unrelated test in this same run and observing it pass reliably in
isolation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…aude CLI in unit tests) The gate failures (agent_phase_mismatch on "Add user authentication" tasks: security-reviewer landing in an Implement phase) were not a planner bug -- ValidationStage's agent_phase_mismatch check was correctly rejecting an invalid plan. The plan was invalid because IntelligentPlanner() built with no explicit task_classifier defaults to FallbackClassifier, which tries a real Sonnet call via HeadlessClaude whenever the `claude` binary is reachable on PATH (_talent_agent_available() only checks binary presence, never an opt-in flag or ANTHROPIC_API_KEY). This sandbox has `claude` on PATH, so every "unit" test in tests/engine/planning and tests/test_engine_planner.py that called create_plan() with no explicit agents/task_type was silently making a live, network- dependent LLM call instead of exercising the documented "mock mode by default" contract -- the model was free to recommend any registered agent for any phase, including a reviewer-class agent for an Implement-type phase. The prior phase-5 gate-repair commit newly registered security-reviewer/devops-engineer in the shared tmp_agents_dir fixture, which put those agents in the live classifier's candidate pool and turned this latent nondeterminism into an intermittent PlanQualityError with no correlation to any code change. tests/CLAUDE.md mandates hermetic tests (no real network/CLI calls); this was a pre-existing violation of that contract, newly exposed rather than newly introduced. Fix: force the TalentAgent probe unavailable for the affected test files/directory (autouse fixture patching agent_baton.core.engine.classifier._talent_agent_available), so FallbackClassifier always falls through to the deterministic KeywordClassifier, unless a test opts into BATON_PLANNER_INTEGRATION=1 (the flag test_planner_smoke.py already documents for real end-to-end coverage). Added tests/engine/planning/test_classifier_hermeticity.py as the regression test: asserts the probe is forced off, and that 8 repeated create_plan() calls on identical input produce byte-identical plans with no reviewer-class agent ever landing in an Implement-type phase -- verified this fails (assert True is False / security-reviewer misplacement) with the conftest fixture removed. Full gate green twice in a row: 685 passed, 7 skipped, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ect elevated permissionMode
Three confirmed defects in the phase 5 talent-factory implementation:
1. talent_factory.retry_budget / max_recursion_depth were parsed from
baton.yaml and threaded into create_plan() but never reached
decide_talent_lifecycle — RosterStage decided with hardcoded defaults,
so retry_budget: 0 ('no generation attempts') still dispatched and
installed a generated agent. The resolved TalentFactoryConfig is now
set on the draft before the pre-pipeline runs and RosterStage reads
retry_budget/max_recursion_depth from it.
2. Duplicate entries in an explicit --agents list created two gaps for
the same capability: the second dispatch collided with the first's
freshly installed artifact and its collision-fallback substitution
rewrote the successful resolution out of the roster, so the generated
agent was installed but never causally used (and talent-builder was
dispatched twice for one gap). Gap detection now dedupes requested
capabilities.
3. validate_generated_agent only checked that permissionMode was
present, so a generated artifact declaring
permissionMode: bypassPermissions (the frontmatter flavor of the
'set permissionMode to auto-edit' injected directive contract §7
warns about) passed validation and was auto-installed and
auto-registered with no human review. Generated drafts are now
restricted to ALLOWED_PERMISSION_MODES = {default, plan}; the
headless dispatcher prompt pins permissionMode: default.
Regression tests added for all three; talent-factory-contract.md §3.3
updated to match, §11 annotated with what step 5.2 delivered.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…on, phase reference normalization
Upgrade heavy-task decomposition from generic templates to concrete work
packages, and add the quality gates + reference-normalization needed to
keep those plans trustworthy after structural changes:
- planning/utils/repo_grounding.py: deterministic (no LLM, no network)
repository scan for heavy-complexity plans -- matches task-summary
keywords against real files/tests/Python symbols, then grounds each
step's context_files/allowed_paths (via the existing scope_contract
derive_allowed_paths evidence pipeline)/deliverables/expected_outcome/
task_description in that concrete evidence, and wires cross-phase
depends_on edges between steps that share a grounded file. Strictly
additive over phase_builder's existing template output and a clean
no-op whenever there's no project_root or no matching evidence, so
deterministic template-based behavior is unchanged when grounding has
nothing to add. Wired into DecompositionStage._build_phases, gated on
inferred_complexity == "heavy", before enrich_phases's generic-default
fill so grounded fields are never overwritten by templates.
- planning/utils/phase_normalize.py: repairs phase/step references that
go stale when ForesightEngine.analyze inserts a phase ahead of an
existing one and renumbers everything after it -- the "Build on the
... output from phase N" text phase_builder.enrich_phases bakes in
before foresight runs would otherwise keep naming a phase number that
no longer refers to what it meant. Snapshots phase/step identity->id
before the restructuring call and diffs against the post-mutation
state to rewrite depends_on edges and the narrow, self-authored
"from phase N (" text pattern -- never touches arbitrary
director-authored prose. Wired into
DecompositionStage._apply_foresight around the ForesightEngine.analyze
call.
- ValidationStage: three new heavy-task-only defect checks in
_detect_shallow_decomposition. generic_placeholder (critical/blocking)
fires on unambiguous literal placeholder markers (tbd/todo/placeholder/
lorem ipsum). bare_agent_template and empty_deliverables/empty_scope
are warnings (surfaced as diagnostics via score_warnings/
plan_diagnostics, non-blocking) -- STEP_TEMPLATES's per-agent/per-phase
coverage has real, currently-legitimate gaps and grounding is
opportunistic (many valid plans have no project_root to ground
against), so blocking on those would reject real, otherwise-fine
plans; confirmed via a regression run against
tests/test_engine_planner.py's TestOriginalProblemScenario before
settling on this split.
Regression coverage: tests/engine/planning/test_repo_grounding.py,
test_phase_normalize.py (including a real ForesightEngine integration
case), and a new TestShallowDecompositionDetection class in
test_validation_stage.py.
Verified no regressions beyond the pre-documented ones (gate-scope
stack-detection flakiness, two governance policy-violation tests, one
explicit-phases-guard test, and the 4 known live-claude-nondeterminism
golden-snapshot cases) across tests/engine/planning/,
tests/test_planner_quality.py, tests/test_planner_governance.py,
tests/test_planner_gate_scoping.py, tests/planning/, tests/test_engine_planner.py,
tests/test_archetype_decomposition.py, tests/api/test_specs_api.py,
tests/test_api_pmo.py, tests/test_pmo_forge.py, tests/test_foresight.py,
tests/test_plan_reviewer.py, tests/test_planner.py, and
tests/manager/test_manager_mode_planner.py.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Implements deterministic, configurable CHECKPOINT emission to mitigate context rot in long executions: - ExecutionEngine._checkpoint_trigger evaluates three independent, deterministic thresholds (BATON_CHECKPOINT_PHASE_INTERVAL, BATON_CHECKPOINT_TURN_THRESHOLD, BATON_CHECKPOINT_TOKEN_THRESHOLD; BATON_CHECKPOINT_ENABLED gates the feature) at the only safe, already- persisted boundary: immediately after a phase fully advances (PHASE_ADVANCE_OK / EMPTY_PHASE_ADVANCE) and before anything in the new phase is dispatched. - _build_checkpoint_handoff / _emit_checkpoint build and persist a compact CheckpointHandoff (goal, completed outcomes, decisions, scope, changed files, unresolved risks, next actions, exact resume command) onto the new ExecutionState.checkpoints list, and advance dedup markers (last_checkpoint_phase/turn_count/tokens, checkpoint_count) so the same phase boundary can never checkpoint twice -- including across an investigative-archetype retry_phase loop. - next_actions() (plural) withholds its dispatchable batch when a checkpoint is due but not yet emitted, so every existing caller (TaskWorker, the REST API's _collect_next_actions, `execute next --all`) naturally falls back to next_action() (singular), the only place that actually emits + persists the checkpoint. - TaskWorker, the CLI's _print_action/_run_loop, and the PMO pending-gates scan all treat CHECKPOINT as a non-terminal paused-for-refresh signal -- never as COMPLETE or FAILED. - Closes a pre-existing gap where ExecutionState.turn_count had no to_dict()/from_dict() roundtrip support at all (needed for the turn-count threshold to survive a reload across CLI processes). Verified end-to-end via ad-hoc scripts (fresh-process resume with no redispatch, plural-dispatch withholding, dedup across sequential checkpoints, worker non-terminal handling, disabled flag) since tests/ is outside this step's allowed_paths; full existing suites (tests/engine, tests/test_executor.py, tests/models, tests/runtime, tests/cli, tests/api -- 1557 tests) pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…mendment publishing Adds agent_baton.core.manager.rebuild (new): validate_manager_artifacts() cross-checks a proposed plan against a freshly-built ManagerArtifacts set (scope contracts <-> context bundles <-> steps, blueprint role assignments, knowledge-plan step references), and rebuild_and_publish() stages every sidecar's final bytes to same-directory temp files and only renames them into place -- plus a monotonic revision manifest -- after validation and every staged write succeed. A failure leaves every previously published file, and the immutable decision-log/decisions/ scope-evidence trees, untouched. artifacts.py gains render_all() (pure (path, text) rendering, extracted from write_all so rebuild.py can stage without writing); paths.py gains revision_manifest. Wires this into every executor.py path that adds phases/steps to a manager_mode plan: amend_plan() (covers scope-expansion phase generation, feedback dispatch, approval-feedback remediation, and manual/CLI amendments) snapshots state.plan/gate_results/approval_results/ feedback_results, mutates as before, and only commits once the rebuild reports ok=True -- otherwise everything is rolled back and ManagerArtifactPublishError is raised. The goal round-out path in _evaluate_goal_after_gate gets the same snapshot/rollback treatment inline (it can't call amend_plan -- see its own docstring). approving a scope-expansion decision now also runs a best-effort full rebuild after its existing narrow contract-sidecar patch, so the rest of the artifact set stays current too. Non-manager_mode plans take none of these new code paths and are behaviorally unchanged. Along the way, found and fixed two adjacent defects the new coverage surfaced: (1) _process_pending_expansions called amend_plan() (which has no `state` param and persists its OWN reloaded copy) without ever refreshing its own `state` reference, so its trailing _save_execution(state) silently reverted every scope-expansion amendment it had just applied -- not manager-mode-specific, fixed by pulling the freshly-amended fields back onto the same state object after each successful amend_plan() call; (2) record_feedback_result's dispatched_step_id lookup matched on amendment.phases_added (documented as the PRE-renumber placeholder id) against the POST-renumber plan reloaded from disk, so it could never match and dispatched_step_id was always left empty -- fixed by matching on the inserted phase's (renumber- stable) name instead. Also wires dispatch-outcome correlation into the resolver's own knowledge telemetry: knowledge_telemetry.py gains record_dispatch_outcome(), called from record_step_result() on every terminal StepResult so KnowledgeUsed rows get an outcome_correlation instead of staying NULL forever. And context_bundles.py's phantom-knowledge-pack fallback (a required pack name that never landed in the knowledge plan's selected_packs) now surfaces a truncation_warning instead of silently attaching a content-less reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…ckpoint/sidecars Adds the test coverage 6.1-6.3 shipped without (6.2 explicitly deferred it; 6.3 covered rebuild/rollback/telemetry in isolation but not the full dispatch-time integration): - tests/cli/test_execute_run_resume.py: CHECKPOINT threshold (phase_interval, turn_threshold, token_threshold, independently), dedup (same phase boundary never checkpoints twice), the BATON_CHECKPOINT_ENABLED=0 escape hatch, and next_actions() withholding its dispatchable batch when a checkpoint is due -- at the bare-engine level, through `baton execute run` (a checkpoint stops the CLI loop cleanly and a wholly independent second invocation resumes past it without redispatching phase 1 or re-checkpointing), and through TaskWorker (treats CHECKPOINT as non-terminal, and a fresh worker/engine pair resumes cleanly). - tests/engine/test_manager_context_prompt.py: the checkpoint + scope- amendment scenario -- a step's scope is widened and its sidecars republished (new revision) *after* a checkpoint has already fired and *before* the amended step is ever dispatched; a brand-new ExecutionEngine instance (simulating the fresh session the checkpoint's own resume command points at) must dispatch with the amended scope contract, not the sidecars that were current when the checkpoint fired. Also pins that checkpoint dedup survives the amendment. - tests/engine/planning/test_repo_grounding.py: a full-pipeline create_plan() snapshot for a heavy task against a real synthetic repository, proving the assembled plan carries no placeholder markers and at least one step is grounded in concrete repo evidence -- the existing 6.1 coverage in this file only exercised the grounding helpers directly, not the whole seven-stage pipeline the way `baton plan` calls it. - tests/manager/test_context_bundles.py: the two distinct phantom- knowledge-pack diagnostics added in 6.3 (confirmed missing from the registry vs. present but never selected for this plan) plus a control case proving neither fires for a normally-selected pack. - tests/knowledge/test_telemetry_production_wiring.py: drives ExecutionEngine.record_step_result's dispatch-outcome wiring (added in 6.3) through the real production call site rather than only the isolated KnowledgeTelemetryStore-level coverage already in tests/knowledge/test_lifecycle_telemetry.py -- complete correlates 1.0, failed correlates 0.0, a non-terminal status leaves outcome NULL, and no knowledge_resolver at all is a safe no-op. Multi-amend artifact versioning, injected-failure rollback, and cross-sidecar integrity already had solid coverage from 6.3's tests/manager/test_rebuild.py -- not duplicated here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…cope in archetype builders; exempt explicit phase overrides from auto-routing checks
Three independent latent bugs (all predating phase 6, confirmed by running
the same failing tests against the end-of-phase-5 commit) were surfaced by
this phase's gate command:
- RiskStage._ensure_safety_roster only extended classified_phases with a
Review/Audit slot for an injected auditor/code-reviewer when
draft.classified_phases was already a concrete list. When it was None
(the common case -- DecompositionStage falls through to
default_phases(inferred_type, ...) instead), an auto-injected auditor had
nowhere to land, got force-assigned into the Implement phase, and
ValidationStage's agent_phase_mismatch hard gate then rejected the whole
plan. Fixed by materializing the same default phase-name template
(rules/phase_templates.PHASE_NAMES) RiskStage would otherwise fall
through to, whenever DecompositionStage will actually consult
classified_phases for this draft (no explicit phases/subtask_data, phased
archetype), so the Review/Audit-slot correction has a concrete base to
extend.
- ValidationStage's PHASE_BLOCKED_ROLES check (bd-0e36: guards against the
planner's own auto-routing landing an architect on Implement) did not
distinguish auto-routed steps from a caller's explicit
phases=[{"name":..., "agents":[...]}] override, re-rejecting the very
choice RosterStage/EnrichmentStage already agreed to preserve
(TestEnrichmentStageExplicitPhasesGuard). Scoped the check to skip only
the exact (phase name, agent) pairs the caller explicitly wrote, so an
empty-agents dict (which still gets auto-routed via
assign_agents_to_phases) keeps the full check.
- DecompositionStage._build_direct_phases / _build_investigative_phases
hardcoded `PlanGate(command="pytest --tb=short -q")` on the
Implement/Fix phases, which meant EnrichmentStage._apply_gates (the only
place gate_scope/stack-aware defaults are computed) skipped those phases
entirely (`if phase.gate is None`). full/smoke gate_scope requests on
DIRECT/INVESTIGATIVE-archetype plans were silently ignored. Left the gate
unset so the existing gate_scope-aware default_gate() path applies
uniformly.
Verified: tests/engine/planning tests/test_planner_quality.py
tests/test_planner_governance.py tests/test_planner_gate_scoping.py
tests/manager tests/e2e/test_manager_mode_planning.py
tests/e2e/test_manager_mode_execution_dry_run.py
tests/engine/test_manager_context_prompt.py tests/knowledge
tests/test_knowledge_integration.py tests/test_knowledge_resolver.py
tests/cli/test_execute_run_resume.py -- 850 passed, 7 skipped, 0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…nt renumbering amend_plan's _renumber_phases rewrites phase_ids/step_ids after a mid-plan phase insertion but left depends_on edges and the baked 'from phase N (<agents>)' task_description text pointing at the OLD ids — after the insert, those ids belong to different steps/phases (typically the freshly inserted remediation phase), so dispatch prompts named the wrong phase and dependency edges pointed at the wrong step. Contract 6.1 explicitly covers 'no stale references after foresight or amendment changes its structure', and phase_normalize's module docstring prescribes the snapshot/normalize bracket for amendment call sites — wire it around the new_phases mutation. Regression: TestAmendmentReferenceNormalization in tests/test_approval_and_amendments.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…eholder product nouns
The blocking generic_placeholder marker regex fired on 'todo' and
'placeholder' used as legitimate product/feature nouns — a heavy plan for
'build a todo list application' (verified end-to-end) or 'add placeholder
text to the search box' was hard-rejected by ValidationStage's quality
gate. Scope the two ambiguous markers with negative lookaheads for the
compound-noun usages (todo app/list/item/..., placeholder
text/image/value/...); bare or annotated markers ('TODO: scope this',
'details tbd', 'this is a placeholder') still block, and 'tbd' /
'lorem ipsum' are unchanged.
Regression: test_todo_and_placeholder_as_product_nouns_are_not_flagged in
tests/engine/planning/test_validation_stage.py.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…e slots The phase-6 gate-repair commit (5cbb4a4) made RiskStage._ensure_safety_roster materialize the default phase template and append an 'Audit' slot for an injected auditor even when classified_phases was None. EnrichmentStage's concern-splitting treats 'audit' as a splittable phase name (for genuine audit-as-work plans), so on a multi-concern compliance-flavored task the freshly appended safety slot was split into per-concern implementation steps — the auditor step evaporated and ValidationStage hard-blocked the plan on audit_missing. Deterministic regression: tests/test_engine_planner.py ::TestConcernSplitting::test_four_concern_summary_produces_four_parallel_steps passed at every phase-6 commit up to 2457aaa and failed from 5cbb4a4. Record the safety-appended phase names on the draft (PlanDraft.safety_appended_phases) and have the splitter skip exactly those slots; genuine Audit/Assess work phases (never in that list) keep splitting, pinned by the pre-existing tests in test_decomposition_fanout.py. Regression: TestSafetyAppendedPhasesNotSplit (unit + full-pipeline) in tests/engine/planning/test_decomposition_fanout.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Forge now runs the same ManagerModePlanner post-processing CLI planning
uses: POST /pmo/forge/plan can stamp manager_mode=True on the generated
plan, and POST /pmo/forge/approve threads it through
ForgeSession.save_plan -> ManagerArtifactPaths rebuild_and_publish so a
manager-mode plan created via the PMO gets the identical, validated,
version-tracked sidecar set (charter, scope map, team blueprint, role
cards, knowledge plan, scope contracts, context bundles) a CLI
`baton plan --manager-mode --save` produces.
Add a new manager-mode PMO API (agent_baton/api/routes/pmo_manager.py):
read endpoints for charter, scope map, workstreams/phases, team
blueprint, role cards, knowledge plan, scope contracts, context bundle
metadata, reports, decision packets, published artifact version, and a
version-consistency validation check -- all scoped to a single card_id,
reading only through ManagerArtifactPaths' sanitized path builders
(never a raw client-supplied path), with 404 for missing
cards/artifacts vs 409 for a plan that isn't manager_mode. A narrow
mutation endpoint (POST .../decisions/{id}/resolve) approves/denies a
scope-expansion decision through ExecutionEngine.resolve_scope_expansion,
the existing transactional rebuild-and-publish path.
Promote agent_baton.core.manager.rebuild's plan-fingerprint digest to a
public helper (plan_fingerprint) reused by the new validation endpoint,
and add ContextManager.load_plan() so callers can load plan.json back
into a MachinePlan through one conventional path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
… API
Adds ManagerWorkspaceView (pmo-ui/src/views/), a single accessible PMO
surface that lets an operator pick a plan and see its intent (charter),
phase/workstream health, scope boundaries + step scope contracts,
assigned team + role cards, knowledge/context provenance, artifact
version/validation, execution progress, and a decision inbox -- all
sourced from the Phase 7.1 /pmo/manager/{card_id}/... read API plus the
existing card/execution/decision endpoints. Non-manager-mode plans get
an explicit banner instead of a silent gap, and every manager-artifact
fetch is independently fault-tolerant (Promise.allSettled) so one
missing sidecar never blanks the rest of the workspace.
The decision inbox resolves both the generic execution decision queue
(APPROVAL/FEEDBACK/INTERACT, with a rationale field) and durable
scope-expansion decisions (approve/deny with optional additional
allowed paths), and reuses GateApprovalPanel for awaiting_human cards
and the existing ExecutionProgress modal for the full step timeline
and pause/resume/cancel/retry/skip controls. A new pure
utils/executionStatus.ts derives a single paused/resuming/failed/
completed/... display status from column + error + local pause/resume
action, documented against the execution-detail endpoint's actual
(column-mirroring) status field so the UI never conflates a paused
worker with a failed one.
Extends pmo-ui/src/api/types.ts with the full Manager* response/domain
shapes (charter, scope map, workstreams, team blueprint, role cards,
knowledge plan, scope contracts, context bundles, decisions,
version/validation) and the generic execution-decision-inbox and
card-execution-detail types, plus matching client.ts methods -- all
additive, no existing wire shapes changed.
Evidence rows pair a reason/label with its path rather than surfacing
a raw path alone (knowledge packs, context bundle must-read/reference
entries), and scope contract / context bundle detail is lazy-loaded
on expand via the per-step manager endpoints.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
…d browser-journey layers Adds the test-engineer deliverables for the director-console manager-mode surface built in 7.1/7.2: - tests/api/test_pmo_manager_journey.py: full POST /forge/plan -> approve -> GET-every-artifact-category journey through a real ForgeSession; missing/ corrupt/mixed-version (stale) sidecar handling; the generic per-card decision inbox exercised on a manager-mode plan with rationale round-tripping and headless-resume; and cross-card task isolation for both manager artifacts and the decision inbox. - tests/e2e/test_manager_mode_cli_pmo_parity.py: asserts baton plan --manager-mode --save (CLI) and POST /pmo/forge/approve (PMO) produce the same plan.json step shape and byte-identical sidecars for the same deterministic input. Also discovers and pins a confirmed, currently-live gap: ManagerModePlanner.build() (the PMO path) estimates context-bundle token counts before its scope-contract/role-card sidecars are written to disk, so every must-read reference comes back with a 0 token estimate and a spurious "Missing file for token estimate" warning that the CLI path (build_and_write, persist_sidecars_early=True) never hits. Fixing this is out of this step's allowed paths (agent_baton/core/manager/); flagged for follow-up. - pmo-ui/src/views/__tests__/ManagerWorkspaceView.test.tsx: deny (reject) flows for both the generic decision inbox and scope-expansion decisions, the stale-artifacts banner, per-section error resiliency when one manager-mode fetch fails, task isolation when switching plans, and an accessible-name/role/live-region check. - pmo-ui/e2e/tests/manager-workspace-journey.spec.ts: a self-contained Playwright journey (open a manager-mode plan, render every artifact category, approve a scope expansion, deny a decision with rationale, resume execution, refresh status, task isolation, axe WCAG scan). Could not be executed in this sandbox (playwright browser download is blocked by the network policy -- verified pre-existing specs hit the same failure) but type-checks clean and was reviewed against the live component/vitest coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Contract 7.1 requires a manager-mode plan created in PMO to carry the same validated sidecars as CLI planning, but ManagerModePlanner.build() (the rebuild_and_publish path used by POST /pmo/forge/approve and every runtime amendment) estimated must-read tokens by re-reading scope- contract/role-card files that had not been written yet: every PMO-created bundle carried token_estimate=0 plus a spurious 'Missing file for token estimate' truncation warning, which the new Manager Workspace then rendered as a red 'truncated' flag on every step. The parity suite had pinned this as a known gap instead of fixing it. ContextBundleBuilder.build now accepts the rendered contract_text / role_card_text and estimates from that in-memory content (byte-identical to the published file's size), and ManagerModePlanner passes both. CLI, PMO/Forge, dry-run previews, and amendment rebuilds now produce identical token accounting; amendment rebuilds also stop inheriting a previous revision's stale file sizes. Parity suite strengthened per its own instruction: manager brief and every context bundle are now compared byte-for-byte, and the pinned-gap test is replaced by a regression test asserting nonzero, matching estimates on both paths. Unit regression tests added for the in-memory estimator and for the unchanged file-based fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
Root CLAUDE.md and api/CLAUDE.md mandate docs/api-reference.md updates when REST routes change; phase 7 added 18 routes with no doc entry. Adds section 4.15 (route table, shared envelope/error semantics, resolve request/response contract) and updates the forge plan/approve entries for the new manager_mode request field and manager_mode/manager_revision response fields and 422 condition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD
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
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.
ClaudeCodeLauncher._git_rev_parse/_git_diff_files were hardcoded to
config.working_directory, ignoring cwd_override. For a Wave 1.3
worktree-isolated dispatch this meant pre/post HEAD capture always
inspected the parent repo, so a real commit made inside the worktree
was invisible: commit_hash/files_changed came back empty even though
the agent committed, and the executor's "no commit -> clean up" path
then deleted the worktree, silently discarding the work.
Thread the launch's effective cwd (cwd_override when set, else
config.working_directory) through pre-launch HEAD capture, post-launch
HEAD capture, and diff/file discovery so all git inspection for a
launch targets the same directory as the subprocess itself.
Add a defense-in-depth fail-closed guard in WorktreeManager, independent
of what the launcher reports:
commit_hash actually exists in the worktree and is not an ancestor of
(or equal to) base_sha before folding/deleting anything.
dirty status before the "no commit reported" cleanup path is allowed
to permanently delete it.
executor.py wires both into record_step_result's worktree lifecycle
handling: on failure the worktree is left intact and the step is marked
failed with a precise WorktreeProvenanceError instead of being folded
or discarded.
Regression tests added under tests/test_claude_launcher.py (effective-cwd
git probing) and tests/test_worktree_manager.py (provenance guard).
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01N4NSwmbL6nNUrdkGCmu8AD