feat(carve S8): wire the orchestration arc into the stack and activate it

[isadeks]

The final slice of the staged carve. Stacked on #659 (carve/s7-orchestration-plane) — review that first; this PR's diff is only its own commit.

What this does

Instantiates everything the previous slices added, and turns it on. Before this, S5–S7 shipped modules and constructs that nothing referenced; after this, a labelled Linear issue actually runs as an orchestrated sub-issue graph.

Stack wiring — cdk/src/stacks/agent.ts

• Instantiate OrchestrationTable, OrchestrationReconciler, and the scheduled StrandedOrchestrationReconciler; grant each what it needs.
• Pass ORCHESTRATION_TABLE_NAME to the Linear webhook processor. That variable is the activation switch — absent, the processor skips the orchestration path entirely, which is exactly what kept S5–S7 dormant. This is why stack wiring and webhook activation could not be split into two PRs: either one alone ships a half-wired system.

Webhook processing — cdk/src/handlers/linear-webhook-processor.ts

• Route a labelled parent issue to graph discovery and a plain issue to a single task.
• Route an @bgagent comment to the right destination: an iteration on a sub-issue, new work on a PR-less task, an epic retry, or a nudge when the intent is ambiguous or the issue has no linked task.

The plan-proposal routes (verdict / revision / plan commands) were part of this slice originally; they went with auto-decomposition, which stays on the development branch as experimental. The webhook drops from 4502 to 3156 lines.

Also here

• github-webhook-processor — fold a PR preview screenshot into the iteration's maturing reply instead of posting a standalone comment.
• fanout-task-events — route terminal-state feedback through the orchestration panel when the task belongs to an epic.
• linear-task-by-issue — resolve a Linear issue to its newest task and PR, for a comment on an issue that has no orchestration row.
• Slack — channel-to-repo mapping so a channel can have a default repo (CLI command + table).
• jest worker limits in cdk/package.json, so the full suite doesn't exhaust memory on a developer machine.
• Docs — ADR-001 (stacked PRs), ADR-018 (agent-session interaction), the compute and rightsized-planning design notes, the surface-agnostic orchestration design, the Linear setup guide, and the reconciler-correctness worksheet.
• Operator scripts for inspecting and tearing down an orchestration run.

Three files were merged, not replaced

jira-integration.ts, ADR-016, and USER_GUIDE.md carry a change from both sides of the fork: this repo added the Jira app-identity work while the source branch added its own change to the same file. Taking either version wholesale would have silently reverted the other. They were 3-way merged against the common ancestor, so both survive — notably jira-integration.ts keeps the app-actor wiring and gains the pdf-parse bundling carve-out its PDF-screening handler needs (cdk/test/contracts/pdf-parse-bundling.test.ts enforces that every construct bundling such a handler has it).

Verification

• tsc --noEmit clean on cdk and cli
• CDK: 3637 tests / 179 suites passing (up from 3491/173 on the base)
• CLI: 680 tests / 54 suites passing
• Agent: 1437 tests passing; ruff check, ruff format --check, ty check all clean
• eslint clean on cdk and cli

Not yet deployed or live-verified. This is synth-and-test-green only. Since this slice is the one that actually activates the path, a dev deploy and a live run are worth doing before it merges.

🤖 Generated with Claude Code

---

Tracking

