Skip to content

fix: composed-workflow determinism + test-suite hardening (2.5.60) - #730

Merged
apackeer merged 5 commits into
v2from
fix/gate-stability
Aug 11, 2026
Merged

fix: composed-workflow determinism + test-suite hardening (2.5.60)#730
apackeer merged 5 commits into
v2from
fix/gate-stability

Conversation

@apackeer

@apackeer apackeer commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Test-suite hardening driven by two full 8-slice live gates on clean v2: every persistent red and flake observed was root-caused and fixed - four are product defects the gates exposed, the rest are test pins that raced live conductors or the box. Both gates are green on this branch's lineage, including the first fully-green live Claude TUI slice (19/0) on record.

Product fixes (each grounded in live trace evidence)

  • Compose stock-match is now a validator-computed number. aidlc-graph.ts validate-grid returns nearest_stock (stock scopes ranked by grid distance from the proposal, reusing the ars subcommand's diff loop). The composer routes matched-vs-custom on nearest_stock[0].diff <= 2, a matched proposal adopts the stock grid verbatim, and conductor prose in all 5 harnesses forbids re-deriving the verdict. Before: the conductor could compare grids itself post-approval, decide "custom", and write a scope file + scope-grid.json entry the human never approved (observed 3/3 on clean v2).
  • The composer scores a scan report as a fix, not a project. Framing stages are screened out (the report is the captured intent) and pre-existing infrastructure gaps (no tests, no CI) no longer inflate the plan out of the stock bugfix/security-patch scopes.
  • Isolated single-stage runs no longer stall on question-free stages. The --single pre-generation summary stop now applies only when the stage body actually ran a file-backed Q&A; a first-scan reverse-engineering proceeds straight to artifacts. Before: non-interactive runs (Kiro ACP, codex exec) hung forever at a manufactured question (journey-workspace twins red on clean v2).
  • Mid-flow freeform prose gets a deterministic routing question. With a workflow active, prose passed to /aidlc used to be silently discarded by the engine's happy path; conductors that skipped their own classification poured new-work descriptions into the active intent (observed: stage diary pollution, fabricated gate rejections, park attempts, hand state edits). The engine now answers with an ask naming the active work, the typed text, and three routes (continue / separate new work with its inferred scope / reshape). Flagged and verb forms are unaffected.

Test-suite hardening

  • Runner log dirs are per-process (tests/logs/<stamp>-p<pid>, non-recursive mkdir): two same-second runners previously shared one dir and mutually deleted each other's _results (a concurrent sliced gate crashed ENOENT mid-run). Claude-gate discovery also tolerates transient sibling-planted test files vanishing between readdir and read.
  • t163 (reaper steal-race): win evidence is identity-aware - each winner reports the pid it reaped plus a predecessor liveness probe. Legitimate serial re-acquisitions under load are tolerated; robbing a live holder still fails; exactly one seeded-sentinel winner. (The residual product race documented in reapStaleLock is out of scope; follow-up candidate.)
  • t72: durable outcome pins (RE checkbox started + cursor at-or-after RE in the state file's own order + audit gate evidence on overshoot) replace a moving-cursor equality.
  • t238: lead-persona evidence accepts the run-stage directive's structured inline_context_paths delivery or an explicit Read; the supports-not-read half of the mob contract is unchanged.
  • t240: test 9 launches 108 real hook subprocesses and exceeded Bun's default 5s per-test timeout under gate load - explicit 60s timeout, assertions untouched.
  • t73/t74: learnings-ritual pins anchored on the protocol-mandated DECISION_RECORDED options row (verbatim "Nothing to add"/"Add a note") instead of paraphrase-sensitive answer wording; t73's artifact-ordering pin narrowed to generated deliverables (the audit hook legitimately logs the diary and questions file before the checkpoint).
  • t29: syncs on turn end + the durable no-state-file contract instead of a pane string the TUI collapses.
  • Kiro TUI recognizer: answers one summary confirmation per checkpoint-bearing stage (keyed on the prompt tail) instead of once per journey.
  • New t275 pins the nearest_stock output shape and the diff<=2 boundary (the t193 scenario).

Verification

Two full 8-slice gates (smoke+unit / integration / deterministic e2e / live Claude TUI / live Kiro ACP / live Kiro TUI / live codex exec / Kiro IDE skip-gate), plus per-fix solo and under-load reruns. Final gate: all 8 slices green; previously-red tests re-verified individually (t193 2x consecutive, ACP journey-workspace 4x consecutive, t163 solo x3 + 2-core torture runs, t72/t238/t240 solo + load). Evidence stamps under tests/logs/ are cited in the commit message.

bun scripts/package.ts --check, bun run typecheck, and the coverage-registry check are green; CHANGELOG 2.5.60 + README badge bumped per the changelog policy.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 978c76a5: CI is green and the focused tests pass, but the approval/routing contracts still have blockers.

  1. P1 - a matched proposal can present and approve a different grid from the one that will run (core/agents/aidlc-composer-agent.md:563, :576-580, :630-640). The composer copies summary from validation and builds the decision table from the mechanical/folded proposal, then may replace that proposal with the stock grid without revalidating or regenerating either surface. The new t275 scenario demonstrates the mismatch: bugfix plus two stages validates as 9 EXECUTE, but the <=2 match rule adopts stock bugfix, which has 7. The human can therefore approve counts/rows that are not the workflow subsequently started. The same contract also says mode is FINAL while still allowing edits; edits to a matched plan have no persistence path because matched approval writes no scope. Revalidate and rebuild summary/table after stock adoption, and convert edited stock plans to custom (or disallow edits that cannot be applied).

  2. P1 - incomplete grids are accepted as exact stock matches (core/tools/aidlc-graph.ts:1019-1023, :1121-1133). Validation checks only supplied keys, and distance ignores missing proposal entries. I reproduced validate-grid with {}: it returns valid: true, a zero-stage summary, and every stock scope at diff: 0, with bugfix first alphabetically. Since the input is LLM-produced and the new number is the deterministic routing authority, require exactly the compiled stage set (or count missing stages as differences) before computing a match.

  3. P1 - the new mid-flow ask has contradictory response protocols (harness/claude/skills/aidlc/SKILL.md:80,130; mirrored in all six harnesses). The central ask dispatch contract and AskDirective type say the human answer goes back through report, while the new branch-specific instruction says route it directly and never use report. This ask has no report result that can represent its three choices, so following the central table fails the flow. Give this question a typed response route/ask subtype and one authoritative continuation contract.

  4. P1 - rich new-work prose produces an offer without the scope required by its confirmation path (core/tools/aidlc-orchestrate.ts:2965-2972). inferScopeFromText() returns source: freeform for no match or >5 words, and the question omits the inferred scope in that case. The harness then requires next --new-intent --scope <the scope the ask named>, but none was named. The ACP test explicitly calls the inferred scope non-deterministic. Always include the proposed scope (or an explicit compose route) in the directive rather than leaving the conductor to invent it.

  5. P2 - t163 can pass when a contender steals the live holder (tests/integration/t163-reaper-steal-race.test.ts:108-119, :243-265). The live-holder case never creates reap-evidence.json; only seedStaleLock() does. If a broken implementation lets a contender acquire, that child throws while reading the missing file before printing WON. Child stderr/status are ignored, winnerEvidence() reports zero winners, and the replacement lock still makes existsSync(lockDir) true, so both assertions pass. Seed the evidence state and fail on every nonzero/malformed child result.

  6. P2 - t238 no longer proves the conductor read the lead persona (tests/integration/t238-user-stories-mob.sdk.test.ts:494-506). inline_context_paths is only a path manifest emitted by buildRunStageDirective; the skill explicitly requires the conductor to read those paths. Accepting manifest presence as an alternative to a Read makes the assertion pass even when the conductor ignores the persona. Keep behavioral evidence that the file content was actually loaded.

  7. P2 - t72 accepts inconsistent stage advancement (tests/integration/t72-stage-reverse-engineering.test.ts:234-257). On an overshoot it permits RE to remain [-] or [?] and accepts STAGE_AWAITING_APPROVAL as sufficient evidence that Current Stage moved later. Advancement should require RE [x] plus STAGE_COMPLETED; otherwise a broken state transition passes as a timing variation.

  8. P2 - the ACP journey no longer proves the new-work offer was rendered (tests/e2e/t-acp-kiro-journey-workspace.serial.test.ts:346-384). Turn 3a now asserts only no birth/no state mutation. Turn 3b itself explicitly says to start a second intent, so it can create one even if turn 3a rendered no question. Capture/assert the routing question or another observable offer surface before sending confirmation.

Verification on this head: 49 focused t114/t275 tests passed, both t163 tests passed, package parity, typecheck, lint, and git diff --check passed. The empty-grid reproduction above was run directly against the shipped graph tool.

@apackeer

Copy link
Copy Markdown
Contributor Author

@leandrodamascena Thanks for the detailed review. I pushed 0f025c69 with the requested fixes:

  • Revalidated adopted stock grids and rebuilt their summary/decision table; edits to matched stock plans now convert to custom so they persist.
  • Made validate-grid require the complete compiled stage set and count missing/extra entries in stock distance.
  • Added a typed new-work-routing ask contract, including a deterministic proposed scope for rich prose, and aligned all harness response routing.
  • Hardened t163, t72, t238, and the Kiro ACP journey against the false-positive paths you identified.
  • Updated t190's fixture proposals for the complete-grid contract; the full unit tier is green (196 files, 4,131 assertions).
  • Made inline persona loading a blocking first-read precondition across all six harnesses and the shared protocol. The live t238 rerun is green, with the lead persona read before the stage file and consume.
  • Scoped Codex question-rendering's report --user-input rule to ordinary asks; typed new-work routing goes through next.

Package parity, typecheck, lint, and diff checks are green. Please re-review when you have a chance.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 0f025c69: the previous eight findings have corresponding fixes, and focused checks pass. Three routing issues remain.

1. P1: stock adoption makes valid in-flight recomposition impossible

core/agents/aidlc-composer-agent.md:570-588 applies stock matching and full-grid adoption to all three composition moments, including in-flight. This conflicts with the in-flight contract at lines 57-65 and core/tools/aidlc-orchestrate.ts:840-847, where only pending, ahead-of-cursor suffix flips may be applied through recompose.

A deterministic example is a running feature workflow where the user asks to skip the still-pending market-research and team-formation stages. Strict validation succeeds with 30 EXECUTE / 2 SKIP and reports enterprise at distance 2, differing on exactly those stages. Step 7 then requires adopting the full stock grid, restoring both stages and producing a 32-stage proposal. The intended pending flips disappear. Approval then either invokes recompose without any flips, which fails its mandatory-argument guard, or attempts differences involving frozen stages, which recompose rejects.

The identical feature and enterprise grids also allow an in-flight feature workflow to be presented as a matched enterprise proposal even though recompose cannot change the workflow scope or its depth metadata.

Restrict stock matching/adoption to front/report composition. In-flight composition should preserve every frozen action and return only validated --skip/--add changes against the current effective plan. Add a deterministic test covering the existing t196 “drop two pending stages” scenario through the new match logic.

2. P2: composed scopes participate in the “stock” ranking

core/tools/aidlc-graph.ts:1015-1019 implements nearestStockScopes() by iterating every entry returned by loadScopeGrid().

That grid does not contain only stock scopes. mergeComposedScopes() explicitly preserves custom entries written by the composer (core/tools/aidlc-graph.ts:1411-1418), and Step 10 writes every approved custom scope into the same registry.

After the first custom scope is created, subsequent ARS and validation calls can rank it at diff: 0 or within the two-flip boundary. Step 7 will then treat that user-owned scope as prevalidated stock, set mode: matched, reuse its possibly task-specific name and depth, and skip the custom persistence path. Matching results therefore depend on which unrelated custom workflows happen to exist in the installation.

Build the candidate set from graph-authored/plugin-authored scopes, for example the fresh transpose before mergeComposedScopes(), or persist explicit scope provenance. Add a test that injects a composed grid entry and confirms it is excluded from nearest_stock.

3. P2: the final match rule has contradictory distance authorities

Several contracts say matching is based on the validator’s distance from the final proposal: core/agents/aidlc-composer-agent.md:566-568, core/knowledge/aidlc-composer-agent/composing.md:50-55, core/tools/aidlc-orchestrate.ts:865-866, and tests/unit/t275-validate-grid-nearest-stock.test.ts:9-12.

However, Step 7 at core/agents/aidlc-composer-agent.md:572-598 uses the smaller of the mechanical ARS screen’s stock distance before evidence-driven folds and the validator’s distance from the final proposal.

If the mechanical screen is two flips from stock but evidence-based folding moves the final proposal much farther away, Step 7 still forces the stock scope and discards every fold. There is no limit on how many final decisions can be erased; only the earlier mechanical distance is bounded.

This contradicts the knowledge contract’s “when no stock scope fits, synthesize; do not force a bad match” rule and leaves the LLM with two authoritative routing instructions. Depending on which instruction it follows, the same validated proposal can become either matched or custom.

Use validate-grid.nearest_stock from the final proposal as the sole routing authority. Keep the mechanical distance as advisory evidence if desired. If using the minimum is intentional, update the conductor, knowledge, and tests to state that contract explicitly and add a divergence test where the mechanical grid is near stock but the final folded proposal is not.

Verification

  • Focused unit suite: 156 passed
  • Recompose suite: 17 passed
  • Reaper concurrency suite: 2 passed
  • bun scripts/package.ts --check: passed
  • bun run typecheck: passed
  • bun run lint: exited successfully with three informational notices
  • git diff --check: passed
  • t72-stage-reverse-engineering: exceeded the five-minute local verification window; no pass/fail conclusion

@apackeer
apackeer force-pushed the fix/gate-stability branch from 0f025c6 to 5df77e3 Compare August 10, 2026 23:05
@apackeer

Copy link
Copy Markdown
Contributor Author

@leandrodamascena Thanks for the follow-up review. I pushed 5df77e38 with fixes for the three remaining routing issues:

  • Restricted stock matching/adoption to front/report composition. In-flight proposals now preserve the running scope, depth, full effective grid, and frozen actions, and return exact changes.skip / changes.add arrays for recompose.
  • Excluded composer-authored scope-grid entries from nearest_stock; only graph/plugin-authored scopes participate in stock ranking.
  • Made the final proposal's validate-grid.nearest_stock result the sole front/report match authority. The earlier mechanical ARS distance is advisory and cannot erase evidence-driven folds.

Added deterministic coverage for the t196 two-pending-stage reshape, mechanical/final distance divergence, composed-scope exclusion, emitted in-flight directives, and all six conductor skills. The branch is rebased onto current v2; version remains 2.5.64.

Verification: focused unit tests passed at tests/logs/2026-08-10T22-19-17Z-p2 (Result: PASS, 44 assertions), along with typecheck, lint, package parity, and diff checks.

Could you please re-review when you have a chance?

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 5df77e38: the three previously reported routing issues are resolved. I found no additional blocking code findings.

Verification passed: 62 focused tests, typecheck, package parity, git diff --check, and remote CI.

Approved. The current merge conflict with v2 still needs resolution before merge.

@apackeer
apackeer force-pushed the fix/gate-stability branch from 5df77e3 to 8553a8d Compare August 11, 2026 20:27
@apackeer
apackeer merged commit e3bf9be into v2 Aug 11, 2026
5 checks passed
@apackeer
apackeer deleted the fix/gate-stability branch August 11, 2026 20:38
alokgp pushed a commit to alokgp/aidlc-workflows that referenced this pull request Aug 12, 2026
Answers two independent reviews of PR awslabs#731. Grouped by the invariant each
class violated rather than patched per finding, because a per-case fix in
this file has repeatedly closed the named site and missed its sibling.

CONTAINMENT. A committed symlink at `documentkb/.journal` let a plain `sync`
recursively delete an external directory while printing "Up to date." and
exiting 0; a symlinked `documentkb/<id>` let sync write outside the project;
and the `aidlc/active-space` cursor was read unvalidated, so `..` escaped the
`spaces/` jail. Containment is now re-checked per path component through one
funnel, enforced by two independent layers: a biome `noRestrictedImports`
override that makes a raw mutating `node:fs` import in this tool impossible,
and a TypeScript-AST completeness property that treats an unrecognised fs
binding as a mutator until a human classifies it. Mutations route through
`ensureDirSync`/`renameIntoPlace`/`removeTreeSync`. Four earlier attempts
enumerated known-good and failed open one level up (parameter names, then
primitive names, then regex-parsed imports); t277's header records the
source-code-level routes that remain out of reach of any in-repo check.

ROW IDENTITY. Re-onboarding an edited file created two live rows for one
path; now the live row at that path IS the identity, refreshed in place and
reported as `edited`. Two identical-byte rows competing for one replacement
had the winner decided by array order; digests are now resolved globally and
the ambiguous case fails closed, requiring `rebind`. `content` and
`summary.path` are bound canonically to the row's own id, so a spliced
catalog can no longer cite one document while serving another's text.
`removed_at` must be a non-empty ISO string, so a malformed tombstone cannot
read as a live row. `onboard --intent` on an unchanged document applied the
association it previously dropped.

PUBLISHED CLAIMS. `rm -rf documentkb/` then `sync` was documented as a
recovery; measured, a full wipe loses ids, tombstones and intent links. The
claim is narrowed to index-only recovery in four places and pinned by a pair
of tests -- one proving what survives an index loss, one proving what does
not survive a tree wipe. `resolveIntentFlag` now accepts a record-dir name
and a canonical UUID, so the remedy its own error message suggests works.
The usage line lists all seven verbs. The audit-shard scoping gap is
documented rather than fixed: document events land in the space shard, but an
unscoped `readAllAuditShards` resolves through the active intent, so
`--doctor --export` omits them once an intent exists.

TEST HYGIENE. `t275` becomes `t286` (PR awslabs#730 ships a different t275). `t279`
no longer mutates the shared checkout -- it works on a scratch copy, so a
killed run orphans a temp dir instead of dirtying `dist/`. The t28/t81
audit-count prose now derives 85; both pins were already correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants