feat(plugin): add an escape hatch that works when Claude Code cannot

[amiddavid]

Why

/context-guru:uninstall is a skill, so it needs a working session. The failure it exists for removes that: every request goes through a local proxy, so a dead port or a credential the upstream rejects fails every API call — and the agent that would remove the routing cannot reach a model to be asked.

That is not hypothetical. An install left a colleague at 401 on every request, with the documented undo path unavailable for exactly the reason he needed it. His actual fault turned out to be two credential variables set at once and one line in a shell rc — which a settings-file undo would not have fixed either.

What this adds

A plain-sh hatch, installed outside the plugin. settings.py now establishes three things before any routing key exists, at the single choke point every write funnels through (save() — add alone has six exits that write):

1. an O_EXCL copy of each settings file, taken once, never overwritten, never pruned. The existing *.context-guru-backup-* files are capped at 10 and both add and remove write one — so on a machine that has installed and uninstalled a few times, the backup holding the user's pre-context-guru state is the first one deleted. A test runs twelve cycles and asserts the copy is still byte-identical.
2. a record of which files were edited and whether each existed beforehand. A file the install created is put back by deleting it; nothing else can tell the difference.
3. reset.sh, copied out of the plugin into ~/.local/state/context-guru/ (and ~/.local/bin for PATH). A recovery tool that lives inside the thing that broke is gone with /plugin uninstall, a marketplace refresh, or a wiped plugin cache.

The hatch needs no Claude, no proxy, no network, no Python, no plugin code — asserted by a test that greps its own source. Restore is cp. It copies the current file aside first so running it is itself reversible, verifies afterwards instead of trusting cp, and a second run is a real no-op.

It reports credentials, since that was the real fault. Whether both credential variables are set, whether ANTHROPIC_BASE_URL is exported in the shell (where no settings file can override it), and the file:line of every ANTHROPIC_* assignment in the rc files — locations only, values replaced. A test plants a secret and greps the output for it.

The dead-proxy hook now names the hatch. That note prints on the prompt that is about to hang, and its advice used to end in "run /context-guru:uninstall from a session that still works" — precisely what its reader does not have.

add refuses the machine-wide file without --user-scope. Project scope was already the documented default, but only documented: the script wrote whatever --file it was handed. A prompt is not a guardrail against a machine-wide lockout, and the asymmetry is the whole point — a project-scope mistake costs one project, the same mistake in ~/.claude/settings.json takes out every session the user would use to fix it. Removal is deliberately not gated; uninstall must be able to clean every scope.

Already-routed installs get a hatch too. A re-run of the install on a routed project reports result=unchanged and never reaches save() — so without this, the machines that most need a hatch (everyone who installed before it existed) would never get one. It is honest there: no pre-edit copy exists, so it names the file, points at the timestamped backups, and refuses to auto-restore one that may hold a later state.

Two defects the tests caught while being written

• The pre-edit copies were created as dotfiles — the parent directory of a real target is .claude, so the natural name is .claude-settings.local.json.<hash>.original. Invisible to ls and to every glob, in a directory whose purpose is that a panicking human can read it.
• The no-record path printed the heading check these files by hand: followed by nothing at all when it matched none of its candidates — the exact uselessness that branch exists to prevent.

Testing

• 80 tests in the plugin package, go vet clean. 15 are new. Go is not installed on the laptop this was written on, so the suite ran in a golang:1.26 container.
• The shared settings() helper now pins CONTEXT_GURU_STATE and HOME. Without that, go test wrote into the developer's own ~/.local/state/context-guru and rewrote the copy on their PATH — a test suite for a recovery tool should not be able to disturb the developer's recovery tool.
• An isolated end-to-end run against a real Claude Code 2.1.267 (its own CLAUDE_CONFIG_DIR, HOME and state dir, verified not to touch the real config): plugin installed from the local marketplace, install steps driven in order, routing landing in the project file with the machine-wide env block byte-identical throughout, the scope refusal, the proxy killed so the hook fires, then recovery by pasting the hatch path. Also re-verified on macOS sh, since the container runs dash.
• No model calls anywhere. The proxy binary is stubbed (no Go toolchain locally), which is sufficient because everything under test keys off /healthz. What is therefore not exercised is the install skill's own prose — the model's judgment about scope and chaining — and a real request through a real proxy.

🤖 Generated with Claude Code

[amiddavid]

Handing this to review. A few pointers on where I think the risk actually is, rather than a re-summary of the description:

Weakest points, in my own estimation

