fix: deterministic session resolution for Inference --auto-state#1467
Closed
jbmml wants to merge 1 commit into
Closed
fix: deterministic session resolution for Inference --auto-state#1467jbmml wants to merge 1 commit into
jbmml wants to merge 1 commit into
Conversation
…tically
synthesizeAdvisorState() read work.json expecting active/current/
activeSession top-level keys, but the registry's shape is
{sessions: {slug: row}} — those keys never exist, so resolution always
fell through to the most-recently-modified MEMORY/WORK/ directory. That
fallback carries no session identity: it can attach a completed,
unrelated session's ISA as the advisor state blob.
Resolution is now deterministic and fail-safe:
1. Row matching CLAUDE_SESSION_ID, any phase (a complete row of the same
session is still this session — the advisor fires at the completion
boundary). Set-but-unmatched returns no state instead of guessing.
Note: Claude Code does not export CLAUDE_SESSION_ID to tool
subprocesses today; this path is forward-looking defense-in-depth.
2. No env: a complete row updated within the last 5 minutes is the
just-finished caller's completion-boundary advisor call — most recent
wins, unless a fresh live row also exists (ambiguous).
3. Else exactly one live row updated within the last 60 minutes (older
non-complete rows are crash leftovers; the registry stale sweep only
prunes them after ~7 days).
4. Anything else returns an ADVISOR STATE UNAVAILABLE marker and warns
on stderr — wrong state is worse than no state for a reviewer.
Adds a self-contained bun test suite covering all resolution branches.
Owner
|
Thank you @jbmml — the deterministic session-resolution intent is sound. This specific path ( |
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
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.
Part 1 of 2 for #1466 (the
Inference.tshalf; theAlgoPhase.tsfail-closed change follows in a separate PR).What
synthesizeAdvisorState()(the--mode advisor --auto-statepath) readwork.jsonexpectingactive/current/activeSessiontop-level keys. The registry's actual shape is{sessions: {slug: row}}— those keys never exist — so resolution always fell through to "most recently modified directory inMEMORY/WORK/". That fallback is usually right by accident, but it carries no session identity: under concurrency or right after another session completes, it attaches an unrelated session's ISA as the advisor state, and the reviewer reviews the wrong work.How
Deterministic resolution, documented in the function's doc comment:
CLAUDE_SESSION_ID, any phase — aphase: completerow of the SAME session is still this session, and the verification doctrine fires the advisor at the completion boundary itself. Set-but-unmatched returns no state rather than falling through to a guess. (Honesty note: Claude Code does not exportCLAUDE_SESSION_IDto tool subprocesses today, so this path is forward-looking defense-in-depth; the operative resolution today is steps 2–4.)completerow updated within the last 5 minutes is treated as the just-finished caller making its mandatory completion-boundary advisor call — the most recent such row wins, UNLESS any fresh live row also exists (then we cannot tell which session is calling → ambiguous). Without this window, fixing the mtime guess would have broken the doctrine's advisor-after-complete flow for every single-session user.ADVISOR STATE UNAVAILABLEmarker and warns loudly on stderr. Wrong state is worse than no state for a reviewer, so the mtime-based directory fallback is removed rather than fenced.Behavior change disclosure: previously
--auto-statealways attached SOME ISA. Now, when the caller's session cannot be determined (multiple live sessions, stale-only registry, unmatched env), the advisor runs with an explicit no-state marker instead of a possibly-wrong blob.No CLI contract change: flags, positional args, output framing, and exit codes are untouched.
Verification
Self-contained bun suite added alongside the tool (
Inference.autostate.test.ts, runs in-process against a throwaway$HOME): env-match incl. complete row; env unmatched → no state; single fresh live → its ISA; two live → no state; recent-complete alone → used; two recent-completes → most recent; recent-complete + live → no state; 1h-old complete → no state; 2h-idle live → no state; native placeholder rows excluded. 10/10 pass;tsc --noEmit --strictclean.