feat(carve S7): the orchestration compute plane — reconciler, release, rollup by isadeks · Pull Request #659 · aws-samples/sample-autonomous-cloud-coding-agents

isadeks · 2026-07-27T12:45:01Z

Slice 7 of the staged carve. Stacked on #657 (carve/s6-decompose-iteration) — review that first; this PR's diff is only its own commit.

What this adds

The engine that actually runs a sub-issue graph. It watches child tasks reach a terminal state, releases whatever they unblocked, keeps the parent's status panel current, and re-stacks dependents when a branch moves.

orchestration-discovery — the composer that turns a trigger into a seeded run: read the graph, validate it, append the integration node if it fans out, persist it, and hand back the root set to start.
orchestration-reconcile — pure gating. Given a child that just went terminal, decide which dependents are now releasable, which must be skipped, and when the parent itself is done. Two recovery shapes are covered too: comment-fixing a failed child un-fails it and re-releases the dependents that were skipped behind it, and re-triggering a finished epic retries only its failed/skipped children.
orchestration-release — create a child's task, behind a claim so two concurrent releasers can't both launch it. A guardrail rejection or an un-onboarded repo is terminal with a reason the user can act on; a 5xx or a duplicate replay rolls back to ready so the sweep retries.
orchestration-rollup — the parent's single maturing panel: one comment that matures in place rather than a stream of updates. A failed row carries an indented sub-line saying what failed and where to read it.
orchestration-restack — when a child's branch changes, work out which dependents need their base moved.
orchestration-parent-comment — route a comment on the epic to the sub-issue it's actually about, and say so plainly when it looks like new work rather than a change to existing work.
orchestration-reconciler (handler + construct) — the TaskTable-stream consumer that drives all of the above. A DLQ for poison records, a FilterCriteria so only terminal-status writes invoke it (RUNNING/heartbeat churn is the bulk of TaskTable writes and would otherwise wake it constantly), and partial-batch reporting so one bad record can't re-drive its healthy siblings.
reconcile-stranded-orchestrations (handler + construct) — a scheduled backstop for a run whose stream event was lost.

⚠️ Unlike S5 and S6, this slice is NOT fully dormant

TaskTable gains a LinearIssueIndex GSI and has its stream enabled (NEW_IMAGE). TaskTable is instantiated in the stack, so these two changes do land on deploy.

Both are required by this slice and can't be deferred: the reconciler consumes that stream, and it queries that index to total an issue's iteration cost. Notes for review:

Enabling a stream and adding a GSI are both in-place CloudFormation updates — no table replacement.
The GSI projection is deliberately narrow (pr_url, pr_number, status, repo, user_id, channel_metadata). A GSI's projection cannot be changed in place afterwards, which is why the cost query does a per-task GetItem for cost_usd rather than widening it.
The stream is on TaskTable rather than TaskEventsTable because the latter is already at its 2-consumer limit.
The reconciler constructs themselves are not instantiated in any stack yet — that wiring is the next slice — so nothing consumes the stream on deploy.

Verification

tsc --noEmit clean
CDK: 3491 tests / 173 suites passing (up from 3213/164 on the base)
eslint clean

🤖 Generated with Claude Code

Tracking

Slice S7 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 #657 — review that first; this PR's diff is against its branch.

isadeks · 2026-07-27T16:05:39Z

Automated review — carve S7 (#659)

Reviewed as its own diff only (carve/s6-decompose-iteration..carve/s7-orchestration-plane, 21 files, +10274/−4). This PR does not claim dormancy — it enables a DynamoDB stream and adds a GSI on the live TaskTable — so the CFN-safety questions got priority. Governance 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 run, not inferred). Only 4 deletions in the whole diff. Every new source file ships with its test in this same slice — no source/test split at this boundary.

CFN safety — the questions worth asking before this merges

Stream enablement: in-place, no replacement. CONFIRMED.
task-table.ts adds stream: dynamodb.StreamViewType.NEW_IMAGE. StreamSpecification is an in-place update on AWS::DynamoDB::Table, and I checked that nothing else in the same diff touches a replacement-forcing property — partitionKey (task_id), sortKey, billingMode (PAY_PER_REQUEST), and tableName (still props.tableName passthrough) are all unchanged. The comment in the code makes the same claim and it holds up.

GSI addition: one per deploy. CONFIRMED.
DynamoDB permits one GSI create per update, and this diff adds exactly one (LinearIssueIndex), taking TaskTable from 4 GSIs to 5:

