Conversation
…ontracts FUSI-034 Step 1 (R1): the shared merge-lane fake was missing two ProjectEngine fields, so a test-double defect surfaced as an apparent production crash. R1 is fixed in the FAKE, not in the assertion and not in the product. project-engine.ts:586-587 (mergeRetryResetTaskIds, mergeEnqueueDeferredByRetryReset) are correct shipped behaviour; the fixture simply predated FN-9317 (706c155). internalEnqueueMerge threw TypeError: Cannot read properties of undefined (reading 'has') at project-engine.ts:3123 because Object.create(ProjectEngine.prototype) runs no class-field initializers. merge-abort-clears-transient-status.test.ts: 8 failed / 21 passed (29) + 5 unhandled errors -> 29 passed (29), 0 errors, stable across 3 runs. The 5 unhandled errors were the same defect on a second path, so their disappearance is independent corroboration. FN-5893 invariant enumeration (not just the reported site): walked all 62 instance fields of class ProjectEngine against MergeLaneState. The drift ratchet at project-engine-merge-lane-fixture-drift.test.ts independently caught the two FN-9317 fields (it was RED before this commit and is green after), which confirms the ratchet works and the fixture had drifted, not that the ratchet is broken. Full diff of 39 unseeded instance fields is reported in the completion summary. Two further real gaps found and closed here: - mergeBodySettleTimeoutMs (project-engine.ts:1015) is read by awaitPriorMergeBodySettle, but is declared OUTSIDE the auto-merge state block the ratchet scans, so the ratchet is blind to it. Unseeded, it collapses the 60s drain latch to ~1ms via setTimeout(fn, undefined). Its three consumers each override it to 1 by hand, which is the smell of a gap patched around rather than closed. Seeded with the production default; their overrides are left untouched. - 36 remaining unseeded fields are non-merge-lane collaborators with no prototype-fake consumer (runtime, notifier, oauth*, planner*, automation*, settingsHandlers, legacyAutoMergeStampAdvisoryEmitted). Recorded as findings, not added: a field no prototype drain path reads needs no fake, and seeding 36 collaborator-shaped fields would make the fixture a second, hand-drifting copy of the whole engine. The three already-allowlisted lane fields (unregisterMergeAdmissionProvider, autostashSweepTimer, mergeActiveReconcileTimer) remain correctly allowlisted and are NOT seeded. No timeout widened, no retry added, no assertion loosened or deleted, no .skip or .only. No changeset: the only file touched is a test helper in the private @fusion/engine package, which AGENTS.md exempts. Co-authored-by: Fusion <noreply@runfusion.ai> Fusion-Task-Id: FUSI-034
…ontracts FUSI-034 Step 2 (R2): fixed in the FAKES, not the assertions and not the product. post-landing-worktree-cleanup.test.ts: 11 failed -> 29 passed (29). OPTION CHOSEN: (a) declare the evidence satisfied. The spec prefers (a) because it keeps FN-9370's guard on the exercised path and proves finalization still completes once evidence exists. Option (b) (no gate enabled) would have exercised only the guard's early-out and left the new guard untested in this file. The gate id is IMPORTED from @fusion/core as POST_MERGE_VERIFICATION_GROUP_ID, not pasted from a captured log, so a future rename of the built-in post-merge group cannot silently strand this fake on a literal. The fixture also sets an explicit enabledWorkflowSteps list rather than relying on defaultOn: true, so the fake's position does not move if that default is ever edited. A SECOND, DISTINCT FAKE GAP surfaced while fixing R2 and is fixed here too. The finalizer no longer calls moveTask: FN-9371 (HEAD 6c57344) moved terminal finalization onto a conditional fence, so the fake was missing store.moveTaskIf and store.updateTaskAtomic and every completion path threw 'store.moveTaskIf is not a function' at auto-merge-finalization.ts:406 before any assertion ran. Both are modelled on the real contracts: - moveTaskIf (packages/core/src/task-store/moves.ts:284): awaits the predicate and returns { task, moved: false } WITHOUT moving on refusal. The predicate is honoured, not stubbed true, so the refusal cases genuinely exercise the fence. - updateTaskAtomic (packages/core/src/task-store/task-mutation-ops.ts:380): runs the updater against the live row; a null/undefined patch is a no-op. The pre-existing moveTask mock also gained its third MoveTaskOptions parameter, which eight existing assertions already expected. NEW COVERAGE (5 cases, 24 -> 29, all additive; nothing removed): - 3 assert the guard's REFUSAL path (outcome 'blocked' plus the exact reason string) for: no report at all, a REVISE verdict, and a failed gate status. Each also asserts the move did not happen, so a refusal cannot half-apply. - 'finalizes without post-merge evidence when the gate was explicitly disabled' covers the isWorkflowOptionalGroupEnabled boundary (this is NOT option (b) - the default fixture keeps the gate enabled and approved). - 'accepts APPROVE_WITH_NOTES as satisfying the post-merge gate' pins the second approving verdict the guard allows. Without the refusal cases, a future change that disabled or short-circuited the guard would leave this file fully green while merged work reached done without the required post-landing Full Suite evidence. FN-5893 SWEEP (all already-green, NONE edited): every engine test referencing finalizeProvenAutoMergeTask was enumerated, not just the named candidates. Swept and confirmed green: confirmed-merge-must-finalize (exposes the guard trigger), merge-proof-reason-renamed-complete-lane (exposes), merger-trait-rekey (exposes), no-merge-workflow-completion (exposes), self-healing (exposes), self-healing-landed-worktree-cleanup (exposes), merger-merge-lifecycle, merge-orphan-body-durable-writes. The 3 PG e2e files do not expose the trigger. Because all conditional File Scope candidates are green, no conditional entry was edited. One flake observed once in merge-orphan-body-durable-writes ('aborts during a real second workspace-repository land and partitions successor writes') and it did NOT reproduce in 3 subsequent identical invocations; that file is outside this family, so it is reported rather than quarantined here. No timeout widened, no retry added, no assertion loosened or deleted, no .skip or .only. No changeset: test file in the private @fusion/engine package, AGENTS.md-exempt. Co-authored-by: Fusion <noreply@runfusion.ai> Fusion-Task-Id: FUSI-034
…gate child
FUSI-034 Step 3 (R3): realigned the ASSERTMENT to the shipped contract, after
reading the collapse implementation and confirming the classification is correct.
workflow-graph-merge-region-collapse.test.ts: 8 failed -> 10 passed (10).
The spec required confirming the collapse logic BEFORE extending the expectation,
because adding a node id to an expectation without confirming the classification
would defeat the test. Confirmed on two independent grounds:
- MERGE_REGION_KINDS (workflow-graph-executor.ts:490) contains only the seven raw
merge primitives. The "optional-group" kind is NOT a member, so
"post-merge-verification" is a post-merge ENTRY node reached after the collapsed
"merge" seam, not an independent merge-region boundary.
- its inner "prompt" child is likewise not a region kind, so it is traversed as a
child of that entry and can never be classified as a region member.
Because the classification is right, the fix belongs in the stale literal and this
step did NOT exceed a test-file change. No product source was touched.
The id is DERIVED from the IR source, not pasted from a captured failing log:
postMergeOptionalGroupNode names the inner node with a template expression
evaluated to ${spec.id}-step (builtin-post-merge-group.ts:125) and the executor
records an optional-group child as <container>::<node.id>
(workflow-graph-executor.ts:147), giving
"post-merge-verification::post-merge-verification-step". That is the same shape as
the "plan-review::plan-review-step" and "code-review::code-review-step" entries
already in the expected list, so a reviewer can see where the value came from.
The test's actual subject is preserved: expectNoRawMergeRegionVisits still proves
none of the seven raw merge primitives is ever visited, and the collapse-to-one-merge
assertion (calls === ['merge'], one merge dispatch) is untouched. This is a stale
literal in a test, not a collapse-logic bug. No assertion deleted or loosened.
Co-authored-by: Fusion <noreply@runfusion.ai>
Fusion-Task-Id: FUSI-034
FUSI-034 Step 4 (R4): classified all 4 red cases, then realigned them. No product
change was required, so nothing outside File Scope was touched.
workflow-step-notes-repair.test.ts: 4 failed / 10 passed -> 14 passed (14).
CLASSIFICATION: the 4 cases have ONE root cause, not the two the spec hypothesised.
A review gate's identity is its optional GROUP id, not the inner template step id.
The executor resolves
effectiveWorkflowStepId = optionalGroupId ?? workflowStep.id.replace(/^graph:/,"")
at execute-workflow-step.ts:262, and reuses that single value (sameGateStepId, :325)
for BOTH the run-audit workflowStepId (:1275) AND the prior-record lookup
findReusableReviewResult (:620, matching result.workflowStepId at :134). Because
optionalGroupId is present, the GROUP id wins. The fixtures and assertions here were
still keyed to the inner step id ("code-review-step" / "plan-review-step"), which is
the pre-group shape.
- 3 cases ("keeps repaired review outcomes unchanged with an throwing / rejecting /
hanging run-audit sink"): STALE MATCHER. The call already carried a complete event
object; only metadata.workflowStepId was wrong. The matcher is TIGHTENED, not
weakened: it now pins agentId, runId, domain, mutationType, target and taskId in
addition to every metadata field. The previous bare top-level objectContaining
asserted nothing about the agent/domain identity, so it would have matched almost
any call.
- 1 case ("narrates an unchanged legacy review whose persisted output and notes are
empty"): STALE FIXTURE ROW. The persisted prior record was keyed to the inner step
id, so findReusableReviewResult never matched it, a SECOND review dispatch ran, and
the reused-empty notice the test exists to cover was never reached. Keying the row
to the gate id makes the record match; because the fixture genuinely persists
notes:"" and output:"", storedReusedNotes is "" and the notice IS produced
(:640-641). The test's own premise is now reachable through the product's real
reuse path rather than around it.
FAIL-SOFT SIGNAL PRESERVED: all 4 cases still assert the outcome is unchanged
(success/verdict/notes/output) BEFORE the audit assertion, which is the actual subject
of this file. No assertion was deleted. The fail-soft surface is unchanged.
NOT APPEASEMENT, PROVEN BY MUTATION: I verified the tightened matcher can still fail.
Changing the asserted agentId, and separately reverting workflowStepId to the old
inner step id, each turn 3 cases red (11 passed). The file was restored byte-identical
after each probe and re-verified green.
Gate ids are named values, not repeated literals: PLAN_REVIEW_GROUP_ID is imported from
@fusion/core (it is re-exported). CODE_REVIEW_GROUP_ID is mirrored as a documented local
constant because @fusion/core does NOT re-export it from its barrel, and the fixture's
own optionalGroupId is the field the product actually reads.
No timeout widened, no retry added, no .skip or .only. No changeset: test file in the
private @fusion/engine package, AGENTS.md-exempt.
Co-authored-by: Fusion <noreply@runfusion.ai>
Fusion-Task-Id: FUSI-034
…fecycle rule
FUSI-034 Step 5 (R5): resolved by isolation experiment, then realigned. NOT a flake,
so no quarantine entry and no vitest.config change (both files are untouched).
executor-paused-abort-todo-benign.test.ts: 5 failed / 53 passed -> 58 passed (58).
STEP 5 BRANCH TAKEN: branch (2), DETERMINISTIC. The file was run alone three times and
gave an identical 5 failed / 53 passed (58) every time, so the spec's order-dependence
hypothesis is disproved and the bisection in branch (3) was not needed. The spec's
prime suspect (resetExecutorMocks leaking state via createMockStore) is not involved.
ROOT CAUSE, established by probing the product rather than by inference. An instrumented
run of the exact failing inputs recorded ZERO updateTask calls, and this log line:
"Workflow graph failed at node 'unknown' - automatic recovery cannot move
'in-progress' backward; card remains in place"
route-graph-failure-to-execution-resume.ts:409 refuses to claim a WIP card when it has
no forward move for it, and returns false so the live row is left untouched. This is the
FN-207/FN-217 lifecycle-containment rule: automatic recovery no longer holds
backward-move authority, so the `status: "failed"` park this block asserted NO LONGER
HAPPENS for these inputs. The spec anticipated exactly this ("containment already removed
backward-move authority from recovery reasons, so a status: failed park may itself be the
stale expectation") and directed the fix be to the TEST, never a restoration of the park.
FIX: the 5 cases now assert the shipped outcome instead of the old park, following the
established in-repo pattern at executor-prompt.test.ts:1680-1686 (assert the row was not
parked and not moved, and assert the retention was narrated):
- updateTask was NOT called with status "failed"
- moveTask was not called at all
- the column is still "in-progress"
- the log narrates the containment ("...card remains in place")
The test's real subject is PRESERVED and still proven per variant: no transient in-place
retry. graphResumeRetryCount is not bumped and execute() is never re-dispatched, which is
the point of the block. The durable inputs (lastError, failureReason, exhausted
graphResumeRetryCount) are what make each variant a terminal non-retryable failure, and
that non-retry contract is still what every variant asserts. This STRENGTHENS the
coverage: the previous expectation only proved a park happened, the new one also proves
the row was left intact and the retention was explained.
NOT APPEASEMENT, PROVEN BY MUTATION: reverting the assertion to the old `status: "failed"`
expection turns all 5 cases red again (53 passed). The file was restored byte-identical
after the probe and re-verified green across 3 runs.
No product source edited. No timeout widened, no retry added, no .skip or .only, no test
removed: the block still has all 5 variants and the suite still has 58 cases. No changeset:
test file in the private @fusion/engine package, AGENTS.md-exempt.
Co-authored-by: Fusion <noreply@runfusion.ai>
Fusion-Task-Id: FUSI-034
…ng tells
FUSI-034 Step 7: appends a dated entry to the store-fake catalogue matching the
existing FN-8949 section shape (short heading plus prose). No gate script, no
automated fixture-drift ratchet, and no shared fake helper: the catalogue's own
"Recommended next step" proposes that helper as cross-unit work needing adopters,
so recording the pattern is this task's job and building the helper is the fleet's.
Recorded, tightly:
- The tell: a `TypeError: Cannot read properties of undefined (reading 'has')`
from deep inside project-engine.ts is a TEST-DOUBLE defect, not a product
crash. Only `Object.create(ProjectEngine.prototype)` skips class field
initializers. Second instance of this fixture family falling behind production;
the first was FUSI-030's missing getTask collaborator.
- The generalising rule: adding a class field to a class faked via
`Object.create(Prototype)` plus a hand-maintained state fixture makes the
fixture wrong, and tsc cannot see it because the engine tsconfig excludes
src/__tests__ and the fakes are cast. The only detector is running the suites.
- The second latent gap the FN-5893 class-field walk found, which the existing
drift ratchet misses: `mergeBodySettleTimeoutMs` lives at project-engine.ts:1015
and is read at :1037, outside the region the ratchet scans. Undefined, the
settle collapses to ~1ms instead of the production 60s and would race silently.
- The post-merge evidence shape (R2/R3): exposing `getTaskWorkflowSelection` on a
test store opts the fake into the FN-9370 guard, which early-outs when the
method is absent. These are COMPLETENESS defects, the mirror of the
missing-method defects already catalogued: exposing a method is not free.
- The environment shape (R6): `password authentication failed for user
"<os-user>"` during CREATE DATABASE is a provisioning gap. Cite the CI
counterpart (full-suite.yml:44-45, pr-checks.yml:206-207 pass an explicit
postgres user) and name the three forbidden workarounds.
- A fourth drift of the same class: a review gate's identity is its optional
GROUP id, so derive an asserted id from the fixture's own field instead of
retyping a literal that can drift in agreement with its other copy.
Co-authored-by: Fusion <noreply@runfusion.ai>
Fusion-Task-Id: FUSI-034
ThreatCrush Security Scan4589 finding(s) HIGH/CRITICAL: 43 | MEDIUM: 4035 | LOW: 511
…and 4539 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR for FUSI-034.
REGRESSION follow-up (FUSI-020 Step 5, family 5 of 7):
assertion-no-error-line- 15 cases across 6 engine test files whose shard log did not include the error text, so the classification is incomplete. These are NOT dispositioned yet: the census could not name why they fail, and per AGENTS.md an unnamed failure may not be assumed to be either a flake or a regression. First step is to reproduce each one locally and capture the real error, not to guess a class. Reproduce: pnpm --filter @fusion/engine exec vitest run --reporter=verbose (drop --silent so the assertion diff and stack are printed). Then classify each as flake or regression and apply the matching disposition: a flake is quarantined on sight (scripts/lib/test-quarantine.json entry + matching one-line exclude in packages/engine/vitest.config.ts, same commit), a regression joins the appropriate family card. Census: docs/solutions/test-failures/main-full-suite-census-2026-09-25.md (familyassertion-no-error-line). First red run: main push Runfusion#3158 (b37d0fe). Note: if a named case turns out to sit inside the merge gate's engine-core allow-list, evict it from that allow-list rather than quarantining it (AGENTS.md gate rule).