Skip to content

Attach declares the gateway first-party so Claude keeps its real context window - #438

Merged
platypii merged 3 commits into
masterfrom
fix/issue-437
Jul 29, 2026
Merged

Attach declares the gateway first-party so Claude keeps its real context window#438
platypii merged 3 commits into
masterfrom
fix/issue-437

Conversation

@philcunliffe

@philcunliffephilcunliffe commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Problem

With Claude Code attached to the HypAware gateway, a fresh session reports ~18% context after a one-word prompt; detached, the same setup reports ~4%.

The tokens are not the problem. Claude Code grants a native-1M model its 1M context window only when the ANTHROPIC_BASE_URL host is api.anthropic.com (or _CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL is set). Behind any proxy it assumes 200k, so the same ~40k of startup context reads as 18% instead of 4%. It is not only cosmetic: context warnings and auto-compact fire against the wrong denominator and trigger far too early.

Root cause

The claude attach writer (hypaware-core/plugins-workspace/claude/src/settings.js) already compensates for one non-first-party default (ENABLE_TOOL_SEARCH, LLP 0045) but not for the assumed-window one, so pointing settings at http://127.0.0.1:<port> silently cuts the assumed window.

Fix

Attach also writes env._CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL = "1", under the same ownership rule as ENABLE_TOOL_SEARCH:

  • only ever added when absent, or when a prior _hypaware marker recorded it as ours (so a re-attach keeps owning it),
  • recorded in managed.env so the single core undo (detachClientFromDisk) removes exactly what attach added,
  • a value the user set themselves is left untouched and stays out of the undo record.

The declaration is accurate: the gateway is a byte-transparent pass-through to api.anthropic.com, which is what the flag asserts, so the other first-party gating it flips (traceparent propagation, the context-1m beta header, an extended usage-limit header, and Anthropic-direct channels like error reporting, org policy limits, and memory-sync eligibility) is correct rather than a tolerated side effect. It does not gate credential choice: the oauth beta header and bearer/API-key selection ride the active session independent of this flag, so the declaration sends no secret anywhere it was not already going. (Corrected post-review: an earlier version of this line claimed the flag also gates "oauth beta headers", which round 2 of review found false; the code comment and LLP 0045 were fixed then, this body line is fixed here as part of triage.)

The two managed env additions are now one MANAGED_ENV_ADDITIONS table plus a single manageEnvAdditions ownership helper, so a third key cannot drift from the undo record.

LLP 0045 Part 3 gains the matching section (with the @ref from the code), including the honest caveat: the key is underscore-prefixed and undocumented (last verified against Claude Code 2.1.220), it fails soft, and the mitigation is to re-verify it if attached sessions start reporting an inflated percent again. The rejected alternative ([1m] model-name suffix) is recorded there too.

Tests

Regression tests fail on master and pass with the fix:

  • test/plugins/claude-settings-attach.test.js
    • attach writes env._CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL = "1" and records it in the marker undo record
    • a user-owned value is left untouched and stays unmanaged
    • a re-attach keeps managing the key it owns
    • the existing managed-env assertions updated to the new managed set
  • test/core/client-detach-disk.test.js
    • the core disk undo removes the managed key and does not stamp the restored base URL onto it

Verified failing-then-passing: on the pre-fix source these run 7 failures across the two files; with the fix, 31/31 pass.

Local checks

  • npm test: 2796 pass, 8 pre-existing failures in test/core/leave-command.test.js that fail identically on unmodified master in this environment (environment-dependent, unrelated to this change).
  • npm run typecheck: clean.
  • npm run smoke -- claude_attach_detach fails on this branch and on unmodified master (pre-existing, on the SessionStart hook assertion), so it is not a regression from this change.

Note

Already-attached machines pick the key up on the next attach (re-attach on endpoint drift, rejoin, or a manual hyp attach); this change does not rewrite existing settings on its own.

Fixes#437