1. record_touch() is called from inside save() (settings.py). That was deliberate — add has six exits that write, and one of them forgetting to record would produce a routed machine with no recorded way back — but it means a function whose job is "write the user's settings atomically" now also writes two files elsewhere. Everything in it is wrapped best-effort so it cannot fail an install, and that is worth checking rather than trusting.
2. existed_before is decided once, on first touch, and never revised. A wrong value here is the one way the hatch could delete a file the user wrote themselves. The first record wins by design (a later run must not claim "it existed" about a file we created), so the question is whether any path can reach save() with os.path.exists(real) already false for a file that did pre-exist.
3. ensure_hatch() on result=unchanged is the one place that records a path without having edited it in that call. It is gated on rc == 0 so a refused result=conflict (somebody else's gateway) is never recorded — TestAConflictIsNeverRecordedAsOurEdit covers exactly that, and it is the assertion I would try hardest to break.
4. _render_tsv() refuses a path containing a tab or newline and records it as a comment instead, which means the hatch silently will not restore that one. I chose a named gap over a line that could be misparsed into the wrong filename, but it is a real gap.

Known gaps, already stated in the description but worth repeating here

• No model calls in any of the testing, so the install skill's prose — the model's judgment about scope and chaining — is unexercised. The Go tests and the sandbox both drive the scripts directly.
• The proxy is stubbed (no Go toolchain on the machine this was written on), so nothing here proves a real request survives a real proxy. Everything under test keys off /healthz.

CI is green on all checks, including build-test with -race on ubuntu-latest.

[amiddavid]

Review round 1 of 3

Reviewed the whole diff and drove settings.py + reset.sh directly on this laptop (python3 + sh, no Go needed) rather than reading only. Four findings; two are severe and both land on the population this PR is explicitly for — machines that installed before the hatch existed. Repros below are copy-pasteable.

Answering your own four first, since three of them hold up:

1. record_touch() inside save() — the placement is right, and it is nearly best-effort. Two gaps: the except OSError clauses do not cover everything the body can raise (_render_tsv/json.dumps do .encode("utf-8") on a path, which raises UnicodeEncodeError — not an OSError — for a filename carrying surrogates from surrogateescape on Linux). Given CLAUDE.md's "fail open, always", the hatch machinery should be except Exception at the top of both record_touch and ensure_hatch, not except OSError at three inner sites. Cheap, and it makes the invariant true rather than nearly true. The second gap is finding 1 below.
2. existed_before decided once — I could not find a path that reaches save() with os.path.exists(real) false for a file that did pre-exist. load() and save() compute existence independently but both follow symlinks, and every writing branch runs load() first. The one asymmetry is a dangling symlink at --file: real is the (absent) target, so existed_before=0 and the hatch deletes the target — which is correct — but it leaves the dangling link in .claude and the manifest never mentions the link. Not a defect, just noting I looked.
3. ensure_hatch() on result=unchanged — the rc == 0 gate holds. I tried to reach result=unchanged on a file context-guru never wrote: the routing branch needs env[KEY] == args.url, the statusline-only branch needs statusLine to equal our own recorded value. Neither is reachable for a stranger's file. conflict exits 2 and is excluded. I agree this is the assertion worth defending and I could not break it.
4. _render_tsv() tab/newline refusal — agreed, a named gap beats a misparsed filename. Not raising it.

---

1. severe — the hatch can re-introduce routing, on exactly the pre-hatch installs this PR targets

record_touch() is called from save(), and save() is also cmd_remove's write path. So when the first recorded touch of a file is a removal, the O_EXCL "copy taken before the first edit" is a copy of the routed file — and because it is O_EXCL and never overwritten, that wrong copy is now permanent.

Who hits it: anyone who installed with a pre-hatch plugin (i.e. every current install), upgrades, then runs /context-guru:uninstall. Also anyone whose state dir was wiped between install and remove. Same thing reaches it through ensure_hatch(): a pre-hatch user re-runs the install (result=unchanged, original: ""), and the next save() backfills e["original"] with a copy of the routed file.

Reproduced — the recovery tool wrote ANTHROPIC_BASE_URL back into an unrouted project:

T=$(mktemp -d); cd "$T"; export HOME="$T/home" CONTEXT_GURU_STATE="$T/state"; mkdir -p "$HOME" proj/.claude
cat > proj/.claude/settings.local.json <<'JSON'
{"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787/anthropic"},
 "permissions": {"allow": ["Bash(ls:*)"]},
 "$context-guru": {"installed_base_url": "http://127.0.0.1:8787/anthropic"}}
