Skip to content

Upstream sync: test isolation, Cursor crash fix, pricing corrections - #36

Merged
CallumAcorn merged 6 commits into
mainfrom
upstream/safe-batch-2
Sep 29, 2026
Merged

CallumAcorn merged 6 commits into
mainfrom
upstream/safe-batch-2

Conversation

@CallumAcorn

Copy link
Copy Markdown
Owner

Reviews all 61 upstream commits waiting since the last sync. Seven are taken, each is either security-positive or a correction with no feature impact, and every one of the 61 now has a ledger row. ./scripts/upstream-status.sh reports zero unreviewed.

Two test-suite defects, measured on the maintainer's machine

The test suite was rewriting the user's real usage cache. LocalUsageCache defaults to the real Application Support/PokeTokenBar/usage-cache.json and had no gate, so any test that ran real providers read and rewrote it. With the app quit, one full suite run changed the real file from 726,929 to 763,881 bytes, and a new checksum. Every test run in this fork had been modifying user data.

Path injection cannot catch this, because it only covers the test doing the injecting, not the whole test process. AppEnv.persistsToUserLocation now owns the rule: an injected path always persists, the default path only inside the app bundle. After the gate, I snapshotted every state file across a full run and none changed. The companion save was never at risk, since its tests all inject a path.

Tests could call Anthropic with the user's real token. UsageStore defaults its limits providers to the real ones, and about ten tests build a store without a stub. Whenever the Keychain grant happened to be live, a refresh in one of those spent the user's real rate limit, which the running app then backs off from. AppEnv.allowsLiveLimitsFetch now gates the real Claude and Antigravity fetches before any Keychain read, allowing them only in the app or with PTB_PARITY=1.

The gate had a side effect worth calling out: it made both Keychain-discipline tests hollow. Blocking before the provider runs makes "the automatic path never reads the Keychain" true for the wrong reason. Both now open the gate with KeychainReader stubbed, so the provider's real logic runs against a fake Keychain. The manual-path test had been querying the real Keychain, prompts allowed, during swift test.

