feat(PTYCAP-01): PTY token accounting — live mid-turn usage, interactive/programmatic bucket split - #395
Open
finedesignz wants to merge 26 commits into
Open
feat(PTYCAP-01): PTY token accounting — live mid-turn usage, interactive/programmatic bucket split#395finedesignz wants to merge 26 commits into
finedesignz wants to merge 26 commits into
Conversation
… interactive Tracer slice for PTYCAP Phase 1 (SC-1/SC-2/SC-3): the supervisor now tails an interactive PTY session's own on-disk Claude transcript JSONL (supervisor/src/usage/pty-transcript-tail.ts, a verbatim port of the hub's telegram transcript tailer) and, for each assistant record carrying a usage block, emits the existing usage_event WS frame additively tagged runner_type: 'pty-interactive' (supervisor/src/usage/pty-usage-emitter.ts). session-bridge.ts's ensurePtyRunner() owns the tailer's lifecycle end to end (start alongside the PTY, stop at all three teardown sites), never on the critical path for PTY startup. The hub's AgentUsageEvent zod schema, the usage_event handler, and recordTokenUsage() all thread the tag through into a new, additive token_usage.runner_type column (idempotent DDL, safe default, CHECK constraint, index) so a PTY turn's spend is visible in the ledger while the turn is still running, instead of never. Fixed a real bug found in inherited partial work from an interrupted prior run: hub/src/ws/agent-protocol.ts had named the new zod field `runner` instead of `runner_type` — since zod strips unknown keys, every PTY row would have silently landed mislabelled 'stream-json' with no error anywhere. Caught by re-running the plan's own grep acceptance criterion before trusting the earlier diff review. RECORD-only: no gate, block, deny, or dispatch call site added. Scoped to cli_kind='claude' only (Codex is a documented fast-follow). No API key, no PTY write, no hub-side filesystem read of a homedir-derived path. Verified: supervisor/test/pty-usage-tail.test.ts (17/17), bun run schema-lint, hub/test/usage-event-handler.test.ts, and the no-api-key/no-streamjson-pty + default-backend-selector + pty-reattach-persistence guard suites all green unchanged (pty-byte-relay/pty-orphan-teardown fail only on missing native node-pty on this host — pre-existing, unrelated to this change).
…ree config - 01-01-SUMMARY.md documents the tracer slice, the inherited runner_type naming bug found and fixed, and the two pre-existing/ambient test issues ruled out as unrelated to this phase. - STATE.md / ROADMAP.md updated via gsd-tools (phase begun, plan 01-01 marked complete). - .planning/config.json: workflow.use_worktrees=false for the remainder of this phase run — this session resumed with uncommitted partial edits from an interrupted prior run, and a fresh isolated worktree per plan would fork from HEAD without that diff, risking duplicated or conflicting work on merge. All four plans in this phase execute sequentially in this worktree.
… and Postgres hub/test/token-usage-runner-type.test.ts (new, DB-free): - recordTokenUsage() with differing runnerType produces distinctly-tagged ledger rows; omitted runnerType defaults to 'stream-json'; both buckets still upsert ONE combined token_usage_daily row (P1-D-D held). - AgentUsageEvent zod PRESERVES runner_type on a PTY-shaped frame (this is the exact case the inherited `runner` typo, fixed in 01-01, would have failed); an old-shape frame with no runner_type parses to undefined; a third enum value fails zod parsing outright. - Source assertion against schema.sql: token_usage_runner_type_check names exactly 'stream-json' and 'pty-interactive', comments stripped first. hub/test/e2e/schema-double-apply.e2e.test.ts (extended, REMO_E2E_DB_URL-gated): one case proving the CHECK constraint is LIVE against real Postgres — an out-of-enum runner_type value is rejected, 'pty-interactive' is accepted. Skips cleanly without REMO_E2E_DB_URL (9 skip, 0 fail locally); the accept/ reject path runs in Woodpecker's postgres:16 service. Verification-only — zero hub/src or supervisor/src changes.
…records stream-json Extends hub/test/usage-event-handler.test.ts (3 pre-existing cases unchanged) with two cases: runnerType omitted records 'stream-json' (the pre-PTYCAP supervisor shape), and runnerType:'pty-interactive' is captured verbatim (so the default is a genuine default, not a hardcode). fakeSql's INSERT destructuring gets the trailing runner_type bind so it reads the current column order rather than a stale shifted one.
01-02-SUMMARY.md documents the SC-2 proof; ROADMAP.md plan checkbox flipped.
…session-close needed hub/test/pty-usage-midflight-visibility.test.ts (REMO_E2E_DB_URL-gated, real Postgres): getTodayTokenTotal() for a fresh user starts at 0; six incremental pty-interactive writes produce a strictly monotonically increasing sequence of readings whose final value equals the exact arithmetic sum, with the ceiling crossing observable at the expected index (SC-3's "detectable mid-flight" claim, expressed as an observation, not a gate call); a stream-json row for the same user adds to the SAME total (no second aggregation path); a cache-read-only record still increases the total (the 2026-07 incident shape an I/O-only sum would have missed). No exit, kill, or session-close event anywhere in the suite. Imports/invokes no gate — RECORD/ OBSERVE only. Skips cleanly without a DB (6 skip, 0 fail locally).
…system read hub/test/no-hub-side-transcript-fs.test.ts: walks every .ts file under hub/src recursively, strips block then line comments, and asserts the set of files whose remaining code references the home-directory resolver equals exactly the single allowlisted prefix (hub/src/telegram/transcript/, Phase 20's pre-existing, flag-gated-OFF Telegram tail). The matcher is proven non-vacuous (it finds both known adapters before filtering), the allowlist size is asserted so growing it is a deliberate act, and a comment mentioning the pattern neither trips nor satisfies the check. Dependency-free (node:fs/node:path/bun:test only), no DB, no network.
…fused before any read supervisor/test/pty-usage-path-containment.test.ts: a relative projectDir, one carrying a parent-directory segment, and one carrying a NUL byte each cause PtyUsageEmitter.start() to log a warn and return without creating any watcher, timer, or emitted frame. When the located transcript's real path resolves outside the projects base (simulated via the projectsBase seam pointing at an unrelated temp dir — no symlink needed, unreliable on Windows without elevation), start() refuses with a path_escape-shaped log and NEVER reads the file, even though it holds a valid usage record. A well-formed, properly contained path still tails and emits normally, proving the guard is selective. Every refusal path leaves stop() safe to call. Confirms resolveSessionDir/realPathContained are imported from ../commands/session-read verbatim, never re-derived.
.woodpecker/qc.yaml: check-baseline walks hub/test only, so it added a dedicated step running supervisor/test/pty-usage-tail.test.ts (the SC-1 proof) so it actually gates the PR, not just a developer's laptop. tools/regression-baseline.json: fresh floor-configuration measurement (no REMO_E2E_DB_URL, no DATABASE_URL) — actual pass=2059 skip=255 fail=0 total=2314 (from recorded 2010/247/0/2257). New _skip_note_ptycap names every delta: token-usage-runner-type.test.ts (7), no-hub-side-transcript-fs.test.ts (4), and 2 new usage-event-handler.test.ts cases add 13 DB-free passes; pty-usage-midflight-visibility.test.ts (6) and one new schema-double-apply.e2e.test.ts case add 7 REMO_E2E_DB_URL-gated skips. skip_max raised 248 -> 256 (+8, not +7) because the measured delta also carries 1 pre-existing DB-gated skip in coolify-webhook-triage-e2e.test.ts already on this branch before Phase 1's planning commit (unrelated scheduler/triage work) — named explicitly rather than silently absorbed. pass_min left at 1850 (~10.2% headroom, consistent with the WORK milestone's precedent of not tightening on every re-measurement). fail_max stays 0; no unconditional skip introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/usage-cost.md gains a PTYCAP Phase 1 section: the end-to-end flow (interactive CLI's own transcript JSONL -> PtyUsageEmitter tail -> usage_event tagged runner_type:'pty-interactive' -> same recordTokenUsage() path), why the tail lives supervisor-side (no ~/.claude/projects in the hub's Coolify container, same reason REMO_TELEGRAM_TRANSCRIPT_TAIL stays OFF there) with a pointer to the no-hub-side-transcript-fs.test.ts guard, the runner_type domain + its live CHECK constraint, and the four decisions a future reader would otherwise mistake for oversights: token_usage_daily's PK stays unsplit, sessions.transcript_path/setSessionPtyIdentity() stays unwired to this path, accounting is claude-only with Codex as a verified fast-follow, and PTY cost is always cost_source:'estimated'. Preserves 01-RESEARCH.md Assumption A2 as a named operator smoke check, not a CI gate. CLAUDE.md's Usage cost ledger Docs map row is extended in place (one sentence) naming the new PTY source and the runner_type split; nothing else in the row or table changed. No HTTP route changed, so docs/openapi.json and docs/api.md are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All 4 plans in PTYCAP-01-pty-token-accounting now executed and summarized. ROADMAP.md: 01-04-PLAN.md checked off, 4/4 plans executed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code review of Phase 1 (18 files, standard depth): 1 critical, 3 warning. CR-01: supervisor/test/pty-usage-path-containment.test.ts (the ASVS V4 containment negative test) is never executed by any CI gate. WR-01: async race in the ported tailJsonl's fromStart:false initial-offset assignment could replay a resumed transcript's history as new PTY usage. WR-02: docs/usage-cost.md misattributes the runner_type default to zod instead of the hub handler. WR-03: new PTY-tailer tests are real-timer-dependent (flakiness risk, not novel to this phase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
qc.yaml's explicit belt-and-suspenders step only ran pty-usage-tail.test.ts, leaving the ASVS V4 path-traversal/symlink-escape negative test for the PTY tailer unexecuted by any CI gate despite the file's own comment claiming coverage was needed for exactly this reason. Also corrects the misleading regression-baseline.json narrative that implied this file was already CI-gated.
fromStart:false set the initial byte offset via an un-awaited stat().then(), while fs.watch and the poll timer were armed synchronously right after. For a resumed transcript whose recent mtime is caused by the CLI's own append landing right as tailing starts, a watch notification could fire pump() before offset resolved, replaying the file's entire history as new usage. Offset now settles before the watcher/poll timer are armed. Ported to both the supervisor copy and its hub source of truth per the file's own header comment.
The doc claimed zod defaults the omitted runner_type field to 'stream-json'; the field is actually optional with no zod default and resolves to undefined when absent — the fallback lives in the hub's usage_event handler (msg.runner_type ?? 'stream-json'). Reworded to match what the schema and handler actually do, per token-usage-runner-type.test.ts.
Owner
Author
AI Review GateGate verdict: FAILURE — blocking finding from Codex Claude Code (QC): pass
Codex: BLOCK
Policy: both reviewers are blocking — a genuine blocking finding from either fails the gate. An infrastructure failure (quota exhausted, timeout, auth failure, no parseable output) is ADVISORY and never blocks: it means the reviewer never saw the code, which is not a verdict about the code. |
…OCKING) Codex found that resolveTranscriptPath's mtime/slack window can pin a SIBLING session's actively-typed-in transcript instead of the new session's own file, silently attributing another session's token spend under this session_id -- the worst failure mode for a metering system. Fix: session-bridge.ts now snapshots the .jsonl basenames already present in the project's Claude session dir BEFORE spawning the PTY process (snapshotPreExistingTranscripts). That snapshot is threaded into PtyUsageEmitter as preExistingNames and excludes those names from resolveTranscriptPath's candidates outright, regardless of how fresh their mtime looks -- only a genuinely new file (absent from the pre-spawn snapshot) can qualify. When nothing new has appeared yet the locator keeps polling instead of guessing, matching the fail-closed principle: no usage emitted beats another session's usage misattributed. Also fixes the QC WARNING: cliKind was derived from a second, independent resolveHumanBackend() call instead of the id already resolved for the PTY runner itself -- now resolved once and reused, so the two can never disagree. The QC INFO (TOCTOU between the one-time realPathContained check and tailJsonl's repeated reopens) is judged, not fixed: both sides of the window are supervisor-local reads on the same host, never hub-reachable -- an attacker able to plant a symlink there already has local write access to this user's home dir, which subsumes the check. Documented inline rather than adding ceremony that buys no real containment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ck (QC round 2)
Round-1 fix excluded pre-existing sibling transcripts via a pre-spawn snapshot,
but snapshotPreExistingTranscripts() returned an empty Set whether the session
dir genuinely had no pre-existing files OR simply didn't exist yet / wasn't
readable at snapshot time — indistinguishable, so resolveTranscriptPath() would
still accept any fresh file in the latter case, reopening the same
mis-attribution door through a different route (round-2 Codex BLOCKING finding).
snapshotPreExistingTranscripts() now returns { reliable, names } instead of a
bare Set. reliable:false (dir absent/unreadable at snapshot time) makes
PtyUsageEmitter.start() refuse outright for that session's whole lifetime —
no timer, no watcher, nothing ever pinned — rather than fall back to
"accept any fresh file." resolveTranscriptPath() also now refuses when more
than one qualifying new candidate exists at once (previously silently picked
the lowest mtime, which is exactly the kind of guess between two candidates
that causes mis-attribution); it keeps polling until it resolves to exactly
one, or gives up.
Governing principle unchanged: emitting NO usage for a session is strictly
better than emitting ANOTHER session's usage under it.
Remaining accepted gap: a project directory's very first-ever PTY session
(before ~/.claude/projects/<dir> exists) gets zero usage accounting for that
one session, since the pre-spawn snapshot can't be proven reliable. This is a
one-time-per-project gap, not a recurring one — every subsequent session in
that project dir snapshots a readable directory.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st-session PTY usage DEFECT 1: the ADD CONSTRAINT idempotency guard for sessions_cli_kind_check, sessions_runner_type_check, and token_usage_runner_type_check checked information_schema.check_constraints by bare constraint_name, unscoped by table. A same-named constraint anywhere on the search_path (a decoy table, another schema) satisfies IF NOT EXISTS and the real per-table constraint is silently never created. Switched all three to pg_constraint scoped by conrelid. Added an e2e proving a same-named constraint on an unrelated decoy table no longer fools the guard, both sessions and token_usage constraints actually bite, and a second full apply stays a clean idempotent no-op. DEFECT 2: snapshotPreExistingTranscripts() returned reliable:false both when the project's session dir didn't exist yet (normal for a project's very first PTY session) and on genuine read/permission failures, conflating "we don't know" with "nothing was there." start() then permanently refused to locate any transcript, so first-session PTY token usage was never recorded -- undercounting the daily token cap's own inputs. Now distinguishes ENOENT (dir absent -> reliable:true, empty set: nothing to skip, nothing ambiguous) from a genuine read failure (reliable:false, unchanged). Added unit tests: dir-absent now yields reliable:true+empty and the session's own usage IS recorded once its transcript lands; a genuine unreadable dir (ENOTDIR) still yields reliable:false. Both new assertions were confirmed to fail against the pre-fix code before the fix landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Milestone PTYCAP phase 1 of 9. Blocks phases 2-4, which in turn gate 7-9.
Goal
The hub knows what a PTY turn is spending while it spends it, not after. Post-hoc accounting
is too late — an interactive TUI turn can run for many minutes, which is exactly how the
2026-07-11 incident burned 2.83B cache-read tokens before anything noticed.
Success criteria
supervisor/test/pty-usage-tail.test.ts(17 tests, passing). Wired as its own Woodpecker step in412ee0dbecausecheck-baselineonly walkshub/test.hub/test/token-usage-runner-type.test.ts+hub/test/usage-event-handler.test.tspass locally. The live-Postgres CHECK-constraint proof is DB-gated and runs in CI.hub/test/pty-usage-midflight-visibility.test.ts— DB-gated, runs against CI'spostgres:16.On SC-2/SC-3: the DB-backed assertions are marked
status: unknownin their SUMMARYsbecause no local Postgres was reachable — deliberately not claimed as passing. They execute
for real in Woodpecker via
REMO_E2E_DB_URL. This is the PGlite-vs-real-Postgres divergencethat bit #367 and #371; green CI here is the actual proof, not the local run.
What changed
New supervisor-side emitter (
supervisor/src/usage/pty-usage-emitter.ts,pty-transcript-tail.ts) tails the CLI transcript and threadsrunner_typeend-to-end:zod field in
agent-protocol.ts-> handler inws/agent.ts->token-usage-dal.tsINSERT.Invariants held
token-cap-coverageandmount-orderare byte-identical to main —git diff origin/main --statover those paths is empty. Ran directly: 34/34 and 22/22 pass.changes land in Phase 2.
cache_read.Verification
bun run check-baseline->pass=2059 skip=255 fail=0 total=2314, exact baseline match.(First run showed one failure in
transcript-adapter-codex.test.ts; isolated 8/8 pass and are-run was clean — transient, consistent with the known Bun
mock.modulecross-file pollution.)hub/typecheckreports 418 errors — all pre-existing, proved by running the same commandin a scratch worktree off bare
origin/mainand diffing the logs: identical error set, onlypath text and line shifts differ.
Provenance
These 22 commits were cherry-picked off
feat/ptycap-token-gate, which had accidentallyaccumulated two unrelated workstreams. The triage/scheduler commits on that branch are already
on main via #394 and were discarded here. That branch is untouched.
🤖 Generated with Claude Code