Slice S8 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). Deliberately not Closes — no single slice closes it; it closes when S8 activates the arc. The auto-decomposition issue (#299) is no longer part of this carve — that feature stays on the development branch as experimental.

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

[isadeks]

Automated review — carve S8 / activation (#662)

Reviewed as its own diff only (carve/s7-orchestration-plane..carve/s8-activation, 70 files, +13439/−565). This is the activation slice — the only PR in the stack that changes deployed behaviour — so it got the highest bar. Governance (branch naming / no linked issue) is noted once for the whole stack, not repeated here.

Slice standalone-ness: PASS

tsc --noEmit clean at this tip on its own base (compiler actually run). Every symbol newly referenced here is defined at s7 — the four constructs, TaskOrchestratorProps.maxConcurrentTasksPerUser, MAX_TOTAL_ATTACHMENT_SIZE_BYTES, renderPreviewBlock, appendOnceToComment, extractLinearIdentifierFromBranch, cleanupAllAttachments.

Activation is unconditional — worth a deliberate decision

new OrchestrationTable(this, 'OrchestrationTable') is instantiated with no context flag, and orchestrationTable is passed to LinearIntegration unconditionally, so ORCHESTRATION_TABLE_NAME is always set. Every existing deployment flips from dormant to active on the first cdk deploy after merge, with no opt-out. The repo has an established precedent for exactly this situation — compute_type and blueprintRepo are both tryGetContext-gated. Not a defect, but for a change of this size a context gate would let operators stage adoption.

Reverts: one real one, found below. #643 is clean.

I checked the #643 Jira app-actor work line-by-line since the fork was behind main on it: all 173 added lines in jira_reactions.py, all 9 new files, and every added line across config.py/pipeline.py/channel_mcp.py/policy.py/prompt_builder.py/server.py/jira-integration.ts/jira-feedback.ts/contracts/constants.json survive at this tip. ADR-016 grew 96→176 lines; the USER_GUIDE.md Linear line was properly replaced, not dropped. yarn.lock is unchanged from s4 onward and never regresses against main. The 3-way merges did their job — on the files that were being watched.

---

1. BLOCKING — Jira fan-out final-status comments are silently disabled (a genuine revert of main's #603)

This PR deletes jiraWorkspaceRegistryTable and jiraOauthSecretArnPattern from the new FanOutConsumer(...) call in cdk/src/stacks/agent.ts. Those props were added to main by 65e96a8e ("post final status comments with cost, turns, and duration (#573) (#603)").

Verified per-slice — s8 is the only ref where they are absent:

ref	Jira props present in agent.ts
main	2
s1 (#647)	2
s4 (#655)	2
s7 (#659)	2
s8 (#662)	0

The FanOutConsumer construct still declares both (fanout-consumer.ts:116, :128) and still gates real behaviour on them: props.jiraWorkspaceRegistryTable.grantReadData(this.fn) + the table-name env var (:261-265), and the secretsmanager resource grant (:268-274). Because both props are optional, dropping them produces no synth error and no test failure — the capability just goes away.

Failure: an operator with Jira onboarded merges this stack. A Jira-labelled issue runs a task that dies before the agent posts its own terminal comment (e.g. error_max_turns). The task reaches terminal state, the stream fires, routeEvent routes to the jira channel — and the fan-out Lambda has neither the registry table name nor permission to read the OAuth secret. The user's issue never gets its final status comment.

This is the highest-priority fix in the stack: it is a silent capability regression on a path main currently supports, and it is invisible to CI. Restore both props.

Note on the class: this is the second instance of main-side work being dropped by a fork-derived file — and it is not the #643 work that was being watched. Worth a mechanical sweep of agent.ts against main for any other construct call that lost props, since optional props make this failure mode silent by construction.

2. MAJOR — the log-delivery pin shim fires on the DEFAULT stack name

maybePinChurnedLogResources (agent.ts:1463) overrides CFN logical IDs and account-unique Name properties on six AWS::Logs::Delivery* resources whenever stack.stackName matches a key in PINNED_LOG_DELIVERY_BY_STACK. The only key is 'backgroundagent-dev' — which is the repo's default stack name.

The code comment asserts the shim is "OFF by default" and that "A fresh stack (a new env, CI, this PR on a clean account) has NO pre-existing resources to collide with and MUST synth the current alpha's natural ids." That premise is false: a new operator who runs mise //cdk:deploy with no context override gets a stack named backgroundagent-dev, matches the table, and synthesizes six log-delivery resources carrying one specific operator's hardcoded logical IDs and account-unique names. Key the shim off an explicit opt-in context value rather than a stack name that everyone gets by default.

3. MAJOR — activation turns on a GSI-backed lookup that no pre-existing task row can satisfy, and the miss is silent

This PR is the first to set ORCHESTRATION_TABLE_NAME, which turns on the Comment webhook path. The standalone comment trigger resolves issue → newest task via resolveTaskByLinearIssue, a Query on the sparse LinearIssueIndex GSI keyed on the top-level linear_issue_id attribute — and that attribute is written for the first time by the hoist added in this same PR (create-task-core.ts:720-725).

So no task created by the currently-deployed code carries it, there is no backfill, and resolveTaskByLinearIssue returns null on error or miss, which the caller treats as "not an ABCA-owned issue, ignore".

Failure: a user labelled ENG-412 yesterday and got PR #88. The operator deploys this PR. The user comments @bgagent also rename the button on ENG-412. The comment webhook now flows through (it was ignored before), the GSI lookup misses because that task predates the hoist, and the comment is silently dropped — no reply, no reaction, nothing. This will affect every in-flight issue at the moment of adoption. At minimum the miss should tell the user; better, fall back to extractLinearIdentifier over recent tasks.

4. MAJOR — scripts/linear_epic.py hardcodes the author's private workspace and a /tmp PAT path

The new script's own docstring claims "All workspace ids are ABCA-demo defaults but overridable by flag" — there are no such flags. create-epic takes only --spec; TEAM_ID, PROJECT_ID and TRIGGER_LABEL are module constants with no CLI override and no env fallback. The values are the author's private workspace: team UUID 8ab50246-…, project UUID f369205b-…, with an inline comment naming abca-demo -> isadeks/vercel-abca-linear (the private fork). It also reads a PAT from a /tmp path.

A public user who follows the docs to this script gets a GraphQL error against UUIDs that do not exist in their workspace, with no way to point it at their own. Either parameterize it properly (flags + env fallback, as the docstring promises) or leave it out of the public repo.

5. MAJOR — public docs leak private context the cleanup pass missed

All absent at s7, introduced here, and each is duplicated into docs/src/content/docs/ by sync-starlight.mjs, doubling the published surface:

• ADR-016 grounds its new revision note in "Engram observation RFC: ADR-009 — Security posture and blast radius for development-time agents #140 (not re-fetchable in the authoring session; the reviewing session should cross-check this revision against it)" — a pointer into the author's private memory store. A prospective adopter reading the published ADR to evaluate ABCA's credential model finds the decision's central rejection unauditable.
• docs/design/SURFACE_AGNOSTIC_ORCHESTRATION.md and its published mirror carry a ## Divergence from linear-vercel + verification section — linear-vercel is the private fork branch.
• The author's Linear workspace slug appears in published docs.

6. MINOR — MAX_TURNS doubled 100 → 200, docs still say 100

agent.ts:353 changes the runtime env MAX_TURNS from '100' to '200', and agent-side config.py/server.py fall back to "200" here (main: "100"). But validation.ts:34 still exports DEFAULT_MAX_TURNS = 100, and that is what the orchestrator writes into every dispatch payload — so the two defaults now disagree depending on which path supplies the value, and docs/guides/DEVELOPER_GUIDE.md:251 still documents 100. An operator sizing a budget on the documented ceiling can get twice the turns on an env-reading path.

7. MINOR — orchestration reconciler is granted read on the entire trace/artifacts bucket

traceArtifactsBucket.bucket.grantRead(orchestrationReconciler.fn) (agent.ts:958) grants s3:GetObject on <bucket>/*. That bucket holds two disjoint key spaces: artifacts/<task_id>/ (plan artifacts — all the reconciler needs) and traces/<user_id>/<task_id>.jsonl.gz (full agent trajectories including tool I/O). The construct's own doc calls out the per-user prefix scoping precisely because get-trace-url authorizes presigned reads by matching the caller's Cognito sub to the record's user_id.

The reconciler reads artifact_uri verbatim off the task record and splits it into bucket+key with no prefix validation (orchestration-reconciler.ts:1532-1536). Scope the grant to artifacts/* — the least-privilege fix is one argument.

---

Reviewed with Claude Code. Verification: tsc --noEmit on this slice's own base; #643 survival checked line-by-line at every slice tip; the FanOutConsumer revert confirmed by counting the props at six refs and reading the construct's prop handling; ADR-016/USER_GUIDE/yarn.lock diffed against main directly.

[isadeks]

Verdicts on the three previously-flagged, deliberately-unfixed defects

Each was independently traced rather than restated. Two of the three land differently than described.

---

1. handleSingleTaskVerdict settles the comment marker to ✅ on paths where nothing was approved — CONFIRMED, and worse than described

The caller (linear-webhook-processor.ts, the pending && (verdict === 'approve' || 'reject') block) wraps the verdict handling in try { … } finally { await settleVerdictMarker(); }, and settleVerdictMarker unconditionally sets the reaction to 'succeeded' for any non-reject verdict. handleSingleTaskVerdict is called inside that try and returns with a bare return on three separate bail paths:

bail path	what the user sees
!taken — pending plan raced/expired	log only, then ✅
!singleHydrated.ok — attachment screening failed	❌ <message> and then ✅ on the trigger comment
result.statusCode !== 201 — task creation failed	failure message and then ✅

So the originally-flagged case (raced/expired) is real, and the attachment-screening path is worse than flagged: the user gets an explicit failure comment and a success tick on the comment they are watching. The third path does the same for a failed createTaskCore.

The finally is deliberate and its comment explains why ("a verdict is fully resolved by the time these paths return … Leaving 👀 made the comment read as 'still thinking about it' forever") — that reasoning is sound for the paths it was written for. The gap is that handleSingleTaskVerdict's early returns are not "fully resolved verdicts". The fix is for it to report its outcome (return a discriminated result) and for settleVerdictMarker to choose the marker from that, rather than from verdict alone.

---

2. sumIterationCostForIssue unpaginated Query + serial GetItem in a stream consumer — LARGELY REFUTED

The concern as framed does not hold, because the expensive path is not per-record:

• The Query is unpaginated (no LastEvaluatedKey loop) and the GetItem loop is serial — both confirmed.
• But three gates precede it in replyToStandaloneTrigger: it requires a trigger comment id; it excludes orchestration iterations (cm?.orchestration_iteration === 'true' → the reconciler owns those); and it takes a one-shot claim — UpdateExpression: 'SET ack_replied_at = :now' with ConditionExpression: 'attribute_not_exists(ack_replied_at)'. So a redelivered batch (fan-out batchSize: 100, reconciler batchSize: 10) is claimed away and the sum runs once per terminal iteration task, not once per stream record.
• Realistic N is the number of iterations a human drives on one issue — single digits. Serial GetItem at that size is milliseconds.
• Both consumers set reportBatchItemFailures: true, so a poison record cannot block the shard.

Residual issue, small but real: at the 1 MB Query page limit the function silently undercounts — total is summed over a truncated Items list and returned as if complete, so the user is shown a cost number that is quietly wrong rather than an error. That needs roughly thousands of tasks on one issue, so I would not block on it. Cheap fix if you want it: a LastEvaluatedKey loop, or an explicit "first N iterations" caveat in the copy.

Also worth noting: sumIterationCostForIssue exists twice with the same shape — fanout-task-events.ts and orchestration-reconciler.ts:1252. Two copies of a cost-accounting function will drift.

---

3. 422 REPO_NOT_ONBOARDED treated as terminal — REFUTED; terminal is correct, and the two updated comments are now accurate

Terminal is the right call, for the reason isDeterministicCreateFailure documents: rolling back to ready hands the row to the 10-minute stranded-orchestration sweep, which would re-release it forever with no terminal state, no ❌, and the epic stuck on 👀 — an infinite silent strand. A repo does not onboard itself, so the retry can never succeed on its own.

The three properties that make terminal safe here are all present:

1. The user is told, specifically. deterministicFailureReason(422) renders "Couldn't start — this repo isn't onboarded to ABCA. Onboard it, then reply @bgagent retry on the epic to re-run."
2. The remedy is real. computeEpicRetryPlan (orchestration-reconcile.ts:327) resets failed → ready/blocked based on predecessor state and re-releases, so @bgagent retry genuinely re-runs a terminally-failed child after onboarding. The work is recoverable, not stranded.
3. 409 is correctly excluded from the deterministic set, so an idempotent replay still rolls back and finalises.

No change needed. The distinction the code draws — deterministic-but-user-fixable → terminal with a remedy, versus transient → rollback — is the right one, and the updated comments now match the behaviour.

[isadeks]

Review findings addressed

BLOCKING #1 — Jira fan-out final-status comments silently disabled. Confirmed and fixed. Verified independently: main and every slice s1–s7 carry both props; s8 was the only ref with zero. The construct still gates the registry grant, the table-name env var and the OAuth secret grant on them, and because both are optional the capability just vanished with no synth error.

Your note on the class was the important part — this was not the Jira work that was being watched, so the watching was mis-targeted. I have added a test asserting both surfaces' registry env vars and both OAuth ARN patterns on the fan-out consumer, and mutation-tested it: removing the props now fails the suite. That guard, not the restore, is what stops a silent optional-prop loss recurring. I also swept agent.ts against main for other construct calls that lost props — no others found.

MAJOR #2 — the log-delivery pin shim fires on the DEFAULT stack name. Confirmed and fixed. main.ts defaults stackName to exactly the shim's only table key, so the "OFF by default" claim in its own comment was false and any operator deploying without an override inherited another account's hardcoded logical ids and account-unique Names. Now requires explicit -c pinnedLogDeliveryStack=<name>; the misleading comment is corrected. Guarded by a test that fails if the ?? stack.stackName fallback returns.

MAJOR #4 — scripts/linear_epic.py hardcodes a private workspace. Confirmed and fixed. You were right that the docstring promised flags that did not exist. Team id is now required via --team/$LINEAR_TEAM_ID with a message saying where to find it; project and label are optional with the platform default. I also dropped the /tmp PAT fallback — a credential should not be read from a world-readable path — so it is environment-only.

MAJOR #5 — published docs leak private context. Confirmed and fixed, in both the source docs and their generated mirrors. The Engram pointer was the worst of the three for exactly the reason you gave: an adopter evaluating the credential model finds the decision's central rejection unauditable. The fork-divergence section and the workspace slug are gone too.

MINOR #7 — reconciler granted read on the entire trace/artifacts bucket. Confirmed and fixed. Scoped to artifacts/*. Two things beyond the one-argument fix: the prefix is now a named constant sitting beside its traces/ sibling with the reason they must stay disjoint, and the handler rejects an artifact URI pointing outside the prefix rather than relying solely on IAM — so a bad or tampered URI produces a usable log line instead of an AccessDenied. Verified by reading the synthesized policy: the object grant is <bucket>/artifacts/*, and a test fails if it widens to the bucket root.

Activation is unconditional — your framing accepted, not changed. You are right that the repo has a tryGetContext precedent and that a change of this size could stage adoption. I have left it unconditional deliberately: a context-gated activation means the dormant slices sit merged-but-unreachable indefinitely, which is the state this carve exists to end, and the gate would then need its own removal PR. Flagging it as a live decision for whoever merges rather than closing it silently — if you want the gate, it is a small change.

MAJOR #3 — activation turns on a lookup no pre-existing task row can satisfy. Confirmed; NOT fixed, and this is the one I most want a second opinion on. Your failure walkthrough is correct: linear_issue_id is hoisted for the first time in this PR, there is no backfill, and resolveTaskByLinearIssue returning null is treated as "not ours, ignore" — so every issue in flight at the moment of adoption silently drops its first follow-up comment. Your minimum ask (tell the user) and your better ask (fall back to extractLinearIdentifier over recent tasks) are both right.

I have not built it because it is a new behaviour rather than a correction, and it needs a deliberate choice about how far back to look and what to say when the fallback also misses. Tracked as the top follow-up. If you would rather this block the merge, say so — it is the one finding here where "silently drops user input" argues for fixing before adoption rather than after.

Gates at the tip: 3653 CDK tests / 180 suites, 680 CLI, 1440 agent; tsc/eslint/ruff/ty clean.

[isadeks]

Correction: the log-delivery pin shim is NOT obsolete — I was wrong, and the deploy proved it

I deleted the shim, reasoning that it was dead code. That was wrong. Reverted.

My analysis compared the resource NAMES. Those genuinely no longer overlap — the alpha naming scheme moved on, live is cdk-applicationlogs-source-… while the current code generates backgroundagentdevRuntimeApplicationLogsDeliverySource…. I concluded create-before-delete could not collide, and deployed.

The stack rolled back:

AWS::Logs::DeliverySource ... Resource handler returned message:
"This ResourceId has already been used in another Delivery Source in this account."
(HandlerErrorCode: AlreadyExists)

A DeliverySource is keyed on the resource it delivers from, not on its name. Both sources point at the same AgentCore runtime ARN, and AWS refuses a second source for a resource that already has one — so the collision happens regardless of naming. That is precisely what the shim prevents. (The rollback left the live resources intact, and the stack is healthy.)

So the shim stays, opt-in as this PR had it.

On the "you now need a flag on every deploy" objection — that is solved outside the repo, which is the right place for it. A pin is only ever correct for the account that already owns those resources, so it belongs in that account's own gitignored cdk.context.json, not in shipped code and not on the command line:

{ "pinnedLogDeliveryStack": "backgroundagent-dev" }

A bare cdk deploy then does the right thing for the affected stack, while a fresh stack in any other account still synths pristine ids. Verified by deploying this stack flag-free with the six log-delivery resources untouched.

---

Deployed and live-verified

Deployed the full stack (S8 tip, whole arc active) to dev and verified each fix by artifact, not exit code — the first two deploy attempts reported exit 0 while failing at ECR asset push, so this matters:

Fix	Live evidence
Jira fan-out restored (BLOCKING)	JIRA_WORKSPACE_REGISTRY_TABLE_NAME present on the fan-out Lambda; both bgagent-{linear,jira}-oauth-* grants on its role
Model grant (BLOCKING, #654)	iam simulate-principal-policy → InvokeModel on us.anthropic.claude-opus-4-8 = allowed; a live task on a repo with no pinned model ran on claude-opus-4-8, completed in 29s / 8 turns / $0.16 and opened a correct single-file PR. Before the fix this died at turn 0.
S3 grant scoped	simulation on the reconciler role: artifacts/… = allowed, traces/<user>/… = implicitDeny
Log-delivery	six resources unchanged after a flag-free deploy
S7 infra	TaskTable stream = NEW_IMAGE; GSIs = the pre-existing four + LinearIssueIndex (one-index addition, no table replacement)
Activation	ORCHESTRATION_TABLE_NAME set on the webhook processor; reconciler event-source Enabled with the terminal-status filter
Revoked-marker opt-in (#655)	a genuinely revoked workspace token hit this path live: the failure was logged honestly and no registry write was attempted — zero AccessDenied in the window. Before the fix this fired a doomed swallowed write every time.
Parameterized linear_epic.py	created and tore down a real two-child epic via --team, env-only PAT, no hardcoded workspace

One thing I could not verify end-to-end: a full Linear-triggered orchestration. The maguireb workspace's OAuth refresh token is revoked (invalid_grant), so the webhook drops the event before orchestration — a known environmental grant-expiry issue, unrelated to this stack. I verified the orchestration wiring by artifact instead (stream consumer enabled + filtered, activation env var set, table and GSI live) and drove the agent path via the API. A Linear-triggered run should be re-tested after that workspace is re-onboarded.

[scottschreckengaust]

Review — carve S8 / activation (#662)

Reviewed as a Principal AWS Solutions Architect, scoped only to this slice's incremental diff (carve/s7-orchestration-plane..carve/s8-activation, 75 files, +13687/−565). This is the activation slice — the one PR in the stack that flips deployed behaviour from dormant to live — so it gets the highest bar. I read the author's own automated-review thread and independently re-verified every prior blocking claim against the current tip before finalizing.

1. Verdict — Request changes

The five previously-flagged blocking/major items (Jira fan-out revert, log-delivery shim default-name, linear_epic.py private workspace, doc leaks, over-broad S3 grant) are genuinely fixed at this tip and I confirmed each. Two confirmed defects remain unfixed, both on the exact path this PR turns on, and both fail silently or misleadingly — the failure mode this repo's tenets ("fail closed", "surface failures", "reviewable outcomes") exist to prevent. The author explicitly left #3 open asking whether it should block; for an activation slice that regresses in-flight user issues on first deploy, it does.

2. Vision alignment — fits, with a caveat

Activating the orchestration arc directly advances the VISION north star (fire-and-forget, escalate-by-policy) and lands approved P0 issues #247 and #299. Blast radius is real but bounded (concurrency table, HITL plan gates, terminal-state routing). The unconditional activation — no tryGetContext gate, every existing deployment flips live on first cdk deploy — is a deliberate, documented decision (the carve exists to end the merged-but-unreachable state); I accept the framing but it raises the bar on the two silent-failure defects below, because they land on every operator's existing issues at the moment of adoption.

3. Blocking issues

B1 — Comment-verdict success marker (✅) is set on paths where nothing succeeded (cdk/src/handlers/linear-webhook-processor.ts:2340, :2412, and handleSingleTaskVerdict at :2856).
The pending && verdict∈{approve,reject} block wraps verdict handling in try { … } finally { await settleVerdictMarker(); }, and settleVerdictMarker sets the trigger comment's reaction to 'succeeded' for any non-reject verdict. handleSingleTaskVerdict runs inside that try and returns with a bare return on two failure bail paths that are new in this PR:

• !singleHydrated.ok (attachment screening failed → safeReportIssueFailure('❌ …')) then the finally stamps ✅ on the same comment.
• result.statusCode !== 201 (task creation failed → failure message) then ✅.

The user watching the comment sees a ❌ failure message and a ✅ success tick — contradictory, and the tick is the durable signal. The finally's own comment justifies settling for "fully resolved verdicts", but these early returns are not that. Fix: have handleSingleTaskVerdict return a discriminated outcome and let settleVerdictMarker choose the marker from the outcome, not from verdict alone. (Independently confirmed; matches the author's own "CONFIRMED, and worse than described" trace.)

B2 — Activation silently drops the first follow-up comment on every in-flight Linear issue at adoption (cdk/src/handlers/shared/linear-task-by-issue.ts:49-80; caller linear-webhook-processor.ts:3457).
This PR is the first to set ORCHESTRATION_TABLE_NAME, which turns on the standalone comment path. That path resolves issue→newest-task via resolveTaskByLinearIssue, a Query on the sparse LinearIssueIndex GSI keyed on the top-level linear_issue_id attribute — an attribute written for the first time by the hoist added in this same PR (create-task-core.ts:720-725). No task created by the currently-deployed code carries it and there is no backfill, so the Query misses; on both miss and error resolveTaskByLinearIssue returns null, which the caller logs at info and treats as "not an ABCA issue — ignore". Result: a user comments @bgagent … on an issue ABCA opened a PR for yesterday, and after this deploy the comment is dropped with no reply and no reaction. This affects every issue in flight at the adoption boundary. Minimum fix: tell the user (a reaction/comment) instead of a silent info log; better: fall back to extractLinearIdentifier over recent tasks so pre-hoist rows still resolve. The author confirmed this and left it as a tracked follow-up — for an activation slice that silently drops user input on adoption, resolve it (even just the "tell the user" minimum) before merge.

4. Non-blocking suggestions / nits

• N1 — MAX_TURNS default drift. agent.ts:353 raises the runtime env MAX_TURNS 100→200 (and agent config.py/server.py fall back to "200"), but cdk/src/handlers/shared/validation.ts:34 still exports DEFAULT_MAX_TURNS = 100 (what the orchestrator writes into dispatch payloads) and docs/guides/DEVELOPER_GUIDE.md:251 still documents 100. The effective ceiling now depends on which path supplies the value; reconcile the two defaults and the doc.
• N2 — sumIterationCostForIssue silently undercounts at the 1 MB Query page boundary (fanout-task-events.ts, orchestration-reconciler.ts:1252). The Query is unpaginated and total is summed over a possibly-truncated Items list, then shown to the user as if complete. Needs thousands of tasks on one issue, so not blocking, but the reported cost is quietly wrong rather than an error. Also: the function exists twice with the same shape — two copies of a cost-accounting routine will drift; extract one.
• N3 — file size / proportionality. linear-webhook-processor.ts is now 4411 lines. It is coherent and well-commented, but it is well past the 800-line heuristic; consider splitting the verdict/revision/standalone-trigger flows into siblings for reviewability.

5. Documentation

Docs are thorough and the prior private-context leaks (Engram pointer in ADR-016, the linear-vercel fork-divergence section, workspace slug) are gone at this tip — confirmed in both docs/design|decisions/ and the generated docs/src/content/docs/ mirror, and the mirror is in sync (both sides updated in the diff). New ADR-001/016/018 and design notes are appropriate. Only doc gap is N1 (DEVELOPER_GUIDE.md:251 MAX_TURNS). Issue tracking is correct: epic #668 plus approved P0 #247/#299.

6. Tests & CI

CI green (build agentcore, secrets/deps, dead-code, title). Strong new coverage: linear-webhook-processor-orchestration.test.ts (+1199), linear-webhook-plan-command.test.ts, mode-a attachments, orchestration-e2e.test.ts, agent.test.ts, and a mutation-tested guard that the Jira fan-out props can't silently vanish again. Gap: no test asserts the marker outcome on the handleSingleTaskVerdict failure bail paths (B1) — a test there would have caught the ❌+✅ contradiction. Bootstrap synth-coverage: not applicable — this slice instantiates constructs (Lambda, DynamoDB SlackChannelMappingTable, events.Rule) whose CFN resource types already exist in the stack; no new resource type is introduced, and the bootstrap suite runs in the passing build.
CDK test perf: spot-checked new suites — no re-enabling of aws:cdk:bundling-stacks; jest worker limits added to cdk/package.json to bound memory. Good.

7. Review agents run

Operating in a subagent context without the interactive pr-review-toolkit agents; I applied each agent's lens by hand over the in-scope diff and cite findings accordingly: code-reviewer (B1, N3, duplication in N2), silent-failure-hunter (B1, B2, N2 — the core of this review), type-design-analyzer (LinearIssueTask/resolveTaskByLinearIssue return shape reviewed — clean, though null-on-both-miss-and-error conflates two cases, feeding B2), comment-analyzer (the settleVerdictMarker and log-delivery-shim comments were re-checked against behaviour — shim comment now accurate; verdict finally comment is accurate for its intended paths but doesn't cover the bail returns), pr-test-analyzer (§6 gap), /security-review lens (S3 grant now scoped to artifacts/* with in-handler prefix rejection — confirmed; Jira/Linear OAuth secret ARN grants scoped to bgagent-*-oauth-*; no new IAM over-grant found). None omitted for scope.

8. Human heuristics

• Proportionality — concern: linear-webhook-processor.ts:4411 lines (N3). Otherwise complexity matches an activation slice.
• Coherence — concern: sumIterationCostForIssue duplicated across two handlers (N2). Naming/structure otherwise consistent.
• Clarity — concern: B1/B2 — error handling hides failures behind a success marker (B1) and a silent ignore (B2); MAX_TURNS magic value diverges from DEFAULT_MAX_TURNS (N1).
• Appropriateness — pass: integration paths are covered by e2e/orchestration suites and the author reports live artifact-verification (Jira env var, scoped IAM simulation, GSI addition without table replacement); tests assert intended behaviour, with the B1 outcome-assertion gap noted.

---

Reviewed with Claude Code. Prior self-review blocking claims re-verified against the tip at 844b27e; B1/B2 confirmed present and unfixed.

[isadeks]

Both blocking defects fixed, plus N1 and N2

Head is now 8fb38f54. You were right to block on B2 in particular — I had left it as a tracked follow-up and you pushed back that an activation slice silently dropping user input at the adoption boundary should not ship that way. Agreed on re-reading it.

B1 — verdict marker claimed success on its own failures

Confirmed exactly as described, including the third path you did not list: the raced/expired-plan bail also fell through to the ✅ stamp.

handleSingleTaskVerdict now returns a discriminated outcome and the marker is derived from what happened, not from the verdict that was requested:

bail path	before	after
attachment screening failed (❌ posted)	✅	❓
task creation non-201 (❌ posted)	✅	❓
raced / expired plan (nothing done)	✅	marker untouched
approved + dispatched	✅	✅
rejected	❓	❓

The third case is worth calling out: skip deliberately leaves the reaction alone rather than settling it, because another verdict already consumed that plan and settled that comment — re-stamping would overwrite a real outcome with this run's guess.

Three tests, one per bail path. Verified by reverting the marker to verdict-keyed and watching all three fail while the pre-existing "marker still settles when handling THROWS" test kept passing, so the finally is intact.

B2 — activation dropped the first follow-up on in-flight issues

Fixed properly rather than with the minimum. Your type-design note fed straight into the fix: null-on-both-miss-and-error was the root, so the lookup is now three-valued.

found / none / error, because a miss and a broken Query are different facts. The second cannot conclude anything about the issue, and reporting it as "not an ABCA issue" is a guess presented as a conclusion — while also hiding a real fault (throttling, a missing index) behind a silent no-op.

Reaching that code requires an explicit @bgagent mention, so the user is provably waiting. Both outcomes now get a reply and settle the comment to ❓:

👋 I don't have a task linked to this issue, so there's nothing for me to iterate on here. If I worked on it before this feature shipped, that link doesn't exist yet — re-apply this project's trigger label and I'll pick it up as a fresh run.

⚠️ I couldn't look up whether I have a task for this issue, so I haven't acted on your message. This is a transient fault on my side, not something wrong with your request — mention @bgagent again to retry.

Both were read as rendered output, not reviewed in source. It says what to do next, not only what failed. Redelivery-guarded with the same ack-claim the near-miss path uses, so a Linear retry does not post it twice.

I did not add the extractLinearIdentifier fallback you offered as the better option. It needs an ENG-123 string, but the webhook only has the issue UUID, and pre-hoist rows keep linear_issue_id nested inside channel_metadata with no index on it — so resolving them means a full table Scan per dropped comment. Not worth it to save a re-label. Happy to revisit if you disagree.

One existing test asserted the old silent behaviour; it now asserts the notification.

N1 — MAX_TURNS

Reverted to 100 across all three sites. main is uniformly 100, so the carve had raised it in the stack env, config.py, and server.py without flagging it — and the first slice's own body explicitly promises to defer this bump to its own PR. So the series was telling reviewers this change was not here while it was. All five sites now agree, including the doc. The bump is a real intended change; it belongs in its own PR against the shared constant, where it applies to both paths at once.

N2 — sumIterationCostForIssue

Extracted to one implementation. The duplication had already caused drift, which makes your "two copies will drift" point concrete: one copy parsed a string cost_usd and added it without a finite check, so a stringified value poisoned the whole total to NaN; the other guarded correctly. Cost accounting that disagrees with itself depending on which handler ran is worse than either behaviour alone.

The shared version paginates. Both copies summed a single 1 MB page as if it were everything, and a capped or partial sum now reports itself as partial rather than presenting a short number as the total.

N3 — file size: not doing it

Real (4497 lines, more than 2x the next-largest handler) but I am declining it in this PR. Splitting it would rewrite a diff reviewers have already read, for zero behavioural change, on the slice with the highest blast radius. Filed as follow-up work instead.

---

CI green: 3688 CDK + 1444 agent tests at the tip. Not yet redeployed — B1/B2 are live behavioural changes on the activation path, so deploy + live-verify is next before I would consider this mergeable.

[isadeks]

Scope change: auto-decomposition is no longer part of this carve

Heads-up before you re-read this — every slice was force-pushed, so a review in progress is against a stale diff. Sorry for the churn; the reason is a scope decision, not a fix.

Auto-decomposition is staying on the development branch as experimental and is not landing on main. It presupposes one way of working: an issue split into a sub-issue graph, with a human approval gate on a proposed plan. main should not presuppose that. Everything else in this arc is workstyle-neutral — running a graph a human already authored, iterating on a PR from a comment, heartbeats, clarify-resume, the Channel abstraction, the compute plane — and those all stay.

What moved

~7,900 lines removed. 23 dedicated files, the agent workflow and its prompt, the admission-table entry, the CLI flags, and about 314 lines embedded in files that survive.

slice	effect
#647 S1	unchanged
#653 S2	comments now describe artifact workflows by their contract, not by one workflow
#654 S3	workflow YAML, prompt, and admission entry removed
#655 S4	--decompose-allowed / --max-sub-issues / --max-parent-budget-usd removed from linear onboard-project
#656 S5	note-sweep renamed for what it does
#657 S6	8538 → ~2000 lines. Now purely the iteration slice; retitled
#659 S7	graph CREATION removed from the reconciler; graph EXECUTION kept
#662 S8	the plan-proposal surface removed; webhook 4502 → 3156 lines

Three things worth knowing as a reviewer

Some of the review fixes from the last round are gone with the feature. The verdict-marker fix (B1 on #662) and the retry-hint trigger_label fix (#659) were both decomposition-only. They were real fixes and they live on the development branch — they are not being dropped as wrong.

Two things looked decomposition-only and were not. DEFAULT_LABEL_FILTER and hasHelpLabel lived in a decompose-named module but are plain label plumbing, so they moved to trigger-label.ts rather than being deleted. And the note sweep is prefix-based and general, so it was renamed, not removed. Both would have been silent breakage.

Removing the feature exposed dead IAM. The reconciler held an s3:GetObject on the artifacts prefix plus put/delete on the attachments bucket, all for plan seeding. They had no caller left. Removed — a net least-privilege gain, and there is now a test asserting the reconciler has no object grant at all, which is the stronger claim since S3 does not normalize keys.

Verification

• All 8 slices CI-green, chain integrity confirmed (each slice's base is its parent's tip)
• 3436 CDK / 681 CLI / 1436 agent tests at the tip
• Zero decompose references in every slice individually, not just at the tip — an earlier pass looked clean at the tip while intermediate slices still carried them, which would have landed decompose text on main temporarily during the serial merge
• Deployed to dev and live-verified by artifact: the deployed webhook bundle contains no plan surface; coding/decompose-v1 is now rejected at admission (Unknown workflow_ref, 400); a normal coding task still runs end to end (PR opened with the requested file, exact contents); coding/pr-review-v1 still admits; the reconciler's IAM shows no S3 statements