Skip to content

test: tolerate retired sessions in the fixture teardown flush (refs #154) - #156

Closed
spacexun2 wants to merge 1 commit into
NanmiCoder:mainfrom
spacexun2:fix/teardown-flush-race
Closed

spacexun2 wants to merge 1 commit into
NanmiCoder:mainfrom
spacexun2:fix/teardown-flush-race

Conversation

@spacexun2

Copy link
Copy Markdown
Contributor

What this fixes

The runtime fixtures share one teardown idiom:

for (const agent of ctx.agents.list()) {
    await agent.whenIdle();
    await ctx.sessions.flush(agent.session);
}

If a member's continuable session has already been retired from the store by the time the loop reaches it, ctx.sessions.flush throws session "<id>" is not live in this store and the scenario exits 1 — after the behavior under test has already been demonstrated. Observed once on CI in captain-idle-wakeup (Real Harness (0.1.2-alpha.5), run 34555573487, PR #147 head 071e2e5, which touches no fixture): member executed, task terminal, captain yielded and woke after idle were all true, while exit0 / productMarker were false purely because of the crashed teardown. Full trace in #154.

On the documented alpha.5 flow the scenario passed 7 of 7 repeats both on the PR candidate and on a build from plain 426024f, so this reads as a structural teardown race rather than a behavioral regression: the loop flushes whatever agents.list() returns, without tolerating a session that retired mid-teardown.

Change

The flush now skips exactly the retired-session failure ("not live in this store" — a retired session has nothing left to flush) and still propagates every other error, including a session/flush callback failure.

Applied to the four fixtures that carry the identical idiom — harness-runtime-idle.mjs, harness-runtime-resume.mjs, harness-runtime-entry.mjs, harness-runtime-web-approval.mjs — because the compatibility gate treats any scenario failure as fatal (verify.yml asserts runs.every(run => run.passed)), so the same window can turn any of them red. The same loop also exists in harness-model-driver.mjs (model benchmark, not run by CI); left untouched to keep this scoped to the runtime fixtures.

Honest limit of the guard: no assertion in these scenarios depends on the flush succeeding (they read team state and the trace), so it cannot mask a currently-asserted property — but the message is the only discriminator, so a genuine premature session retirement would be swallowed rather than surfaced. If you would rather have it stricter (e.g. re-check liveness through the store API instead of matching the message), say so and I will rework it; I avoided sessions.get because the fixtures run against four host versions and I could not confirm that lookup exists in all of them.

Verification

  • node --check on all four fixtures.
  • Full scenario suite against host 0.1.2-alpha.5 (the version that flaked), patched fixtures: 9 runs — the seven scenarios (lifecycle, fallback, failure, captain-idle-wakeup, progressive-entry, web-approval, protocol-compatibility) plus the two cold-restore legs — all passed: true, every run exit 0. result.json records the fixture hashes, which match this commit.

Refs #154

The runtime fixtures flush every session from agents.list() during
teardown. A member's continuable session can be retired from the store
while that loop runs; the flush then throws "session ... is not live in
this store" and the scenario exits 1 after its behavioral assertions
already passed (observed once on CI in captain-idle-wakeup, run
34555573487).

The flush now skips that specific failure - a retired session has
nothing left to flush - and still propagates every other error. Applied
to the four fixtures sharing the idiom (idle, resume, entry,
web-approval): the compatibility gate treats any scenario failure as
fatal, so the same window can turn any of them red.

Refs NanmiCoder#154
@spacexun2

Copy link
Copy Markdown
Contributor Author

Closing this out — @NanmiCoder's liveness guard in 68fe529 (ctx.sessions.get(agent.id) === agent.session before each teardown flush, across the same four fixtures) addresses the same retirement race from #154 via a pre-check instead of a tolerant catch, and shipped with v0.1.18. Our four-host CI runs on this branch (all green, including the alpha.5 captain-idle-wakeup scenario) can serve as an independent cross-validation of that scenario. Thanks!

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.

1 participant