A crash on corrupt Cursor data (chattymin#307)

LocalAdditionalUsageProvider had its own copy of intValue, separate from the LocalUsageReader one that got an upper bound after an earlier overflow crash. This copy had a floor but no ceiling, so parseCursorBubble's input + output trapped on a corrupt record. Verified: SIGTRAP with the old helper, clean parse with the clamp.

Pricing corrections (chattymin#304, chattymin#309, Opus 5.5)

This fork kept the model-family price fallback, so nothing showed as Unavailable, but the fallback was wrong for two models:

Model Fallback charged Correct Error
claude-opus-5-5 Opus 5 rate (4, 20, 5, 0.2) 25% high, 2.5x on cache reads
claude-sonnet-5 Sonnet 4 rate (2, 10, 2.5, 0.2) 1.5x high

Cache reads dominate real usage, so the Opus 5.5 error mattered. claude-opus-5 happened to match the fallback and is now explicit so it cannot drift.

Declined on security grounds

chattymin#327 (track several Claude accounts, 3,419 lines) and the per-account session key are both built on the session key declined as chattymin#241, which stores a full account cookie in plaintext.

Deferred

Forty of the attempted commits conflicted, concentrated in Localization, CompanionStore, PopoverView and CompanionView: the files this fork has diverged most. Porting features into them is where existing behaviour breaks, so they are recorded as a new deferred disposition (wanted in principle, needs a deliberate port) rather than forced in. The most valuable of these is protecting legendary companions from accidental egg discard, which prevents data loss and is worth its own change.

Upstream's chattymin#334 independently arrived at exactly the Rare Candy re-arm design from #29, which is recorded as already present.

Verification

1072 tests, 0 failures, 90.27% coverage. All 24 hardening checks pass. Each fix verified by injection.

🤖 Generated with Claude Code

chattymin and others added 6 commits September 29, 2026 10:32
Two defects, both measured on the maintainer's machine. Upstream fixed the
same two (d6d24a0, 671c1ec); these are ports against this fork.

The usage cache defaults to the real Application Support usage-cache.json
and had no gate, so any test that ran real providers read and rewrote it.
With the app quit, one full suite run changed the real file from 726,929 to
763,881 bytes. Every test run in this fork had been modifying user data.
Path injection cannot catch this: it covers the test that injects, not the
process. AppEnv.persistsToUserLocation now owns the rule (an injected path
always persists, the default path only inside the app) and the cache obeys
it. Measured again after the gate: no state file changes across a full run.
The companion save was never at risk, since its tests all inject a path.

UsageStore defaults its limits providers to the real ones, and about ten
tests build a store without a stub. Whenever the Keychain grant happened to
be live, a refresh in one of those called api.anthropic.com with the user's
real token and spent their rate limit. AppEnv.allowsLiveLimitsFetch now gates
the real Claude and Antigravity fetches before any Keychain read, allowing
them only in the app or with PTB_PARITY=1.

The gate made the two Keychain-discipline tests hollow: blocking before the
provider runs makes 'the automatic path never reads the Keychain' true for
the wrong reason. Both now open the gate with KeychainReader stubbed, so the
provider's real logic runs against a fake Keychain. The manual-path test had
been querying the real Keychain with prompts allowed during swift test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
LocalAdditionalUsageProvider has its own copy of intValue, separate from the
LocalUsageReader one that got an upper bound after an overflow crash. This
copy had a floor but no ceiling, and NSNumber.intValue saturates at Int.max,
so parseCursorBubble's 'input + output' trapped on a corrupt Cursor record.
Verified: SIGTRAP with the old helper, clean parse with the clamp.

Uses LocalUsageReader.maxParsedTokenValue rather than a second limit.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
chattymin#309)

This fork kept the model-family price fallback, so nothing showed as
Unavailable, but the fallback was wrong for two models. claude-opus-5-5 was
billed at Opus 5's rate (5, 25, 6.25, 0.5) instead of (4, 20, 5, 0.2): 25%
high on input, output and cache writes and 2.5x high on cache reads, which
dominate real usage. claude-sonnet-5 fell back to Sonnet 4's rate, 1.5x high.
Opus 5 happened to match the fallback and is now explicit so it cannot drift.

Also adds gemini-2.5-flash-lite. Verified by injection: without the Opus 5.5
row a million cache reads cost 0.5 instead of 0.2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Seven taken or ported (the Cursor token clamp, two test-isolation fixes,
pricing rows, hiding zero-token providers). Two declined on security grounds,
both built on the session key declined as chattymin#241. The rest are skipped (tool
not installed, or already present), declined (release process, or changing
existing behaviour by design), or deferred.

Adds a 'deferred' disposition: wanted in principle but colliding with how far
the fork has diverged, so it needs a deliberate port rather than a
cherry-pick. Forty of the attempted commits conflicted, concentrated in
Localization, CompanionStore, PopoverView and CompanionView, which is where
porting would risk existing features.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
testStartTransitionsThroughConnectingToWatching failed intermittently on CI
('expected watching, got connecting'), including on main at fac7ea0, a commit
that only bumps a version string. It passed locally.

The stub serves responses from a shared static queue, and each store polls
every 10ms. Only one of the four spectator tests stopped its store, so stores
from earlier tests kept polling and removeFirst() took the next test's queued
response first. That test's own store then got an empty body and never left
connecting. Timing-dependent, hence CI-only.

Every store created by these helpers is now stopped at teardown, and the queue
is reset per test. BattleStoreTests copied the same pattern and stopped none
of its stores; not yet triggered, but the same mechanism, so fixed there too.

25 consecutive runs of both classes: 0 failures.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@CallumAcorn
CallumAcorn merged commit 509c0b2 into main Sep 29, 2026
2 checks passed
@CallumAcorn
CallumAcorn deleted the upstream/safe-batch-2 branch September 29, 2026 00:56
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.

2 participants