neutral-loopand others added 2 commits July 29, 2026 03:58
…ext window
Claude Code only grants a native-1M model its 1M context window when
ANTHROPIC_BASE_URL points at api.anthropic.com. Behind the HypAware
gateway it assumes 200k, so an attached session reports ~18% context
where the same session direct reports ~4%, and context warnings and
auto-compact fire against the wrong denominator. The token usage itself
is unchanged; only the assumed window is.
Attach now also writes env._CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL="1",
managed by the same ownership rule as ENABLE_TOOL_SEARCH: only ever
added when absent or already recorded as ours, recorded in the
`_hypaware` marker so the core undo removes exactly what attach added,
and never clobbering a value the user set themselves. The declaration is
accurate - the gateway is a byte-transparent pass-through to
api.anthropic.com.
The two managed env additions are now one table (MANAGED_ENV_ADDITIONS)
plus a single ownership helper, so a third key cannot drift from the
undo record.
LLP 0045 Part 3 gains the matching section, including the caveat that
the key is underscore-prefixed and undocumented (last verified against
Claude Code 2.1.220) and what to re-verify if the symptom returns.
Co-Authored-By: Claude <noreply@anthropic.com>
…rop an em dash
The _CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL declaration asserts a property of
whatever the gateway forwards to, not of the gateway itself (unlike
ENABLE_TOOL_SEARCH). The anthropic upstream's base_url is ordinary config, so
repointing it makes the declaration false while attach still writes it
unconditionally. Record that as a stated precondition in LLP 0045 Part 3 and at
MANAGED_ENV_ADDITIONS rather than leaving it implicit.
Also drop the em dash this PR reintroduced into the undo-record comment
(CLAUDE.md forbids U+2014 anywhere).
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Verdict: approve after the two fixes pushed below (8f8b5da)

The change is correct. The ownership refactor is behaviour-preserving for ENABLE_TOOL_SEARCH, detach removes exactly what attach added, and the regression tests genuinely fail without the fix. Two findings, both Low, both fixed on the branch. Nothing blocking.

What I verified (not just read)

1. The refactor did not silently change the pre-existing ENABLE_TOOL_SEARCH path. The old predicate was weOwnToolSearch || typeof env.ENABLE_TOOL_SEARCH !== 'string'; the new one skips on !weOwnIt && typeof env[key] === 'string'. Those are the same condition. The three unchanged ENABLE_TOOL_SEARCH behaviour assertions (test/plugins/claude-settings-attach.test.js:143, test/core/client-detach-disk.test.js:133, test/core/client-detach-disk.test.js:162) still pass untouched.

2. The env-ownership logic is sound. Attach adds a key only when absent or when the prior marker recorded it (hypaware-core/plugins-workspace/claude/src/settings.js:184-192); a user-set string is skipped and kept out of managed.env, so the generic core undo (src/core/config/client_detach_disk.js:163-183) never sees it. Detach removes a managed key only while the live value still equals what we wrote, restores prev_base_url for ANTHROPIC_BASE_URL only, and warns-and-leaves on an external override. I looked specifically for the strand/clobber cases and found none:

  • user owns the key, then re-attach: still skipped, still unmanaged (test at :194)
  • we own it, user deletes it, re-attach: re-added, still ours
  • prior marker from the pre-PR version (no _CLAUDE_… recorded): weOwnIt false, user value respected
  • manageEnvAdditions now runs beforeenv.ANTHROPIC_BASE_URL = baseUrl, but the table contains neither that key nor anything the base URL touches, so the reorder is inert.

3. The tests have teeth. On the pre-fix settings.js with the new tests: 7 failures / 31. With the fix: 31/31. I also mutation-tested the ownership guard — deleting the typeof env[key] === 'string' line makes both "leaves a user-owned … untouched and unmanaged" tests fail. So they are real regression tests, not constant assertions. (Note test/plugins/claude-settings-attach.test.js:194 passes vacuously on master, which is expected for a negative guard; the mutation test above is what proves it guards something.)