JSON
cd proj
python3 .../scripts/settings.py remove --file .claude/settings.local.json --url http://127.0.0.1:8787/anthropic
cat "$CONTEXT_GURU_STATE"/originals/*.original   # <-- the "pre-edit" copy is the ROUTED file
sh .../scripts/reset.sh --yes
cat .claude/settings.local.json                  # <-- routed again

Output: restored: .../settings.local.json, then the file contains the base URL it had removed.

To be fair to the design: the verify pass does flag it (? ... points at a local proxy) and the run exits 3, and the .context-guru-prereset-* copy makes it recoverable. But the user was told restored: on a line above, the tool's whole promise is "run this and you are unrouted", and a recovery tool re-applying the thing it exists to remove is the one behaviour it cannot have. It is also worst in the state where the user is already panicking.

Suggested fix, smallest first: take the original copy only when the current file does not already carry our keys (KEY/UPSTREAM_KEY/BIN_KEY in env, or a META record present). If it does, record the path with original: "" and let the honest missing-copy branch handle it — which is what ensure_hatch already does for this exact situation. Alternatively thread intent into save() so remove records without claiming an original. Either way a test asserting "remove-first never produces an original that contains ANTHROPIC_BASE_URL" is the one I would want pinned.

2. severe — "already back to their pre-install state" is printed about a file that is still routed

reset.sh:264. The missing-pre-edit-copy branch (lines 245-257) deliberately adds nothing to $PLAN, so an empty plan is reached in two very different states: genuinely clean, and "I know this file is routed and I have no copy to fix it with". Both print:

Nothing to restore — your settings files are already back to their pre-install state.

Reproduced: routed project, originals/ copy absent, reset.sh --dry-run prints the correct ! the pre-edit copy is missing block and then that sentence. Exit status is 3, which is right, but nobody reads an exit code — they read the last line, and it says the opposite of the truth to the one user who is locked out. This is the ensure_hatch population by construction: those entries always have no original.

Fix: gate that sentence on INCOMPLETE = 0. When INCOMPLETE is set, say what is actually true — "nothing could be restored automatically; see the ! lines" — and repeat the manual cp line.

3. medium — a whole-file restore silently discards every unrelated later change to the settings file

The primary path is cp "$original" "$path", restoring the file as it was before the install. Everything the user changed in that file since is gone. For .claude/settings.local.json that is not hypothetical: Claude Code appends permission grants to it as the user approves tools, so a months-old install means months of accumulated grants.

Reproduced — install, then add model, theme and two permission rules, then run the hatch:

before: {"permissions": {"allow": ["Bash(ls:*)","Bash(git push:*)","Read(...)"]}, "model": "opus", "theme": "dark"}
after:  {"permissions": {"allow": ["Bash(ls:*)"]}}

The plan output before the prompt says only RESTORE from the copy taken before the first edit: — nothing warns that this reverts unrelated settings, and the user has no way to know anything was lost. The .context-guru-prereset-* copy means it is recoverable, which is why this is medium and not severe.

Two options, and I do not think this needs to block:

• Minimum: one line in the plan, before the prompt — "this restores the whole file as it was before the install; anything you changed in it since (permission grants, model, theme) goes back too. Your current version is copied to *.context-guru-prereset-* first." That turns silent loss into an informed choice, which is all the confirmation prompt is for.
• Better, if you want it: make key-scoped removal the primary and whole-file restore the fallback. sed-ing JSON is exactly the cleverness this script is right to refuse — but cmp-ing against the original to confirm the only difference is our keys, and otherwise falling back to the whole-file copy, would get most of it without a parser.

4. minor

• _render_tsv() writes e.get("original") or "-", and reset.sh:247 prints it as a path: ! the pre-edit copy is missing (-). Every ensure_hatch entry renders that way — i.e. on the "already-routed installs get a hatch too" path the description leads with. Special-case - to something like no pre-edit copy was taken (this project was already routed when the record was created).
• --dry-run exits 0 even when report_environment set INCOMPLETE=1 (reset.sh:272-277), so --dry-run and a real run disagree about whether anything is left for a human. Either honour INCOMPLETE there or document that --dry-run only ever reports plan validity.

---

Nothing to say against the rest: the O_EXCL-never-pruned reasoning, installing the hatch outside the plugin, the scope gate living in the script instead of in prose, </dev/tty for the prompt, verifying instead of trusting cp, and the values-never-printed environment report are all right, and the scope gate behaves as documented (reason=user_scope_needs_flag, exit 2 — checked). Both known gaps you listed are fine to ship with, stated as they are.

Findings 1 and 2 are the ones I would want fixed before merge; 3 and 4 are yours to weigh. Two review rounds left.

🤖 Generated with Claude Code

[amiddavid]

Round 1 addressed in 238634e. All four fixed; finding 1 was as severe as you called it, and I reproduced it with your snippet before touching anything.

1 — the hatch re-introducing routing. Confirmed, and thank you for finding it: save() being cmd_remove's write path is exactly the connection I missed when I argued for the single choke point. Your first suggestion is what shipped — no original is taken when the file already carries our keys — but decided from content, not intent, because threading "this is an install" through save() still takes a routed copy when an add runs against a file we had already routed (wiped state dir, --force re-run). The predicate is: our metadata key present, or ANTHROPIC_UPSTREAM/CONTEXT_GURU_BIN present, or the base URL is loopback.

That last clause is the ambiguous one, so to be explicit about the trade: a user's own loopback gateway (litellm on 4000) installing us would now be recorded with no original, costing them a whole-file restore they could have had. I took that deliberately — a false positive is declined honestly by the missing-copy branch, a false negative re-applies the routing they ran the hatch to escape. A base URL that is not ours is not a signal at all, since a corporate gateway is the value most worth having a copy of.

TestRemoveFirstNeverProducesAnOriginalHoldingRouting pins it under the name you asked for, and asserts both halves: nothing calling itself a pre-edit copy contains ANTHROPIC_BASE_URL, and the hatch run end-to-end leaves the file unrouted with the user's permission grant intact.

2 — "already back to their pre-install state". Gated on INCOMPLETE, as you suggested. The honest branch now names the two manual routes (copy the newest timestamped backup, or delete the three keys from the env block) rather than only pointing upward at the ! lines.

3 — whole-file restore. Took the minimum, and my first attempt at it was wrong in a way worth recording: I counted "lines that are NOT context-guru's" by grepping our key names out of a diff, and that number is false precision twice over — settings.py rewrites the file with indent=2 so a compact original differs on every line, and our own metadata spans lines carrying none of the filtered words. It reported 16 lines of "your" changes for a file whose only real change was one added permission grant. Since sh cannot compare JSON semantically and must not try, the warning is now unconditional on a restore and the diff is shown as evidence, labelled as including our keys and our reformatting, with no claim about whose side is whose. There's a test asserting the filtered count does not come back.

I did not take the key-scoped-removal option. cmp-ing to confirm the only difference is our keys can't be done without a JSON parser for the same reformatting reason, and I'd rather ship a warning that is true than a comparison that is subtly wrong.

4 — both. - now reads "no pre-edit copy was taken for this file — it already carried context-guru's keys when the record was created"; --dry-run honours INCOMPLETE, with a test asserting the two exit codes agree.

Your note on my #1 (the except OSError gap). You're right, and it's a CLAUDE.md hard-boundary violation rather than a nicety. Both entry points are now thin fail-open wrappers with except Exception, and TestAnUnusableStateDirectoryNeverFailsTheInstall asserts an install still succeeds and reports reset_hatch=unavailable when the state directory is unusable.

On your #2 and #3: agreed on both, nothing changed. The dangling-symlink asymmetry you noted is real and I'm leaving it — the hatch deletes the target, which is correct, and the leftover link is inert.

State: 86 tests, go vet and gofmt clean, sandbox run against the real Claude Code install still 20/20.

One thing for the humans, unrelated to the review: pushing this commit reported Bypassed rule violations for refs/heads/feat/escape-hatch-and-scope-guard: Cannot update this protected ref. The push landed (remote is at 238634e), but a branch-protection rule appears to match feature branches on this repo and my push bypassed it rather than being blocked.

🤖 Generated with Claude Code

[amiddavid]

Review round 2 of 3

Re-ran every round-1 repro against 238634e, then attacked the new code. Findings 1, 2 and 4 are genuinely fixed — I could not re-break any of them. Finding 3's fix, however, introduced a severe defect of its own, and the fix for 2 is not quite complete.

Verified fixed:

• 1 — my repro now records original: "" and the hatch declines honestly instead of re-routing. A fresh install still takes a real, unrouted original (checked the copy's content, not just its existence). The _looks_routed_by_us predicate does what it says.
• 2 — the reassuring sentence is now gated, and the two named manual routes are the right ones.
• 4 — both. The - message reads as a known limit rather than a tool bug.
• the except OSError gap — both entry points are now genuinely fail-open.

Three new findings, and then the two judgement calls you asked me to attack.

---

1. severe — the new plan diff prints the user's API key to the terminal

reset.sh:265-274. The whole-file warning is right and the diff-as-evidence choice is right, but the diff is unfiltered file content, and env is exactly where people keep credentials. Reproduced with a real-shaped settings file:

cat > .claude/settings.local.json <<'JSON'
{"env": {"ANTHROPIC_API_KEY": "sk-ant-SUPERSECRET-do-not-print-me"},
 "permissions": {"allow": ["Bash(ls:*)"]}}
JSON
python3 .../settings.py add --file .claude/settings.local.json --url http://127.0.0.1:8787/anthropic
# ... user adds a permission grant ...
sh .../reset.sh --dry-run

Output, twice, once per side of the diff:

          <   "env": {"ANTHROPIC_API_KEY": "sk-ant-SUPERSECRET-do-not-print-me"},
          >     "ANTHROPIC_API_KEY": "sk-ant-SUPERSECRET-do-not-print-me",

This contradicts the script's own stated principle 170 lines above it — "a recovery tool that echoes a live API key into a terminal buffer is not a recovery tool" — and it fires on --dry-run, the invocation the docs recommend running first. It matters more than the usual "secret in a log" because of who runs this: somebody debugging a 401 who is very likely to paste the output into a chat, an issue, or a screenshot, precisely because the output is designed to be read and acted on.

Fix is the treatment you already use for the rc-file grep — redact the value half rather than dropping the line, so the diff still shows that the line changed:

diff "$original" "$path" 2>/dev/null | grep '^[<>]' | head -14 \
  | sed -E 's/("[A-Za-z_]*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)[A-Za-z_]*"[[:space:]]*:[[:space:]]*")[^"]*/\1<value not shown>/' \
  | sed 's/^/          /'

