Skip to content

fix(usage): show a Claude Enterprise seat's monthly spend cap when it has no 5h/weekly window - #33655

Merged
nwparker merged 3 commits into
mainfrom
nwparker/agint-claude-enterprise-spend
Oct 25, 2026
Merged

nwparker merged 3 commits into
mainfrom
nwparker/agint-claude-enterprise-spend

Conversation

@nwparker

@nwparker nwparker commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​105 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​105
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​60 $\color{#cf222e}{\Huge{\mathbf{−}}}$​9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​51

ELI5

Claude Enterprise seats have no 5-hour or weekly limit, only a monthly spend cap. Orca already downloads that cap but never drew it, so the Claude meter in the status bar and the account switcher rows stayed empty. Now the cap shows as a monthly bar (e.g. 80% 30d), the same way Orca shows monthly limits for Grok, Cursor and OpenCode Go.

This is contributor PR #31065 by @frank-bee, rebased onto current main. Their two commits and authorship are kept. The only change is a test-file conflict against #33542's new InlineUsageBars cases (both sides kept).

Root cause

  • /api/oauth/usage for an Enterprise login returns five_hour: null, seven_day: null and carries the limit in spend / legacy extra_usage (sample payload in Status bar: show Claude monthly spend when an account has no 5h/weekly windows (Enterprise) #31061).
  • fetchClaudeOAuthUsage (src/main/rate-limits/claude-oauth-usage-request.ts) mapped only the 5h/7d/Fable windows; spend went to extraUsage, which the status bar shows only as an overage balance once a plan window is capped (isExtraUsageActive). With no window the chip, tooltip bar and switcher rows had nothing to draw.

What changed

  • claude-usage-credits.ts: mapClaudeSpendLimitWindow turns a currency spend cap (spend, falling back to legacy extra_usage) into the existing monthly window. No window for a zero/missing cap or a balance-only response.
  • claude-oauth-usage-request.ts: sets monthly only when there is no 5h and no 7d window. On Pro/Max, spend is overage after a plan cap and keeps today's balance display.
  • Legacy extra_usage percent now prefers used/limit over utilization, which has been seen as a 0–1 fraction (contributor's real-account observation).
  • InlineProviderUsage.tsx: switcher rows draw the monthly bar under the same "no 5h/7d" rule; isUnavailableInactiveUsage counts monthly so such an account is not treated as signed out.
  • Everything downstream (status bar chip, tooltip "Monthly" row, tone, overflow chip, stale policy, withoutResetWindows) already handles monthly. The live statusline ingest returns early when a post has no 5h/7d window, so it cannot overwrite this.

How verified

Risk / trade-offs

Fixes #31061
Refs #29944
Supersedes #31065

Co-authored-by: Frank Bernhardt 275327+frank-bee@users.noreply.github.com

Frank Bernhardt and others added 2 commits October 25, 2026 15:36
…y window

Enterprise seats have no five_hour/seven_day windows, so the Claude meter stayed empty although the usage response carries the monthly spend cap. Map it to the existing monthly window in that case only, and render that window in the account switcher's inline usage rows too.

Fixes #31061
… percent from amounts

A balance-only spend object no longer hides the cap from legacy extra_usage, and legacy utilization reported as a 0-1 fraction no longer understates the meter.

Co-authored-by: Frank Bernhardt <275327+frank-bee@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 378afe7f-64df-4ee0-b0f2-0af0bdbc31b5
Commits

Reviewing files that changed from the base of the PR and between 8709b6e and ceb66ff.

Files selected for processing: 1
  • src/renderer/src/components/status-bar/inline-usage-bars.test.tsx
Walkthrough

Claude usage mapping now creates a 30-day monthly window from spend data or legacy extra_usage when session and weekly windows are absent. The status bar displays monthly usage in that case, includes its reset timestamp in the countdown, and does not classify monthly limits as unavailable inactive usage. Tests cover data mapping, fallback conditions, and rendering.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Moderate · up to 8709b

Static analysis fails on the new test fixture. Replace its wall-clock timestamp before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the change: Claude Enterprise seats with no 5h/weekly window now show the monthly spend cap. It matches the raw summary, which covers mapping spend to a monthly window and the switcher…
Description check Passed The description follows the template well. It has an ELI5 section, What Changed, Root cause, Why-style risk and trade-off notes, verification notes, and a 'Fixes #31061' linked issue. The Visual Proof…
Linked Issues check Passed Issue #31061 requires a monthly Claude usage bar when both 5-hour and weekly windows are absent. fetchClaudeOAuthUsage sets monthly only in that condition. mapClaudeSpendLimitWindow maps rich `s…
Out of Scope Changes check Passed The changes stay within issue #31061. They add spend-cap mapping, preserve existing Pro/Max overage behavior when a plan window exists, render the monthly window in the status bar and account switcher…
Full details: Title check

Explanation

The title names the change: Claude Enterprise seats with no 5h/weekly window now show the monthly spend cap. It matches the raw summary, which covers mapping spend to a monthly window and the switcher rows. It uses the fix() convention and is specific. Its length (93 characters) is at the long end but still readable.

Full details: Description check

Explanation

The description follows the template well. It has an ELI5 section, What Changed, Root cause, Why-style risk and trade-off notes, verification notes, and a 'Fixes #31061' linked issue. The Visual Proof section is missing. The author gives before/after screenshots only by reference to #31065 and does not attach them here. The Testing checklist and the AI Disclosure, Review, Agent skill boundary, and Checklist sections are not clearly filled in. The description also does not say which platforms were tested.

Full details: Linked Issues check

Explanation

Issue #31061 requires a monthly Claude usage bar when both 5-hour and weekly windows are absent. fetchClaudeOAuthUsage sets monthly only in that condition. mapClaudeSpendLimitWindow maps rich spend data and the legacy extra_usage cap to a 30-day rate-limit window. InlineUsageBars renders that window and its percentage. The supplied test summary reports coverage for spend, legacy fallback, plan-window exclusion, and monthly-only rendering. No reset date is available in the mapped API data, so no reset date requirement remains unmet.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #31061. They add spend-cap mapping, preserve existing Pro/Max overage behavior when a plan window exists, render the monthly window in the status bar and account switcher, and add focused automated tests. No unrelated production behavior is demonstrated.

Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 72ae593b-cb8d-4389-8aff-acf7df85f799
Commits

Reviewing files that changed from the base of the PR and between 9639426 and 8709b6e.

Files selected for processing: 5
  • src/main/rate-limits/claude-oauth-usage-extra-usage.test.ts
  • src/main/rate-limits/claude-oauth-usage-request.ts
  • src/main/rate-limits/claude-usage-credits.ts
  • src/renderer/src/components/status-bar/InlineProviderUsage.tsx
  • src/renderer/src/components/status-bar/inline-usage-bars.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@nwparker
nwparker merged commit cffb527 into main Oct 25, 2026
37 checks passed
nplez1 added a commit to nplez1/orca that referenced this pull request Oct 28, 2026
… verdict answerable (#188)

* chore(branches): retire the ready-for-upstream branch set, 28 branches archived

Branches were kept near-upstream-ready so a fix could be offered to stablyai/orca on demand. That
premise is gone: a branch 2,800 commits behind is a snapshot, not a ready state, and the ready state
decays faster than it is maintained. If upstream work is wanted it gets cut fresh from nplez1/main,
because the code that lands there is the only version worth offering.

So: 28 branches retired, and every one carries a pushed archive/2026-10-27/<branch> tag first, so
"retired" means unreferenced by a branch, never gone. 8 are zero-loss -- the PR merged and the branch
tip equals the PR head, or the patch is already in main. 20 are archived with unlanded content: the old
nplez1 ideas and all twelve upstream-bound fix/* and feat/* branches, which sat ~2,800 commits behind
and were never opened.

nplez1/AI-Prompts is the one with real unlanded code -- PR #184 was closed unmerged and it creates
src/shared/ai-prompts/**, which main does not have. The archive tag points at its local tip, which
carried a commit the PR never got.

nplez1/main, nplez1/Fork-Opt and the three backup/* snapshots stay. Deleting a safety ref to tidy a
branch list is the worst trade available.

The mobile question the plan had left open is answered by measurement: exactly one archived branch
carried mobile work, and all ten of its files are already on main. mobile/ is upstream's, 3980 files.

* feat(usage): take upstream's Claude spend cap, keep only the Codex half

Upstream shipped the Claude Enterprise spend cap as cffb527 (stablyai#33655); the fork had implemented the
same half itself in dd12489 and kept it. Take upstream's, byte for byte -- all three of its files hash
identically to that commit -- and retire the fork's parallel parser and its separate allowance readout.

Three things upstream does that the fork did not, and two of them were the fork being wrong:

  - the legacy extra_usage percentage is computed from the amounts first, so a 0-1 fraction 0.0958
    reads as 9.58% instead of 0.0958%
  - monthly needs a real cap -- a currency, a percentage and a limit above 0 -- not a bare percentage
  - the window comes from the same ExtraUsageBalance the Usage credits section renders, so the bar and
    that section cannot disagree

The duplicate allowance line goes with it: the Credits section already shows the same bar and amounts.
Net +163/-443 across 5 files, 231 of the deletions a fork-only test.

The Codex spend_control half stays fork-only -- spend_control.individual_limit to monthly plus a
count-unit allowance -- and its 23 cases pass unchanged. #134's OAuth refresh was already isolated in
claude-accounts/claude-oauth-token-refresh.ts and is untouched; upstream deliberately never refreshes.

Two shared comments named Claude as an allowance producer, which stopped being true here, and the usage
doc claimed Claude shows "$192.68 of $8,000.00" and that Orca never computes the percentage from the
amounts. Both are now false, so both say what is true: Copilot and Codex produce the allowance, and
Claude's percentage is one of the two computed ones.

* refactor(renderer): move the fork's styling and transient state out of upstream's files

Both stylesheets are now byte-identical to the merge base: main.css and rich-markdown-editor.css carry no
fork lines, so a conflict there resolves by taking upstream outright. 157 fork lines moved out verbatim
except one comment that said "further down this file", which is no longer true.

  - components/pr-comment-inline/pr-comment-thread.css holds the .orca-pr-comment-* block, imported by
    PRCommentThreadCard.tsx the way upstream imports AgentKanbanBoard's own stylesheet
  - components/comment-composer.css holds the 31 composer lines that were in rich-markdown-editor.css,
    imported by comment-composer.tsx. Not folded into the PR block: CommentComposer is rendered by the
    Jira issue workspace too, and it is the only caller that passes compact or disabled

The load order cannot change the result: no moved selector ties on specificity with a remaining rule that
touches the same element and property. The only overlaps outrank in either order --
.orca-diff-comment-inline > .orca-diff-comment-card (0,2,0) over .orca-pr-comment-thread (0,1,0), and
.monaco-editor .glyph-margin-widgets .cgmr (0,3,0). The composer's own rules (.rich-markdown-editor.
comment-composer-editor at 0,2,0 and its p/ul/ol/blockquote at 0,2,1) outrank every base
.rich-markdown-editor rule, and the rules that would tie are scoped to .linear-issue-markdown-editor and
.rich-markdown-editor-shell, which the composer is never inside. The block was unlayered and stays
unlayered, so it still beats Tailwind's layered utilities.

Transient fork state moves to store/slices/fork-ui.ts -- settingsProjectAddHostRequest,
prCommentInlineDisplayById, their actions and the ProjectHostAddRequest type -- registered beside
worktree-setup-runner. ui.ts, ui-slice-contract.ts, ui-slice-contract-contextual.ts and
ui-slice-surface-actions.ts now carry no fork lines for it; the only one left in
ui-slice-settings-actions.ts is the request clear in closeSettingsPage. store-test-helpers.ts composes
the slice too: a hand-built AppState there fails tc:web without it, which is the check that catches a
future slice registered in one place only. The persisted hiddenWorkspaceStatusIds and
mergeSameBranchWorkspaces stay in upstream's UI slice because they ride its hydration.

task-page/jira/Content.tsx is rebuilt on upstream's nested-ternary shape: one hook (use-jira-board-mode.ts)
plus one early return that mounts the configured board, so its fork delta is 63 changed lines where it was
384.

* test(vm): pin the stdin EPIPE guard the PR #187 review asked about

The review's NIT was right that the guard had no direct regression test: its only coverage was the
context-menu case exiting 9. Adding one, verified both ways -- 8/8 with the guard, and the new case fails
when the guard line is removed.

The emit is synthetic on purpose. A real failing write surfaces as an unhandled error rather than a clean
assertion failure, so emitting error on a stream with no listener tests the same contract and reports
it as an assertion.

Also records why Copilot's hasData is not hasUsageData: that module imports this one, so reusing it would
cycle, and the allowance is Copilot's own readout rather than one term among several.

* docs(plan): record the branch retirement, the spend cap and the renderer seams

Three plan items close in this branch:

  - § 6.7 is decided, not deferred. Branches are no longer kept upstream-ready: the branches it left
    open are retired, the twelve upstream-bound fix/ and feat/ branches among them, each with a pushed
    archive tag so "retired" means unreferenced rather than gone. BRANCHES.md carries the table, the
    restore commands and the per-branch reasoning; § 6.7 keeps the policy and the measurement.
  - § 6.4 records what taking upstream's Claude spend cap actually changed, including the two cases
    where the fork was wrong rather than merely different.
  - § 6.6 records the 157 lines moved out of upstream's stylesheets, why the load order cannot matter,
    and the store slice that tc:web catches if a future one is registered in one place only.

LOCAL-PATCHES.md gains local(usage) and local(renderer) so the next sync knows which files are
byte-identical to upstream and can be resolved by taking theirs.

* feat(ai-vault): give the list tier its own tables, so the version cannot collide again

The fork had added 8 columns plus an FTS table to upstream's sessions table, and both sides bumped the
same version integer (fork 6->9, upstream 6->8). A sync forced a merged value of 10, and the same shape
had already hit the parse cache. SESSION_SEARCH_SCHEMA_VERSION is upstream's 8 again, and the fork's
list state is its own:

  - session_list_facts (session_row_id plus the 8 columns) and session_metadata_fts live beside
    upstream's sessions table, versioned under meta.session_list_schema_version
  - triggers on sessions keep both in step, so upstream's writer, retention purge and replacement delete
    need no fork hunks at all -- session-search-retrieval.ts, session-search-retention-delete.ts and
    ai-vault-host-leg-cache-retention.test.ts are byte-identical to upstream again
  - what is left in upstream's files is 3 lines in session-search-schema.ts (two hooks and an import),
    2 in session-search-file-records.ts, and 5 hunks in ai-vault-host-leg-cache.ts for two features that
    are not the sync delta: the unbounded key and the row-capped prune a paged reply needs

A v10 database is migrated, not dropped. Upstream deletes a file whose version it does not know, so the
migration runs before its opener: it renames sessions_fts, copies the 8 columns into the facts table,
drops them from sessions, sets the version to the literal '8' that upstream content had and writes the
fork key. Verified on real temp-file SQLite, fresh and v10-with-fork-data, plus both discard paths, and
the migration test fails if the migration is removed -- upstream deletes the file instead.

The host-leg delta moves to ipc/ai-vault-host-leg-sync.ts, wrapping scan at the ai-vault.ts call sites,
so the cache's scan signature is upstream's again. One behaviour difference, recorded rather than hidden:
after a delete upstream's cache cannot expand an unchanged reply, so the next plain refresh rescans --
4 where the fork had 3. The held list makes that a small diff request rather than a full re-download, and
a new sync test asserts it.

Also keeps two cross-cutting inventories in step, which is how this change would otherwise have been
caught by gate 1 rather than here: the WSL mover's table list gains session_list_facts and
session_metadata_fts, and its SQL-owning file map gains the new schema module.

* docs(plan): §6.5 records the fork-owned schema, its migration and its expiry

* feat(sync): commit the proof rule the 10-25 sync used, and forecast what a merge breaks

The 10-25 sync resolved 133 hunks by hand under one rule -- upstream never touched this region -- with a
throwaway script that was never committed. This is that rule, generalised, plus the two checks that catch
the class of fork behaviour that has silently disappeared in past syncs.

conflicts.mjs --propose now attaches a per-hunk proof read from each side's TypeScript syntax tree, never
from a regex: upstream changed only comments or whitespace (take ours); the fork did (take theirs); the
fork changed only identity literals (take theirs with the literals put back token for token, which is
stronger than taking theirs and re-running the sweep, because the sweep only rewrites some literals); and
imports only on every side (a three-way union that honours removals, refused if it would import one
module twice or bind one name twice). A ledger or manifest deletion is printed as a git rm command and
never run.

--apply-proven <decision.json> re-proves every file from disk, writes only files where every hunk is
proven, and refuses one edited since the proposal. It stages nothing.

Measured on HEAD against upstream/main 8d72873: 245 of 421 hunks proven, 159 of 267 files with hunks
fully proven. On the 281cccf sync: 187 of 347, 113 of 216.

--rename-forecast <old-base> <new-base> names, before the merge, the fork-added imports, vi.mock paths and
path strings that upstream's renames and deletions will break -- 32 real targets on the 281cccf sync in
2 s, 23 today across 9 modules. fork-symbols.mjs lists the fork-added names a merged file no longer
declares; a symbols entry in expected-deletions.json acknowledges one with a reason. It compares names,
not behaviour, and says so.

Also fixes two things found on the way. deletedForkModules read an acknowledged object that 6357e7b
replaced with the deletions[] manifest, so it reported FAILED on every run; it now reads the manifest
through the shared parser, and asks whether an asserted deletion is present again rather than whether this
one sync deleted it -- the second question reads all 22 settled entries as stale every time. Verified
against a real tip: passed, no false stale lines, and it correctly lists the two files deleted today.

The plan's account of that sync was off in two places, recorded in the plan: the 133-hunk note is at
LOCAL-PATCHES.md:1552, not :1207, and its "50 identity-only hunks" does not reproduce -- only 1-2 qualify
on either range measured, and only 7 unproven hunks in the 281cccf merge contain an identity literal
at all.

* docs(plan): §5.9 records the measured proof coverage and what the tooling found

* feat(ci): judge the nightly against a Linux red set, so one upstream race cannot mute it

The Mac allowlist names this machine's environment; the nightly runs ubuntu-24.04-arm and has its own.
Without a set for it, a nightly is red forever on one upstream test, and a nightly nobody can read is
worse than no nightly.

local/nightly-linux-known-red.json holds the Linux set. An entry needs file, test, class and evidence;
an entry with no evidence is a hard problem, because that is the difference between a verdict and a mute
button. check-nightly-linux-parity.mjs --failures judges every failing assertion in the shards against
it: no entry fails the run, a listed test that passed is reported as prunable but never enforced, and a
file-level failure -- a beforeAll or import error with no assertion rows -- can never be absorbed by a
name-matching entry.

Its first entry is the daemon history-recovery test that failed on Linux in BOTH runs, the 04:53Z
schedule and the dispatch, with DaemonConnectionLostError out of disconnectOnly, while passing 3/3 here.
Every file in that stack is upstream-owned with zero fork hunks, so it is upstream's race under Linux
runner timing; the entry records its exit condition rather than hiding it.

Verified both directions on real artifacts: --failures exits 0 on the current run (1 known, 0 unexpected)
and exits 1 on the earlier one, naming the three branch-gap reds the merge has since fixed. The parity
mode is unchanged and still exits 0 on the same five shards, all 17 Mac entries passing on Linux.

Five new fixture tests, 20/20 in local/lib/linux-parity.test.mjs.

* fix(sync): type the status the reason string interpolates, so the changed-code gate passes

* test(vault): name the schema inventory for its domain role, not its structure

* fix(ci): make the Linux red set fail closed on what the reports cannot show

Three holes an Opus 5.5 review of the branch found in the red-set checker, all of the same kind: a night
could print "every Linux failure is a listed one" while something was red outside the assertion rows.

  - A shard that died or timed out uploads no unit-report.json, so four of five reports read as a clean
    night. --expect-reports N now fails the run when fewer arrived.
  - A report declares its own failing counts. More declared failing tests than could be attributed to a
    row or a file is now a problem, as is a shard whose success is false with no failing test at all --
    an unhandled error or a dead worker, which no assertion row records.
  - A file-level error was recorded only when the file had no failing assertion, so an afterAll or
    teardown error in the very file an allowlisted test failed in was absorbed. It is now its own red.

Also anchors the name match. It was a bare substring, so any sibling test whose title contained the
entry's name was absorbed; it now requires the exact name or the same name as a suffix after the
describe prefix, which is also what parity mode already did -- the two modes agreed by accident before.

A listed entry that stopped failing is now classified rather than assumed prunable: passed on Linux
means the entry can go, but a file that never ran means there is no evidence either way, and calling
that prunable would invite deleting a real entry on the night a shard failed to upload.

Suite-level failures that carry no failing test are printed as a note and never enforced. Vitest counts
a suite as failed for worker-teardown noise (observed on last night's shard 4, one failed suite and no
failing row), and failing the night on that would make the verdict permanently red on something no entry
could honestly claim.

--allowlist and --red-set are now usage errors in the wrong mode instead of silently ignored flags, and
the header documents both modes and their exit codes. 26 fixture tests.

* docs(renderer): say why one fork line stays in upstream's UI slice, and load the thread CSS where the marks are

The review's MINOR and NITs: fork-ui.ts now names the one fork line that stays in
ui-slice-settings-actions.ts and why; the gutter marks import the thread stylesheet directly instead of
reaching it through four modules; Content.tsx gets the blank line after its imports; the usage doc's
percentage claim covers both Claude cases rather than only the fraction; and monthly's doc comment lists
Claude enterprise spend caps among its producers.

* fix(vault): rebuild rather than stamp when the list tier cannot be checked

Four MINORs and two NITs from the review of the schema change, each with a test that fails without it.

A wrong or missing fork key on a file that already holds sessions now rebuilds instead of adopting a
stale tier. Before, when the pre-open check threw (a lock timeout, a transient error), it returned "keep"
and ensureSessionListSchema stamped the current version over whatever was on disk: on a fork-built v8
file that left the fork tables empty and the key claiming 1, so existing sessions never got facts or
metadata rows, because unchanged files are never re-read. It now throws SessionListTierMismatchError with
code SQLITE_CORRUPT, which upstream's openVersionedCacheDatabase already treats as delete-and-rebuild
once, so no new lines in upstream's opener. If upstream changes that rule the test fails rather than the
index degrading quietly.

Adds the INSERT trigger. Metadata search depended on upstream's createSessionRow staying "insert blank,
then UPDATE" -- and the whole point of this change is to survive upstream's writer changing. The body is
shared with the UPDATE trigger and existing files pick it up through IF NOT EXISTS, so no version bump.

Qualifies the fact columns with session_list_facts. in COLUMNS and QUERY_COLUMNS: unqualified, upstream
adding a model column to sessions would fail every list query with "ambiguous column name".

Requires all eight legacy columns before migrating rather than one, so a partial layout is discarded
cleanly instead of failing partway and being deleted by the opener with a "check skipped" warning. And
corrects the host-leg cap comment: the 40k-row cap applies to each of the two maps separately.

* fix(sync): close the prover holes the review found, including one that dropped upstream's change

An independent review of the tooling found one BLOCKER and four SHOULD-FIX items. Every one is a case
where the tool would have resolved a conflict confidently and wrongly.

The BLOCKER: the hunk-edge check looked only at tokens, and comments are not tokens. Upstream can disable
a function by opening /* in one hunk and closing it in the next, with the function in the context
between -- both hunks read "upstream unchanged", the file is fully proved, and --apply-proven writes it,
losing upstream's change. Comments are now collected from the gaps between tokens and a hunk is refused
when a comment crosses its edge on any side, in both directions. A hunk wholly inside one comment on
every side still proves, so conflicts inside a JSDoc block stay resolvable.

Three more soundness holes, all of the same kind:

  - identity re-application rebranded literals the fork had deliberately left alone: positions where the
    fork kept the base literal were skipped, then every matching token in theirs was swapped. It now
    refuses when a mapped literal also appears unchanged, and requires theirs to contain each key exactly
    as many times as base does.
  - JSX whitespace was trimmed on single-line text, where it actually renders -- and a second bug meant
    JSX text's leading whitespace never reached the normalizer at all. Only runs containing a newline are
    collapsed now.
  - fork-symbol-check accepted HEAD as "merged", which during conflict resolution is the pre-sync tip: it
    compared the tip with itself and reported clear. Both ancestry directions are now required, and a
    relocated name fails the scan instead of counting as a pass -- the probe also excludes the fork's own
    added lines, so a dropped function kept alive only by its own test is no longer hidden.

post-sync no longer treats a pending-decision entry as acknowledged. It means "not settled", and
acknowledging an unreferenced export is what expected-deletion is for.

Plus the review's MINORs: an import dropped on one side and changed on the other is refused, duplicate
modules and bindings are checked against imports outside the hunk, a trailing comment after the last
import survives, --apply-proven refuses a path git does not list as unmerged and refuses a symlink, and a
deletion prints plain git rm.

64 node --test cases across the eleven files; each new fixture was proved to fail with its fix removed,
and the relocated-only fixture exists because the first mutation survived the one that was there.

* fix(ci): type the red-set options so the changed-code gate can see expectedReports

* fix(ci,vault): make the red-set note factual, and stop a concurrent open from throwing

Two MINORs and a NIT from the delta re-check.

The red-set note claimed the shards' failed-suite count minus the failing-file count was "usually Vitest
worker teardown", which the check cannot know: Vitest's suite count is not a count of files with a
failing row, and it can include a suite that failed for a reason no assertion row records. It now reports
the comparison and says that it does not attribute the difference. The check that does fail the night --
a shard reporting failure with no failing test -- stays, because a red shard is a red night whatever the
rows say.

The fresh-index stamp is INSERT OR IGNORE. Two processes opening a fresh index can both see the key
absent, and `key` is a primary key, so a plain INSERT threw a constraint error the opener does not treat
as unusable and that open failed. Never overwrite was the intent; not throwing is the fix.

And the README no longer says `merge-base --is-ancestor` is checked "both ways": it checks that both the
tip and the new base are contained in the merged ref, once per ref.

* fix(sync): record trailing comments, and stop refusing every JSX hunk

A delta re-check of the previous round found the BLOCKER only partly closed and one fix over-refusing.
Both were reproduced against proveFile directly, by the reviewer and again here.

The comment-edge check asked TypeScript only for leading comments, but a comment that starts on the same
line as the token before it is that token's *trailing* comment, which getLeadingCommentRanges never
returns. So upstream disabling a function with `const x = 1 /* disabled` in one hunk and `*/ c()` in the
next still proved both hunks "upstream unchanged", marked the file fully proven and wrote a resolution
that dropped upstream's change -- the exact failure the check exists to stop. Trailing ranges are now
collected too, deduplicated by position, with ranges starting inside JSX text dropped: a trailing scan
from a tag's `>` otherwise reads `<p>/* text` as a comment that runs on to the next `*/` in real code and
refuses an unrelated hunk.

The previous fix read JSX text from `pos` in the geometry test, which made the whitespace-only text node
between two JSX elements span the hunk edge: every hunk among JSX children was refused with "the hunk
edge splits a token". A prover that refuses .tsx is a prover nobody runs. The geometry now uses a
region start that skips whitespace-only JSX text, while the normalizer still starts at `pos`.

Plain getStart was not enough for that: it shrinks a rendered single space (`</b> <i />`) to zero width,
so upstream deleting that space would prove "upstream unchanged" and the deletion would be lost. The
region start is getStart for every node except JSX text, which starts just after its last leading
newline -- same-line leading whitespace always sits on the hunk's own line, so it cannot create a
split-token refusal.

31 tests. Five mutations, each failing a different assertion: dropping the trailing scan, dropping the
JSX-text filter, the old pos-based start, plain getStart, and dropping the whitespace-only skip. I
reproduced the last one myself: with the trailing scan removed by an exact-string edit the suite fails
30/1, and the file restores byte-identical.

* fix(ci): find a red-set entry by its short title, so a passed test reads as prunable

The stale classification reused judgeParityEntry, which compares the test name exactly because the Mac
allowlist stores Vitest's full name including the describe prefix. A red-set entry stores the bare title,
so when the daemon history-recovery test PASSED 17/17 on the branch nightly the tool reported "no
evidence ... did not run" and told the operator to keep an entry that could be pruned. It now matches the
way failures are matched, and the same run reads "prunable".

Also records that third run in the entry. It failed twice and passed once, which is what "intermittent"
looks like: one pass is not the two consecutive ones the exit condition asks for, so the entry stays, and
the evidence now says failed, failed, passed rather than implying it always fails.

* fix(ci): a known Linux red stops mailing an alarm, and a new one still does

The nightly reds the shard job on Vitest's exit code, so a failure this repo already triaged emailed an
alarm every time it recurred. At 07:19 today that was the daemon disconnect race -- an upstream,
intermittent failure recorded as a known red with its evidence -- reported as "Fork main unit tests /
test / tests node 24 4/5 Failed". A lane that cries wolf on its own allowlist teaches everyone to ignore
it, which is the failure mode the whole red set exists to prevent.

The shard verdict now reads `local/nightly-linux-known-red.json` when it runs on Linux. A failing test
that matches an entry is reported as a tolerated known red and the shard passes; a failing test that
matches nothing still fails, by name. So the only email left is a failure nobody has classified yet.

Four things cannot be absorbed, because each one is a red the entry does not describe:

- a file-level failure (nothing ran to failure) when the entry names a test. The tolerated test did not
  fail, so something else did.
- a report that declares more failing tests than it lists, which is a red with no name to match.
- an unhandled error that is not the tolerated Vitest teardown race.
- a shard that failed with nothing in the report or log to explain it.

An entry whose file ran here and did not fail is reported as prunable, as a notice -- the same claim the
cross-shard checker makes, and it stays a notice so a stale entry cannot fail a night on its own.

Both matchers are now one function (`entryFileMatches`, `entryMatchesFailure` in `local/lib/linux-parity.mjs`)
shared with the cross-shard checker, so the per-shard and whole-night readers cannot drift apart on the
same artifacts. A path must match across a separator, or `src/a.test.ts` would absorb
`src/other/a.test.ts`.

Verified by replaying real artifacts through the new verdict, not only fixtures. The 07:19 run's shard 4
now exits 0 with the race named and tolerated. Run 40485647652's shards 1, 2, 3 and 5 -- which carry the
three branch-gap reds and two unhandled errors -- still exit 1, each naming its own reds. Four mutations
each kill a different test: dropping the manifest, letting a named entry absorb a file-level failure,
dropping the whole-file entry form, and matching a path without the separator.

* fix(ci): a file-level error beside a known red is its own red, and prune only on a pass

Review found the classification could pass a new failure green, and one notice disagreed with the
cross-shard checker on the same input.

The BLOCKER: the file-level check only looked at files where no row failed, so a file whose afterAll or
teardown threw *and* whose listed test failed that night read as green -- the row was absorbed and the
file's `message` was never read. It now fires on `file.message || no failing row`, the same condition
`local/lib/linux-parity.mjs` already used, so the per-shard verdict and the cross-shard reader agree on
the one input where they disagreed. The test for it needed the fixture to build what Vitest actually
reports -- one file entry carrying both a failed row and a message -- because a second entry for the same
file let the old condition fire and the test passed against the bug.

The prunable notice now requires a *passed* row matching the entry. Before, it fired whenever the
entry's file appeared, so a night where the file crashed before the test ran, or the test was renamed or
skipped, invited deleting an entry that is still doing work.

A file-level failure is now `test: null` on both sides rather than the string '(file-level failure)' in
one and null in the other: the sentinel was only safe until some entry's title ended with it.

Also from review: `--known-red` with no path was silently ignored and is now a usage error, and the test
helper always writes a manifest so a Linux run of the suite cannot fall back to the repo's real one and
mean something different on CI.

What it still cannot see, and now says so: a describe-level hook failure Vitest attributes to a suite
rather than to the file's `message`. `numFailedTestSuites` counts failing describes as well as failing
files -- 2 for one failing file, measured on the 07:19 run -- so it cannot be used to catch that without
failing every night.

* docs(patches): record what a red nightly shard now means, and what it still cannot see

A behaviour this fork owns belongs in the ledger beside its evidence. Says that the shard verdict reads
the Linux known-red manifest, the four inputs it can never absorb, the describe-level hook failure it
cannot see and why, and that the night's verdict is still the cross-shard checker's.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Status bar: show Claude monthly spend when an account has no 5h/weekly windows (Enterprise)

1 participant