4. The @ref anchor resolves.LLP 0045#_claude_code_assume_first_party_base_url-keep-the-models-real-context-window matches the heading added at llp/0045-client-attach.design.md:270. The LLP section is honest about the flag: underscore-prefixed, undocumented, version-stamped (2.1.220), fail-soft, with the rejected [1m] alternative and an upstream tracking link recorded.

5. No other site enumerates the managed env keys. Grepped ENABLE_TOOL_SEARCH repo-wide: only this writer, the core undo's explanatory comment, the LLP, and tests. No drift-detection or status surface needed updating alongside.

6. npm test in a clean worktree: 2796 pass / 8 fail, and the 8 are the test/core/leave-command.test.js failures unrelated to these files (they fail on the central-config-layer teardown exit code). Identical before and after my fixes.

Findings

[Low] hypaware-core/plugins-workspace/claude/src/settings.js:144 — added line reintroduced a U+2014 em dash. The undo-record comment was rewritten by this PR and the rewritten line carries — leaving no orphaned …. CLAUDE.md forbids em dashes "anywhere: code, comments, JSDoc, strings, or docs". Fixed in 8f8b5da: replaced with a comma. I deliberately left the four pre-existing em dashes in that file (lines 16, 24, 123, 148) alone — out of scope for this PR.

[Low] llp/0045-client-attach.design.md:284-288 — the "declaration is accurate" claim is stated as unconditional, but it depends on config this code cannot see. This is the one substantive point, and it is the difference between the two keys in the table:

  • ENABLE_TOOL_SEARCH asserts a property of the gateway (it forwards tool_reference untouched). Always true.
  • _CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL asserts a property of whatever the gateway forwards to. That is ordinary config: upstreams[].base_url is a free-form validated string (hypaware-core/plugins-workspace/ai-gateway/src/config.js:47), and https://api.anthropic.com is merely what the hyp init preset writes (hypaware-core/plugins-workspace/claude/src/index.js:358).

Attach writes the declaration unconditionally — attach(opts) only has port/version/stateFile/settingsPath/binPath in scope, not the gateway's upstream config — so an operator or org config that repoints that upstream makes the declaration false, and the other first-party behaviour the LLP says the flag gates (traceparent propagation, oauth beta headers) then applies to traffic that never reaches Anthropic.

I do not think the settings writer should police this; plumbing upstream config into it would be a real design change and is disproportionate. But the LLP asserting accuracy flatly leaves the assumption implicit. Fixed in 8f8b5da by recording it as a stated precondition: a paragraph in LLP 0045 Part 3 (llp/0045-client-attach.design.md:290) and a three-line note at MANAGED_ENV_ADDITIONS so the code and the doc agree.

Not findings, recorded so the next reviewer does not re-derive them

  • Re-attach overwrites a value the user changed on a key we own (e.g. user sets ENABLE_TOOL_SEARCH=false while attached; the next attach restores true). Pre-existing and documented as "a re-attach keeps owning it" — this PR extends it to a second key without changing the rule.
  • Downgrade orphans the key. Attach with this version, downgrade, re-attach: the old writer's marker drops _CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL from managed.env while the key stays live, so a later detach strands it. Inherent to adding any managed key; downgrade is not a supported path.
  • key in priorManagedEnv walks the prototype chain. Harmless — neither table key exists on Object.prototype, and JSON.parse will not set one. Pre-existing form.
  • Key order in the user's settings.json shifts on a first attach (added keys now land before ANTHROPIC_BASE_URL). Cosmetic; left alone rather than churn the diff.

Pushed

2edd315..8f8b5da on fix/issue-437. Both fixes are comment/doc only — no behaviour change, no test change. Verified in the committed tree: the em dash is gone from settings.js:144, and llp/0045-client-attach.design.md:290 contains the new precondition paragraph. npm test after the fixes is byte-identical in outcome to before (2796 pass, same 8 unrelated failures).