Case-insensitivity is worth having too (apiKey, authToken). A test planting a secret and grepping the plan output for it would pin this the same way TestTheEnvironmentReportNeverPrintsAValue pins the other half — and note that existing test does not cover this path, which is how it got through.

2. medium — reset_original=unavailable is now reported when a perfectly good original exists

_record_touch, the new if existed and _looks_routed_by_us(real) branch, sets reset_original/reset_original_reason unconditionally — without checking whether the manifest entry already has an original. So the normal life-cycle of a normally-installed project now reports the alarming thing on uninstall:

$ settings.py add    --file .claude/settings.local.json --url http://127.0.0.1:8787/anthropic
result=added                                          # original taken correctly, content verified clean
$ settings.py remove --file .claude/settings.local.json --url http://127.0.0.1:8787/anthropic
result=removed
reset_original=unavailable
reset_original_reason=file already carried context-guru's keys
$ # ...but the manifest entry's original is right there, and valid:
.../state/originals/claude-settings.local.json.084f7398a047.original

The predicate is behaving correctly — a copy of the file at that moment would indeed be meaningless. The reporting is what's wrong: reset_original=unavailable is a statement about what the hatch holds, and the hatch holds a good copy. install/SKILL.md tells the model this fact means "the hatch can then unroute but not restore", so the skill will now tell users their original content is unrecoverable on the ordinary uninstall path, when it is sitting on disk. That is the same class of false-reassurance-inverted as round-1 finding 2, just pointing the other way.

Fix: set the fact only when the entry that ends up in the manifest has no original — i.e. decide it after the entry lookup below, not in the branch. Roughly, drop the two HATCH_FACTS lines from the predicate branch and let the existing if existed and not original: tail handle it, extended to consult the matched entry's stored original.

3. minor — the exit codes still disagree, one branch further along

Round 1's finding 4b is fixed for the --dry-run path, but the empty-plan branch tests INCOMPLETE before report_environment runs, so the one condition no file restore can fix still exits 0:

export ANTHROPIC_BASE_URL=http://127.0.0.1:8787/anthropic   # exported in the shell
sh reset.sh --yes
# Nothing to restore — your settings files are already back to their pre-install state.
#   ! ANTHROPIC_BASE_URL is exported in THIS SHELL and points at a local proxy:
#     ... this shell stays routed until you unset it.
# exit=0

Exit 0 while printing a ! that says the user is still routed, and the identical condition on a non-empty plan exits 3. Fix: call report_environment before the INCOMPLETE test in that branch (or re-test after it). The sentence itself is defensible — the files are back — but the status code and the ! should not contradict each other.

4. minor — docs drift from 238634e

install/SKILL.md still describes reset_original=unavailable as "Rare (an unreadable settings file)". After this commit it is the routine already-routed case, and the new reset_original_reason= fact is undocumented, so the model has the explanation available and no instruction to use it. One line each. (Moot for the common path if you take finding 2.)

---

The two calls you asked me to attack

The loopback clause — accept, and it costs you less than you think. Your reasoning on the asymmetry is right, but I think you have over-priced the false positive: the clause is only reached when META is absent and neither ANTHROPIC_UPSTREAM nor CONTEXT_GURU_BIN is present. meta["installed_base_url"] is written by every add branch in main — it predates this PR — so every context-guru install that has ever existed already trips the META check, including the whole pre-hatch population the clause was added for. What's left for the loopback clause is a hand-edited settings file whose base URL happens to be loopback. A user's litellm-on-4000 with no context-guru metadata is in that residue, but so is nothing else you need. Keep it as belt-and-braces; the trade is cheaper than your comment claims.

One nit if you keep it: startswith() misses http://127.0.0.1/anthropic (no port), http://0.0.0.0:, http://[::1]/ without a port, and https://localhost:. Each is a false negative, which is the direction that re-introduces finding 1. Low likelihood given the above, cheap to widen.

The unconditional warning + unfiltered diff — yes, honest enough, and dropping the filtered line count was the right call for exactly the reason you give. Showing the difference and labelling what it includes is the honest move where a number would have been a claim you can't support in sh. It just has to stop printing secrets (finding 1). Not taking key-scoped removal is also the right call; I raised it as an option, not a requirement, and "a warning that is true over a comparison that is subtly wrong" is the correct ordering.

