perf(windows): Node hot paths for the check-inbox hook and CLI entries (implements #449) - #463
Draft
koujimatsumoto76-spec wants to merge 1 commit into
Draft
Conversation
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.
What
Implements the proposal from #449. On Windows, the measured hot hook / CLI entries today
walk bash -> many short-lived processes (sed / tr / sqlite3 / identity walk),
which costs 40-80 s per invocation on a stock Git Bash install (measured
breakdown in #449). This PR adds first-class Node implementations for the hot
paths and dispatches to them where it pays:
scripts/lib/identity-resolver.js) — purelookup: project + type -> identity candidates. Reused by every entry below;
replaces the per-call
identities.shspawn (~70 s of a 77 s SessionStart).check-inbox.js— the Stop-hook path (cooldown, actas-lock respect,unread select, read_at on displayed rows, block/status JSON) via
node:sqlite.agmsg-fast.js) —team/send/history/inbox/whoami, byte-parity with the bash output contracts (raw-compared; seeBehavior notes). Only commands with an existing upstream
.shentry areincluded — no upstream-unused code.
(Native SessionStart is deliberately NOT in this PR: open PR feat(windows): native Codex hook transport (queue + out-of-sandbox dispatcher) #305 already
reworks the Windows hook transport across the same files
(hooks-json/delivery/_delivery). Our measurements strongly support that
direction — the Git Bash entry alone costs 6-9 s, and our local native
SessionStart cut 77 s to ~1-2 s — so we would rather bring those numbers and
review energy to feat(windows): native Codex hook transport (queue + out-of-sandbox dispatcher) #305 than collide with it. If feat(windows): native Codex hook transport (queue + out-of-sandbox dispatcher) #305 stalls, a follow-up PR
can carry it.)
Dispatch contract (uniform): Node path only on Windows; each entry
(
check-inbox.jsand the fast CLI) probesnode:sqliteand falls back tothe existing bash path only on a failed probe (exit 78), i.e. before any
side effect. The gating is
deliberately narrow: the bash paths remain the contract of record on every
platform, so the Node paths can be extended to other OSes later without this
PR taking that risk now.
node:sqliteis still marked experimental upstreamin Node; because every native entry is probe-gated, an API change degrades to
the bash path instead of breaking anything. After
the Node implementation has started, a non-zero exit is loud and does NOT
fall back (a mid-run bash retry could double side effects such as a send
INSERT or read_at updates). Non-Windows platforms are untouched.
Measured (Windows 11, Git Bash/MSYS2, Node v25)
Behavior notes
of representative outputs (LF-normalized).
read_atis set only on therows actually displayed — this matches current main (check-inbox.sh /
inbox.sh already restrict to displayed IDs), so no behavior divergence
against the tree this PR targets.
probe-gated, so an older Node simply keeps the bash path.
node:sqliteneeds Node >= 22.5; the capability probe (not a versioncompare) decides, and older/absent Node keeps the bash path.
(CreateFileMapping error 5), the native path is an availability fix, not
just performance.
Not in this PR
launcher's role-session/thread authority or the request-file contract,
and — matching current behavior — the codex SessionStart still skips the
post-request cleanup/GC half (upstream's codex plug already exits 0 after
the request hand-off); other types' cleanup semantics are untouched.
walk from the hot paths. delivery.sh status reports 'stale pidfile' for a live bridge on Windows (MSYS kill -0 cannot see native node PIDs) #415 is untouched here (our local install also has
a native read-only
mode/pairsfast path, but upstream has no wired entryfor them today, so they are left out of this PR rather than shipped as
unused code).
config.sh3-level key defect found during this work will be filed asa separate issue (not yet filed; no existing upstream issue matches).
Structure (patchless boundary, against main 3f87d60)
This PR does NOT port our local patch artifacts (full-file replacements /
anchor edits were derived against v1.1.6 and are not portable — e.g. the
request file grew from 5 local fields to the current 3-field contract, with
identity authority moved into the launcher). Instead it implements the same
behavior first-class in the current tree:
scripts/lib/identity-resolver.js+scripts/check-inbox.js+scripts/agmsg-fast.jsas new files (no runtime deps,node:sqliteonly);hook generation (
commandWindows) and the.shentry heads;NOT modified.
Minimal seam (5 change areas — 4 runtime files + tests — per on-tree verification against 3f87d60):
scripts/lib/identity-resolver.js(new)scripts/check-inbox.js(new) +scripts/check-inbox.shheadscripts/agmsg-fast.js(new) + the 5 CLI.shentry headsNo changes to hooks-json.sh / delivery.sh / _delivery.sh — zero overlap with
open PR #305 (Windows hook transport), by design.
Fallback semantics: diagnostics to stderr and read-only probing are allowed
before falling back (exit 78); once the Node implementation has started, a
non-zero exit is loud and never falls back — a bash retry could double side
effects (a second send INSERT, repeated read_at updates).
Validation gate (before submission): the implementation is built and verified
on an isolated clone of current main. Nothing is transplanted from our
v1.1.6-based local install. Per-leg time boxes (a run is terminated at 5 min
of silence or its cap, and switched to an alternative gate instead of
retried): Node units <= 3 min; real
commandWindowsPowerShell smokes <= 60 seach; static checks /
bash -n<= 1 min; full Bats on Linux/macOS (CI or anequivalent environment) <= 10 min. Windows Bats suites are NOT part of the
gate — measured on this machine they cost 4-7 min for 3 cases (delivery
subset aborted at 15 min), which is the very spawn tax this PR addresses;
the Windows delivery behavior is covered by the PowerShell smokes instead,
and that substitution is recorded openly.
Compatibility gates: shared-path shell code is bash-3.2-safe (macOS ships
3.2); a fixture pins the POSIX
commandfield byte-unchanged whencommandWindowsis added; full Bats pass on the macOS leg.🤖 Generated with Claude Code