feat(daemon): attest human presence to LORE for the Codex path (lr-f7c100) - #421
Conversation
…c100) Codex console sessions have no equivalent to Claude's Stop-hook pulse attestation (LORE PR #1897), so 29 of 31 substantive Codex rollouts on this host since 09-01 mint no engram (lore lr-37c8cc). The Console is the component that knows a human is present -- a person is typing into it -- so it now declares that directly via LORE's platform-agnostic verb: lore session attest-human <session_id>. Human presence is marked structurally, not heuristically: session.humanOriginated is set only inside project-user-message.js's WS "message"/ "send_scheduled_now" handlers, the one code path in this codebase reachable exclusively by a live client turn. Headless session drivers (project-loop.js's Ralph Loop iterations, project-external-trigger.js, scheduler.js) build history and call sdk.startQuery directly and never pass through that handler, so they can never set the flag -- a scheduled or autonomous Codex dispatch is never attested. The attest-human call fires in sdk-message-processor.js the moment the Codex thread/rollout id first becomes known (parsed.sessionId, gated on session.vendor === "codex" && session.humanOriginated), which is before any turn completes -- i.e. before LORE's sentinel evaluates a dispatch decision for that session id. Claude console sessions are deliberately NOT duplicated here: the pulse hook already covers them (verified live: this session's own sidecar carries source=hook). Fail-open by construction (lib/lore-attestation.js): the call is fire-and-forget, never awaited by the caller, and any failure -- including the lore binary being entirely absent (ENOENT) -- is swallowed after a log line so session create is never affected on a host without LORE installed. TASK: lr-f7c100 Tests: npm test, 1588/1588 passing (6 new: 4 in test/sdk-message-processor-codex-human-attestation-lr-f7c100.test.js, 2 in test/lore-attestation-lr-f7c100.test.js).
PEACHES — Clean (0 blocking findings)Reviewed commit: 73b3286 (feat/lr-f7c100-codex-human-attestation) SummaryThe implementation is structurally sound. All six verification checks pass: 1. HUMAN/HEADLESS BOUNDARY — VERIFIED
2. ORDERING — VERIFIED
3. FAIL-OPEN CORRECTNESS — VERIFIED
4. IDEMPOTENCE — VERIFIED
5. CORRECT SESSION ID — VERIFIED
6. CLI-ONLY REQUIREMENT — VERIFIED
Scope NoteThe task narrowing is correct: Claude sessions are already covered by LORE's pulse hook (PR #1897). This PR scopes to Codex only, which was the uncovered half. Testing ObservationThe unit tests (lore-attestation-lr-f7c100.test.js, sdk-message-processor-codex-human-attestation-lr-f7c100.test.js) are comprehensive for the call-invocation contract but do NOT verify that an actual sidecar file appears on disk. Per retro tome #845, reports success while nothing happened is the dominant failure mode. However, this is a responsibility boundary issue: the console calls the LORE CLI correctly; LORE's own test suite must verify the sidecar is created. The console code cannot reasonably test LORE's file-writing behavior. No blocking issue. |
|
Audit of lib/lore-attestation.js, lib/project-user-message.js, lib/sdk-message-processor.js, and the two new test files (lr-f7c100). Reviewed base 8302c71..73b3286.
2/3. Over-attestation / auth: session.humanOriginated is set only at project-user-message.js:384 (inside the WS {type:"message"} handler, reachable only via a connected client's handleUserMessage dispatch) and :335 (send_scheduled_now, same WS path, requires nowSession.scheduledMessage already set by a prior authenticated schedule_message call). Verified project-loop.js (Ralph Loop iterations/crafting/judge), project-external-trigger.js (file-watcher spawnSession/pushMessageToSession), and scheduler.js (cron-triggered loop registry) all call sdk.startQuery/pushMessage directly on sessions from sm.createSession() and never touch humanOriginated -- headless paths cannot reach the flag. The gate at sdk-message-processor.js:254 (session.vendor==="codex" && session.humanOriginated) is structurally sound given that. WS authentication itself (ws._clagenticUser, set in project-connection.js before handleConnection) is pre-existing infrastructure this PR does not modify or weaken.
No blocking or nit findings. Clean, well-scoped diff; provenance and fail-open properties for a security-relevant attestation path are all traceable to a trusted local subprocess boundary. scanners_run: gitleaks/trufflehog/semgrep/osv-scanner not available in this sandbox (no scanner binaries wired for this invocation) -- judgment-only review performed instead, diff obtained via bobbie-github-security read (GitHub API) against #421, base 8302c71 verified resolvable. |
… (lr-f7c100) PEACHES fnd (via NAOMI/coordinator CI-red report on PR #421 at head 73b3286): the unit test for attestHumanSession's fail-open PATH case mutated the real process.env.PATH and restored it in a synchronous finally, but execFile's child-process spawn is asynchronous. The restore ran before the spawn it triggered actually resolved, so the test (a) never verified the ENOENT branch it claimed to -- the spawn very likely raced against the ALREADY-restored real PATH, which has a real lore binary on most dev/CI hosts -- and (b) left an un-awaited, unaccounted-for child process in flight past the end of the test. That is exactly the leaked-handle failure class check-test-count.js (lr-795882/lr-a7b03e) exists to catch, and a strong candidate for the observed CI failure: two clean local npm test runs at the flagged head (73b3286) both passed 1609/1609, consistent with a race that resolves differently under CI's process/scheduler timing than on an idle sandbox. Fix: lib/lore-attestation.js's attestHumanSession() gains an optional, test-only onSettled hook, invoked once the execFile callback (or the synchronous invalid-input early return) actually completes -- never passed by any production call site, so production behavior and signature are unchanged for every existing caller. The test now awaits real completion via this hook instead of touching global process.env.PATH at all, so it is deterministic regardless of whether a real lore binary is present in the test environment, and never leaves a child process unaccounted for. Ruled out lr-eb0d5e (the daemon-spawn/waitForServer flake pool -- shell-asset-public-before-auth-lr-2895ea, ws-subprotocol-echo-cookie-auth-lr-4c58ae, boot-smoke-lr-1a5f, project-asset-auth-fallback-lr-e33776): none of those four files touch lib/lore-attestation.js, lib/sdk-message-processor.js, lib/project-user-message.js, or process.env.PATH, and their signature symptom (daemon did not respond within 20000 ms) is a different harness and a different failure shape than an unmocked async CLI-spawn race in a brand-new test file. TASK: lr-f7c100 Tests: npm test, 1609/1609 passing (2 local clean runs at this fix).
|
PEACHES — clean (0 blocking, 1 design observation) The fix closes the race condition documented by the user, and the re-verified contract items hold at new head SHA 6764e37: 1. Production signature unchanged, fire-and-forget preserved:
2. Race condition closed, genuinely awaited:
3. No leaked handles:
4. Prior contract items confirmed at new SHA:
Design observation (peaches.nit.observation): |
|
BOBBIE re-audit of PR #421 at head 6764e37 (base 8302c71, self-fetched from the live PR via bobbie-github-security read, not hand-counted). SCANNER SWEEP (base..head, 8302c71..6764e37):
MANUAL VERIFICATION (re-confirmed at 6764e37):
FINDING: none blocking. No RULEBOOK.md rule triggered. The onSettled async-exception-context gap (item 2, line 63) is real code-shape debt but has no citable reachable production path today -- fails the Pre-Report Gate severity test, not reported as a finding. |
|
Merged via clagentic-loadout v0.2.0
|
What changed
On a Codex session driven by a human through the console, the console now calls LOREs platform-agnostic human-presence declarer: lore session attest-human SESSION_ID, keyed on the Codex thread/rollout id, the moment that id first becomes known.
Why
LOREs engram-mint gate is default-deny on a positive human-presence attestation (lore lr-6b2f6e). Claude console (SDK) sessions are already covered by LOREs own pulse hook (PR 1897): Claude Codes Stop-hook lifecycle fires for an SDK-driven query exactly as it does for the claude CLI; verified live on this very session (its sidecar carries source=hook, and lore engram list shows it already scribed). Codex has no equivalent hook: the console drives Codex directly over the app-server JSON-RPC protocol, so 29 of 31 substantive Codex rollouts on this host since 2026-09-01 mint no engram (lore lr-37c8cc). This closes that gap from the console side, per lore lr-f7c100 comment 1s explicit scope narrowing (the Claude half may already be satisfied; the half NOT covered is the consoles Codex path).
How
Human-vs-headless distinction is structural, not heuristic. session.humanOriginated is set only inside project-user-message.jss WS message/send_scheduled_now handlers: the ONE code path in this codebase reachable exclusively by a live client turn. Headless session drivers (project-loop.jss Ralph Loop iterations, project-external-trigger.js, scheduler.js) build history and call sdk.startQuery directly from their own functions and never pass through that handler, so they structurally cannot set the flag: a scheduled or autonomous Codex dispatch is never attested. This was confirmed by an explore pass over the session-creation call graph (all sm.createSession/sdk.startQuery call sites enumerated) before writing any code.
Ordering (load-bearing per the tasks acceptance criteria): the attest-human call fires in sdk-message-processor.jss existing _isNewSessionId branch, the moment parsed.sessionId (the Codex thread id, captured from thread/starts result) is first seen, gated on session.vendor being codex and session.humanOriginated being true. This is before any turn completes, i.e. before LOREs sentinel evaluates a dispatch decision for that session id. Demonstrated directly: ran lore session attest-human against a synthetic id with no prior sidecar, confirmed via find that the sidecar did not exist beforehand and does exist immediately after, at the exact path LORE writes to at runtime on this host (/tmp/lore-runtime-0/, discovered via find, not hardcoded from the task descriptions prose).
Fail-open by construction: lib/lore-attestation.js is a new, single-purpose module using execFile with a bounded 5s timeout, fire-and-forget (caller never awaits it), and any failure, including ENOENT when the lore binary is entirely absent, is swallowed after a log line. Session create is never affected on a host without LORE installed.
Claude is deliberately NOT touched. Calling attest-human for Claude too would be harmless but is out of scope per comment 1 and code-craft rule 1 (minimal change for the assignment): the pulse hook already covers it, confirmed live on this session.
Verification (per retro tome 845, demonstrated failure first)
Test status
npm test: 1609 of 1609 passing (6 new).
TASK: lr-f7c100
Update: CI-red fix folded in
Coordinator/NAOMI reported PR 421 CI RED at head 73b3286 (check-test-count FAIL, named test failure, pull_request check-suite, 56s run). Diagnosed and fixed on this same branch rather than opening a new PR.
DIAGNOSIS: category (a), a bug in this diffs own new test file, not (b) unrelated environment divergence and not (c) the lr-eb0d5e daemon-spawn/waitForServer flake pool.
test/lore-attestation-lr-f7c100.test.js originally mutated the real process.env.PATH to simulate a host without the lore binary, then restored it in a synchronous finally block, before the async execFile call it triggered had actually settled. That is a genuine race: the restore ran before the spawn resolved, so the test never verified the ENOENT branch it claimed to, and left an unawaited child process in flight past the end of the test, exactly the leaked-handle failure class check-test-count.js (lr-795882/lr-a7b03e) exists to catch. This matches PEACHES own finding that the tests should verify real settlement rather than only mocking attestHumanSession.
Ruled out lr-eb0d5e explicitly: none of its four tracked files (shell-asset-public-before-auth-lr-2895ea, ws-subprotocol-echo-cookie-auth-lr-4c58ae, boot-smoke-lr-1a5f, project-asset-auth-fallback-lr-e33776) touch lib/lore-attestation.js, lib/sdk-message-processor.js, lib/project-user-message.js, or process.env.PATH, and that pools signature symptom (daemon did not respond within 20000 ms) is a different harness and a different failure shape than an unmocked async CLI-spawn race in a brand-new unit test file.
FIX: lib/lore-attestation.js gains an optional, test-only onSettled hook on attestHumanSession, invoked once the real execFile callback (or the synchronous invalid-input early return) actually completes. No production call site passes it, so production signature and behavior are unchanged for every existing caller. The test now awaits real completion through this hook and never touches global process.env.PATH, so it is deterministic regardless of whether a real lore binary exists in the test environment, and never leaves a child process unaccounted for.
VERIFICATION: two clean local npm test runs at the original flagged head (73b3286) both passed 1609 of 1609, which is itself consistent with a race that resolves fine on an idle local sandbox but differently under CI process/scheduler timing. After the fix, two more clean local runs also pass 1609 of 1609. I could not pull the CI check-run log directly (loadout-git-host-api is scoped away from AMoS for that read; it is PEACHES/BOBBIE/NAOMI surface), so this diagnosis is built from direct code inspection of the exact hazard PEACHES flagged, not a captured failing-test name from the CI log itself. If CI still shows the same named failure at the new head, that would falsify this diagnosis and needs a different root cause.
TASK: lr-f7c100