feat(carve S3): agent runtime — deterministic issue feedback, build/lint gate, plan and restack workflows

[isadeks]

Stacked on #653 → #647. Review those first; this PR's diff is only its own slice.

What this changes

The Python agent runtime. Three related shifts:

1. Issue feedback becomes deterministic instead of tool-mediated. The agent previously reached the issue tracker through an MCP server, which made a status update a model decision — it could be skipped, duplicated, or worded differently each run. Feedback now happens in the pipeline, so a reaction or comment is a consequence of the run reaching a state, not of the model choosing to report it. Fewer moving parts at runtime and no tracker credentials in the model's tool surface.

2. A build/lint gate before delivery. The runtime can run a repo's configured build and lint commands and treat the outcome as part of whether the work is deliverable, rather than opening a pull request and letting CI discover the breakage.

3. Two new workflows — decompose-v1 (produce a plan for a piece of work) and restack-v1 (rebase a stack of dependent branches after an earlier one changes).

Why this is one slice

The 49 files are one atomic unit. The no-MCP, build-gate, plan and restack changes interleave within pipeline.py and repo.py — attempting to take a subset produced 39 unresolved-symbol errors, because each piece needs models, hooks and shell helpers introduced by the others. Splitting it would mean shipping something that doesn't compile.

Behavioral changes to be aware of

• The default model moves to a newer generation, along with some runtime defaults. Intrinsic to this runtime rather than separable — calling it out explicitly rather than burying it.
• Comments and test names throughout are rewritten to explain their reasoning directly instead of referencing internal tracker ids.

Verification

ruff + ty + pytest (1421 tests) all green.

---

Tracking

Slice S3 of the linear-vercel → main carve. Tracking issue: #668 (slice table, why it is sliced rather than merged, and review guidance).

Lands part of #247 (parent/sub-issue orchestration) and #299 (auto-decomposition). Deliberately not Closes — no single slice closes either; they close when S8 activates the arc.

Stacked on #653 — review that first; this PR's diff is against its branch.

[isadeks]

Automated review — carve S3 (#654)

Reviewed as its own diff only (carve/s2-ecs-rightsized-planning..carve/s3-linear-nomcp-attachments, 43 files, +5314/−900), on the understanding that this carves an already-live arc from a private branch onto main, so the questions are is the slicing correct and is this fit to be public — not is the feature a good idea. Governance (branch naming / no linked issue) is noted once for the whole stack and not repeated here.

Every finding below was reproduced independently before reporting. Findings I could not reproduce were dropped rather than reported as "possible".

Slice standalone-ness

Typecheck: PASS. tsc --noEmit is clean at this slice's tip on its own base (verified by installing this slice's own yarn.lock and running the compiler — not inferred). Symbols that look undefined here are defined on the base branch. yarn.lock is byte-identical to main at this slice, so no dependency is dropped or downgraded.

Reverts: CLEAN on the named risk. I checked the #643 Jira app-actor work line-by-line, since this slice contains 3-way merges and the fork was behind main on it. Every line #643 added to config.py, jira_reactions.py, pipeline.py, channel_mcp.py, policy.py, prompt_builder.py, server.py, jira-integration.ts, jira-feedback.ts, jira.ts and contracts/constants.json is still present at this slice tip, and all 9 files #643 created still exist. One main-side definition is gone — channel_mcp._linear_server_entry — and that removal is correct: it is the point of the slice, and the jira builder entry is properly preserved beside it.

---

1. BLOCKING — the default agent model changes here, but the IAM grant for it lands 5 slices later

agent/src/config.py:553 (and the TaskConfig default in models.py) flip the fallback model from us.anthropic.claude-sonnet-4-6 to us.anthropic.claude-opus-4-8. The Bedrock grant is derived from DEFAULT_BEDROCK_MODEL_IDS in cdk/src/constructs/bedrock-models.ts, and at this slice that list is still [sonnet-4-6, opus-4-20250514-v1:0, haiku-4-5-…] — no opus-4-8. opus-4-8 is only added to the grant list in #662.