The PR asserted the flag gates "traceparent propagation, oauth beta headers".
Read off the shipped Claude Code bundle (2.1.215), the oauth part is wrong and
the list is materially incomplete. `_CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL` is
one branch of a single is-first-party predicate that also gates the context-1m
beta header, an extended usage-limit header, Anthropic error reporting, the org
policy-limits fetch and memory-sync eligibility. The oauth bearer token and its
`oauth-2025-04-20` beta header ride an active oauth session, not this predicate,
so the flag changes no credential's destination.
That distinction is what bounds the documented precondition: a repointed
`upstreams[].base_url` leaks no secret it was not already going to receive, and
the real failure is the assumed window being too large for a 200k upstream, which
fails loudly. Record both in LLP 0045 and at MANAGED_ENV_ADDITIONS so the next
reader re-verifies against the right list.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Neutral review, round 2 of 2 (final)

Head reviewed:8f8b5da9edf81a54d2794443067d7a191f240cb6
Verdict: approve with one finding, fixed in-round. New head after fix: baaaa382a9a765355955401eeffa01bbe995dd02.

Round 1 record: #438 (comment)


Round 1's own two fixes: both verified correct and complete

  1. Em dash.git diff origin/master...8f8b5da has zero U+2014 in any added line. hypaware-core/plugins-workspace/claude/src/settings.js:147 now reads loading this plugin, leaving no orphaned .... The em dashes still present in the touched files (settings.js:16,24,123,148, most of llp/0045) are all unchanged pre-existing lines, so the rule is satisfied for this diff. Complete.

  2. The LLP precondition wording matches what the code does. Checked against the code rather than taken on trust:

    • attach(opts) destructures { port, version, stateFile, settingsPath, binPath } only (settings.js:109). No upstream config is in scope, exactly as LLP 0045 states.
    • hyp init writes upstreams: [{ name: 'anthropic', base_url: 'https://api.anthropic.com', path_prefix: '/' }] as ordinary user-editable config (hypaware-core/plugins-workspace/claude/src/index.js:355-360). The gateway_upstream manifest contribution (hypaware-core/plugins-workspace/claude/hypaware.plugin.json:43) is consumed only by the walkthrough proposal path (src/core/cli/walkthrough.js:645-649); nothing pins the host at runtime.

    So "precondition, not invariant" is accurate. Accepted.


The substantive open question, judged independently

Round 1 left the unconditional first-party declaration as a documented precondition rather than a code check. I reach the same disposition, but on evidence rather than deference, and the PR's justification for it was partly wrong.

I read the predicate off the shipped Claude Code bundle installed in this environment (/usr/local/lib/node_modules/@anthropic-ai/claude-code/bin/claude.exe, 2.1.215):

functionNd(){if(Z._CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL)return!0;returnGUn()}functionGUn(){lete=process.env.ANTHROPIC_BASE_URL;if(!e)return!0;returntPe(e)}functiontPe(e){try{lett=newURL(e).host;return["api.anthropic.com"].includes(t)}catch{return!1}}

The env var is one branch of a single is-first-party predicate. What it gates:

