test: verify final memory review fixes - #21
Draft
twoimo wants to merge 29 commits into
Draft
Conversation
…ispose flake (Yeachan-Heo#3144) * test(sdk): make teardown ordering witness production-observable The Phase 2 rewrite of "session teardown drains admitted direct gate resolution" was accepted with a narrowed claim because a mutation probe survived: replacing `await rt.waitForGateResolutionQuiescence()` with `void ...` still passed. The test fully mocked `resolveGate`, so the delayed operation never touched the real terminal controller, and it released the resolver after a single setImmediate while `stopSession()` was independently awaiting the native `pushFrameAndWait(session_closed)` barrier — resolution therefore completed before teardown reached detachment even without the quiescence await. Now the test calls through the original `registerGateTerminalController` and the original `resolveGate` (wrapping the latter only with a deferred pre-terminalization gate), makes `pushFrameAndWait(session_closed)` an explicit test-controlled pre-drain barrier, asserts the controller is still attached at the quiescence point, then observes real accepted terminalization and gate continuation before detachment. Mutation-proved: the void-await mutation now FAILS at the pre-detachment assertion (controllerAttached false). Passes 10/10 unmutated. * test(runtime): poll for the TERM marker in the concurrent-dispose redteam Caught live by the stabilization soak: main-nontag rehearsal run 30149261910 failed on shard 11 with (fail) process-lifecycle adversarial owned-process invariants > double and concurrent dispose share one settled result and issue one terminating signal expect(received).toHaveLength(expected) Expected: 1 Received: 0 The child's TERM trap appends its marker asynchronously (`trap 'echo term >> $tmp; exit 0' TERM`), so under shard load `awaitExit` can return before that write lands and the single-sample read observes an empty file. The file already has a `waitForAsync` helper for exactly this shape; the marker assertion just wasn't using it. Polls for the single terminating signal before asserting, preserving the original invariant (exactly one `term` line — not "at least one"). Verified: 15/15 reruns, 4x parallel contention clean, whole file 9/9, typecheck and biome clean. This test was NOT in the 62-suspect audit shortlist: it did not fail or retry during the mined two-week window, so it is a genuinely new observation the soak surfaced. --------- Co-authored-by: Yeachan-Heo <yeachan-heo@gajae.dev>
`sh` runs a TERM trap only after the current foreground command returns. With the child looping on `sleep 1` and `gracefulMs: 500`, dispose could escalate to SIGKILL before the handler wrote its `term` marker, so the one-terminating-signal assertion saw an empty file (observed in CI run 30149261910, coding-agent shard 11). Shorten the loop interval to 0.05s and use the module's own `DEFAULT_GRACEFUL_MS` (2000ms) so the trap has a deterministic window. Under 18-worker CPU contention the old shape wins the race 8/20; the new shape wins 20/20. All assertions are unchanged. Also record the missing `## [Unreleased]` changelog entries for Yeachan-Heo#3109, Yeachan-Heo#3127, and Yeachan-Heo#3131, found while auditing release scope after v0.11.9.
…turns (Yeachan-Heo#3147) Lifecycle `session_shutdown` started `controller.stopCurrentSession(ctx)` but discarded the promise, awaiting only `stopSession(id)`. Once startup has settled the host is broker-visible and can accept `session.close` while the startup handler's post-start `reconcileCurrentSession` is still running. That reconciliation can mint a replacement notification-root token; `ensureTelegramDaemon` then unregisters it asynchronously. Shutdown could therefore return — and disposal exit — before that unregister's file lock and atomic registry write settled, leaving a stale `sessions[id]` row that the retained older token is correctly fenced from removing (`unregisterNotificationRoot` rejects token mismatches by design). Now shutdown snapshots `sessionStartPromises.has(id)` first and awaits the settled controller stop after `stopSession` whenever startup was NOT pending, so completed-start reconciliation and its replacement-token cleanup are joined. The intentional nonblocking path is preserved exactly where it matters: a genuinely pending startup entry (the `/notify on` case) still leaves the controller stop fire-and-forget. Surfaced by the flaky-CI stabilization soak. The regression test "Telegram root release failure is retained and retried through lifecycle shutdown" failed deterministically on darwin-arm64 at dev head while Linux CI stayed green (run 30147146988, 34/34 shards) — a completion- ordering divergence, not a `/var` canonicalization or native-addon issue: both registry and notification-root paths are lexical `path.join` with no realpath or case folding. Verified on darwin-arm64: the previously-failing test now passes 10/10, the whole sdk-host-wiring file is 72/72 (first fully green run of this file on Darwin), and telegram daemon + btw-e2e are 463/463. The assertion was kept intact rather than replaced with polling, which would have hidden the lifecycle-return bug. Co-authored-by: Yeachan-Heo <yeachan-heo@gajae.dev>
Merge upstream dev 2a7f33d into the memory guard successor branch before applying the final review fixes.
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
2 participants
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.
Internal one-shot verification PR. The trusted base workflow adds the two remaining Yeachan-Heo#3137 regressions, regenerates native declarations, runs all focused memory suites and package checks, then pushes only the verified commit.