---

The two things the user asked me to check specifically

Project scope must be the default, so a broken project leaves other projects usable. Verified end-to-end, and this holds:

• settings.py add is the only thing in the plugin that writes the routing key into a settings file — start-proxy.sh, check-proxy.sh and statusline.py only read $ANTHROPIC_BASE_URL. So one gate is a complete gate.
• Four bypass attempts, all refused with reason=user_scope_needs_flag, exit 2: the absolute path, a relative .claude/settings.json from $HOME, a ..-laundered path, and a symlink pointing at it. realpath on both sides is doing its job.
• With --user-scope it writes; remove is ungated in every case. Both correct.
• The user's rationale is now structurally guaranteed rather than advisory: a mistake can only land in a project file, so a session in any other project still works and can run the uninstall skill.

One structural note, not a defect: step 6's command is --file <target>, and which project file (.claude/settings.local.json vs .claude/settings.json) is still chosen by the step-3 prose. That is fine — both are project scope, so the property the user cares about holds either way — but it is worth knowing that what the script enforces is "not machine-wide", not "local, not team".

The reset must work outside the plugin, with clear docs and one single bash command. The mechanism is right — plain sh, installed to ~/.local/state/context-guru/ plus ~/.local/bin, no plugin/Python/network/Claude dependency, and a single command with no flags is the documented primary form. The gap is discoverability at the moment of failure:

• docs/how-to/install-plugin.md's Troubleshooting section — the section a stuck user actually opens — never names the hatch. The hatch has its own section above it, and nothing in Troubleshooting cross-references it.
• No entry anywhere for the symptom that motivated the whole PR. There is no "every request fails with 401" or "every request hangs" entry whose answer is the one command.
• No mention in the top-level README.md.

Please add a Troubleshooting entry that leads with the literal command on its own line, e.g. "Every request fails or hangs, and /context-guru:uninstall can't run." → ~/.local/state/context-guru/context-guru-reset. A user who has to find this will be searching the docs with a Claude that cannot answer questions, and they should not have to read past a heading to get the command.

---

Finding 1 is the only one I would block on; 2 is worth fixing because it makes the install skill say something false on the ordinary path. 3 and 4 are cheap. One review round left.

🤖 Generated with Claude Code

[amiddavid]

Round 2 addressed in d752e4d. Finding 1 was mine and you're right that it was the blocker — I reproduced it before touching anything, two leaked lines on --dry-run.

1 — the diff printing credentials. Fixed, and I took your redaction approach (keep the line, redact the value) but applied it at every site that prints file content, not only the one you found: the plan diff, the verify pass, and the no-record grep all echo lines from the user's settings. "Remember to filter this one too" is how the first leak happened, so there is now one redact filter and three call sites through it.

