fix(coding-agent): assert goal_updated payload instead of emit arity

[twoimo]

Summary

packages/coding-agent/test/goals/goal-mode-integration.test.ts fails on current dev (b6198e748) with a stale call-arity assertion. This is a test-only repair; no product file changes.

The immutable per-attempt scope facility (#3608) routes every session extension event through the three-argument shape:

await this.#extensionRunner.emit(
    { type: "goal_updated", goal: event.goal, state: event.state },
    undefined,
    deliveryScope,
);

The test still asserted the single-argument shape:

expect(emit).toHaveBeenCalledWith(expect.objectContaining({ type: "goal_updated" }));

toHaveBeenCalledWith matches the full argument list, so the extra undefined, undefined makes it fail with Number of calls: 2.

I checked whether deliveryScope === undefined is itself the defect, and I do not believe it is. goal_updated is emitted from #emitSessionEvent (agent-session.ts:2768) with no attempt scope attached, and deliveryScope resolves as scope ?? event.scope. Goal state is session-level rather than attempt-level, so no scope is the correct value here. The product looks right and the assertion is the stale part.

Change

Assert the delivered payload instead of the call arity. The test now locates the terminal goal_updated event the throwing hook actually received and proves it carries the completion state:

const goalUpdates = emit.mock.calls.map(([event]) => event).filter(event => event.type === "goal_updated");
expect(goalUpdates.length).toBeGreaterThan(0);
const terminalUpdate = goalUpdates[goalUpdates.length - 1];
expect(terminalUpdate?.goal?.status).toBe("complete");
expect(terminalUpdate?.state?.mode).toBe("exiting");

This is what the test's name already claims, and it is stronger than what it replaced: the old line only proved some goal_updated fired, while this proves the throwing hook received the terminal completion payload. expect() calls in the file go from 80 to 82. The spy's parameter type is widened structurally, so there is no cast and no private access.

Verification

On b6198e748, darwin-arm64:

• bun test packages/coding-agent/test/goals/goal-mode-integration.test.ts — 17 pass / 0 fail, 82 expect() calls (was 16 pass / 1 fail, 80 calls)
• bun run check:types (tsc -p packages/coding-agent/tsconfig.json --noEmit) — exit 0
• bun x @biomejs/biome check packages/coding-agent/test/goals/goal-mode-integration.test.ts — exit 0
• Exactly one changed file

Two-sided proof, since a fixture-side change deserves the scrutiny:

1. Fail-on-revert. Reverting only this diff on the same tree returns 16 pass / 1 fail; restoring returns 17 pass / 0 fail.
2. Product-coupled. With the fix in place, dropping state: event.state from the goal_updated emit in agent-session.ts makes the new assertion fail (16 pass / 1 fail); restoring the product returns 17 pass / 0 fail.

The second proof is the one I care about. I did not want to hand you an assertion that merely accommodates whatever the code currently does — it is bound to product behaviour and fails when that behaviour regresses.

Notes

I found this while checking why test:@gajae-code/coding-agent:shard-1-of-8 is red. It was the only remaining failing test in that shard on my open PRs (#3618, #3620) after the earlier topology failures cleared. The file is not in CODING_AGENT_SHARD_ONE_COVERAGE_PATHS and, as far as I can tell from the open PR list, is not claimed by any open PR — if it overlaps work you already have in flight, please close this and I will drop it without argument.

Happy to adjust the assertion shape if you would prefer a different convention here; there was no existing precedent in the repo for asserting the three-argument emit shape, so I picked one.

[twoimo]

Heads up on an overlap I noticed after opening this PR, so you can close whichever one is redundant.

#3638 ("AttemptScope post-merge compatibility repairs") touches the same file and the same assertion. The two changes differ:

#3638's approach — pin the new arity:

expect(emit).toHaveBeenCalledWith(expect.objectContaining({ type: "goal_updated" }), undefined, undefined);

This PR's approach — assert the payload, ignore arity:

const goalUpdates = emit.mock.calls.map(([event]) => event).filter(event => event.type === "goal_updated");
expect(goalUpdates.length).toBeGreaterThan(0);
const terminalUpdate = goalUpdates[goalUpdates.length - 1];
expect(terminalUpdate?.goal?.status).toBe("complete");
expect(terminalUpdate?.state?.mode).toBe("exiting");

Both make the test pass. The difference in what they detect:

• If a future change adds a fourth argument to emit(...), fix(coding-agent): AttemptScope post-merge compatibility repairs (#3592) #3638's form breaks again and this form does not.
• If the product silently stopped attaching state to the goal_updated payload, this form fails and fix(coding-agent): AttemptScope post-merge compatibility repairs (#3592) #3638's form still passes. I verified that by deleting state: event.state from the goal_updated emit in agent-session.ts — this assertion catches it (16 pass / 1 fail), and the arity form does not.

That said, #3638 is the more complete change — it carries the product-side repairs for forceAbort and resetAttemptBudget that this PR does not touch, and I confirmed locally that its head fixes agent-session-abort-timeout (4 pass / 0 fail) and this goal test (17 pass / 0 fail).

So: please treat #3638 as authoritative and close this one if you prefer to keep your version. I am not attached to it. If you would rather keep the payload assertion, the two changes are a clean textual merge — only the one expect(emit) line and the spy's parameter type differ.

Sorry for the duplicated review effort. I opened this before #3638 existed and did not re-check ownership immediately before pushing.

Two things I found while verifying, which may be useful either way:

1. At fix(coding-agent): AttemptScope post-merge compatibility repairs (#3592) #3638's head, agent-session-message-pipeline.test.ts (3 fail) and agent-session-auto-compaction-continue.test.ts (2 fail) are still red on the same root cause. I opened fix(coding-agent): assert message-pipeline callback payloads instead of arity #3641 for the message-pipeline file (test-side arity only). The auto-compaction pair looks product-side — resetAttemptBudgetSpy receives 0 calls where 1 is expected — and fix(coding-agent): AttemptScope post-merge compatibility repairs (#3592) #3638's head adds a resetAttemptBudget call site, so I have deliberately left that one alone as yours.

2. An earlier comment I left on fix(session): complete descriptor-bound cleanup and live migration leases #3596 claimed that PR repaired the sdk-machine-lifecycle-topology failures. That claim was wrong and I have since retracted it there — the local failure I attributed to fix(session): complete descriptor-bound cleanup and live migration leases #3596 was a stale prebuilt natives binary in my worktree. Apologies for the noise.

[Yeachan-Heo]

receipt: gajae-ci/merge-verdict-3637
pr: #3637
head: a58814e3d4ba0ae46af370fad8df2d9bb51ea28d
base: b6198e74840a603adbc425148e322ecb6ba8820f
ci: 13/13 green, mergeable clean
hostile-verdict: MERGE_READY (P0=0, P1=0)
reviewed:
  - test-only: enriches mock emit fn type (goal?, state?) and replaces
    toHaveBeenCalledWith with explicit call extraction asserting the
    terminal goal_updated event has status=complete, mode=exiting
  - product-coupled: the 3-arg emit shape (event, continueWhile?, scope?)
    originates from AttemptScope deliveryScope threading in #emitExtensionEvent (#3608)
  - no production mutation, no contract change
disposition: MERGE_READY; merge to dev

PR #3637 hostile exact-head review: MERGE_READY (P0=0, P1=0). Test-only fix, product-coupled to AttemptScope 3-arg emit shape. CI 13/13 green.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]