ref	GSIs	names
`main` / s6	4	UserStatus, Status, Idempotency, JiraIssue
s7 (#659)	5	+ LinearIssueIndex
s8	5	unchanged

Since JiraIssueIndex already exists on main, this is a clean single-index addition, not two-at-once.

GSI projection: correct for its purpose — but four call sites already pay for it.
Projection is INCLUDE with ['pr_url','pr_number','status','repo','user_id','channel_metadata']. I traced every consumer and every attribute it reads:

linear-task-by-issue.ts resolveTaskByLinearIssue reads task_id, user_id, repo, pr_url, pr_number, status — all projected ✅
github-webhook-processor.ts reads task_id, channel_metadata.iteration_reply_comment_id — projected ✅

So the routing lookups the index was designed for are fully covered, and the projection is right for them. But four call sites need attributes the index does not project, and each pays a per-item base-table GetItem:

orchestration-reconciler.ts:1266 — cost_usd
fanout-task-events.ts sumIterationCostForIssue — cost_usd
clarify-resume.ts:45 — code_changed, answer_text, task_description, workflow pin
linear-webhook-processor.ts:3747 — the clarify fields (reads the full base row)

Each is deliberate and documented in-code. I am raising it because a GSI projection cannot be changed in place — DynamoDB rejects it, as the code comment itself records from experience. So this list is the permanent cost of the current shape. Four known consumers already working around it, two of them for the same field (cost_usd), is the moment to ask whether cost_usd belongs in the projection. Adding it now is free; adding it later means creating a second index. Worth a deliberate decision rather than inheriting it.

On the previously-flagged `sumIterationCostForIssue` concern — largely REFUTED

The standing concern was an unpaginated Query on LinearIssueIndex plus serial per-task GetItem in a stream-consumer Lambda. I traced it in both copies and the batch-multiplication part does not hold:

The Query is unpaginated (no LastEvaluatedKey loop) and the GetItem loop is serial. Both confirmed.
But it does not run per stream record. Three gates precede it in replyToStandaloneTrigger: it requires a trigger comment, it excludes orchestration iterations, 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.
Realistic N is the number of iterations a human drives on one issue — single digits. At that size, serial GetItem is a few milliseconds.
Both consumers are reportBatchItemFailures: true, so a poison record doesn't block the shard.

Residual risk, small but real: at the 1 MB Query page limit the function silently undercounts rather than erroring — total is summed from a truncated Items list and returned as if complete. That needs roughly thousands of tasks on one issue to trigger, so I would not block on it. If you want the cheap fix, it is a LastEvaluatedKey loop or a Limit plus an explicit "showing first N" note, so the number shown to the user is never quietly wrong.

Duplication 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.

Other findings

MINOR — renderStatusBlock, rollupKindFromChildren, renderRollupComment, postRollup, renderEpicPanel, buildPanelRows and truncateQuote are exported from orchestration-rollup.ts but have zero callers in src/ anywhere in the stack.
Verified at the s8 tip: git grep -l <fn> -- 'cdk/src/**' excluding the defining file returns nothing for all seven. Only upsertEpicPanel and cascadeNodeLabel are actually used. The file's own comment explains why — the maturing panel "supersedes the separate renderStatusBlock + renderRollupComment" — so this is superseded code that shipped alongside its replacement. It is invisible to the dead-code ratchet because each function has a test file referencing it, and knip-baseline.json is unchanged at 78 across the whole stack. ~600 lines of dead rendering logic is a lot to publish; consider deleting the superseded half.

I also rendered the two dead functions to check whether they were merely unused or actually wrong, and they are wrong in ways that would matter if anything called them:

renderStatusBlock([succeeded, failed, blocked, released, skipped])
  → "🔄 **ABCA orchestration** · 3/5 complete"

Three of five "complete" with one success — terminal() counts failed and skipped toward "complete".

rollupKindFromChildren([succeeded, released])  → "complete"
rollupKindFromChildren([])                      → "complete"

A still-released (running) child yields complete, and an empty set yields complete. Another reason to delete rather than keep.

MINOR — a raw internal UUID is shown to users on the fallback path. In renderStatusBlock, when a child has neither display_id nor title the label falls back to sub_issue_id, rendering - ✅ issue-uuid-1 — succeeded. Currently unreachable (dead function), but the same display_id ?? sub_issue_id fallback pattern is worth checking in renderEpicPanel before activation.

IAM

Least-privilege on the new roles looks sound: grantReadData where read-only suffices, grantReadWriteData only on the three tables the reconciler genuinely writes, a DLQ with enforceSSL and 14-day retention for poison records, and cdk-nag suppressions that name the CDK-generated index/* wildcards specifically rather than blanket-suppressing. No wildcard actions. The one over-broad grant I found is in #662 (traceArtifactsBucket.grantRead on the whole bucket) and is reported there.

Reviewed with Claude Code. Verification: tsc --noEmit on this slice's own base; GSI count and stream declaration compared across six refs; every LinearIssueIndex consumer's attribute reads traced against the projection list; the claim gate read as an actual ConditionExpression; the dead rollup functions executed to check their output.

isadeks · 2026-07-27T16:20:20Z

Addendum to the review above — two further findings

A second pass surfaced two things the first review missed.

MINOR (user-copy + public-fitness) — a user-facing retry instruction names the wrong label.

orchestration-rollup.ts:318:

(No reply? Removing and re-applying the `abca` label also retries.)

The trigger label is per-project configurable (label_filter on the project-mapping row) and its default is bgagent — DEFAULT_LABEL_FILTER = 'bgagent' in orchestration-decomposition-mode.ts:44, linear-webhook-processor.ts:35 and jira-webhook-processor.ts:58, and the docs say the same. abca is this project's own label, not the shipped default.

So a user who follows this instruction removes and re-applies a label the webhook does not filter on, and nothing happens. It should interpolate the resolved label_filter for that project, or fall back to DEFAULT_LABEL_FILTER rather than a hardcoded string. This is both a correctness bug in user-facing copy and an instance of the use-case-specific-content class.

NIT — taskTableForWrites is a prop that silently does nothing.

OrchestrationReconcilerProps.taskTableForWrites (orchestration-reconciler.ts:46) is declared and documented ("TaskTable (for createTaskCore writes when releasing children)"), but the constructor never reads it — the write grant actually comes from props.taskTable.grantReadWriteData(this.fn). It is also never passed by the stack at #662.

Worth removing rather than leaving: the only reason a caller would reach for this prop is to point writes at a different table, and doing so would silently have no effect while appearing to work.

NIT — third instance of the "cited artifact lands later" pattern.

reconcile-stranded-orchestrations.ts:39 and orchestration-release.ts both refer the reader to docs/research/orchestration-reconciler-correctness.md for the failure-mode analysis justifying the sweep. That file does not exist at main, s1–s6, or this slice — it is added in #662. So this slice's two most safety-critical modules cite a nonexistent document for their correctness rationale.

This is the same shape as the ADR-016 citations in #654 and the model/IAM split in #654 → #662. See the stack-level note in the summary: the recurring boundary error in this stack is code landing in an earlier slice than the artifact that authorises it.

isadeks · 2026-07-27T17:52:39Z

Review findings addressed

Thank you for the CFN-safety pass — checking stream enablement and GSI count against six refs is exactly the verification this slice needed, and I'm glad the in-place claims held up under it.

MINOR — user-facing retry copy named the wrong label. Fixed. This was the most valuable finding in the review: the hint told users to re-apply a hardcoded label while the shipped default is a different string and the label is per-project configurable, so a user following the instruction got silence. renderEpicPanel and upsertEpicPanel now take the resolved label and fall back to the platform default. I rendered the panel to read the output rather than trusting the source.

Note the existing test asserted the hardcoded string, which is why this shipped — so I fixed the assertion too and added two cases (a custom label, and the default fallback). All three fail if the hardcoded string comes back.

MINOR — taskTableForWrites is a prop that silently does nothing. Confirmed, removed. Your reasoning for removing rather than leaving it is right: the only reason to reach for it is to point writes at a different table, and that would appear to work while having no effect.

GSI projection — cost_usd. Genuinely useful framing: adding it now is free, later means a second index. I have not added it, and want to be explicit that this is a judgement call rather than an oversight. Four consumers pay a GetItem, two for that one field — but widening the projection makes every task write carry cost_usd into the index, and the field is written late and often on iteration-heavy tasks. The GetItem cost is bounded per your own analysis below; the write amplification is not as easy to bound. If you disagree I will take the change — you have looked at the consumers more closely than the original author did.

sumIterationCostForIssue — thank you for refuting the part that did not hold. The claim as I had it (batch multiplication in a stream consumer) was wrong, and reading the claim gate as an actual ConditionExpression is the check I should have made. The residual you identified — a silent undercount at the 1 MB page boundary rather than an error — is the real defect, correctly sized as non-blocking. Not fixed here; tracked, with your suggested fix (page loop, or an explicit "first N" note so the number shown is never quietly wrong).

Duplicated sumIterationCostForIssue. Correct, two copies with the same shape. Not de-duplicated in this slice because the copies live either side of a slice boundary; folding them together is a follow-up, and the page-limit fix should land in one place once they are merged.

MINOR — ~600 lines of dead rendering logic. Confirmed: seven exports with no src/ callers, invisible to the dead-code ratchet because each has a test. Not deleted here — that is a clean subtractive change, and I would rather it be its own reviewable commit than buried in a slice this size. Your rendering of the two dead functions is the part I want to flag as valuable: rollupKindFromChildren([]) returning complete, and terminal() counting failures toward "3/5 complete", would both be real bugs if anything called them. That turns "delete this" from tidiness into correctness, and I have recorded it that way.

MINOR — raw UUID on the fallback path. Checked renderEpicPanel's live path as you suggested; it uses the same display_id ?? sub_issue_id shape, so the exposure is real there too if a row lacks both. Left as-is for now (a row with neither is a data defect in its own right) but noted.

NIT — third instance of "cited artifact lands later". Your stack-level observation is the most useful thing in this review. Two of the three are addressed (the descriptor/YAML split, the model/grant split); the doc citations I have left, with reasoning on #654. The pattern is real and worth stating as a slicing rule: code must not land in an earlier slice than the artifact that authorises it.

isadeks · 2026-07-28T00:12:26Z

Operator note: this slice adds one GSI, but a STALE deployment may need two — DynamoDB allows one per update

Found while deploying this arc to a second environment. Correcting something I asserted earlier in this PR.

What I verified, and why it was the wrong baseline. I checked that TaskTable goes from 4 GSIs to 5 — adding only LinearIssueIndex — and that JiraIssueIndex already existed on main. Both true: JiraIssueIndex came in with #640, which is an ancestor of this carve's base, so it is not this arc's index. This slice adds exactly one GSI relative to main.

But DynamoDB does not constrain the delta against main. It constrains the delta against what is actually deployed:

Cannot perform more than one GSI creation or deletion in a single update

An environment whose stack predates #640 has three GSIs live (StatusIndex, IdempotencyIndex, UserStatusIndex). Deploying this branch there asks for two new indexes at once — JiraIssueIndex and LinearIssueIndex — which DynamoDB refuses. TaskTable goes UPDATE_FAILED and the whole stack rolls back. (The rollback is clean: the table is not replaced, and the newly-created orchestration resources are removed.)

The dev account did not surface this only because its stack was already current with main before the arc went on. That is exactly the kind of thing that hides on the machine where the work was done.

Operator guidance for merging/deploying this:

Confirm the target stack is current with main before deploying this slice. If it is not, deploy main first (which adds JiraIssueIndex alone), then this branch (which adds LinearIssueIndex alone).
This is per-deployment state, not a code change — nothing in the template needs fixing, and splitting the index across slices would not help, since the constraint is about the deployed baseline rather than the source.

Also worth recording, since it bears on how anyone verifies this: on that environment two cdk deploy runs printed exit code 0 while failing at Docker image build (a NodeSource CDN 403 that then poisoned the layer cache — pruning the build cache cleared it). Same trap hit on the dev account, where two runs exited 0 while failing at ECR asset publish. Check the CloudFormation stack status and LastUpdatedTime, not the CLI's exit code.

isadeks · 2026-07-28T01:47:44Z

Second-round review: my retry-hint fix was cosmetic — now actually wired

The reviewer checked whether any production caller passes the labelFilter I added. None did, and none could: all three upsertEpicPanel call sites omit it, and the reconciler has no project-mapping access — no mapping table in its props, and no mapping env var. So every user still saw the platform default and the hint was still wrong for any project that renamed its trigger label. The original finding was still open in production; my fix only moved the parameter into place.

Fixed properly. The label is now captured where the mapping is in hand — at seed time — and persisted on the meta row's release context; the reconciler reads it back and passes it to the panel. Rows seeded before the field existed simply have none and fall back to the platform default, as before.

Only the settled-panel call site is wired, deliberately: the retry hint renders only when the epic is terminal and something failed, and the other two sites render an in-progress panel that has no hint. I verified that rather than assuming it.

Guarded in both directions, and the read half is the one that mattered: dropping the write fails the seed test, but dropping the read hydration originally left the entire 3500-test suite green. Both are now pinned.

On taskTableForWrites — fair criticism that an unexplained API removal was bundled into a copy fix. It is genuinely dead (no caller on this branch, the next, or the source branch) and removing it was the right call, but it belonged in its own commit with the reasoning stated. Noted.

On the labelFilter markdown escaping — accepted as low severity and not changed: the value comes from a project mapping an admin sets, not from user input. Worth a backtick strip for consistency; tracked rather than done here.

Verified and not changed: the panelRow rename is clean and load-bearing (mutating panelLabel to ignore display_id fails 5 of the renamed tests), and the backward dependency on the decomposition module resolves at runtime, not just under tsc.

isadeks · 2026-07-28T10:53:55Z

Upgrade-ordering note added, from a second-environment deploy — and a correction

Deployed this arc to a different account (one whose stack was last updated at pre-#643 main) and hit the GSI limit I had reasoned about earlier from the wrong baseline:

Cannot perform more than one GSI creation or deletion in a single update

I had verified "exactly one new GSI" against main, which is true — this slice adds only LinearIssueIndex, and JiraIssueIndex came from #640, an ancestor of the carve base. But DynamoDB constrains the delta against what is deployed, not against main. A stack predating #640 has three GSIs live, so this branch asks for two at once and CloudFormation rolls the whole stack back.

Operator guidance: confirm the target stack is current with main before deploying this slice. If it is not, deploy main first, wait for JiraIssueIndex to reach ACTIVE (~10 min), then deploy this branch. Verified working in that order — UPDATE_COMPLETE, table TableId unchanged so no replacement, and the resulting GSI set is the four pre-existing plus LinearIssueIndex.

Nothing in the template needs changing, and splitting the index across slices would not help — the constraint is about the deployed baseline.

A correction to something I wrote earlier in this thread. I claimed a stray artifact URI "would fail closed anyway" because the reconciler's grant is scoped to artifacts/*. That is wrong, and I confirmed it against the live policy: S3 does not normalize keys, so artifacts/../traces/u/x is a literal key that an artifacts/* resource matches by string prefix — iam simulate-principal-policy returns allowed for exactly that shape. So the handler-side check was the only thing standing between a tampered task record and another user's trajectory, and a bare startsWith did not cover traversal. Now fixed to reject traversal segments (raw and percent-encoded), with a test — deleting the check previously left all 64 reconciler tests green.

…load the PDF Every PDF attachment was being refused with "Attachment '<name>' could not be stored." The S3 upload threw "Cannot perform Construct on a detached ArrayBuffer", and because attachment handling is fail-closed the whole task was rejected. pdf-parse transfers ownership of whatever it is handed to its worker, which DETACHES the underlying ArrayBuffer. It was handed `new Uint8Array(content.buffer, content.byteOffset, content.byteLength)` — a VIEW, which shares its backing store with the caller's Buffer. The caller uploads those same bytes to S3 after screening returns, so by then its buffer was dead. The existing comment reasoned about this and still got it wrong: it said "slice to the exact PDF bytes so a pooled Buffer's backing ArrayBuffer isn't handed over wholesale", which correctly avoids donating a whole pooled buffer but still hands over the caller's. A view narrows WHAT is transferred, not WHOSE it is. Now `Uint8Array.from(content)` — a copy. That costs one allocation of an already-size-capped payload, against silently rejecting every PDF upload. The unit tests could not catch this: they mock the PDFParse class, so no transfer ever happens and a view behaves like a copy. The new test asserts the OWNERSHIP boundary instead of the outcome — same bytes reach pdf-parse, different backing store — which is checkable without a real worker. Verified by restoring the view and watching it fail.

…straction The dormant foundation for parent/sub-issue orchestration: pure graph logic, the DynamoDB row store, and a surface-agnostic feedback layer. Nothing here is instantiated in a stack yet — the executor and the stack wiring land later, so this slice is synth-and-unit-test only and changes no deployed behaviour. Graph core (pure, no I/O): - orchestration-dag: validate a `depends_on` graph (duplicate id, dangling edge, cycle) and return its topological layering via Kahn's algorithm. The layers are what a reconciler releases children from, in dependency order. - orchestration-graph-source: the seam between "where the graph came from" and the executor. Three tiers — a tracker that has native sub-issues (read it), a caller that supplies the DAG declaratively (CLI/API, or a planner), or a structureless trigger (single task). - orchestration-integration-node: when a graph fans out to several leaves, append a synthetic node depending on all of them so there is one combined artifact instead of N unrelated PRs. - orchestration-epic-tip: where a newly-added, dependency-less node stacks — the leaf frontier, so it inherits the epic's accumulated unmerged work rather than branching off the default branch. - orchestration-base-branch: pick a child's base branch from its predecessors. Row store and table: - OrchestrationTable construct: orchestration_id (PK) + sub_issue_id (SK), with sparse GSIs to resolve a child back from its task id or its head branch. - orchestration-store: the read/write surface over those rows, including the idempotency markers that keep duplicate webhook deliveries from double-acting. - orchestration-log-events: structured events for a run, plus the scheduled sweep's backstop. - linear-subissue-fetch: read a parent's children and their `blocks` relations into a DAG. Fails loud rather than silently truncating an over-size epic. Channel abstraction: - orchestration-channel: the surface-agnostic interface — required operations every channel must implement, optional ones gated by declared capability, so the engine never branches on which tracker it is talking to. - orchestration-channel-{linear,jira,slack} + factory: per-surface adapters selected from the stored row. - orchestration-comment-trigger: parse a mention comment into a command, ignoring the bot's own comments. - iteration-reply: one maturing threaded reply per iteration rather than a stream of new comments; the two async writers of a reply converge instead of overwriting each other.

…it repo-wide The test-scope lint relaxation switched off four rules across all 152 test files to accommodate 13 new ones. `max-len` and `no-magic-numbers` are fair there — literal fixtures and long expected strings are the point of a test. `no-shadow` is not: it is correctness-adjacent in test code, where a shadowed `row` or `mock` inside a nested describe is a common way to assert against the wrong fixture and still pass. So `no-shadow` stays on, and the two shadows it found are fixed rather than silenced. One was a redundant local `makeDdb` identical to the file-level helper (deleted). The other was a local `child` that builds a persisted ROW shadowing a file-level `child` that builds a graph node — genuinely two different shapes under one name, which is exactly the hazard. Renamed to say which it is.

…ontain Review caught two inaccurate comments. Both were worth fixing, and looking for more of the same class turned up two others. The integration-node comment gave the synthetic id as `<orchestrationId>#integration` while the code builds `__integration`. Not a harmless typo: the `#` form is exactly what the comment thirty lines above warns will 400 the child, because the id flows into an idempotency key validated against /^[a-zA-Z0-9_-]{1,128}$/. A reader trusting the example would reach for the one separator that breaks it. Now points at the suffix constant so the two cannot drift again. Three comments referenced symbols that are not in this slice: - `findOrchestrationIds` — real, but in the stranded-orchestration reconciler, which lands later. Replaced the cross-reference with the property it was there to convey: every paginated read of this table must follow LastEvaluatedKey, because a single page is a silent partial answer rather than an error. - `discoverOrchestration` — the consumer lands with the compute plane; now described by role instead of by name. - `renderFailureReply` — called "the existing" renderer when nothing of that name exists here yet, which is the most misleading form: it reads as a claim about the current tree. Now describes it as arriving with the activation slice. A comment naming a symbol a reader cannot grep is a dead end, and in a stacked series it is an easy defect to introduce, since the symbol does exist on the branch the slice was carved from.

…ormant) Turns "one plain issue" into a reviewable sub-issue graph, and makes a long iteration legible while it runs. Like S5, none of this is wired into a stack yet — no handler calls it and no construct is instantiated, so this slice is unit-test-only and changes no deployed behaviour. Auto-decomposition — plan, review, revise, then run: - decomposition-mode: pure label parsing. The plain trigger label runs an issue as one task (or runs its existing sub-issue graph); `…:decompose` plans first and waits for approval; `…:auto` plans and starts immediately. A decompose suffix on an issue that already has sub-issues is a no-op. - decomposition-planner: parse and validate the plan a `coding/decompose-v1` agent emits as an artifact. Budget is DERIVED from each piece's S/M/L size rather than asked of the model, so the Σ is a stable, explainable ceiling. Edges are indices into the plan's own list, validated as a DAG before anything is created. - decomposition-caps: per-project limits. Over-cap REJECTS with a message and never trims — trimming can silently drop a node others depend on. - decomposition-render: the proposal comment, and every note around it. Plain English throughout: no "critical path" or "cost ceiling" jargon, the spend figure is framed as the safety limit it is, and each decline says what actually happened rather than a plausible-sounding stand-in. - decomposition-store: the pending plan survives between the propose webhook and the approve one. Create-once so a redelivery is a no-op; consume is a conditional delete-and-return so two racing approvals can't both write back. - decomposition-writeback: create the real sub-issues and their `blockedBy` relations. Idempotent and resumable — a planned node whose title already exists is reused, so a retry after a partial write-back doesn't double-create. - decomposition-flow: ties the above into caps → propose-or-seed, plus the approve/reject verdict path. All I/O injected, so the control flow is testable without Linear or DynamoDB. - plan-commands / plan-revise / plan-revise-interpret: a reviewer can edit the plan directly ("drop 3", "merge 1 and 2") or in prose. Edits apply deterministically to the CURRENT plan and the "What changed" line is a COMPUTED before→after diff — never a model self-report, which used to let a dropped piece quietly reappear with a fabricated justification. Iteration feedback: - iteration-heartbeat (+ construct and scheduled sweep): a long iteration used to show "starting on this" and then nothing until it finished. The sweep edits the SAME maturing reply in place to show elapsed time and the latest progress note — no new comments. Eligibility keys on the reply-routing fields, so standalone iterations are covered too, not just orchestrated ones. - iteration-reply-claim: claim a reply so two writers converge on one comment. - failure-reply: failure is answerable. A red build points at the build log by task id (the agent runs the build itself, and the target repo may have no CI); an agent crash gives the classified reason, a truncated excerpt, and whether to retry or escalate. Never the raw build output — that's untrusted repo code. - clarify-resume: resume a task that stopped to ask a question before spending.

… a dead regex **A private commit sha was the whole justification for skipping a PROMPT_ATTACK screen.** That is the one sentence a security reviewer needs to be able to follow, and it pointed at a commit no public reader can resolve. Replaced with the actual structural argument: the reviewer's instruction is embedded as delimited data inside a prompt whose only job is to classify it into a fixed edit vocabulary, the output is validated against that closed set before anything is applied, so a jailbreak cannot widen what the caller does — and a note not to reuse this shape where the output is executed rather than matched. **The multi-part conjunction regex could not match its own intended phrasings.** A trailing `\b` after the `plus,` and `;` alternatives requires a word character to follow, which inverts the intent: "the form, plus, a signup page" scored zero while "plus,a signup page" scored one, and "do a; b; c" scored zero while "do a;b;c" scored two. So the correctly-punctuated prose this is meant to catch was the case it missed, and both punctuation alternatives were dead weight. Word alternatives keep their boundaries; the punctuation ones no longer require one. **`latestProgressNote` was documented as a shipped capability but has no producer** anywhere in the stack — the sweep never sets it and there is no persisted attribute to read, so the heartbeat shows elapsed time only. The render path is written and tested, so rather than delete it or half-wire it, the field and the module/construct docs now say plainly that it is reserved and what wiring it needs. **`IterationHeartbeat` shipped without a construct test**, the one place in this slice where source and test coverage parted company. Added, following the repo's per-construct synth-assertion convention — including an assertion for the least-privilege claim the construct's own comment makes but nothing checked. Also: a cross-file line-number reference that pointed at a line which does not exist until a later slice (named the function instead), and the last of the private work-item shorthand.

…t overclaimed Two findings from a second review pass on my own fixes. **The conjunction regex now false-positived on pasted code.** Removing the trailing `\b` fixed the punctuation alternatives, but a bare `;` with no boundary matches EVERY semicolon — and an issue description written for a coding agent routinely contains a snippet, a stack trace or CSS. Four realistic single-task bug reports flipped to "multi-part" and would have nagged the user to decompose them. That is a false positive in the direction that costs attention, and my previous commit message argued for avoiding exactly that. The bare `;` is dropped. It never earned its place in either form: with the boundary it only matched unnatural no-space input, and without one it matched everything. `plus,` keeps the boundaryless form and the word alternatives keep theirs, which is enough for both real multi-part prose shapes; a numbered list is caught by the separate list-item path. A test now pins that a code paste stays single-task, which is what the previous test set never asserted. **The guardrail comment asserted a safety property the code did not have** — which is worse than the private commit sha it replaced, because a reviewer will trust prose. Three claims failed: the instruction was interpolated raw, so reviewer text containing the `"""` fence could close the data block and continue at prompt level; the free-text fields were type-checked but unbounded; and the computed diff is title-keyed, so an edit reusing an existing title reports as "modified" rather than "added" and cannot be relied on to surface a malicious edit. Rather than soften the prose, the first two are now true: the delimiter is neutralized in reviewer text and the instruction and returned fields are bounded. The comment names the real backstop — the downstream guardrail screen at task creation — and says plainly that the diff is a reviewer-facing summary, not a security control. All three new guards are mutation-verified.

…decomposition Decomposition stays on the development branch as experimental rather than landing on main: it presupposes one way of working — an issue split into a sub-issue graph with a human approval gate on the proposed plan — and main should not presuppose that. The iteration work in this slice does not: it applies to any task that opened a PR, however that task came to exist. Removes the 20 decomposition files (planner, caps, flow, store, types, writeback, render, the plan-command and plan-revise handlers, and their tests). What remains is the 12 files this slice is now about: the iteration heartbeat and its sweep, the reply claim, clarify-resume, and the failure reply. Two helpers in the deleted label module were NOT decomposition and had to survive: `DEFAULT_LABEL_FILTER`, which the project-mapping table treats as the default when a project sets no `label_filter` and the rollup renders in operator-facing copy, and `hasHelpLabel`, the one-time explainer label. They now live in `trigger-label.ts`, named for what they are. `triggerLabelVariants` was NOT kept — it existed to return the `:decompose` and `:auto` variants, so with those labels gone it would only ever return the bare base. Tests came across with the module, plus one pinning `DEFAULT_LABEL_FILTER`'s value directly. That constant is load-bearing in a way its size hides: changing it silently stops every project that never set a filter explicitly. Slice size drops from 8538 lines to 1918. The PR title and description need to change to match, since this is no longer a decomposition slice.

…, rollup The engine that actually runs a sub-issue graph: it watches child tasks reach a terminal state, releases whatever they unblocked, keeps the parent's status panel current, and re-stacks dependents when a branch moves. - orchestration-discovery: the composer that turns a trigger into a seeded run — read the graph, validate it, append the integration node if it fans out, then persist it and hand back the root set to start. - orchestration-reconcile: pure gating. Given a child that just went terminal, decide which dependents are now releasable, which must be skipped, and when the parent itself is done. Also covers two recovery shapes: comment-fixing a failed child un-fails it and re-releases the dependents that were skipped behind it, and re-triggering a finished epic retries only its failed/skipped children. - orchestration-release: create a child's task, with a claim so two concurrent releasers can't both launch it. A guardrail rejection or an un-onboarded repo is terminal with a reason the user can act on; a 5xx or a duplicate replay rolls back to ready so the sweep retries. - orchestration-rollup: the parent's single maturing panel — one comment that matures in place rather than a stream of updates. A failed row carries an indented sub-line saying what failed and where to read it. - orchestration-restack: when a child's branch changes, work out which dependents need their base moved. - orchestration-parent-comment: route a comment on the epic to the sub-issue it's about, and say so plainly when it looks like new work instead of a change. - orchestration-reconciler (handler + construct): the TaskTable-stream consumer that drives all of the above, with a DLQ for poison records, a filter so only terminal-status writes invoke it, and partial-batch reporting so one bad record can't re-drive its healthy siblings. - reconcile-stranded-orchestrations (handler + construct): a scheduled backstop for a run whose stream event was lost. Not dormant, unlike S5/S6 — read this part carefully: TaskTable gains a LinearIssueIndex GSI and has its stream enabled (NEW_IMAGE). Both are needed by this slice: the reconciler consumes that stream, and it queries that index to total an issue's iteration cost. The GSI's projection is deliberately narrow because a projection cannot be changed in place later. Enabling a stream and adding a GSI are both in-place CloudFormation updates, so no table replacement. The reconciler constructs themselves are NOT instantiated in any stack yet — the wiring lands in the next slice — so nothing consumes the stream on deploy.

The epic panel's retry fallback told the user to remove and re-apply a hardcoded label. The trigger label is per-project configurable through the project mapping's `label_filter`, and the platform default is a different string — so a user who followed the instruction re-applied a label the webhook does not filter on, and nothing happened. `renderEpicPanel` and `upsertEpicPanel` now take the resolved label and interpolate it, falling back to the platform default when a caller has none. Rendering the panel confirms the output; the existing test had asserted the hardcoded string, which is why it never caught this.

… hint The previous fix made `renderEpicPanel` take a `labelFilter` and interpolate it — but no production caller passed one, so in practice every user still saw the platform default and the hint was still wrong for any project that renamed its trigger label. The fix was cosmetic. The reconciler is where the panel is rendered, and it has no project id to look a mapping up with — it works from the orchestration row. So the label is now captured where the mapping IS in hand, at seed time, and persisted on the meta row's release context; the reconciler reads it back and passes it to the panel. Rows seeded before this field existed simply have no label and fall back to the platform default, as before. Only the settled-panel call site is wired, deliberately: the retry hint renders only when the epic is terminal AND something failed, and the other two call sites render an in-progress panel that has no hint. Guarded in both directions — dropping the write fails the seed test, dropping the read hydration fails the load test. The read half had no coverage until now, so removing it left the whole suite green.

… the default Review flagged trigger_label as only half-wired in this slice. Checking it, the gap is wider than that: the field is read, persisted, hydrated, and threaded to the panel, but NO code path on any branch ever assigns it a value — not this slice, not the activation slice, not the branch this was carved from. The whole plumbing was dead, so every epic's retry hint renders the default label regardless of what the project is configured to trigger on. That makes the hint actively misleading rather than merely generic: telling an operator to re-apply `bgagent` when their project triggers on something else sends them to a label that will not start anything. The seed site is in this slice, so the fix belongs here. The decompose task already carries its configuration through `channel_metadata` (mode, caps, revision round), so the label rides along the same way and is populated onto the release context at seed time. A blank or whitespace value is treated as absent so the renderer's default applies, rather than rendering an empty label in the hint — which also covers a task stamped before this field existed. The producer side (the webhook stamping `decompose_trigger_label` from the project mapping's resolved `label_filter`) lands with the webhook, in the activation slice. Until then this reads as absent and behaves exactly as before, so the two halves are safe to land apart in this order — consumer first, and never the reverse. Verified by deleting the parse and watching the assertion fail.

…raph EXECUTION With decomposition staying on the development branch, the reconciler no longer needs to build a graph — a graph on main is human-authored and seeded by the webhook. This removes the plan-consuming half and keeps the executing half. Gone: the planning-task event shape and its parser, the plan reconciler, the artifact fetch, the :auto seed path, and the seed-time attachment hydration. 656 contiguous lines, plus the six imports of the deleted planning modules and the stream-handler branch that dispatched to them. Kept, and worth naming because the file is now much smaller: terminal-child reconciliation, dependent release, the restack cascade, panel refresh and settle, failed-node recovery, and the iteration reply. Those are what the slice is for. The removal also made three things dead that were only ever used by seeding — the S3 client, the Bedrock screening config, and the artifacts-bucket env — so they went too. Net effect on IAM is a least-privilege improvement: nothing in this slice reads S3 any more. Two consumers needed repointing rather than deleting, since they were never about decomposition: - `orchestration-rollup` imported DEFAULT_LABEL_FILTER from the deleted label module; it now takes it from `trigger-label`. - A test asserted that a planning task falls through `parseTerminalTaskRecord`. The property it was really pinning is that the gate is the presence of orchestration_id, not the workflow id — a task outside a graph must not be mis-gated as a child and released against a graph it has nothing to do with. Rewritten around a plain terminal record so it still guards that. Also drops a probe mock that existed because the seed path failed closed on an attachment probe. With the seed gone the reconciler never probes, so the mock was asserting nothing.

…integration node The epic panel's preview came only from the synthetic integration node, so a chain got no preview at all — the case where it is most obviously wanted, because the reviewer is looking at the parent and a finished node holds exactly the artifact they want. An integration node exists only when a graph has SEVERAL leaves; it is there to merge them. With one leaf there is nothing to merge: everything already converged on the final node, so a synthetic node would re-run work that node just did and its "combined" preview would duplicate the leaf's own. That gate is correct. What was wrong is concluding "no integration node" means "no preview". The panel now falls back to the sole leaf. Deliberately only the SOLE leaf: with several leaves and no integration node (mid-flight, or an older row set) it still shows nothing, because picking one of several would present a single branch's result as the whole epic's — worse than showing nothing. Leaf-finding reuses `computeLeaves`, the same function the seeder uses to decide whether an integration node is needed, so the panel's idea of the final node cannot drift from the seeder's. The label is now conditional. "Combined preview" is only true when leaves were actually merged; on a chain it reads "Preview". Calling a chain's final-node preview "combined" tells a reviewer several branches were merged when none were. Both variants were RENDERED and read as output, not reviewed in source. Verified by mutation, three ways: reverting to integration-only, picking the first child instead of the leaf, and hardcoding the "Combined" label each fail the test and nothing else. Three existing panel tests changed expectations, which is the honest signal that this alters user-visible output rather than only adding a path.

…n child A released child was submitted with no workflow_ref, so it resolved to default/agent-v1 — the repo-OPTIONAL generic workflow. Every other channel (Linear, Jira, Slack) pins CODING_WORKFLOW_ID at its own call site precisely because of this; create-task-core.ts even says so in a comment. The orchestration release path was the one caller that never did. The consequence is not a clean failure. default/agent-v1's setup skips the stacked-base and predecessor-merge path entirely, so: - A CHAIN survives by luck. Its child needs no merge, works from the base it was handed, and the result looks correct. That is why this went unnoticed. - A DIAMOND does not. The integration node exists to merge two predecessor branches; it was handed both in channel_metadata, then given a clean default branch instead. The agent read a task description asking it to integrate, could not see either branch, and hand-wrote an approximation of ONE arm — silently dropping the other's work while producing a plausible-looking PR. Found live: a two-leaf epic's integration PR contained only the first arm's change, and the second arm's was absent from the merged file entirely. Everything upstream was correct — the node was seeded with both dependencies, selectBaseBranch returned shape 'diamond' with both branches, the task record carried both, and the orchestrator's payload wiring was deployed. Only the workflow was wrong. Two tests, one for a normal child and one for an integration node, asserting the REQUEST body rather than the context — nothing had asserted the body before, which is why the whole suite passed with the bug in place. Verified by deleting the pin and watching both fail.

… comment target the combined result Two fixes for one incident. An epic fanned out an API child and a UI child in parallel; each saw only its own sub-issue text and chose its own names — `kyoto` with `checkIn`/`checkOut` on one side, `wander-kyoto` with `startDate`/`endDate` on the other. Both passed their own tests, the branches merged with no conflict, and the deployed flow was broken. Neither child was wrong in isolation. The agreement existed only in the parent, which neither of them could see. Then the follow-up comment about the failure landed on the UI child, where the backend code was not present, so the agent adjusted how the error was displayed. SHARED CONTEXT. The epic's title and body are captured at seed time onto the meta row and prepended to every child's task description, delimited and labelled, with an explicit statement that siblings are working in parallel from the same text and that any name or shape it specifies is a contract rather than a suggestion. Seed time is the only point where the issue body is in hand — the reconciler releases from the stored row with no token to re-fetch it. Truncated at the single write site (4000 chars/field) for two independent reasons that both bind: the 400 KB DynamoDB item limit, and the fact that this text is guardrail-screened and counted against every child's prompt budget. Truncation is marked, so a reader never mistakes half a spec for the whole. The integration child gets it too — it is where a cross-boundary mismatch actually surfaces, and without the contract it cannot tell a real mismatch from two equally plausible conventions. TARGETING THE COMBINED RESULT. `parseParentNodeReference` already looked for `integration`/`combined`, but the keyword only ADMITTED the synthetic node and a title word still had to match. So `integration` worked by accident (it appears in the node's title) while `combined` — the word the panel itself shows, in "combined result", "Combined PR", "Combined preview" — matched nothing and fell through to a "which sub-issue?" reply. The node carries no Linear identifier, so that keyword is the only handle a user has. The keyword is now the match. The disambiguation reply also lists it, with the exact phrasing to type. It is listed but never auto-selected: the combined branch holds every sibling's work, so a misrouted change there is the most expensive kind to unpick, and an ambiguous comment must still ask rather than guess. A test asserts the advertised keyword and the accepted keyword are the same constant, because the failure mode of drift is a reply that tells someone to type a word the parser ignores. Verified by mutation: dropping the context, reverting the keyword to a gate, and hiding the hint each fail their own test and nothing else. Note the pre-existing keyword test passed against the gate behaviour — it asserted `integration` routes, which it did, for the wrong reason.

…l admits the child Caught on the first live run of the change: one child failed at admission with "Task description was blocked by content policy". The shared-context preamble was written as instructions to the agent — "use it exactly", "do not invent an alternative" — and stacked with the epic's own imperatives the whole task description trips the guardrail's PROMPT_ATTACK filter at MEDIUM confidence. The child never ran. That is a worse failure than the bug this feature fixes: the epic previously produced two children with mismatched contracts, and this version produced one child and one that could not start. Isolated it against the deployed guardrail rather than guessing. Each piece passes alone — the delimiters, the epic's own text, the child's own text, and every individual sentence of the preamble. It is the accumulation of instruction-shaped prose that trips it, and swapping only the preamble's voice flips the same assembled description from GUARDRAIL_INTERVENED to NONE with the epic text unchanged. Now descriptive: it says several sub-issues are being built in parallel, that each was given this text, and that the names in it are shared with those siblings — so an alternative chosen here would not match theirs. That carries the same constraint without addressing the model in the second person, and a model acts on it just as readily. A test asserts the VOICE, not the wording: no "use it exactly", no "do not invent", no "you must", while still requiring the parallel-siblings and shared-with framing. The failure mode this guards is subtle — imperative phrasing reads as an improvement and silently makes children unadmittable.

The predecessor slices were squash-merged into the shared base branch, which discards the commit history this branch shared with them. Git therefore sees the row-store module as added independently on both sides (add/add) rather than as one file with two sets of edits. Resolved to this branch's version for both files. That is lossless, not a preference: this branch's copy is a strict superset of the base's (+80 lines in the module and +114 in its suite, with zero deletions), so no line present on the base is dropped. Verified by diffing the resolved file against the base copy and confirming no removals.

…the terminal set Three review findings on the compute plane, all in the same area. The running-total cost sum queried the task index without following the pagination cursor, so past DynamoDB's 1 MB page boundary it summed one page and presented the result as the total. Truncation surfaced as a quietly wrong number rather than an error, which is the worse failure. It now paginates, and reports whether the figure is complete instead of leaving the caller to assume it. It also had no bound on the per-task read fan-out, on the reasoning that iteration counts are small. True today, but nothing enforced it, and the reads happen inside a stream handler with a two-minute budget. There is now a cap that trims the id list rather than merely stopping pagination: the projection is the task id alone, so a single page can hold far more rows than the cap, and breaking out without trimming would still fan out over the whole page. When the cap bites it says so and the total is flagged partial. The implementation moves to a shared module because a second near-copy lives in the fan-out dispatcher, and the two had already drifted — one parsed a string cost without a finite check, poisoning the total to NaN, while the other guarded correctly. Cost accounting that disagrees with itself depending on which handler ran is worse than either behaviour alone. The dispatcher adopts it in the activation slice. Separately, the handler restated the list of terminal statuses that the stream's event filter is built from. Two literal lists that must agree can drift, and either direction is silent: a status added to the filter but not honoured here is dropped on arrival, and one honoured here but absent from the filter never arrives. The set is now derived from the same constant. Tests: 11 for the cost sum, covering the page cursor, the cap bounding the fan-out both within and across pages, string and unparseable costs, read and query failures, and that this task is counted exactly once. Each was confirmed to fail against a deliberately broken version. The terminal set gets a guard driving every filtered status through the parser, plus a negative case so it cannot pass by accepting everything — verified by re-running the mutation that survived before it existed. Also drops an import left unused by the extraction.

isadeks · 2026-07-29T20:36:46Z

Review response — nits (a), (c), §6, and the new conflict

Thanks — this was a useful pass. Current tip: 3ac65d71. Two things reshaped the picture since the review, so I've noted where a finding no longer applies and why.

Decomposition was withdrawn from this carve entirely. It's staying on the experimental branch: it presupposes a single workstyle (issue → sub-issue graph → approval gate on a proposed plan), and main shouldn't. That removes the subject of nit (d) — reconcileDecomposePlan and the ack claim sharing a not-yet-seeded partition are gone, along with the planner, the plan commands and the revise loop. The reconciler is 1445 lines now, which also answers most of §8's proportionality concern: the ~500-line planning branch you identified as a separable concern is no longer in the file.

(c) — the cost fan-out. Fixed, and it was worse than "no cap". You were right that nothing bounded it. Two defects, not one:

The query never followed the pagination cursor, so past DynamoDB's 1 MB page boundary it summed one page and presented that as the total. Truncation surfaced as a quietly wrong number rather than an error.
Adding a cap that only stopped paginating would not have fixed the fan-out you flagged. The projection is task_id alone, so a single page holds far more rows than any sane cap — breaking out of the loop without trimming the id list still fans out over the whole page. The cap now trims.

When the cap bites, the total is flagged partial and the reply renders "total this PR: at least $X" rather than showing a short figure as settled. The implementation moved to a shared module, because your N2 on the activation slice was also right that a second near-copy existed in the fan-out dispatcher — and the two had already drifted: one parsed a string cost_usd without a finite check, poisoning the total to NaN, while the other guarded correctly.

11 tests, each confirmed to fail against a deliberately broken version.

§6 — the TERMINAL drift guard. Taken, but structurally. Rather than assert two literal lists stay equal, the handler now derives its set from the same constant the stream's FilterCriteria is built from, so they cannot drift. I added the guard anyway, driving every filtered status through the parser plus a negative case so it can't pass by accepting everything — worth doing, because the first version of the derived set still passed the full suite when I mutated TIMED_OUT out of it. Nothing exercised that status end-to-end. It does now.

(a) — trigger_label has no production writer in this slice. Correct, and correctly diagnosed as a staged-carve artifact: the seed-time write lives with the webhook wiring on the activation slice. Your read of the fallback is right — DEFAULT_LABEL_FILTER is what renders here, and it's tested.

(b) — the stream + GSI in one deploy. Confirmed, empirically rather than by reasoning. A raw UpdateTable does reject the combination, as you'd expect from the API reference — but CloudFormation serialises them: one UPDATE_IN_PROGRESS → UPDATE_COMPLETE, in place, no replacement. Verified on a live deployment of this stack, not a changeset dry-run.

§5 — the dangling design citation. Good catch. docs/research/orchestration-reconciler-correctness.md lands on the activation slice, so the reference resolves once the arc is merged but not at this commit.

One new thing, worth flagging as process rather than code. This PR was CONFLICTING when you looked, and not because of anything in the diff: the predecessor slices were squash-merged into the shared base, which discards the history this branch shared with them, so git saw the row store as independently added on both sides. Resolved toward this branch and verified lossless by diffing against the base copy and confirming zero removals — its version is a strict superset (+80 lines in the module, +114 in the suite, no deletions).

Full CDK suite green: 3310 tests, 167 suites.

The build gate fails on any file the build itself modifies, and the import added with the drift guard was placed after a lower-sorted path.

aws-samples#695) * feat(carve S1): foundation contracts for the orchestration arc (additive, dormant) Slice 1 of the linear-vercel→main carve. Purely additive shared-contract changes that every later orchestration slice references — nothing here is wired or activated, so behavior on main is unchanged. - types.ts: additive optional TaskRecord fields the orchestration/Mode-B/A6 arc reads (orchestration_id, parent_task_id, depends_on, linear_issue_id, code_changed, head_sha, screenshot_url, answer_text, …). All optional → back-compatible. - validation.ts: attachment MIME maps + size constants. Deliberately keeps the max-turns default at main's value (the arc's bump to a higher default is a live behavioral change and belongs in its own PR, not this dormant slice). - workflows.ts: register the read-only planning + restack workflow definitions + the readOnly flag on the workflow type. Registry metadata only — the workflow definitions + planners land in later slices, unused until then. - repo-config.ts: additive optional build_command / lint_command on the repo + blueprint config (per-repo build/lint verification commands the agent runs before opening a PR; default to the platform commands when unset). Gates: cdk compile + eslint + full jest green. No new imports, no runtime wiring, no behavioral change. * test(validation): cover the UTF-8 rejection path for text attachments The full-buffer UTF-8 check had no test for its failure branch: the existing cases only fed NUL bytes, which the null-byte scan catches on its own, so the `TextDecoder` throw path was never taken. Deleting the rejection entirely left the suite green. Three cases, each of which fails if the check regresses: - bytes that are not decodable UTF-8 but carry no NUL (a truncated multi-byte sequence, and 0xFF which never appears in UTF-8) must be rejected, - a 9 KB ASCII preamble must not launder trailing invalid bytes — this is what the whole-buffer check exists for, and it passes under a prefix-only scan, - legitimate multi-byte text must still be accepted, so the decode can't be "fixed" by rejecting everything non-ASCII. * fix: don't admit a workflow whose definition ships later DESCRIPTORS is the live admission table — create-task-core resolves any submitted workflow_ref against it — so an entry here with no corresponding `agent/workflows/**` file is worse than a missing entry: the submission succeeds with a 201, the caller is told the task started, and the agent then dies when it cannot load the workflow. An accepted task that cannot run is a worse experience than a clean rejection at submission. `coding/restack-v1` and `coding/decompose-v1` were declared here while their YAML lands in a later change, and there is no platform-only submission gate to stop a user asking for them by id. Move both entries to travel with their files. The drift test only walked YAML → descriptor, which is why the orphans were invisible, so add the reverse direction: every declared descriptor must have a shipped workflow file. Verified by re-adding an orphan and watching it fail. * chore: keep a node_modules SYMLINK out of git `node_modules/` with a trailing slash matches directories only, so a symlink of that name — e.g. one pointing at a shared install in a sibling worktree — is not ignored and `git add -A` will commit it. CI then fails at install time with `EEXIST: file already exists, mkdir '.../node_modules'`, which reads as a runner cache glitch rather than a tracked file. Match both forms, and drop the one that had been committed. * docs(validation): say why the two magic-byte functions scan different amounts Review raised that `validateMagicBytes` and `detectMimeTypeFromMagicBytes` now disagree about how much of the buffer to look at, with nothing explaining it — a reader is left to guess whether the 8 KB prefix scan in the detector is an oversight. It isn't, and a future "consistency" fix would make things worse. The detector's answer is a guess used to fill in a missing content type, and everything it returns is re-checked by `validateMagicBytes` over the whole buffer one call later. So scanning more bytes there buys no safety, while making it strict would turn a detection miss into a spurious rejection: it would return null for content that validation would have accepted. Also documents that the workflow model allow-list and the Bedrock IAM grant are independent lists, and that a model must be on both. Allow-listing alone lets a workflow pass admission and then fail at turn 0 with AccessDenied, which is a confusing place to learn about it. Adds a test that every bare allow-listed id is paired with its us- inference- profile form, and vice versa. An id admitted in only one form is a latent rejection, since admission compares the literal string a workflow pinned. Verified by deleting one pair and watching the assertion fail. * fix(validation): full-buffer-check inline attachments whose type was detected Review found the magic-bytes check gated on a DECLARED content_type, so an inline attachment with no declared type — where the type is guessed by detectMimeTypeFromMagicBytes — never got the whole-buffer validation. That leaves the weaker of the two paths unguarded. Detection only scans an 8 KB prefix for NUL bytes, so it is a guess; a declared type at least states an intent. Reproduced before fixing: ~8 KB of clean ASCII followed by 0x00 0xFF 0xFE 0x00, submitted with no content_type, was guessed text/plain and ADMITTED. That is precisely the laundering the UTF-8 hardening was written to stop, reaching the same handlers through the door next to the one that was locked. Now every inline attachment is validated, detected types included. Confirmed this does not cost the detection path its purpose: a real PNG and real JSON with no declared content_type still resolve and pass, so the fix does not trade a laundering hole for spurious rejections. Full suite green with no other test needing a change, which is the useful signal here. Also corrects the comment on the detector. It asserted that whatever detection returns is re-checked whole-buffer by validateMagicBytes — true as a description of intent, false on this path until now. It now records that the re-check is load-bearing and that the call site must not be narrowed back to declared types. Verified by restoring the old gate and watching the laundering assertion fail. * feat(carve S2): configurable ECS Fargate task sizing + read-only planning task (#653) * feat(carve S3): agent runtime — deterministic issue feedback, build/lint gate, restack workflow (#654) * feat(carve S4): Linear issue-context surface — attachments, PDF/image screening, auth health (#655) * feat(carve S5): orchestration DAG core, row store, and the channel abstraction (dormant) (#656) * feat(carve S6): iteration feedback — heartbeat, reply claim, clarify-resume, failure reply (#657) * feat(carve S2): ECS rightsized planning task def + configurable task sizing Slice 2 of the linear-vercel→main carve (stacked on S1 foundation contracts). Adds a second, smaller read-only "planning" Fargate task definition alongside the existing build task def, and routes read-only workflows to it so a clone-and-read task (which never builds) doesn't allocate the full build task's CPU/memory. - ecs-agent-cluster.ts: add the planning task def (2 vCPU / 8 GB) next to the build def; make BOTH task sizes overridable via a new optional `taskSizing` prop, with the current values as defaults. The defaults are generous (tuned for a large monorepo build); a consumer with a lighter repo can shrink the build task to cut Fargate cost, so sizing is configuration rather than a fixed value. - ecs-strategy.ts / compute-strategy.ts / orchestrate-task.ts: select the planning task def for read-only workflows on the ECS substrate (ignored by AgentCore); the read-only flag comes from the workflow registry (S1). - orchestrator.ts: thread the per-repo build/lint commands + read-only routing. Also rewrote the comments in these files to plain what/why so they're understandable without internal tracking-ticket context (this is public-sample code). Comment-only edits — verified the non-comment source is identical to the source branch. Gates: cdk compile + eslint + full jest (135 suites / 2522 tests) green. * fix(carve S2): de-opinionate the build task — modest default, overridable env Two findings from the automated review, both instances of one-deployment's measurements becoming everyone's default. **The default build task was the Fargate maximum** — 16 vCPU / 120 GB plus a 100 GiB root filesystem. The docstring was honest about why (tuned for a large TypeScript + Python monorepo) and pointed at the sizing prop, but a default is what an adopter who changes nothing pays for, and that is roughly 5x the per-build cost of a modest size in us-east-1 on-demand. Now 4 vCPU / 16 GB on Fargate's own 20 GiB disk, with the ceiling reachable through `taskSizing` and a test showing that path. Under-provisioning is a slow or OOM-ing build, which is diagnosable and one prop from fixed; over-provisioning is a silent bill. The planning default (2 vCPU / 8 GB) was already well judged and is unchanged. **Four build-tool env vars were hardcoded with no override.** A verify timeout and parallelism caps for a mise/jest/pytest-shaped build are meaningful only for the toolchain a given repo uses — inert for an adopter who uses none of them, and actively wrong for one who uses mise with plenty of memory, since the cap serialises their build to solve a memory problem they may not have. Added `extraBuildEnvironment`, merged over the defaults so a caller's key wins. The measured reasoning stays in the repo as the comment it always was; it is no longer welded into the construct. Also clarifies that "read-only planning task" describes the workflow's behaviour rather than a reduced-privilege role — both task defs deliberately share one role so grants cannot drift between them, and the name should not be mistaken for a privilege boundary. * fix(carve S2): make the sizing knobs actually reachable, and refuse platform env keys A second review pass found my de-opinionation had swapped one problem for its mirror image, plus a footgun. **The prop had no caller, so the ceiling was unreachable.** Lowering the default to 4 vCPU / 16 GB is only defensible if a large monorepo can raise it without editing the construct — but nothing passed `taskSizing`, `AgentStack` takes no custom props, and no context key read it. So the fix traded "everyone pays for 16 vCPU" for "nobody can reach 16 vCPU", and the second is worse for exactly the repo the old default existed to serve. Sizing and the build-tool env are now read from deploy context, the same shape as the `compute_type` gate beside them: cdk deploy -c compute_type=ecs -c ecsBuildTaskCpu=16384 \ -c ecsBuildTaskMemoryMiB=122880 -c ecsBuildTaskEphemeralStorageGiB=100 A malformed value throws at synth rather than silently falling back — "I set the flag and the build still OOM'd" is a worse afternoon than a failed synth. **`extraBuildEnvironment` could clobber platform wiring.** It spreads over the whole container env, not just the four build-tool vars it was added for, so a mistyped key could reach table names, bucket names, or `AGENT_SESSION_ROLE_ARN` — whose absence makes the agent fall back to ambient credentials with per-tenant scoping silently OFF, which the agent's own session module calls its most dangerous failure mode. Reserved keys are now rejected at synth. Also corrects the docstrings, which still advertised 16 vCPU / 120 GB / 100 GiB two lines above code saying 4096 / 16384 / 21 — whichever an operator read first was wrong. Reachability is asserted end to end (context → resolver → construct → template): unwiring the call site fails that test, where before it left the whole suite green. * fix(carve S2): give the build task real disk margin — 21 → 50 GiB Stress-tested the modest default on the workload the old 120 GB default existed for: a full parallel `mise run build` of this monorepo (agent + CDK + CLI + docs) on ECS, with the build gate actually running. memory 3132 MiB peak of 16384 → ~5x headroom disk 14.68 GiB peak of ~21 → ~1.4x headroom result build_passed, ECS exit 0, no OOM, no ENOSPC So CPU and memory are comfortably right, and for a reason worth recording: `MISE_JOBS=1` serialises the packages, so peak memory is max-single-package rather than sum-of-all. The old ceiling was never the binding constraint. Disk is the outlier. 70% of the requested figure consumed by a single build with a warm clone means a heavier dependency cache, or a second build sharing the task, plausibly runs it out of space — and running out of space surfaces as a spurious build FAILURE rather than an obvious resource error, which is the worst way for this to present. 50 GiB restores real margin, and ephemeral storage is a small fraction of the per-task cost next to vCPU and RAM, so the cost win that motivated the modest default is intact. Sized on measurement rather than on the assumption that the old numbers were all equally over-provisioned. The docstring records the figures, and a test pins the default so it cannot drift back to the floor unnoticed. * fix(carve S2): make the planning task def reachable in the slice that adds it Review found the planning task def inert in this PR: nothing sets ECS_PLANNING_TASK_DEFINITION_ARN, so the strategy's `readOnly && ECS_PLANNING_TASK_DEFINITION_ARN` guard is always falsy and read-only workflows keep running on the build def. The three wiring lines existed, but in the LAST slice of this series rather than this one, so the feature works at the series tip and on the branch this was carved from — which is why it live-tested fine — while this slice ships a task def that is billed for and never used. Merges land one at a time, so there is a real window where the def is deployed and unreachable, and anyone testing rightsized planning in that window would conclude the feature is broken. Moved the wiring here, to the slice that introduces the def. `planningTaskDefinitionArn` is required rather than optional, matching its siblings, so an ecsConfig that omits it fails to compile instead of silently disabling the routing. No IAM change: the def shares the build def's task and execution roles, and the ecs:RunTask grant is scoped by ecs:cluster rather than by task-definition ARN. Adds a stack-level assertion that the orchestrator's environment carries BOTH ARNs and that they differ. This has to be at the stack level: the strategy's unit tests set the env var by hand to exercise the routing branch, so they pass whether or not the stack supplies it. Verified by deleting the wiring line and watching both the test and tsc fail. Also corrects four docstrings that still described a 64 GB / 16 vCPU build default after it was lowered to 4 vCPU / 16 GB, one of which claimed a 64 GB task had been OOM-killed when the measured figure was 32 GB. Routes the ECS MAX_TURNS fallback through DEFAULT_MAX_TURNS instead of a literal 200. main uses 100, so the literal was an unflagged behavioral change, and it disagreed with the hydrate path whenever a payload omitted max_turns — a turn ceiling that depends on which code path filled it in. * docs(carve S2): describe artifact workflows by their contract, not by one workflow These comments used the decomposition planner as the example for the read-only planning task def and the artifacts bucket. With decomposition staying on the development branch, that names a workflow main will not have — so a reader looking for it finds nothing, and the comment reads as describing absent code rather than the general rule. The rule is what matters and is what the code keys on: a repo-ful workflow whose terminal outcome is an ARTIFACT clones for context and delivers a document instead of opening a PR, and a read-only workflow runs on the smaller task def. Reworded around that, using coding/pr-review-v1 where a concrete read-only example helps. Two tests used the planner purely as a convenient read-only example; repointed to pr-review-v1. The properties they pin are unchanged and still real: a read-only task inherits the repo's compute substrate rather than being downgraded to a different substrate family, and it runs on the planning def rather than the build def. * feat(carve S3): the agent runtime (Linear no-MCP, attachments, build/lint gate, plan/restack) Slice 3 of the carve — the full autonomous-agent runtime, landed as ONE unit. Stacked on S2. Unlike the CDK subsystems, the agent Python evolved as a tightly- coupled whole: the changes to pipeline.py / repo.py / post_hooks.py / shell.py / models.py interleave WITHIN shared files (a single pipeline.py carries the no-MCP, decompose-plan, restack, and clarify-resume hunks). Splitting it along the CDK subsystem lines produces non-compiling intermediate states, so it lands atomically: all 27 agent/src modules + their tests + the two workflow definitions. What it brings: - Linear no-MCP: strip the Linear MCP server; the platform pre-hydrates comments + attachments deterministically instead of the agent fetching them at runtime. - Per-repo build/lint verification gate before opening a PR (build_command / lint_command), with a structured verify outcome (passed / timed-out / infra-failed). - Stacked-child branch handling + predecessor re-merge (for the sub-issue DAG). - Agent-native decompose + restack workflows (clone, plan/re-stack, emit artifact). - Attachment download + screening + version-pinned integrity read. Behavioral note for reviewers: this also moves the default agent model and a couple of runtime defaults to their current values — intrinsic to the runtime as it runs today; called out here rather than split out, to keep the runtime internally consistent. Gates: agent ruff + ty + full pytest (1421 tests) green. Files are verbatim from the source branch (combination-verified: 0 diffs vs source). Comment cleanup for public readability follows as a separate commit on this branch. * test: tolerate either frozenset layout when cross-checking the writeable-workflow set The agent-vs-CDK cross-check for the writeable-workflow constant matched only a single-line `frozenset((...))`. The formatter renders the same literal across several lines once it grows past the line limit, and the pattern then found nothing — so the check failed on a purely cosmetic reformat instead of on a real drift between the two lists. Allow whitespace between the frozenset call and its inner tuple so the assertion tests the set's contents, which is what it is for. * fix(carve S3): keep the Jira app-identity work when applying the agent-side changes The agent-side files in this slice were taken wholesale from the branch that carries the orchestration work. That branch predates the Jira app-identity feature already on the target, so copying its versions over silently dropped that feature from all 13 agent files — the app-actor env plumbing, the per-task credential scrub, and their tests — while the CDK, CLI and Forge halves of the same feature stayed. The result would have merged as a half-reverted feature. Reapply the agent-side changes as a three-way merge against the common ancestor instead of a wholesale copy, so both survive: the app-identity path and this slice's own agent changes. Verified per file that the result is exactly "branch version + app-identity delta" with no other drift, and that a symbol removed on purpose here (the Linear MCP endpoint) stays removed. One docstring conflicted, describing how Jira comments get posted; kept the target's wording since it names the app actor and is now correct. * fix(carve S3): review findings — close a clone-guard bypass, grant the model the agent defaults to Three fixes from the automated review of this slice, plus its comment cleanup. **The self-reclone guard let `git clone -b` through.** `-b` is `gh`'s `--body` short form, so it sits in the free-text-argument list, but it is ALSO `git clone`'s own `--branch`. The guard truncated the command at the first free-text flag BEFORE scanning for the clone verb, so the cut landed ahead of the very verb it was looking for and an ordinary `git clone -b main <own repo>` was allowed — reopening the stranded-work failure the guard exists to prevent. Two more shapes slipped through for the same reason: `gh repo clone -b main <own repo>`, and a clone chained after an unrelated `-m` (`git commit -m wip && gh repo clone <own repo>`), where the `-m` belongs to the commit, not the clone. Now each shell segment is checked on its own, and within a segment the verb is located first — a verb only counts as prose if a free-text argument opens before it in that same segment. Prose in a `--body` value is still ignored. **The agent's fallback model was not in the Bedrock grant list.** This slice flips the fallback to Opus 4.8 for a repo that pins no model, but the IAM grant is derived from `DEFAULT_BEDROCK_MODEL_IDS`, and that entry was landing several changes later. Merging this slice on its own would have produced AccessDenied at turn 0 on every task with no per-repo model. The grant now travels with the default, and a drift guard reads the agent's own fallback and asserts the grant covers it, so the two cannot diverge again. **Two workflow descriptors arrived here rather than in the base slice**, matching where their `agent/workflows/**` definitions live. `DESCRIPTORS` is the live admission table, so an entry without its file means a submission is accepted with a 201 and then dies when the agent cannot load it. Also removes private tracker ids and work-item shorthand used as the load-bearing explanation in comments, docstrings, log messages and test names. The reasoning is preserved in plain language — a comment that recorded a real defect still records it, just without an id only this team can resolve. * fix(carve S3): the clone-guard rewrite broke two things it was meant to protect A second review pass on my own fix found that it closed the `-b` bypass while opening a new one and introducing a false positive. Both are worse than the original bug in a security guard, so both are fixed and pinned. **New bypass: a wrapped clone walked straight through.** Splitting on a bare newline separated the verb from the repo, so `git clone \` + newline + url never had the slug in the verb's segment. That command was BLOCKED before my rewrite and ALLOWED after it — including via the `-b` form the rewrite existed to catch. Backslash-continuations are now joined before scanning, which also covers a continuation splitting the verb itself (`git \` + newline + `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` or `-m` value spanning lines is how an agent normally writes a PR or commit 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 — exactly the false positive the free-text list exists to prevent. A bare newline is no longer a separator; the real separators still are, so a clone chained after an unrelated `-m` is still caught. Also removes `_CLONE_VERB_RE`, which was never load-bearing: across every segment shape it was the sole matcher zero times, and it actually MISSES a subshell (`( git clone …`) that the anchored pattern catches. Dead alternatives in a security path are worse than none. Both regressions are mutation-verified: restoring `\n` as a separator fails the multi-line-body test, and dropping the continuation join fails the wrapped-clone test. * fix(carve S3): report a signal-killed build as infra, not as broken code Review found the streamed path returning Popen's raw return code, which for a signal death is the NEGATIVE signal number (SIGKILL -> -9). The OOM classifier keys on the shell's 128+signal convention (137), so the two never met. This is reachable, not theoretical. A verify command only goes through a shell when it contains shell operators, and `mise run build` has none — so it is exec'd directly and its signal deaths arrive as -9. Reproduced: is_infra_failure returns False for -9 and True for 137, on identical input otherwise. The consequence is the exact mislabel that function exists to prevent. The container/cgroup OOM-killer writes "Killed process" to the KERNEL log, not the build's stderr, so an OOM'd build frequently has no stderr signature at all and the exit status is the only evidence. Left raw, that build is reported as a genuine failure — telling the user their code is broken when the box ran out of memory. Given the build tier's default was just lowered, this is the wrong moment to have OOM detection silently not fire. Normalized at the boundary where the signal is still visible rather than teaching each consumer both encodings, so SIGTERM (-15 -> 143) is covered too. Also replaces `proc.returncode or 0`, which mapped an unreaped None to 0. That cannot happen after wait(), but success is the one unsafe default for an unknown outcome, since it would let a build that never ran report as passing. Now -1. Tests assert 137 reaches is_infra_failure as infra, and that None maps to non-zero. Verified by restoring the old expression and watching the first fail. * refactor(carve S3): drop the decomposition workflow from the main-bound slices Decomposition is being kept on the development branch as experimental rather than landed on main. It presupposes one way of working — an issue split into a sub-issue graph with a human approval gate on the proposed plan — and main should not presuppose that. The rest of this arc is workstyle-neutral: iterating on a PR, heartbeats, clarify-resume, and the compute substrate work regardless. Removes from this slice the workflow YAML, its prompt, its registration in the prompt map, its DESCRIPTORS admission entry, and the two prompt-injection blocks that only a planning task used (the warm repo digest and the revise-round edit-in-place directive, both keyed on channel_metadata nothing else writes). Two things deliberately KEPT, because they are contracts rather than features: - The repo-ful artifact branch in the pipeline. It is selected by the workflow contract (terminal_outcomes.primary == artifact AND requires_repo), not by a workflow id, and no shipped workflow currently takes it. Deleting it would remove a declared capability and the failure mode is bad — an empty PR opened for a document task, or a build run that was never wanted. Its test now synthesizes the workflow by copying the real coding workflow and flipping only those two fields, so the test cannot drift from the production contract the way a hand-built stub would. Verified by forcing the gate false and watching it fail. - The trigger-label and :help helpers. Those are label plumbing, not decomposition. Tests that used the planning workflow purely as a convenient read-only or non-PR example now use coding/pr-review-v1, which is the surviving read-only workflow — the properties they assert (a read-only task inherits the repo's compute substrate; a non-PR workflow still takes the default clone path) are real and still worth pinning. Comments across the stack, cluster, orchestrator, and fan-out handlers described this branch in terms of the planning workflow; they now describe artifact workflows generally, since that is what the code actually keys on. No decompose reference survives in this slice's source or tests. * feat(carve S4): Linear issue-context surface — attachments, PDF/image screening, auth health Bring the Linear surface's issue-context and operator tooling onto main. All of this is reachable today through the existing Linear webhook path; none of it depends on the sub-issue orchestration arc, so it stands alone. - linear-attachments.ts / linear-issue-context-probe.ts: pull an issue's file attachments and recent comments into the task context, so the agent sees the screenshots and discussion a human would. - attachment-screening.ts: screen fetched attachments (images and PDFs) before they reach the model. This carries the pdf-parse v2 upgrade as one unit — the source uses the v2 `PDFParse` class, which requires pinning the dependency and dropping the hand-written module declaration that described the v1 function export. Splitting any of the three breaks the build, so they travel together. - error-classifier.ts: distinguish transient compute faults from user-visible build failures, and report whether work was preserved when a run does not produce a pull request. - screenshot-url.ts + github-screenshot-integration.ts: resolve screenshot URLs for the review path. - CLI: `linear-auth-health.ts` + `platform-doctor.ts` surface an expired or revoked Linear authorization as a diagnosable condition instead of silently dropped events; `linear.ts` / `platform.ts` expose it. The comments and test names in these files are rewritten to explain the what and why directly rather than pointing at internal tracker ids, and two CLI strings that printed an internal issue number to user-facing output are reworded. Stacked on the agent-runtime slice. Full CDK suite 2598 passing, CLI 675 passing, both type-checks clean. * fix(carve S4): review findings — two attachment defects, and stop shipping an inert marker **A quadratic regex could blow the webhook timeout.** Making the leading `!` optional in the markdown-link pattern stopped the engine anchoring each attempt on a literal `!`, so the unbounded label quantifier retried from every `[`. Measured on a description of unmatched brackets: 4 KB ≈ 7 ms, 12 KB ≈ 56 ms, 50 KB ≈ 940 ms — and around 150 KB exceeds the processor's 30 s timeout. No crafting is needed; a large pasted table does it, and the existing scan cap does not bound it because the backtracking happens inside a single `exec()` before any match is produced. The label quantifier is now bounded, which caps the work per start position while still matching every real markdown label. **Two attachments whose names differ only by `.` versus `-` silently became one.** The upload id collapsed `.`, `-` and `/` onto the same character and then squashed runs, so `design.v1.png` and `design-v1.png` produced an identical id — and both de-dupe sites resolve a collision by discarding the second file. A user who attached both got one, with nothing logged and no warning, and the agent planned against incomplete material. The id now carries a short digest of the full pathname, keeping the readable stem for logs. Both fixes are pinned by tests that were mutation-verified: reverting either change fails its own test and nothing else. **The revoked-authorization marker was on by default and could never succeed.** Every Lambda that resolves a token holds read-only registry access, and no slice in this arc grants write, so the marker's write failed AccessDenied on every revoked refresh and the failure was swallowed. The feature read as working while being permanently inert, which is worse than being visibly absent. It is now opt-in: a caller that holds the grant passes a recorder explicitly, and the default is to attempt nothing. When the grant lands, flip the default in that same change. * fix(carve S4): bound the scan input, not just the label — the label bound lost attachments A second review pass found my markdown-regex fix was wrong in both directions. **It silently dropped attachments.** A 300-char label bound makes a link with longer alt text stop matching entirely, so the file vanishes with nothing logged and the agent plans against incomplete material. That is the failure this slice argues elsewhere is the worst kind, and I introduced it while fixing a slow scan. The bound is now 4096 — far past any real label — and a test pins that a ~780-char descriptive label still yields its attachment. **And it did not fix the slow scan.** BOTH variable parts of the pattern backtrack per start position, and the URL part dominates: on `[](https://a` repeated, a 100 KB description takes ~820 ms with or without a label bound, and larger inputs still exceed the webhook timeout. Bounding the label only fixed the one shape the new test happened to assert. The real bound is on the input: descriptions longer than 64 KB have their tail left unscanned, and the truncation is LOGGED rather than passed over in silence. That fixes every hostile shape at once. Both are mutation-verified — restoring the tight bound fails the long-label test, removing the cap fails the URL-shape test. Two timing assertions are replaced with assertions on the observable work (the truncation warning). A wall-clock budget inside a parallel suite measures CPU contention as much as the code: both passed alone and failed under full-suite load, which would have been a flaky gate rather than a guard. * refactor(carve S4): drop the decomposition config fields from the table docs The project-mapping table's field list documented decompose_allowed, max_sub_issues, and max_parent_budget_usd. Nothing in the main-bound slices reads them, since decomposition stays on the development branch as experimental, and a doc describing configuration the code does not honour is worse than no doc — an operator would set those fields and wait for behaviour that never comes. The table is schemaless apart from its partition key, so this removes only prose; any row carrying those fields is still read back untouched. * refactor(carve S4): drop the CLI flags for a feature main will not have `linear onboard-project` offered --decompose-allowed, --max-sub-issues, and --max-parent-budget-usd. With decomposition staying on the development branch, those wrote project-mapping fields nothing reads. Worse than inert: the command printed "Auto-decomposition: ENABLED" and told the operator which labels to apply, so the only feedback they got said it had worked. They would then label an issue and get an ordinary single task, with nothing anywhere explaining why. Found by widening the search past cdk/ and agent/, where I had been looking. The table itself is schemaless, so a project row that already carries these fields is still read back unharmed — they are simply ignored. * fix(carve S4): give pdf-parse its own copy so the caller can still upload the PDF Every PDF attachment was being refused with "Attachment '<name>' could not be stored." The S3 upload threw "Cannot perform Construct on a detached ArrayBuffer", and because attachment handling is fail-closed the whole task was rejected. pdf-parse transfers ownership of whatever it is handed to its worker, which DETACHES the underlying ArrayBuffer. It was handed `new Uint8Array(content.buffer, content.byteOffset, content.byteLength)` — a VIEW, which shares its backing store with the caller's Buffer. The caller uploads those same bytes to S3 after screening returns, so by then its buffer was dead. The existing comment reasoned about this and still got it wrong: it said "slice to the exact PDF bytes so a pooled Buffer's backing ArrayBuffer isn't handed over wholesale", which correctly avoids donating a whole pooled buffer but still hands over the caller's. A view narrows WHAT is transferred, not WHOSE it is. Now `Uint8Array.from(content)` — a copy. That costs one allocation of an already-size-capped payload, against silently rejecting every PDF upload. The unit tests could not catch this: they mock the PDFParse class, so no transfer ever happens and a view behaves like a copy. The new test asserts the OWNERSHIP boundary instead of the outcome — same bytes reach pdf-parse, different backing store — which is checkable without a real worker. Verified by restoring the view and watching it fail. * feat(carve S5): orchestration DAG core, row store, and the channel abstraction The dormant foundation for parent/sub-issue orchestration: pure graph logic, the DynamoDB row store, and a surface-agnostic feedback layer. Nothing here is instantiated in a stack yet — the executor and the stack wiring land later, so this slice is synth-and-unit-test only and changes no deployed behaviour. Graph core (pure, no I/O): - orchestration-dag: validate a `depends_on` graph (duplicate id, dangling edge, cycle) and return its topological layering via Kahn's algorithm. The layers are what a reconciler releases children from, in dependency order. - orchestration-graph-source: the seam between "where the graph came from" and the executor. Three tiers — a tracker that has native sub-issues (read it), a caller that supplies the DAG declaratively (CLI/API, or a planner), or a structureless trigger (single task). - orchestration-integration-node: when a graph fans out to several leaves, append a synthetic node depending on all of them so there is one combined artifact instead of N unrelated PRs. - orchestration-epic-tip: where a newly-added, dependency-less node stacks — the leaf frontier, so it inherits the epic's accumulated unmerged work rather than branching off the default branch. - orchestration-base-branch: pick a child's base branch from its predecessors. Row store and table: - OrchestrationTable construct: orchestration_id (PK) + sub_issue_id (SK), with sparse GSIs to resolve a child back from its task id or its head branch. - orchestration-store: the read/write surface over those rows, including the idempotency markers that keep duplicate webhook deliveries from double-acting. - orchestration-log-events: structured events for a run, plus the scheduled sweep's backstop. - linear-subissue-fetch: read a parent's children and their `blocks` relations into a DAG. Fails loud rather than silently truncating an over-size epic. Channel abstraction: - orchestration-channel: the surface-agnostic interface — required operations every channel must implement, optional ones gated by declared capability, so the engine never branches on which tracker it is talking to. - orchestration-channel-{linear,jira,slack} + factory: per-surface adapters selected from the stored row. - orchestration-comment-trigger: parse a mention comment into a command, ignoring the bot's own comments. - iteration-reply: one maturing threaded reply per iteration rather than a stream of new comments; the two async writers of a reply converge instead of overwriting each other. * fix(carve S5): keep no-shadow enforced in tests rather than relaxing it repo-wide The test-scope lint relaxation switched off four rules across all 152 test files to accommodate 13 new ones. `max-len` and `no-magic-numbers` are fair there — literal fixtures and long expected strings are the point of a test. `no-shadow` is not: it is correctness-adjacent in test code, where a shadowed `row` or `mock` inside a nested describe is a common way to assert against the wrong fixture and still pass. So `no-shadow` stays on, and the two shadows it found are fixed rather than silenced. One was a redundant local `makeDdb` identical to the file-level helper (deleted). The other was a local `child` that builds a persisted ROW shadowing a file-level `child` that builds a graph node — genuinely two different shapes under one name, which is exactly the hazard. Renamed to say which it is. * docs(carve S5): fix comments that describe code this slice does not contain Review caught two inaccurate comments. Both were worth fixing, and looking for more of the same class turned up two others. The integration-node comment gave the synthetic id as `<orchestrationId>#integration` while the code builds `__integration`. Not a harmless typo: the `#` form is exactly what the comment thirty lines above warns will 400 the child, because the id flows into an idempotency key validated against /^[a-zA-Z0-9_-]{1,128}$/. A reader trusting the example would reach for the one separator that breaks it. Now points at the suffix constant so the two cannot drift again. Three comments referenced symbols that are not in this slice: - `findOrchestrationIds` — real, but in the stranded-orchestration reconciler, which lands later. Replaced the cross-reference with the property it was there to convey: every paginated read of this table must follow LastEvaluatedKey, because a single page is a silent partial answer rather than an error. - `discoverOrchestration` — the consumer lands with the compute plane; now described by role instead of by name. - `renderFailureReply` — called "the existing" renderer when nothing of that name exists here yet, which is the most misleading form: it reads as a claim about the current tree. Now describes it as arriving with the activation slice. A comment naming a symbol a reader cannot grep is a dead end, and in a stacked series it is an easy defect to introduce, since the symbol does exist on the branch the slice was carved from. * feat(carve S6): auto-decomposition planning and iteration feedback (dormant) Turns "one plain issue" into a reviewable sub-issue graph, and makes a long iteration legible while it runs. Like S5, none of this is wired into a stack yet — no handler calls it and no construct is instantiated, so this slice is unit-test-only and changes no deployed behaviour. Auto-decomposition — plan, review, revise, then run: - decomposition-mode: pure label parsing. The plain trigger label runs an issue as one task (or runs its existing sub-issue graph); `…:decompose` plans first and waits for approval; `…:auto` plans and starts immediately. A decompose suffix on an issue that already has sub-issues is a no-op. - decomposition-planner: parse and validate the plan a `coding/decompose-v1` agent emits as an artifact. Budget is DERIVED from each piece's S/M/L size rather than asked of the model, so the Σ is a stable, explainable ceiling. Edges are indices into the plan's own list, validated as a DAG before anything is created. - decomposition-caps: per-project limits. Over-cap REJECTS with a message and never trims — trimming can silently drop a node others depend on. - decomposition-render: the proposal comment, and every note around it. Plain English throughout: no "critical path" or "cost ceiling" jargon, the spend figure is framed as the safety limit it is, and each decline says what actually happened rather than a plausible-sounding stand-in. - decomposition-store: the pending plan survives between the propose webhook and the approve one. Create-once so a redelivery is a no-op; consume is a conditional delete-and-return so two racing approvals can't both write back. - decomposition-writeback: create the real sub-issues and their `blockedBy` relations. Idempotent and resumable — a planned node whose title already exists is reused, so a retry after a partial write-back doesn't double-create. - decomposition-flow: ties the above into caps → propose-or-seed, plus the approve/reject verdict path. All I/O injected, so the control flow is testable without Linear or DynamoDB. - plan-commands / plan-revise / plan-revise-interpret: a reviewer can edit the plan directly ("drop 3", "merge 1 and 2") or in prose. Edits apply deterministically to the CURRENT plan and the "What changed" line is a COMPUTED before→after diff — never a model self-report, which used to let a dropped piece quietly reappear with a fabricated justification. Iteration feedback: - iteration-heartbeat (+ construct and scheduled sweep): a long iteration used to show "starting on this" and then nothing until it finished. The sweep edits the SAME maturing reply in place to show elapsed time and the latest progress note — no new comments. Eligibility keys on the reply-routing fields, so standalone iterations are covered too, not just orchestrated ones. - iteration-reply-claim: claim a reply so two writers converge on one comment. - failure-reply: failure is answerable. A red build points at the build log by task id (the agent runs the build itself, and the target repo may have no CI); an agent crash gives the classified reason, a truncated excerpt, and whether to retry or escalate. Never the raw build output — that's untrusted repo code. - clarify-resume: resume a task that stopped to ask a question before spending. * fix(carve S6): review findings — explain the guardrail exception, fix a dead regex **A private commit sha was the whole justification for skipping a PROMPT_ATTACK screen.** That is the one sentence a security reviewer needs to be able to follow, and it pointed at a commit no public reader can resolve. Replaced with the actual structural argument: the reviewer's instruction is embedded as delimited data inside a prompt whose only job is to classify it into a fixed edit vocabulary, the output is validated against that closed set before anything is applied, so a jailbreak cannot widen what the caller does — and a note not to reuse this shape where the output is executed rather than matched. **The multi-part conjunction regex could not match its own intended phrasings.** A trailing `\b` after the `plus,` and `;` alternatives requires a word character to follow, which inverts the intent: "the form, plus, a signup page" scored zero while "plus,a signup page" scored one, and "do a; b; c" scored zero while "do a;b;c" scored two. So the correctly-punctuated prose this is meant to catch was the case it missed, and both punctuation alternatives were dead weight. Word alternatives keep their boundaries; the punctuation ones no longer require one. **`latestProgressNote` was documented as a shipped capability but has no producer** anywhere in the stack — the sweep never sets it and there is no persisted attribute to read, so the heartbeat shows elapsed time only. The render path is written and tested, so rather than delete it or half-wire it, the field and the module/construct docs now say plainly that it is reserved and what wiring it needs. **`IterationHeartbeat` shipped without a construct test**, the one place in this slice where source and test coverage parted company. Added, following the repo's per-construct synth-assertion convention — including an assertion for the least-privilege claim the construct's own comment makes but nothing checked. Also: a cross-file line-number reference that pointed at a line which does not exist until a later slice (named the function instead), and the last of the private work-item shorthand. * fix(carve S6): the regex fix over-corrected, and the guardrail comment overclaimed Two findings from a second review pass on my own fixes. **The conjunction regex now false-positived on pasted code.** Removing the trailing `\b` fixed the punctuation alternatives, but a bare `;` with no boundary matches EVERY semicolon — and an issue description written for a coding agent routinely contains a snippet, a stack trace or CSS. Four realistic single-task bug reports flipped to "multi-part" and would have nagged the user to decompose them. That is a false positive in the direction that costs attention, and my previous commit message argued for avoiding exactly that. The bare `;` is dropped. It never earned its place in either form: with the boundary it only matched unnatural no-space input, and without one it matched everything. `plus,` keeps the boundaryless form and the word alternatives keep theirs, which is enough for both real multi-part prose shapes; a numbered list is caught by the separate list-item path. A test now pins that a code paste stays single-task, which is what the previous test set never asserted. **The guardrail comment asserted a safety property the code did not have** — which is worse than the private commit sha it replaced, because a reviewer will trust prose. Three claims failed: the instruction was interpolated raw, so reviewer text containing the `"""` fence could close the data block and continue at prompt level; the free-text fields were type-checked but unbounded; and the computed diff is title-keyed, so an edit reusing an existing title reports as "modified" rather than "added" and cannot be relied on to surface a malicious edit. Rather than soften the prose, the first two are now true: the delimiter is neutralized in reviewer text and the instruction and returned fields are bounded. The comment names the real backstop — the downstream guardrail screen at task creation — and says plainly that the diff is a reviewer-facing summary, not a security control. All three new guards are mutation-verified. * refactor(carve S6): reduce this slice to the iteration work, without decomposition Decomposition stays on the development branch as experimental rather than landing on main: it presupposes one way of working — an issue split into a sub-issue graph with a human approval gate on the proposed plan — and main should not presuppose that. The iteration work in this slice does not: it applies to any task that opened a PR, however that task came to exist. Removes the 20 decomposition files (planner, caps, flow, store, types, writeback, render, the plan-command and plan-revise handlers, and their tests). What remains is the 12 files this slice is now about: the iteration heartbeat and its sweep, the reply claim, clarify-resume, and the failure reply. Two helpers in the deleted label module were NOT decomposition and had to survive: `DEFAULT_LABEL_FILTER`, which the project-mapping table treats as the default when a project sets no `label_filter` and the rollup renders in operator-facing copy, and `hasHelpLabel`, the one-time explainer label. They now live in `trigger-label.ts`, named for what they are. `triggerLabelVariants` was NOT kept — it existed to return the `:decompose` and `:auto` variants, so with those labels gone it would only ever return the bare base. Tests came across with the module, plus one pinning `DEFAULT_LABEL_FILTER`'s value directly. That constant is load-bearing in a way its size hides: changing it silently stops every project that never set a filter explicitly. Slice size drops from 8538 lines to 1918. The PR title and description need to change to match, since this is no longer a decomposition slice. * feat(carve S7): the orchestration compute plane — reconciler, release, rollup (#659) * feat(carve S8): wire the orchestration arc into the stack and activate it (#662) * style: drop a stray blank line left by the merge resolution The build gate rejects any file the build itself modifies, and the union of the two conflicting test blocks left a double blank line. * fix(agent): keep log-delivery ids stable by default, not behind a flag Updating an already-deployed stack failed at ``AWS::Logs::DeliverySource AlreadyExists`` and rolled the whole stack back. The AgentCore Runtime auto-creates a DeliverySource per logging config and names it after the construct path the library uses internally, so a library-side rename renames these resources — and CloudFormation creates a renamed resource before deleting the old one. A DeliverySource is unique per (resource ARN, log type) account-wide and the runtime ARN is untouched by the rename, so the new source is a second source for the same runtime and CloudWatch Logs refuses it. Worth stating plainly, because it is not intuitive: renaming these resources can never avoid the collision. The conflict is on the ARN they point at, not on their own names. Only holding the logical id steady avoids it, because that is what makes CloudFormation update in place instead of creating a duplicate. A mechanism for this already existed but was opt-in, via ``-c pinnedLogDeliveryStack=<name>``. That inverts the default: the safe path was the one an operator had to already know about, and the failure that would teach them is a mid-update rollback whose message never mentions the flag. I hit exactly that on a deploy of this branch, with the code in front of me. The pin now applies on a plain ``cdk deploy``, and the flag is gone. The opt-in existed for a real reason, so to be explicit about why keying on the stack's own name is safe. A stack that owns these resources gets ids matching what CloudFormation recorded, and updates in place — the broken case. A fresh stack or account owns none of these names yet, so they create normally; the ids being ours rather than the library's is the point, and the values carry no meaning beyond being stable. Any other stack name finds no table entry and keeps the library's naming, which is correct for it, so two stacks can never claim one account-unique name. Verified against the live stack: a diff with no flags shows zero delivery-source destroys, where before it destroyed and recreated all six delivery resources. The two remaining destroys are ordinary version churn. The old test asserted the opt-in was required, so it is replaced by tests for the behaviour that matters — pinned with no context, and no leakage into a stack that has no recorded ids. Both are confirmed to fail against a version that reintroduces a gate, and against one where a pinned id is "tidied" into the library's name. * fix(ci): stop a coverage-file race failing the agent image fingerprint, clear an advisory Two CI failures, neither caused by the change they landed on. The build failed in the ECS cluster suite with ENOENT on ``agent/.coverage.<host>.<pid>.<random>``. pytest-cov writes one temp file per process and deletes them as it combines; the agent suite and the CDK suite run in parallel, and the CDK suite fingerprints the agent tree for the image asset. So the fingerprint walk stats a file that coverage then removes, and the build dies on a path nothing will ever read. Timing-dependent, which is why it had not surfaced before — the tree is walked in the same seconds the coverage run is tearing down. Fixed by excluding coverage data from the fingerprint in two places, because the tree is used two ways: the root .dockerignore covers the real image (context is the repo root) and already excluded ``coverage/`` and ``.pytest_cache/`` as DIRECTORIES, which never matched these files; a new agent/.dockerignore covers the four CDK test call sites that fingerprint ``agent/`` on its own, where the root file does not apply. Verified by causation, not just by a green run: with a stray ``.coverage.*`` file present the asset hash is unchanged, and with the new .dockerignore removed the same file changes the hash. So the exclusion is what fixes it. Separately, osv-scanner flagged brace-expansion 5.0.8 (GHSA-rgw5-rvv9-x895) in both lockfiles. Pre-existing — main pins the same version — and an advisory published against a dependency already there rather than anything introduced here. Bumped via the mechanisms already in use for exactly this, yarn ``resolutions`` and npm ``overrides``, so it stays transitive. Using ``yarn upgrade`` / ``npm install`` would have added brace-expansion as a direct dependency of a package that does not use it.

isadeks force-pushed the carve/s7-orchestration-plane branch from 9302c20 to 59ba7e2 Compare July 27, 2026 12:54

isadeks force-pushed the carve/s6-decompose-iteration branch from b51246e to 18bcf34 Compare July 27, 2026 13:28

isadeks force-pushed the carve/s7-orchestration-plane branch from 59ba7e2 to bc71750 Compare July 27, 2026 13:28

isadeks force-pushed the carve/s6-decompose-iteration branch from 18bcf34 to 4581e88 Compare July 27, 2026 16:05

isadeks force-pushed the carve/s7-orchestration-plane branch from bc71750 to c9ab31c Compare July 27, 2026 16:05

isadeks mentioned this pull request Jul 27, 2026

feat(carve S5): orchestration DAG core, row store, and the channel abstraction (dormant) #656

Merged

isadeks force-pushed the carve/s6-decompose-iteration branch from 4581e88 to 8b85d8e Compare July 27, 2026 17:28

isadeks force-pushed the carve/s7-orchestration-plane branch from c9ab31c to f28f5ff Compare July 27, 2026 17:28

isadeks force-pushed the carve/s6-decompose-iteration branch from 8b85d8e to 32e969a Compare July 27, 2026 17:51

isadeks force-pushed the carve/s7-orchestration-plane branch from f28f5ff to 007e462 Compare July 27, 2026 17:52

isadeks force-pushed the carve/s7-orchestration-plane branch from 007e462 to 8f7f73f Compare July 27, 2026 17:56

isadeks force-pushed the carve/s6-decompose-iteration branch from 32e969a to f5afe7e Compare July 27, 2026 18:40

isadeks force-pushed the carve/s7-orchestration-plane branch from 8f7f73f to a510000 Compare July 27, 2026 18:40

isadeks force-pushed the carve/s6-decompose-iteration branch from f5afe7e to 0fb2cc1 Compare July 28, 2026 01:42

isadeks force-pushed the carve/s7-orchestration-plane branch from 62e29fc to 13338b8 Compare July 28, 2026 01:42

isadeks mentioned this pull request Jul 28, 2026

epic(orchestration): land the linear-vercel orchestration arc on main as 8 reviewable slices #668

Closed

4 tasks

isadeks force-pushed the carve/s6-decompose-iteration branch from 0fb2cc1 to b1410ea Compare July 28, 2026 12:17

isadeks force-pushed the carve/s7-orchestration-plane branch from 13338b8 to 3b1863f Compare July 28, 2026 12:17

isadeks marked this pull request as ready for review July 28, 2026 12:55

isadeks requested review from a team as code owners July 28, 2026 12:55

isadeks added 17 commits July 29, 2026 16:30

isadeks force-pushed the carve/s6-decompose-iteration branch from c5a6d3c to e48273b Compare July 29, 2026 15:32

isadeks force-pushed the carve/s7-orchestration-plane branch from 4415f8f to e8aa93c Compare July 29, 2026 15:33

Base automatically changed from carve/s6-decompose-iteration to carve/s1-foundation-contracts July 29, 2026 18:54

isadeks requested a review from scottschreckengaust July 29, 2026 20:11

isadeks added 2 commits July 29, 2026 21:20

style(carve S7): order the new test import as the linter does

e7ddc01

The build gate fails on any file the build itself modifies, and the import added with the drift guard was placed after a lower-sorted path.

isadeks merged commit e2d3f69 into carve/s1-foundation-contracts Jul 30, 2026
4 checks passed

isadeks deleted the carve/s7-orchestration-plane branch July 30, 2026 19:27

github-actions Bot mentioned this pull request Aug 1, 2026

Monthly PR metrics report #691

Open

isadeks mentioned this pull request Aug 3, 2026

feat(orchestration): land the carved orchestration arc on main (S2–S8) #695

Merged

Navigation Menu

Uh oh!

feat(carve S7): the orchestration compute plane — reconciler, release, rollup - #659

feat(carve S7): the orchestration compute plane — reconciler, release, rollup#659
isadeks merged 38 commits into
carve/s1-foundation-contractsfrom
carve/s7-orchestration-plane

isadeks commented Jul 27, 2026 •

edited

Loading

Uh oh!

isadeks commented Jul 27, 2026

Uh oh!

isadeks commented Jul 27, 2026

Uh oh!

isadeks commented Jul 27, 2026

Uh oh!

isadeks commented Jul 28, 2026

Uh oh!

isadeks commented Jul 28, 2026

Uh oh!

isadeks commented Jul 28, 2026

Uh oh!

isadeks commented Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

isadeks commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this adds

⚠️ Unlike S5 and S6, this slice is NOT fully dormant

Verification

Tracking

Uh oh!

isadeks commented Jul 27, 2026

Automated review — carve S7 (#659)

Slice standalone-ness: PASS

CFN safety — the questions worth asking before this merges

On the previously-flagged sumIterationCostForIssue concern — largely REFUTED

Other findings

IAM

Uh oh!

isadeks commented Jul 27, 2026

Addendum to the review above — two further findings

Uh oh!

isadeks commented Jul 27, 2026

Review findings addressed

Uh oh!

isadeks commented Jul 28, 2026

Operator note: this slice adds one GSI, but a STALE deployment may need two — DynamoDB allows one per update

Uh oh!

isadeks commented Jul 28, 2026

Second-round review: my retry-hint fix was cosmetic — now actually wired

Uh oh!

isadeks commented Jul 28, 2026

Upgrade-ordering note added, from a second-environment deploy — and a correction

Uh oh!

isadeks commented Jul 29, 2026

Review response — nits (a), (c), §6, and the new conflict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

isadeks commented Jul 27, 2026 •

edited

Loading

On the previously-flagged `sumIterationCostForIssue` concern — largely REFUTED