Conversation
readSessionUsage quarantines a log it cannot parse, so the guard at the top of the function skips it on later polls, but a log it cannot open records nothing. The next poll opens the same file and fails identically for the life of the scan, and because an unopenable log never produced a thread id, the failure also escapes the thread-tree filter in #readSessions. stop() awaits refresh() unguarded, so the final poll can reject after the turn completed and discard the caller's usage. Quarantine the session on an access refusal the process cannot clear by retrying, mirroring the parse-failure exit. Other open failures keep retrying: a process-wide shortage such as EMFILE clears on its own, and retiring a session for it would stop observing usage that could still be read.
|
Flagging an overlap I should have caught before opening this. Staff PR #465 — function isSessionAccessDenied(error: unknown): boolean {
return (
isRecord(error) && (error["code"] === "EACCES" || error["code"] === "EPERM")
);
}and calls To be precise about the current state: If #465 lands first, though, this one is redundant and should be closed rather than rebased. I have not closed it myself because #465 is still open, so the fix is not on The note in the description about #224 stands: it was closed by its author on 2026-09-01 with no maintainer decision recorded. |
Summary
An inaccessible session log under
$CODEX_HOME/sessionsfails on every cost poll for the rest of the scan, and can fail a scan that already completed.readSessionUsagehas two failure exits and only one of them records that the session is bad. A log it cannot parse setssession.unreadable, so the guard at the top of the function skips that file on later polls: one error, then the tracker moves on. A log it cannot open records nothing, so the next poll opens the same file and fails identically everyCOST_POLL_INTERVAL_MS.The failure also escapes the thread-tree filter in
#readSessions. A deferral is filtered byincludedonly when the thread id is known, and an unopenable log never produced one, soif (session.threadId === null) throw error;rethrows instead of deferring — for a file that can never be attributed to this scan.Because
stop()awaitsrefresh()unguarded, the final poll can reject after the turn already completed, discarding the caller'sfallbackUsageand reporting a successful scan as failed.Reproduced on
mainat70d5b2edwith a realchmod 0o000rollout file: fouronErrorreports within 300 ms, andstop()rejecting withEACCES: permission denied, open '.../sessions/2026/07/26/rollout-prior-thread.jsonl'fromsrc/cost.ts:191.Closes #223. #224 proposed a fix for this and was closed by its author on 2026-09-01 with no maintainer decision recorded; this is an independent implementation against current
main, and it differs in the respect described under Changes.Changes
readSessionUsagenow quarantines the session whenopen()fails with an access refusal the process cannot clear by retrying (EACCES,EPERM), mirroring the existing parse-failure exit. The error is still thrown, so it is still reported — once.quarantineSession()helper. The parse-failure path is unchanged in behavior.EMFILEclears on its own, and retiring a session for it would stop observing usage that could still be read, which would weaken the--max-costenforcement the tracker exists for. No attempt counter or other new limit is introduced.Not changed, deliberately:
stop()still awaitsrefresh()unguarded, so a failure that first appears exactly at the final refresh would still discardfallbackUsage. With inaccessible logs quarantined, the reported failure mode no longer reaches that path. Hardeningstop()against unrelated refresh failures is a separate question and is left out of this change.Testing
From
sdk/typescripton macOS arm64, node 24.19.0, bun 1.3.14, pnpm 11.11.0:bun test --timeout 30000 tests-ts/cost.test.ts -t inaccessibleat70d5b2edwithout thesrc/cost.tschange: 2 fail —expect(received).toHaveLength(expected) / Expected length: 1 / Received length: 4, andstop()rejecting withEACCES: permission denied, open '/…/rollout-prior-thread.jsonl'atasync stop (src/cost.ts:191:16).bun test --timeout 30000 tests-ts/cost.test.ts: 74 pass, 0 fail (72 before the two new tests).quarantineSession(session)call from the open-failure path makes both new tests fail again; restoring it makes them pass. So the tests pin this change rather than the surrounding code.bun test --timeout 30000 tests-ts/cost-context.test.ts tests-ts/cost-model.property.test.ts tests-ts/cli-show-cost.test.ts: 24 pass, 0 fail.pnpm run typesrc=0,pnpm run lintrc=0,pnpm run formatrc=0 (All matched files use Prettier code style!).bun test --timeout 30000 tests-ts/api.test.ts: 54 pass, 4 skip, 109 fail, identical with and without this change — the same three counts measured at base and at head. Those failures areModuleNotFoundError: No module named 'workbench_scan_usage'from the generated_bundled_pluginpayload, which I could not build here:pnpm run build:pluginstops atplugins/codex-security/native/prebuilt/COPYRIGHT-library.html, and preparing that payload needs the pinned Rust toolchain. For the same reasonpnpm run testwas not run locally; the focused module suites above were run instead.The two new tests are gated
test.skipIf(process.platform === "win32" || process.getuid?.() === 0), following the existing permission fixture intests-ts/security-policy-helper.test.ts:203, because the fixture is a POSIX permission bit. Both restore the mode in afinallyblock so the shared temporary-directory cleanup can remove the fixture.Risk and rollout
No public CLI or SDK surface changes, and no new limit, flag, or configuration. Behavior changes only for a session log that cannot be opened: it is reported once and then skipped, instead of being reported on every poll for the life of the scan. A quarantined session is not re-read if it later becomes accessible — that is the contract the parse-failure path already has. Missing files (
ENOENT) are unchanged, and transient open errors keep retrying as before. No release, migration, or rollout ordering impact.Public disclosure review