Verified per-slice:

	fallback model in config.py	opus-4-8 in grant list
main	sonnet-4-6	no
s3 (#654)	opus-4-8	no
s4–s7 (#655–#659)	opus-4-8	no
s8 (#662)	opus-4-8	yes

The fallback is reachable: it fires when the per-repo model_id is absent, and model_id is optional at both layers (repo-config.ts — readonly model_id?: string on both RepoConfig and BlueprintConfig). ANTHROPIC_MODEL is set in exactly one place in the CDK tree at this slice (ecs-strategy.ts:211) and only when blueprintConfig.model_id is present.

Failure: merge any of #654–#659 alone, deploy, onboard a repo without model_id, submit a task → InvokeModel on an inference-profile ARN outside the role's policy → AccessDeniedException at turn 0, on every task on the stack.

This is the clearest slicing defect in the stack: a behaviour change and the permission it requires are on opposite sides of a five-slice boundary. Either move the bedrock-models.ts grant addition into this slice, or defer the model-default flip to #662.

2. MAJOR — 9 sites cite ADR-016 as the authority for "no Linear MCP"; ADR-016 does not say that until #662

This slice adds code comments across 6 files (channel_mcp.py ×4, config.py, linear_reactions.py, pipeline.py, prompt_builder.py, cdk/src/handlers/shared/types.ts) citing ADR-016 as the governing decision for Linear determinism / no-Linear-MCP — including one that calls it enforcement, and one that prints the citation in a runtime WARN the operator sees after the platform deletes an MCP entry from their committed .mcp.json.

docs/decisions/ADR-016-pluggable-identity-and-auth.md contains 0 occurrences of "mcp" at this slice, at s4, and at s7. It gains that content (38 mentions) only at s8/#662.

So #654–#659 ship code justified by a decision the repo does not document, and a user-visible log line pointing at a doc that does not support it. Either move the ADR-016 revision into this slice, or drop the citations until the ADR lands.

3. MAJOR — self-reclone guard is bypassed by git clone -b

_is_self_reclone (agent/src/hooks.py:297) truncates the command at the first _FREE_TEXT_ARG_RE match before scanning for the clone verb, to avoid a false positive on a PR body quoting a clone command. But that regex matches those flags anywhere, including before the clone verb — and -b is git clone's own --branch short form.

I executed the guard's own regexes against real commands (repo acme/app):

command	result
git clone https://github.com/acme/app /workspace/app	BLOCKED ✅
git clone --branch main https://github.com/acme/app …	BLOCKED ✅
git clone -b main https://github.com/acme/app …	ALLOWED ❌
git commit -m wip && gh repo clone acme/app …	ALLOWED ❌
gh pr create --body "do not run gh repo clone acme/app"	allowed ✅ (correct)

git clone -b <branch> <own repo> is an ordinary single command, not a contrived chain. It reopens exactly the lost-work failure this guard was built to prevent: the agent works in the nested clone, the outer tree stays clean, and the delivery gate sees nothing. Fix is to require the free-text flag to appear after the clone verb, or to scan the whole string for the verb and only use the truncation to suppress a match found inside a quoted value.

4. MAJOR — private tracker ids and work-item shorthand used as the explanation for code

Confirmed by sweeping the added lines. These are not fixtures — they are the load-bearing explanation in comments and docstrings, unresolvable to anyone outside the author's team:

• ABCA-### as the reason for a change — 11 sites in agent/src/pipeline.py (lines 559, 639, 671, 718, 1053, 1058, 1112, 1379, 1404, 1518) and agent/src/post_hooks.py:381. Highest-impact: _apply_delivery_gate, where every load-bearing sentence is anchored to "ABCA-815".
• PM-3 as a section marker in linear_reactions.py and pipeline.py.
• Dated incident framing: live-caught on backgroundagent-dev (pipeline.py:1034) — names a private stack.

Test/fixture data carrying tracker ids is fine and I have not reported any. The fix is to state the invariant plainly ("a task that reported success but shipped nothing must fail, or the platform reports success for lost work") and drop the id.

5. MINOR (downgraded) — _NO_CLARIFICATION_WORKFLOW_IDS contains a workflow id that never matches

agent/src/runner.py:355 lists "web/research-v1"; the tree's only research workflow is knowledge/web-research-v1, and git grep 'web/research-v1' returns exactly one hit — this line. The opt-out therefore never fires for research tasks.

I am reporting this minor, not major: it is a stale entry in a deny list, so the consequence is that the clarification tool is offered where it was meant to be withheld — a wasted tool offering, not a broken path. The reviewing agent's claim that it breaks the research flow outright is not something I could confirm, so I am not asserting it.

---

Reviewed with Claude Code. Verification: tsc --noEmit run on this slice's own base; #643 survival checked line-by-line per slice; the clone-guard regexes executed against real command strings; ADR-016 and grant-list contents diffed per slice.

[isadeks]

Note: force-push during review — one finding is now FIXED

My review above was written against 4f947eef; the current head is 77677a00.

Finding 1 (BLOCKING — the opus-4-8 model/IAM grant gap) is RESOLVED. anthropic.claude-opus-4-8 is now present in DEFAULT_BEDROCK_MODEL_IDS in cdk/src/constructs/bedrock-models.ts at this slice, so the default model and the Bedrock grant that permits it now land together. The five-slice window where any deploy would 403 at turn 0 is closed. Thanks — that was the most consequential item in the stack.

The remaining findings all still hold at 77677a00, re-verified individually:

finding	status
1. BLOCKING — opus-4-8 default without its IAM grant	✅ FIXED
2. MAJOR — 9 sites cite ADR-016 for "no Linear MCP"; ADR-016 has 0 mentions of MCP until #662	still holds
3. MAJOR — self-reclone guard bypassed by git clone -b <branch> <own repo>	still holds
4. MAJOR — ABCA-### as the explanation for code (15 occurrences in pipeline.py alone)	still holds
5. MINOR — web/research-v1 is a stale id that never matches	still holds

Of these, #3 is the one I would prioritise: git clone -b main <own repo> is an ordinary single command and it silently reopens the lost-work failure the guard exists to prevent. Verified by executing the guard's own regexes — --branch (long form) is correctly blocked, -b is not.

[isadeks]

Review findings addressed

BLOCKING #1 — model default flips here, IAM grant lands 5 slices later. Confirmed and fixed.

Worth recording how I verified it, because my first check said the opposite: grepping bedrock-models.ts for opus-4-8 matches on main, because the id appears in a docstring example of the context override. Extracting only the actual DEFAULT_BEDROCK_MODEL_IDS array entries reproduces your table exactly — the grant list gains it only at the activation slice, while config.py flips the fallback here. Your finding stands; my initial refutation was a bad grep.

Fixed by moving the grant entry into this slice, alongside the default that needs it. Also added a drift guard that reads the agent's own fallback out of config.py and asserts the grant list covers it. Mutation-tested both directions: removing the grant fails the test; changing the agent default to a model that is granted correctly passes, so the guard catches the dangerous direction without being noise.

MAJOR #3 — the clone guard is bypassed by git clone -b. Confirmed, and it was worse than reported. Executing the guard's own regexes found your case plus two more:

command	before	after
git clone -b main <own repo>	ALLOWED	BLOCKED
gh repo clone -b main <own repo>	ALLOWED	BLOCKED
git commit -m wip && gh repo clone <own repo>	ALLOWED	BLOCKED
git clone -b x git@github.com:<own repo>.git	ALLOWED	BLOCKED
gh pr create --body "…gh repo clone <own repo>"	allowed	allowed ✅

Your diagnosis of the mechanism was exactly right. The fix takes it a step further: the command is split on shell separators first, so a free-text argument only swallows the rest of its own segment — that is what catches the -m wip && chain, where the -m belongs to the commit rather than the clone. Within a segment the verb is located first, and only counts as prose if a free-text flag opens before it. 14 cases now assert the full matrix, and the three new tests fail against the original guard.

MAJOR #2 — 9 sites cite ADR-016 for a decision the repo does not document until the last slice. Confirmed (0 occurrences of "mcp" in that ADR here; 38 at the activation slice). Not moved, and here is why: the ADR revision is bundled with several other doc changes in the activation slice's bucket, and pulling one section forward would either split that file across two PRs or drag unrelated doc changes into this one. The citations are accurate about the final state of the stack, and the stack merges in order. If you would rather the ADR revision move here, that is a clean change and I will make it — flag it and I will.

MAJOR #4 — private tracker ids as the load-bearing explanation. Fixed. ~60 sites across pipeline.py, post_hooks.py, linear_reactions.py, config.py, prompt_builder.py, server.py and 19 test files. Two things beyond what you listed: the delivery-gate log message itself carried an id (reworded after confirming no test asserts it), and five test identifiers did too (test_forwards_channel_fields_ABCA_487, TestDiamondBaseBranchF1, test_a4_*) — those would have shown up in CI output on a red test. All renamed to describe behaviour.

MINOR #5 — _NO_CLARIFICATION_WORKFLOW_IDS contains an id that never matches. Confirmed: the tree's research workflow is knowledge/web-research-v1 and web/research-v1 appears only on that line. Not fixed in this PR — it is a pre-existing stale entry in a deny list rather than something the carve introduces, and your severity call is right that the consequence is a wasted tool offering. Tracking separately. Thank you for downgrading it rather than repeating the stronger claim.

Gates: 2544 CDK tests, 1440 agent tests, ruff/ty/eslint/tsc clean.

[isadeks]

Second-round review: my clone-guard fix broke two things it was meant to protect

A second adversarial pass over my own fix found that it closed the -b bypass while opening a new one and introducing a false positive. Both verified by execution against the pre-fix guard, both now fixed.

New bypass — a wrapped clone walked straight through. Splitting on a bare newline separated the verb from the repo, so the slug was never in the verb's segment:

command	before my fix	after my fix	now
git clone \ ⏎ <own repo>	BLOCKED	ALLOWED	BLOCKED
git clone -b main \ ⏎ <own repo>	allowed	ALLOWED	BLOCKED
git clone --depth 1 \ ⏎ <own repo>	BLOCKED	ALLOWED	BLOCKED

So the -b bypass I set out to close survived via the continuation route, and I newly broke two cases that already worked. In a security guard that is worse than the bug I fixed. Backslash-continuations are now joined before scanning — which also catches a continuation splitting the verb itself (git \ ⏎ clone), the case that genuinely needs the joining since no segment handling reunites those.

New false positive — a multi-line PR body was denied. A --body/-m value spanning lines is how an agent normally writes a PR body, and that text routinely documents a clone command. Treating \n as a command separator put the quoted line in its own segment with no preceding free-text flag, so a legitimate gh pr create was refused with "The repository is ALREADY cloned…" — exactly the false positive the free-text list exists to prevent, and the reason the single-line test kept passing. A bare newline is no longer a separator; the real separators still are, so a clone chained after an unrelated -m is still caught.

Both are mutation-verified: restoring \n as a separator fails the multi-line-body test, dropping the continuation join fails the wrapped-clone test.

Also removed _CLONE_VERB_RE. Across every segment shape it was the sole matcher zero times, and it actually misses a subshell (( git clone …) that the anchored pattern catches. A dead alternative in a security path is worse than none.

Two lower-severity findings, verified and not changed:

• The model-grant drift guard uses a non-global match(), so it validates the first ANTHROPIC_MODEL occurrence. Today the only other one is ("ANTHROPIC_MODEL", "") — inert, since the pattern needs at least one char. Worth tightening to matchAll if that ever gains a non-empty value; not urgent, and the guard fails loudly (not silently) under every reformatting I was shown.
• agent/src/models.py hardcodes the same model default and sits outside the guard's reach. Tracked.

Verified as still-correct across a 50-case attack table: the -b fix itself, gh repo clone -b, scp URLs, subshells, tabs, ;;, |&, git -C dir clone, uppercase hosts, ssh://, git://, and prose in a --body. Pre-existing gaps unchanged by any of this (sudo/env prefixes, git "clone" quoting, $(…), eval) behave identically before and after, so they are not regressions from this work.

Gates: 1442 agent tests, 2545 CDK, ruff/ty/eslint/tsc clean.

[scottschreckengaust]

Verdict: Approve with nits

Slice S3 of the linear-vercel carve. Reviewed only the incremental diff vs the parent branch carve/s2-ecs-rightsized-planning (56 files: agent runtime + tests, two workflow YAMLs, and the CDK Bedrock-grant / workflow-descriptor entries that must travel with the runtime). The change is large but coherent, exceptionally well-documented at the point of every non-obvious decision, and backed by a green suite (1421 tests, +28 new verify-command tests; all 4 CI checks pass). No blocking issues found.

Vision alignment

Advances the fire-and-forget tenet: moving issue feedback from a model-mediated MCP tool into deterministic pipeline steps makes a reaction/comment a consequence of reaching a state rather than a model choice — fewer moving parts, and tracker credentials leave the model's tool surface (blast-radius reduction). The build/lint gate keeps outcomes reviewable by catching breakage before a PR is opened. decompose-v1 (read-only, artifact terminal) and restack-v1 (writeable, PR terminal) are correctly scoped and platform-issued, not user-facing — consistent with policy-gated escalation. Backed by approved P0 issues #247 (parent/sub-issue orchestration) and #299 (auto-decomposition); rolled up under epic #668. Governance gate satisfied.

Blocking issues

None.

Non-blocking suggestions / nits

1. is_infra_failure keys OOM detection on exit 137, but the new streaming path (_run_cmd_streaming) execs via subprocess.Popen and returns the raw signal code — a SIGKILL is -9, not 137 (agent/src/shell.py:302, agent/src/post_hooks.py:107-136). For the default mise run build (direct-exec, not bash -lc-wrapped), if the exec'd process is itself OOM-killed with no ENOSPC/OOM string on stderr, is_infra_failure(-9, ...) returns False and the gate mislabels an infra kill as a genuine build failure — exactly the failure mode the function documents preventing. In practice the OOM killer usually reaps a child (node/jest) and mise propagates a shell-convention 137, and the pre-existing buffered subprocess.run path shares this negative-code behavior, so this is not a regression introduced here — but consider normalizing abs(returncode) / adding -9/-15 to the infra signatures so the direct-exec streamed path is covered symmetrically. Worth a follow-up issue rather than a change in this slice.
2. _run_cmd_streaming(cmd, label, cwd, timeout) accepts label but never uses it (agent/src/shell.py:241-242) — the CMD: prefix line is logged by run_cmd before delegation. Drop the parameter or thread it into the drain-thread log prefix; a dead parameter in a security/observability-critical helper invites confusion.
3. Default agent model bumped to us.anthropic.claude-opus-4-8 (agent/src/models.py, agent/src/config.py:553). Correctly co-landed with the Bedrock grant entry (cdk/src/constructs/bedrock-models.ts) and a drift-guard test that reads the agent's own fallback and asserts the grant covers it — this closes the AccessDenied-at-turn-0 gap the commit describes. Just flagging the cost/behavioral change for downstream awareness; it is disclosed transparently in the PR body rather than buried.

Documentation

Comment/docstring cleanup removes private tracker IDs (ABCA-###, #NNN) and preserves the reasoning in plain language — a net improvement for a public sample repo. No docs/guides/ or docs/design/ source edits in this slice, so the Starlight mirror-sync gate is N/A. Workflow behavior is documented inline in the two new agent/workflows/coding/*.yaml descriptors. Issue tracking is reflected via #247/#299/#668.

Tests & CI

Strong coverage: test_verify_commands.py (28 tests) exercises timeout/inert/infra-137/genuine-failure classification including the dangerous 137-with-plain-output fall-through; test_shell.py covers the streaming tee, stdout/stderr separation, redaction, and check=True raise; test_hooks.py mutation-verifies the clone-guard fixes (restoring \n as a separator fails the multi-line-body test; dropping the continuation-join fails the wrapped-clone test). New CDK descriptors (restack-v1, decompose-v1) have matching YAML files, prompt sections, and admission-table + prompt tests. Bootstrap synth-coverage: N/A — the CDK diff only adds a model-ID string to an existing InvokeModel grant list and two entries to the in-memory DESCRIPTORS table; it introduces no new CloudFormation resource type, so the ADR-002 bootstrap-policy checklist does not apply.

Review agents run

• code-reviewer (principal-architect hand-review over the incremental diff) — ran; findings above.
• silent-failure-hunter — ran (in scope: linear_reactions.py, shell.py, post_hooks.py error handling). Best-effort WARN-and-continue in linear_reactions.py is appropriate for non-critical feedback; the verify path deliberately distinguishes timed_out/inert/infra_failed from genuine failure rather than swallowing — no silent-default traps found. The one residual concern (nit #1) is a mislabel, not a swallow.
• type-design-analyzer — ran (in scope: new VerifyOutcome dataclass, TaskConfig new fields). VerifyOutcome with explicit passed/timed_out/inert/infra_failed flags is a clean, well-justified widening of the old bare-bool; new build_command/lint_command default to "" with sensible fallback. No concerns.
• comment-analyzer — ran. Comments are accurate and reasoning-rich; the tracker-ID scrub preserves substance. No stale/misleading comments found.
• pr-test-analyzer — ran; coverage assessment above. No happy-path-only gaps identified for the new logic.
• /security-review — ran (in scope: the _is_self_reclone clone-guard rewrite and the Bedrock IAM grant). The guard now joins backslash-continuations before segmenting, checks each shell segment independently, and locates the clone verb before the free-text scan — closing the -b/--branch bypass, the wrapped-clone bypass, and the multi-line---body false positive, all mutation-pinned. Fail-closed on non-dict tool input is preserved. Grant remains scoped to the enumerated model IDs. No new secrets, network egress, or input-gateway surface. (semgrep MCP scan attempted but the plugin required an explicit --config; substituted the manual principal security pass over the two in-scope files.)

Human heuristics

• Proportionality — Pass. The atomic 56-file slice is justified (splitting produced 39 unresolved-symbol / non-compiling intermediate states; the no-MCP/build-gate/plan/restack hunks interleave within pipeline.py/repo.py). Abstractions match the problem.
• Coherence — Pass. restack-v1/decompose-v1 follow the existing descriptor + YAML + prompt-section pattern with real substance; VerifyOutcome reuses the established gating vocabulary; the same read_only/requires_repo invariants are threaded consistently CDK↔agent.
• Clarity — Concern (nit #2): the unused label param in _run_cmd_streaming obscures intent in an observability helper. Otherwise names communicate intent and error handling surfaces (not hides) failures.
• Appropriateness — Pass. Integration behavior is verified against real subprocess/shell semantics (streaming, signal exit, shlex vs bash -lc), not only self-written mocks; the clone-guard tests assert what the guard should do and are mutation-verified. Maintainable by this team.

[scottschreckengaust]

Nit (non-blocking): the streamed path returns proc.returncode from subprocess.Popen, which is the raw signal code for a killed process (SIGKILL → -9), not the shell's 128+signal convention. is_infra_failure (post_hooks.py:107) keys OOM detection on returncode == 137, so a direct-exec mise run build OOM-killed with no ENOSPC/OOM stderr string would be classed as a genuine build failure rather than an infra fault — the exact mislabel that function exists to prevent. Not a regression (buffered subprocess.run shares this), but consider normalizing to abs(returncode) or adding -9/-15 to the infra signatures. Also proc.returncode or 0 maps a None (shouldn't occur post-wait) to 0=success — a fail-open in the unlikely edge; prefer an explicit -1 default.