effectevidence
native-1M window + context-1m-2025-08-07 beta (this PR's purpose)`if(n==="firstParty"&&Nd()
ENABLE_TOOL_SEARCH optimistic defaultif(!process.env.ENABLE_TOOL_SEARCH&&vn()==="firstParty"&&!Nd())
traceparent propagation`function a1r(){return Nd()
anthropic-usage-limit: extended header...&&vn()==="firstParty"&&Nd()&&et("tengu_lantern_spool",!1))zo["anthropic-usage-limit"]="extended"
error reporting to Anthropic`function $Gc(){...if(vn()!=="firstParty"
org policy-limits fetch (and the permission defaults it feeds)mFr() returns "custom_base_url" when !Nd(); D9(){return mFr()===void 0} gates the ${BASE_API_URL}/api/claude_code... fetch and Qi()
memory-sync eligibilityfunction vXh(){if(!Nd())return!1;...}

The PR's stated justification is wrong on one item. The code comment, LLP 0045 and the PR body all said the flag gates "traceparent propagation, oauth beta headers". It does not gate oauth beta headers. The beta list builder pushes $et = DT("oauth_auth","oauth-2025-04-20") on qo() (an active OAuth session), not on Nd():

if(qo()||oGn()&&!u1r()&&CH())t.push($et);

and the auth-header resolver wq() picks Authorization: Bearer <accessToken> / x-api-key from qo() / Ahe() without consulting Nd() at all.

Why that makes the precondition proportionate rather than a security problem. Because credential choice is not gated, the flag sends no secret to any host it was not already going to reach. Attach has already pointed ANTHROPIC_BASE_URL at the local gateway, and the gateway already forwards to whatever upstreams[].base_url says; that forwarding decision, credentials included, is made by the operator's config, not by this flag. If base_url is repointed at a non-Anthropic host, the extra traffic that host receives is a traceparent, an anthropic-usage-limit: extended header and a beta header. No secrets. The first-party-only side channels the flag re-enables (error reporting, policy limits, memory sync) target Anthropic's own API host, not the repointed upstream, so nothing leaks sideways there either.

The real residual harm is the window itself, and it runs in the unsafe direction: a repointed upstream that genuinely is 200k now gets warned about and auto-compacted far too late, and an over-long request fails at the upstream. That failure is loud (an upstream API error, not silent corruption), confined to a configuration the product does not otherwise support, and the only code fix is to plumb gateway upstream config into a settings writer that today receives a port. Disproportionate. Precondition is the right call.


Findings

LOW - the enumeration of what the flag gates was inaccurate and materially incomplete.hypaware-core/plugins-workspace/claude/src/settings.js:62-63 and llp/0045-client-attach.design.md:286-288 both claimed the flag gates "traceparent propagation, oauth beta headers" and leaned on that to argue accuracy. The oauth claim is false against 2.1.215 (evidence above) and the list omitted error reporting, the policy-limits fetch, memory-sync eligibility and the usage-limit header. This matters because the same LLP section instructs the next reader to re-verify the key against a future Claude Code release; sending them at the wrong list defeats the mitigation.

FIXED in baaaa38 (comment + doc only, no behaviour change): settings.jsMANAGED_ENV_ADDITIONS now carries the corrected list and the explicit "does not gate credential choice" note; llp/0045 gains a three-way breakdown (sent to upstream / sent to Anthropic / not gated at all) plus a bounded blast-radius paragraph recording the reasoning above.

Positive verification: git diff --stat 8f8b5da HEAD shows exactly settings.js (+13/-6 region) and llp/0045-client-attach.design.md changed; git show HEAD:<file> | grep "oauth beta headers" returns nothing in either file, and the replacement text (does *not* gate credential choice, Not gated by it at all: credential choice, blast radius of a false declaration) is present in the committed tree.

Reviewed and found clean

  • Ownership / undo logic.manageEnvAdditions (settings.js:186-196) preserves the prior ENABLE_TOOL_SEARCH rule exactly (weOwnToolSearch || typeof !== 'string') and generalises it. Upgrade path checked: a pre-PR marker with managed.env = {ANTHROPIC_BASE_URL, ENABLE_TOOL_SEARCH} picks the new key up on re-attach; a marker where the user owned ENABLE_TOOL_SEARCH keeps not owning it. The core undo (src/core/config/client_detach_disk.js:161-185) already iterates managedEnv generically and special-cases ANTHROPIC_BASE_URL for the restore target, so a second managed addition needs no core change. Correct.
  • @ref anchors. Both new refs (settings.js:135,136) resolve to real headings in llp/0045.
  • Suite.npm test at baaaa38: 2796 pass / 8 fail, the 8 being the pre-existing test/core/leave-command.test.js failures that reproduce identically on origin/master in this environment. npm run typecheck clean. node --test test/plugins/claude-settings-attach.test.js test/core/client-detach-disk.test.js: 31/31.

Non-blocking observations, deliberately not fixed

  • settings.js:191 gates ownership on typeof env[key] === 'string'. A user who writes a non-string value (e.g. "ENABLE_TOOL_SEARCH": true) has it overwritten and recorded as managed, so detach deletes it, which contradicts the "a value the user set themselves is left untouched" rule in the helper's own JSDoc. This is pre-existing behaviour that the PR faithfully preserved, the hole is narrow (Claude Code's env is a string map, so a non-string there is already invalid), and tightening it to key in env would change behaviour for ENABLE_TOOL_SEARCH too. Out of scope for a final review round on a different key.
  • client_detach_disk.js:181 assigns warning as a single string, so if two managed env keys were both externally overridden only the last is reported. Pre-existing shape; this PR makes it marginally more reachable (2 to 3 managed keys). Fixing it changes the DetachFromDiskResult.warning contract.
  • The PR body still contains the corrected-away claim ("the other first-party gating it flips (traceparent propagation, oauth beta headers)"). I am not permitted to edit PR bodies; the authoritative text in the code and LLP is now correct.

Unresolved for a human

Nothing blocking. One item to be aware of at merge time: the flag was verified here against Claude Code 2.1.215 (the version installed in this environment); the code comment claims "last verified 2.1.220". I could not check 2.1.220 directly. The predicate is present and behaves as documented in 2.1.215, so the claim is corroborated but not confirmed at the exact stated version.

Review cap reached (2 of 2 rounds). This PR is a draft; per policy I did not mark it ready or merge it.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Neutral triage: PR can merge safely

Head reviewed: baaaa382a9a765355955401eeffa01bbe995dd02

Review hit its 2-round cap (round 1: #438 (comment), round 2: #438 (comment)) with four residual findings recorded as "not fixed in this PR". I re-derived each against the code at head rather than taking the recorded severity at face value.

Classification (all non-blocking)

  1. Ownership gate clobbers/adopts a non-string user value (hypaware-core/plugins-workspace/claude/src/settings.js:196). Reproduced empirically: a hand-written "ENABLE_TOOL_SEARCH": true (JSON boolean) gets silently coerced to "true" and recorded as managed, so a later detach deletes it outright — real data loss, not theoretical. But this predicate is byte-identical to what already ships on master for ENABLE_TOOL_SEARCH alone (confirmed via round 1's mutation testing and my own read of the diff); this PR only extends the same rule to a second key, 1 exposed key to 2, not a new defect class. Preference, not a blocker.
  2. Single warning string drops all but the last override notice (src/core/config/client_detach_disk.js:181). Already reachable pre-PR with 2 managed keys; this PR takes it to 3. Affects only a diagnostic message, not the actual on-disk protection (the externally-overridden value is still left alone either way). Preference.
  3. Verified-version mismatch: settings.js:60-61 says "last verified 2.1.220"; the LLP 0045 paragraph round 2 added says "verified against 2.1.215". I independently confirmed the Claude Code bundle installed in this environment is 2.1.215, matching round 2's own investigation - the discrepancy is real but documentation-only, no runtime effect.
  4. PR body false claim ("oauth beta headers"): confirmed the code and LLP no longer contain this claim (git grep at head returns nothing), but the PR body still did. Since the body is what a human reads at merge time, I corrected that one sentence in place (see the current body) - the rest of the body is untouched, verbatim.

All four are preference-level or documentation-only: none constitute wrong behavior, data loss caused by this PR, a security hole, a crash, or a perf regression relative to what already ships on master. npm test at head: 2796 pass / 8 fail, the 8 being the pre-existing test/core/leave-command.test.js failures - reproduced independently in a clean triage worktree, identical count and names. The two files' own regression tests: 31/31 pass.

Filed a follow-up issue enumerating all four (with file:line and the "why deferred" reasoning) so they don't get lost: #440

Per LLP 0017, this PR is judged safe to ship. Left as draft, not marked ready, not merged - that's a later tick's job.

@philcunliffe
philcunliffe marked this pull request as ready for review July 29, 2026 04:25
@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 29, 2026
@platypii
platypii merged commit a1100f0 into masterJul 29, 2026
8 checks passed
@platypii
platypii deleted the fix/issue-437 branch July 29, 2026 18:34
philcunliffe added a commit that referenced this pull request Jul 29, 2026
…ified-version stamp (#449)
* claude attach: a non-string user env value is the user's, and one verified-version stamp (#448)
Finding 1: `manageEnvAdditions`'s ownership gate tested the *type* of the
existing value (`typeof env[key] === 'string'`), so a hand-written JSON
boolean or number at a managed key fell straight through it. Attach coerced
the value, recorded the key in the `_hypaware` marker's `managed.env`, and
the core undo - doing exactly what that record authorized - then deleted a
setting the user owned. Reproduced end to end: `ENABLE_TOOL_SEARCH: true`
plus `_CLAUDE_CODE_ASSUME_FIRST_PARTY_BASE_URL: 0` came back from attach as
`"true"`/`"1"`, and detach removed both, pruning the emptied `env` block
with them. The `0` case is the sharpest: that is the user turning the flag
off, and attach reversed their intent before deleting it.
The guard now tests presence (`key in env`), which is what the helper's
JSDoc always promised and what LLP 0045's "only manage the key when it is
ours" rule means. Coercion was never intended, so the guard moves to match
the doc rather than the doc moving to bless the data loss.
Finding 2: the three verified-version stamps disagreed - settings.js and
llp/0045:373 said 2.1.220, llp/0045:330 said 2.1.215. Both values entered
in the same commit (a1100f0, #438), so this was never an introduced-at vs
last-verified distinction, just an unverified number that review round 2
corrected in one place of three. 2.1.215 is the release the predicate was
actually read off the shipped bundle, and it is the version installed here
(`claude --version`, `@anthropic-ai/claude-code@2.1.215`). All three stamps
now read 2.1.215. The stamp is load-bearing: the LLP sends a future reader
to re-verify this undocumented, underscore-prefixed flag against a later
release, and an ambiguous baseline defeats that mitigation.
Regression tests fail on master and pass here: a round trip in
test/core/client-detach-disk.test.js proving a non-string user value
survives attach *and* detach byte-for-byte, plus attach-level coverage of
boolean/number/null in test/plugins/claude-settings-attach.test.js.
Co-Authored-By: Claude <noreply@anthropic.com>
* claude undo: report a managed key the user overrode with a non-string
The undo side of the same ownership rule this PR fixes on attach. The
attach guard now decides ownership by the key being present rather than
by the type of its value, so a hand-written JSON boolean at a managed key
is a value the tree expects to meet. The undo's never-clobber notice was
still type-gated:
} else if (typeof current === 'string')
so it stayed silent about exactly that value. The key correctly survived
the detach, but the operator was never told a managed key had been left
behind on disk - which is the whole job of the notice, since LLP 0045
promises "core accumulates one message for each key it left in place",
and those keys stay on disk after a detach that otherwise reports
success. The `json_path` undo in the same file already tests presence
(`current !== undefined`); only the `json` marker branch diverged.
Gated on `key in envObj`, not a bare `else`: a key the user deleted
outright was not left in place and must not be reported. Both directions
are pinned by tests - reverting the predicate fails "reports a managed
key the user overrode with a non-string", widening it to `else` fails
"stays silent about a managed key the user deleted outright".
Doc-conformance only, so LLP 0045 is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
* claude undo: the never-clobber notice tests own presence, everywhere
Round 2 on the same ownership bug class, which now has three instances.
1. `detachLegacyJsonMarker` (the pre-record branch) still gated its notice on
`typeof current === 'string'`. A legacy marker meeting an
`ANTHROPIC_BASE_URL` the user had switched off with JSON's `false` or `null`
left the key on disk - correctly - and said nothing about it. Reproduced:
env before : {"ANTHROPIC_BASE_URL": false, "ANTHROPIC_API_KEY": "sk-x"}
result : {"changed": true} <- no warning
env after : {"ANTHROPIC_BASE_URL": false, "ANTHROPIC_API_KEY": "sk-x"}
with the string case one line away reporting correctly. That is the same
LLP 0045 promise the record-driven branch was fixed against last commit:
"core accumulates one message for each key it left in place ... those keys
stay on disk after a detach that otherwise claims success". The legacy
branch is the one that most needs it, since it reverses by convention and
therefore meets settings this tree never wrote. Reachable today: any
pre-upgrade marker without a `managed` record still dispatches here.
2. The record-driven branch's new presence test is now `Object.hasOwn`, not
`key in`. Its key names come off disk, from whatever `managed.env` a
plugin's attach recorded, so an inherited `Object.prototype` name satisfied
`in` and reported a key that was not on disk at all:
marker managed.env : {"toString": "x"}
settings env : {"ANTHROPIC_API_KEY": "sk-x"}
result : "toString was overridden externally; leaving in place"
which is precisely the false "left in place" report the presence test was
introduced to prevent. The attach-side guard keeps `key in` correctly: its
keys are in-tree literals, not disk input.
Every predicate is pinned in both directions by mutation: reverting either
branch to the type gate fails one test, widening either to a bare `else` fails
another, and swapping `Object.hasOwn` back to `in` fails a third. No test
fires for more than its own mutant.
Doc-conformance only; LLP 0045 already states the rule, so it is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
* claude attach: back up the base URL by presence, not by JSON type
The fourth and worst instance of issue #448's bug class, on the one key the
PR had not swept: `ANTHROPIC_BASE_URL` itself. Unlike the managed additions
this key has no ownership guard to fall through, because attach always
repoints it. The backup IS the guard, and it was type-gated:
const liveBaseUrl = typeof env.ANTHROPIC_BASE_URL === 'string' ? ... : undefined
so a hand-written non-string read as nothing-to-back-up. Attach then recorded
the key in `managed.env` with no `prev_base_url`, and the core undo, finding a
managed key with no prior to restore, deleted it. Reproduced end to end on
871016e, for `8080`, `false` and `null` alike:
BEFORE ATTACH : {"ANTHROPIC_BASE_URL": false, "ANTHROPIC_API_KEY": "sk-x"}
MARKER : prev_base_url absent, managed.env.ANTHROPIC_BASE_URL present
AFTER DETACH : {"ANTHROPIC_API_KEY": "sk-x"} <- user's value gone
result : {"changed": true, "removed": "http://127.0.0.1:4123"}
No warning, no backup, nothing to recover from: strictly worse than the
managed-additions bug this PR opened on, where the value at least survived on
disk. `null` and `false` are the sharpest cases again, being how a user
switches an override back off.
Fixed across all three points the value passes through, since fixing any one
alone still loses it:
- attach takes the backup unconditionally (JSON cannot encode `undefined`, so
`undefined` already means absent, and the existing `!== undefined` checks are
the presence test the type test was standing in for),
- re-attach carries the recorded prior forward on the field being present
rather than on its type,
- the core undo reads `prev_base_url` by presence, so a backup the marker is
holding can no longer be discarded into the delete branch.
The marker keeps the real JSON value; only the human-readable `prevValue` /
`restoredValue` report coerces to a string, the pattern the undo already used
for `removed`.
LLP 0045 stated presence-not-type for "every env key attach adds beside the
base URL" and left the base URL's own backup unstated, which is the gap that
let this through. It now states the rule for the backup too, and why this key
needs it more rather than less.
Mutation-pinned in all three places: reverting the attach backup fails 8
tests, reverting the undo read fails the 3 round-trips, and reverting the
re-attach read fails the two-attach round-trip.
npm test: 2862 pass / 8 fail (the known leave-command baseline). typecheck clean.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: neutral-reconciler <neutral@example.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approvedneutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attached Claude sessions report inflated context percent: proxy base URL drops the assumed window from 1M to 200k

2 participants

@philcunliffe@platypii