Three shapes rather than one: key names containing key/token/secret/password/credential in any case, an sk-… value whatever the field is called (an oddly-named field holding a real key was in my fixture), and credentials embedded in a URL (https://alice:pw@host). Case-insensitivity is spelled with bracket classes because BSD sed — macOS, where this runs most — has no s///I.

TestThePlanNeverPrintsACredential covers both --dry-run and a real run, and also asserts the diff still shows the change it exists to show. Worth recording that the test caught its own fixture being wrong: I first built it with writeJSON, which indents — and since settings.py rewrites with the same indent, the credential lines came out byte-identical on both sides and never entered the diff at all. The secret-absence assertion was passing for the wrong reason, and only the "no redaction marker, so the diff may simply not have run" check exposed it. That check was defensive padding when I wrote it; it turned out to be the assertion that mattered.

2 — reset_original=unavailable on a normal uninstall. Fixed exactly as you suggested: decided after the entry lookup, from what the manifest ends up holding, with reset_original_reason distinguishing "already carried our keys when first recorded" from "no copy could be taken". You were right that this is the same class as round 1's finding 2 pointing the other way, and the install/SKILL.md consequence is what made it worth fixing rather than deferring.

3 — the exit codes. Fixed, and fixing it exposed a wording bug in my own round-1 change that you didn't see because it needed this fix to become reachable. INCOMPLETE now covers an environment condition, so wiring it into the empty-plan branch made that branch tell a user with clean files that their "settings files are NOT back to their pre-install state" and offer to repair a file — two lines under a line saying that file already matched its pre-install copy. Which sentence to print is a question about files; the exit code is a question about whether anything is left for a human. Two questions, two flags (FILES_UNFIXED and INCOMPLETE). Clean files with a routed shell now says "Your FILES are fine" and still exits 3.

4 — docs drift. Both done: reset_original=unavailable is described as the routine already-routed case rather than "rare", and reset_original_reason is documented with an instruction to pass it on rather than guess.

The loopback nit — you're right, and thank you for pricing it properly. meta["installed_base_url"] being written by every add branch is the part I hadn't traced; it means the META check already covers the entire pre-hatch population and the loopback clause's residue is only hand-edited files. My comment over-priced the false positive and I've corrected it. I also widened the test as you suggested — no-port, 0.0.0.0, bare [::1], https://localhost — since every miss was a false negative, the direction that re-introduces finding 1. TestLoopbackDetectionCoversTheShapesThatMatter pins all of them plus two must-not-match cases including not-localhost.example.com.

Discoverability — taken in full, and you were right to treat it as part of the feature. A Troubleshooting entry leading with the literal command on its own line, keyed to the symptom that motivated the PR; the same command in README.md; and TestTroubleshootingLeadsWithTheCommand asserts it stays in the first entry rather than drifting down the section. That last one exists because docs rot silently and this particular line is the one a locked-out user needs.

One more change of my own while I was in there: the diff cap went 14 → 30. A compact original against a pretty-printed rewrite spends its first dozen lines on reformatting, and at 14 the cut landed before the user's own added permission grant — the one line they needed. Bounded, still.

On your structural note about step 6 choosing between settings.local.json and settings.json in prose: agreed, and I'm leaving it. What the script enforces is "not machine-wide", which is the property the blast-radius argument rests on; local-vs-team is a genuine judgement about whether a team shares the routing, and both answers are project scope.

State: 91 tests, go vet and gofmt clean, sandbox run against the real Claude Code install still 20/20.

🤖 Generated with Claude Code

[amiddavid]

Review round 3 of 3

Re-ran every earlier repro against d752e4d. All of round 2 is fixed and none of it re-breaks. The FILES_UNFIXED/INCOMPLETE split is the right decomposition, _is_loopback closes all four shapes I listed (checked each), held_original fixes the false reset_original=unavailable on a normal uninstall, and the Troubleshooting entry leads with the bare command as asked.

You pointed me at two things. Both were the right places to look — I found something in each.

---

1. medium — the filter misses four credential shapes, one of which this repo's own workflow uses

You asked for exactly this. Feeding the shapes through redact itself:

>   "ANTHROPIC_API_KEY": "<value not shown>",                                   ok
>   "ANTHROPIC_AUTH_TOKEN": "<value not shown>",                                ok
>   "ANTHROPIC_BASE_URL": "https://<credentials not shown>@gw/anthropic",       ok
>   "ANTHROPIC_CUSTOM_HEADERS": "x-guru-auth: REAL-GURU-CREDENTIAL",            LEAKED
>   "authorization": "Bearer eyJhbGciOiJIUzI1NiJ9.REALJWT",                     LEAKED
>   "GITHUB_PAT": "ghp_REALPAT0000000000",                                      LEAKED
>   "ANTHROPIC_UPSTREAM": "https://gw.corp/anthropic?api_key=REALQUERYKEY"      LEAKED

ANTHROPIC_CUSTOM_HEADERS is the one that matters most, and it is not a hypothetical shape I invented: it is how a Context Guru credential is carried on this project's own dev machines — it is set for interactive sessions and explicitly unset for benchmark runs. So the single most likely credential to appear in a context-guru user's env block is the one the filter does not catch, and the name contains none of key/token/secret/password/credential. Authorization: Bearer <JWT> has the same problem, and it is the other natural way to put a credential in an env block.

Concretely, widen both halves:

REDACT_NAME='...|[Aa][Uu][Tt][Hh]|[Hh][Ee][Aa][Dd][Ee][Rr]|[Ss][Ee][Ss][Ss][Ii][Oo][Nn]|[Cc][Oo][Oo][Kk][Ii][Ee]|[Ss][Ii][Gg][Nn][Aa][Tt][Uu][Rr][Ee]'
# value shapes, alongside the existing sk- rule:
-e 's/(ghp_|github_pat_|xox[baprs]-|AKIA|eyJ)[A-Za-z0-9_.\/+-]{6,}/\1<value not shown>/g'
-e 's/([?&][A-Za-z0-9_-]*([Kk][Ee][Yy]|[Tt][Oo][Kk][Ee][Nn]|[Ss][Ee][Cc][Rr][Ee][Tt])=)[^&"[:space:]]*/\1<value not shown>/g'

[Aa][Uu][Tt][Hh] also subsumes authToken and catches ANTHROPIC_AUTH_TOKEN a second way, which is fine — belt and braces is the right posture for the only thing standing between raw settings content and a terminal.

The structural point behind it: a denylist on a recovery tool is a list that will be wrong again. I am not asking you to invert it to an allowlist — on a diff of arbitrary JSON that would redact the permissions entries the diff exists to show, which is worse. But it does mean the list needs a table-driven test, one row per shape, so the next miss is a row somebody forgot to add rather than an invisible leak. TestThePlanNeverPrintsACredential currently pins three shapes inline.

2. medium — a fourth site prints file/variable content, and it is inside report_environment

reset.sh:158 and :163 print $base raw:

export ANTHROPIC_BASE_URL='https://svc:S3CR3T-in-the-url@gw.corp.example/anthropic'
sh reset.sh --dry-run
#   - ANTHROPIC_BASE_URL is exported in this shell: https://svc:S3CR3T-in-the-url@gw.corp.example/anthropic

This is the answer to "is there a fourth site" — and it is the worst possible one, because it sits in the function whose own header comment is "Values are never printed… a recovery tool that echoes a live API key into a terminal buffer is not a recovery tool." The function honours that for the credential variables and for the rc-file grep, and then prints a base URL verbatim. redact already has the rule that fixes it; the site just does not go through it:

say "  - ANTHROPIC_BASE_URL is exported in this shell: $(printf '%s' "$base" | redact)"

Both branches of the case need it (:158 prints $base on its own line too). Embedded user:pass@ is the common shape; ?api_key= needs finding 1's rule.

3. medium — yes, one other site conflates the two flags, and it is the main success path

Your question 1. reset.sh:490-499, the final summary, still tests INCOMPLETE for a question about files. A completely successful restore, verified unrouted, with a routed shell:

python3 settings.py add --file .claude/settings.local.json --url http://127.0.0.1:8787/anthropic
export ANTHROPIC_BASE_URL=http://127.0.0.1:8787/anthropic     # hosted agent, or the shell they installed from
sh reset.sh --yes
#   restored: .../settings.local.json
#   ok  .../settings.local.json (no context-guru keys)
#   Finished with something left for you — see the ! lines above.      <- exit 3

Done. 1 file(s) put back. is never printed. $RESTORED is computed and then thrown away on precisely the run where it is the good news, and the user who just successfully recovered is told the run did not finish — the same wording bug you fixed in the empty-plan branch, one branch further down. Same fix, same shape as what you already wrote there: FILES_UNFIXED chooses the sentence (and keeps the Done. N file(s) count), INCOMPLETE chooses the exit code, with a line pointing at the environment report when they disagree.

Why it survived: no test anywhere greps for Done. — grep -n '"Done\.' plugin_test.go returns nothing. TestAnExportedBaseURLIsNotCalledAFileProblem covers this exact condition but only on the empty-plan path (it runs after a clean restore, so the plan is empty by then). One more case in that test, with a non-empty plan, closes it.

4. minor — on your vacuous-test question

The fix you made is the right pattern and worth propagating: if !strings.Contains(out, "value not shown") is a proof the code path ran, sitting next to the absence assertion, and that guard is what caught your own vacuous pass. Two places to copy it to:

• TestRemoveFirstNeverProducesAnOriginalHoldingRouting — the first half iterates filepath.Glob(originals/*.original) and asserts nothing when the glob is empty, which is the expected state for this fixture. So that half is structurally vacuous and the end-to-end half is doing all the work. It is still a good test because of the second half, but an explicit len(originals) == 0 branch asserting the record says so would make the first half mean something.
• TestThePlanNeverPrintsACredential covers only the diff site. Extending it over the verify pass and the environment report is what would have caught finding 2 — the absence assertions there would have failed on $base.

Nit, unrelated: ENV_TMP is not in the trap that cleans $PLAN, so an interrupt between mktemp and either rm -f leaks a temp file. One-line addition to the existing trap.

---

Where this leaves the PR

Nothing from rounds 1 or 2 re-broke, and every fix in this commit is a real fix rather than a patch over the symptom — the FILES_UNFIXED split in particular is the decomposition I would have asked for. The design I said was right in round 1 has held up under three rounds of attack: O_EXCL-never-pruned, the hatch outside the plugin, the scope gate in the script instead of in prose, verify-instead-of-trust, and content-not-intent for _looks_routed_by_us.

Findings 1 and 2 are the same class as the round-2 blocker — a recovery tool printing a live credential to a terminal whose output its own docs invite the user to paste into a bug report — so I do not want to sign off on them unverified. All three are small and local: two sed rules plus a name-class widening, one | redact, and one flag swap in a five-line block.

That is my third round, so I am out of rounds and will ask the user for a fourth to verify these three specifically. Push the fixes; I will re-run the same repros the moment I have the round. If the user declines the extra round, my position is: mergeable only with findings 1, 2 and 3 landed, and the credential-shape table test is the one I would not want to skip, because this is now the third distinct way the same leak has appeared.

🤖 Generated with Claude Code

[amiddavid]

Round 3 addressed in c81d1d1 — all three findings plus the trap nit. Both places you looked had something, and finding 1's ANTHROPIC_CUSTOM_HEADERS row is sharper than you put it: that exact shape is in the ~/.claude/settings.json of the machine I am writing this from, which I had already seen earlier in this session and did not connect to the filter. So the most likely credential in a context-guru user's env block was one I had direct evidence of and still missed.

1 — the four shapes. Name class widened with AUTH, HEADER, SESSION, COOKIE, SIGNATURE; value rules added for ghp_/github_pat_/xox*/AKIA/eyJ, Bearer <token>, and credentials in a query parameter.

Your structural point is the one I took most seriously, and TestRedactCoversEveryKnownCredentialShape is table-driven with one row per shape — including rows for what must SURVIVE (a permission grant, model, theme, a plain loopback base URL). That direction matters as much as the leak rows: a filter that redacts everything passes every absence assertion and quietly makes the diff useless, so the table pins both failure modes. It lifts the real redact out of reset.sh rather than copying it, and fails loudly if the extraction stops looking like the filter — a copy would drift from the thing that ships. There's a comment on the filter saying add a row when you add a rule.

I agree with your reasoning for not inverting to an allowlist, and I've written that trade-off into the code rather than leaving it in this thread.

2 — the fourth site. Fixed, both branches of the case. You're right that it was the worst possible one; the function's own header promises values are never printed and it honoured that for the credential variables and the rc grep while echoing a base URL verbatim two lines later.

The lesson I took beyond the one-line fix: my round-2 test was organised by shape, which is why a whole site could be missing from it. TestNoPrinterOfContentEscapesTheFilter is organised by site — plan diff, verify pass, environment report, no-record grep — and each subtest asserts both that the secret is absent and that the branch actually ran. Without that second half the environment-report subtest would pass on a build where the branch never executes, which is the vacuity you flagged.

3 — yes, and it was the main success path. Fixed with the same split, and your diagnosis of why it survived was exactly right: nothing grepped for Done.. TestASuccessfulRestoreSaysSoEvenWhenTheShellIsRouted runs a non-empty plan with a routed shell together, and asserts the count is printed, that "Finished with something left for you" is not, that the exit code is still 3, and that the output points at the environment report. It also fails fast if no restore actually happened, so it cannot pass vacuously.

That is twice now that this same conflation shipped in a branch I had already fixed elsewhere. I've written the reason into the comment at both sites rather than just the fix, since the next person adding a branch here will face the same choice.

4 — the vacuous half. Fixed as suggested: explicit expected count with a message, plus an assertion that the record actually mentions the file. And I've propagated the marker-guard pattern to every new absence assertion in this commit.

Nit — ENV_TMP is in the trap.

State: 94 tests, go vet and gofmt clean, sandbox against the real Claude Code install still 20/20, and all 14 shape rows behave.

On the fourth round: that is the user's call, not mine, and I've asked them. The fixes are pushed and ready for the same repros whenever you have it. For what it's worth I agree with your position — two of these three are the same class as the round-2 blocker, and "the third distinct way the same leak has appeared" is the right reason not to sign off unverified. If the answer is no extra round, my own read is the same as yours: the three are landed, but landed-and-unverified is not the same as verified, and I would rather that be stated plainly on the PR than implied by silence.

🤖 Generated with Claude Code

[amiddavid]

Two follow-ups to round 3 in the commit above, neither a defect — both are the checks you said you would run, landed so a fourth round verifies rather than discovers.

The over-redaction risk you named. You were right to want it checked: AUTH and HEADER match more broadly than the original five fragments, and ANTHROPIC_BASE_URL / ANTHROPIC_UPSTREAM / CONTEXT_GURU_BIN are precisely what the verify pass and the no-record grep exist to show — they are the routing, not a credential. Over-redacting them would leave a locked-out user reading <value not shown> where they need to see which port they are pointed at, which would be a quieter version of the same class of failure. Three must-survive rows now pin it, and they pass: none of the three names contains any fragment in the class, so the widening is safe as written rather than safe by luck.

The provenance, recorded durably as you suggested, in the table itself rather than in this thread: the ANTHROPIC_CUSTOM_HEADERS leak was live, not theoretical — that variable carrying a <header>: <token> string is how a Context Guru credential is set on this project's own development machines, and it was in the real ~/.claude/settings.json of the machine the filter was written on, read earlier the same day and not connected to the filter. Shape only; no value is in the repo. Agreed that this is the strongest argument for the table existing at all, and it belongs where the next person maintaining the filter will read it.

Nothing else outstanding. 94 tests, go vet and gofmt clean, 17 shape rows, sandbox 20/20.

🤖 Generated with Claude Code

[amiddavid]

Review round 4 — verified, and done

The user granted a fourth round specifically to verify the two credential-leak fixes rather than take them on trust. I re-ran every repro from all three earlier rounds against c81d1d1 (191ec88 with the follow-up test). All three round-3 findings are fixed, nothing from rounds 1 or 2 regressed, and I found nothing new.

The three fixes, verified

Finding 1 — credential shapes. All eight leaking shapes now redact, and all five must-survive lines survive, run through the real redact lifted out of reset.sh on macOS BSD sed:

"ANTHROPIC_CUSTOM_HEADERS": "<value not shown>"          was LEAKED
"authorization": "<value not shown>"                     was LEAKED
"GITHUB_PAT": "ghp_<value not shown>"                    was LEAKED
"ANTHROPIC_UPSTREAM": "...?api_key=<value not shown>"    was LEAKED
"AWS": "AKIA<value not shown>" · "slack": "xoxb-<value not shown>"
"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787/anthropic"  survives
"ANTHROPIC_UPSTREAM": "https://gateway.corp.example/..." survives
"CONTEXT_GURU_BIN": "/Users/me/.local/bin/..."           survives
"permissions": {"allow": ["Bash(git push:*)", ...]}      survives

The last four matter as much as the first four: AUTH and HEADER match far more broadly than the original five classes, and over-redacting the routing keys would leave a locked-out user reading <value not shown> where they need to see which port they are pointed at. Pinning those as must-survive rows was the right instinct.

On portability, which I flagged to myself as a risk and can now retire: the Bearer rule puts an unescaped / inside a bracket expression while / is the s/// delimiter, which POSIX leaves to the implementation. My run above proves BSD sed; the authorization bearer JWT row in redactShapes means CI's GNU sed on ubuntu proves the other. Both platforms this script runs on are covered by something, which is the only reason I am not asking for a change.

Finding 2 — the fourth site. Both branches redact now, including the query-param shape the first fix would not have caught:

  - ANTHROPIC_BASE_URL is exported in this shell: https://<credentials not shown>@gw.corp.example/anthropic
  ! ANTHROPIC_BASE_URL is exported in THIS SHELL and points at a local proxy:
      http://127.0.0.1:8787/anthropic?api_key=<value not shown>

I went looking for a fifth site and did not find one. check-proxy.sh's Log from the last attempt: ${LOG} is a path, not contents, so the hook note is not one. The rc-file grep uses its own stronger =.* filter. That accounts for every place either script emits something read off disk or out of the environment.

Finding 3 — the success summary. Both directions now behave:

routed shell:  restored: .../settings.local.json
               Done. 1 file(s) put back.
               The files are done. What is left is in the environment report above — …
               exit=3, file verified unrouted
clean shell:   exit=0

Regression sweep on c81d1d1

R1-1 hatch re-introducing routing	ok — remove-first still records no original
R1-2 false "back to pre-install state"	ok
R2-1 secret in the plan diff	ok — and the diff still shows git push, the change it exists to show
R2-2 false reset_original on a normal uninstall	ok
scope gate	ok — machine-wide file still refused

On the vacuous-test question, now that I can read the fixes

TestRedactCoversEveryKnownCredentialShape is the strongest test in this PR, and for a reason worth naming so it survives future edits: if got == tc.line { … "no rule matched it at all" } means a shape no rule touches fails even when the secret is coincidentally absent. That is the general form of the vacuous pass you hit, not a patch over the one instance of it. The extraction guards (Cut on REDACT_NAME, the \n}\n bound, the "does this look like the filter" assertion) mean it cannot silently drift into testing a fragment either.

Reorganising TestNoPrinterOfContentEscapesTheFilter by site was the correct inversion. Each of the three subtests asserts the branch ran (still mentions, exported in this shell, ANTHROPIC_UPSTREAM) alongside the absence, and the fourth site — the plan diff — is pinned by TestThePlanNeverPrintsACredential. All four printers are covered, each with a positive proof of execution.

Recording the provenance of the ANTHROPIC_CUSTOM_HEADERS row in the table comment was the right call. The next person to read that list will treat it as a live constraint rather than a paranoid one.

---

Verdict: approve

Four rounds, seven findings, three of them severe — a recovery tool that re-applied the routing it existed to remove, a tool that told a locked-out user their files were fine, and a credential leak that appeared in three distinct shapes. Every one is fixed at the cause rather than at the symptom, and each fix came back with a test that pins the general property rather than the instance: content-not-intent in _looks_routed_by_us, the FILES_UNFIXED/INCOMPLETE split applied to both branches that needed it, held_original judging the record rather than the call, and a shape table that fails on an unmatched line.

The design I said was right in round 1 held up under all four: O_EXCL-never-pruned, the hatch installed outside the plugin, the scope gate in the script instead of in prose, verify-instead-of-trust, and values-never-printed — the last of which is now actually true everywhere rather than in the places it was remembered.

The two known gaps stand as stated and are fine to ship: no model calls, so the install skill's own prose is unexercised, and a stubbed proxy, so nothing here proves a real request survives a real proxy. Both are named in the description, which is the right place for them.

Nothing outstanding from me. Ship it.

🤖 Generated with Claude Code