feat(plugin): a Claude Code plugin for context-guru, with the #141 review's blockers fixed

[amiddavid]

The Claude Code plugin: /plugin marketplace add rossoctl/context-guru → /plugin install → /context-guru:install.

#141 has merged, so this is no longer stacked — it is two commits on main, 17 files, and the diff is the plugin and its docs. Three skills over four scripts and two hooks.

Default routing scope is .claude/settings.local.json: one repo, gitignored, --global an explicit opt-in. A base URL pointing at localhost breaks Claude Code wherever it is routed when the proxy is down, so blast radius is the thing the default optimises for.

Read this first: it cannot work yet, and that is not a defect in the diff

install.sh resolves a GitHub release, and no tag has published assets. On main today the third command ends at no_release_found.

So the merge order matters more than the code review: run release.yaml via workflow_dispatch (snapshot, publishes nothing — it has never executed, and #141 landed wrap_in_directory: true whose archive layout only a real goreleaser run confirms), cut v0.1.0, then merge this. Landing it first puts a README on main telling strangers to run three commands, the last of which fails.

The plugin has therefore never run end to end in a real session. Everything below was verified by unit tests, by driving the scripts directly, and by reading — not by installing it.

What the review of the combined PR found here, and what changed

All six blocking findings were in this half:

#	Finding	Fix
1	uninstall killed the user's own session and left the proxy running	pidfile + socket-owner fallback; the port is in argv via --listen; no pattern matching
2	install.sh could not install, and its documented go install fallback was absent	implemented, plus download_failed as a reported outcome and a key=value contract curl no longer breaks
3	A dead proxy hangs silently and status cannot diagnose it	UserPromptSubmit hook — the only thing that runs without a model call
4	cache advertised context_guru_expand	fixed in #141 where the gate belongs; the claims here corrected
5	The backup destroyed the user's undo; uninstall did not restore what it replaced	O_EXCL + microsecond stamps; the replaced value is recorded and restored
6	The atomic write widened a credential file's mode, and replaced symlinks	mode preserved, path resolved first

On finding 1 the mechanism is worth stating, because the fix follows from it: the port was passed through LISTEN_ADDR in the environment, so pkill -f "context-guru-proxy.*$PORT" matched no proxy — and did match the shell running it, which is the session's own Bash tool. The narrow pattern the skill offered as the safe option was the one that bit. So the fix is a handle, not a better pattern.

On is_ours I did not take the review's suggestion. A URL-shape rule ((127.0.0.1|localhost|[::1]):\d+/anthropic) makes litellm's default read as ours, so uninstall would delete somebody else's routing — the existing test failed the moment I tried it. add now records the URL it wrote and later runs read that record; anything unrecorded stays a conflict, for add and remove alike.

One change since that review

install.sh finds the binary in the tarball rather than assuming the archive root. #141 set wrap_in_directory: true — deliberately, because a flat archive plus the documented tar xzf with no -C overwrites the README.md and LICENSE of whatever directory the user is standing in, which for an evaluator is their own project. Without this change the first install anyone attempted would have failed with binary_not_in_tarball, reading as a broken release rather than a moved file. Found by checking #141's packaging change against this script, not by waiting for it to fail.

Verification

The shell and Python helpers are tested from Go (context-guru-plugin/plugin_test.go) so go test ./... and CI cover them. Seven mutations, each proven to have landed in the source before its result counted, each failing its named test and passing when restored — full output in the commit body. Coverage includes: settings merge/conflict/removal/backup/mode/symlink behaviour, the hook's silence in unrouted projects, its idempotence, its non-failure when the binary is missing, and its wait for /healthz.

One process note kept in the open: my first attempt at the backup mutation reverted only the timestamp granularity and left the O_EXCL loop, so the filename was still unique and the test passed — proving nothing. Reverting half a fix is its own route to a vacuous result.

Worth a reviewer's attention

• The SessionStart hook runs in every project the user has, because the plugin installs at user scope. It self-gates on $ANTHROPIC_BASE_URL naming its own port — matching the port, not "localhost", so somebody routing to litellm on 4000 is not hijacked. It is synchronous (closes the race with the session's first request), idempotent (SessionStart also fires on clear/compact/resume/fork), and exits 0 on every path: a hook that fails here is a plugin that can brick every session on the machine.
• settings.py edits the user's real settings.json. It merges exactly one key, backs the file up first (and prunes to 10), refuses to overwrite a base URL it did not write, restores what it replaced on removal, preserves mode, and follows symlinks. It refuses rather than rewrites a file it cannot parse.
• The zero-value cases are documented where a first-run user reads them, and status checks the commonest one at runtime: outside a git repository there is no environment snapshot, so cachesplit skips and the saving is exactly zero. status also no longer reads acted: 0 / savings_pct: 0 as a verdict — those count content removal, and this component relocates a breakpoint.

🤖 Generated with Claude Code

[amiddavid]

Review of #160 — plugin only, 2 commits, 17 files

Reviewed at 5d37e0c in a separate worktree, rebased scope (origin/main...HEAD), so #141's
changes are excluded. I took your steer and spent the depth on settings.py and the
SessionStart/UserPromptSubmit hooks rather than the docs.

The six blocking findings from #141 are genuinely fixed, and the tests are not vacuous — each
one drives the real script and asserts an observable (the sentinel file, the recorded argv, the
pidfile naming a live PID, the file mode, the symlink target). TestInstallRefusesAnUnverified Download in particular tests the fail-closed property rather than the happy path. The
is_ours-by-record decision is right, and your reasoning for declining the URL-shape suggestion
holds: litellm's 127.0.0.1:4000/anthropic really does make a shape rule unsafe.

Two findings below are, I think, blocking, and both are on the surfaces you named. I verified each
by running the scripts rather than by reading — measurements are in the comments.

1 — the UserPromptSubmit hook cannot finish inside its own timeout. Measured 18s on the exact
dead-proxy path it exists for, against a timeout: 10. The hook is the whole fix for blocking
finding #3, and in the scenario it was written for it is killed before it prints anything.

2 — settings.py remove without --url deletes any base URL, including a stranger's. Measured:
a corp gateway with no context-guru record was removed with result=removed, exit 0. The safety
property currently lives only in the uninstall skill's prompt — a model remembering a flag — while
this script is the layer that is supposed to be deterministic and to "refuse to guess."

Four medium items and a tail of small ones follow. Nothing here needs a real install to close
except the go install PATH item, which I could not exercise: there is no Go toolchain on this
machine, so I could not run go test ./.... CI's build-test was still pending when I checked;
make cover does run ./... so the new package is genuinely gated (the piped go test | sed step
at ci.yaml:34 is only the coverage log, not the gate).

[amiddavid]

Blocking — this timeout is shorter than the work the hook does, on the path the hook exists for.

Measured, with routing configured, no proxy listening, and a binary present that never binds:

$ time check-proxy.sh   # via CLAUDE_PLUGIN_ROOT, routed port, CONTEXT_GURU_BIN=a sleep script
ELAPSED=18s   (UserPromptSubmit timeout is 10s)

The 18s is structural, not a fluke: check-proxy.sh probes /healthz, then invokes
start-proxy.sh, which probes again and then runs its own 15s health-wait loop. The
diagnostic cat <<EOF block is the last thing in the script, so at 10s the hook is killed and
the user sees nothing at all — the identical symptom (a prompt that produces no output) that
this hook was added to replace with an explanation.

The comment at start-proxy.sh:99 says "the hook's own timeout (60s in hooks.json) is the real
backstop." That is true for SessionStart and false here; this path has 10s.

Two fixes, and I think you want both:

1. Print the note before attempting recovery, not after. The note is the deliverable; the
   restart is opportunistic. Emitting it first means a kill at any point still leaves the user
   informed.
2. Bound the wait when invoked from this hook — e.g. have start-proxy.sh honour a
   CONTEXT_GURU_HEALTH_BUDGET (seconds) that check-proxy.sh sets to ~5, and raise this
   timeout to 30 so a slow-but-successful recovery still gets to report success.

A test here would have caught it; see my comment on check-proxy.sh.

[amiddavid]

Blocking — remove with no --url deletes a base URL that is not ours, and reports success.

args.url defaults to "" (line 286), and this guard is if args.url and .... With --url
omitted the entire conflict check is skipped and the key is deleted unconditionally. Measured
against a file with someone else's gateway and no context-guru record:

$ settings.py remove --file settings.json      # note: no --url
result=removed
was=https://gateway.corp.example.com
restored=
exit=0

The gateway is gone, restored= is empty because there was no record to restore from, and the
exit code says success. The module docstring advertises exactly this invocation as supported —
settings.py remove --file PATH [--url URL] — and states two lines later that a base URL that is
not ours "is a CONFLICT and exits non-zero."

What makes this blocking rather than cosmetic is where the safety currently lives. uninstall/SKILL.md
does pass --url, and its prose says "Passing --url is what keeps this safe" — so the property
that protects the user's gateway is a model remembering a flag in a prompt. This script is the
deterministic half precisely so that it is not. It is also the failure mode with the worst blast
radius you named: it edits ~/.claude/settings.json.

Fix, either way round:

• make --url required for remove (mirroring the add check in main()), or
• treat a missing --url as "remove only if is_ours(data, current)" — i.e. fail toward leaving
  the user's configuration alone, which is what is_ours' own docstring says it is for.

Worth a test: remove with no --url over a foreign URL must exit 2 and leave the file byte-identical.

[amiddavid]

Medium — worst-case loop duration is ~138s, against a 60s SessionStart timeout.

The comment says "Up to ~15s", which holds only when the port is refused — then curl returns
instantly and 60 × 0.25s ≈ 15s. If something is listening but not answering (a hung proxy, a
half-open socket, a port taken by an unrelated service that accepts and stalls), each curl burns
its full --max-time 2. Measured against a listener that accepts and never replies:

one curl took 2054ms  ->  60 iterations ≈ 138s   (SessionStart timeout is 60s)

Consequence: the hook is killed at 60s, so the block at lines 109-116 never runs — the user gets
no log path, no /context-guru:status pointer, no tail of the log. And a hung port is one of the
likelier ways to reach that block, since a cleanly-absent proxy exits the loop fast either way.

Budget on wall clock rather than iteration count, and drop the per-probe timeout:

deadline=$(( $(date +%s) + ${CONTEXT_GURU_HEALTH_BUDGET:-15} ))
while [ "$(date +%s)" -lt "$deadline" ]; do
  curl -fsS --max-time 1 "$HEALTH" >/dev/null 2>&1 && { ...; exit 0; }
  sleep 0.25
done

That also gives you the knob check-proxy.sh needs for the 10s-timeout finding on hooks.json.

[amiddavid]

Medium — the go install fallback skips both the mkdir and the on_path report that the
tarball path does.

try_source_build returns 0 and line 126 exits 0 immediately, so everything from line 178 down is
bypassed on this path:

• mkdir -p "$DEST" (line 178) never runs, yet GOBIN="$DEST" is set here. I could not test
  whether go install creates a missing GOBIN — there is no Go toolchain on this machine — so
  this needs confirming rather than taking from me; if it does not, the fallback fails on a clean
  box where ~/.local/bin does not exist yet, which is the common case for the audience this
  whole script targets.
• on_path is never emitted (lines 193-197). So a user who lands on the fallback gets no
  warning when $DEST is not on PATH — and ~/.local/bin frequently is not. They then hit
  start-proxy.sh:56: "routing is configured for port N but the proxy binary is not on PATH",
  from a hook, in a later session, with nothing connecting it back to the install. install/SKILL.md
  reads on_path and tells the user about it, so the skill is silent here too.

Both are fixed by making the fallback fall through to the shared tail instead of exiting: have
try_source_build set the path and break/skip to line 188, or hoist mkdir -p "$DEST" above
line 121 and factor the on_path case into a function both paths call.

Minor, same function: emit "fallback=go_install" is printed before the attempt, so it appears
even when the build fails. install/SKILL.md documents reading it together with the result, which
is correct — just noting the line is "attempted", not "used".

[amiddavid]

Medium — this manual-start command reintroduces the dead dashboard link the PR lists as fixed.

It omits --dashboard and --dashboard-db, which start-proxy.sh:89-90 passes deliberately.
--dashboard defaults to false (cmd/context-guru-proxy/main.go:105), so a proxy started this
way does not serve /dashboard/ — and step 6, ~25 lines below, tells the user

Dashboard: http://127.0.0.1:<port>/dashboard/ — the four billed token tiers are where the cache
effect is visible.

which is a 404. That is the same defect as the "dead dashboard link — it advertised /dashboard/
and never passed --dashboard" item in the PR description.

It also does not self-heal, and that is the part worth fixing rather than documenting:
start-proxy.sh is idempotent on /healthz (line 51), so once this hand-started proxy is up the
hook will never replace it — and with --idle-exit 24h it holds the port for a day. The user's
dashboard is broken for that whole window with no signal as to why.

check-proxy.sh:48 has the same omission in the command it prints to the user.

Simplest fix: make both places invoke start-proxy.sh rather than restating the command line —
the script already self-gates on ANTHROPIC_BASE_URL, which step 5 correctly explains is unset in
the installing session, so it would need an override (CONTEXT_GURU_FORCE=1, or just documenting
ANTHROPIC_BASE_URL=http://127.0.0.1:$PORT/anthropic start-proxy.sh). Failing that, add the two
dashboard flags in both places.

Credit where due: step 5's explanation of why the hook does nothing in the installing session is
exactly right, and it is the kind of thing that usually gets left out.

[amiddavid]

Low — glob.glob treats [, ? and * in the path as pattern syntax, so pruning silently does
nothing for a settings file under a directory containing them (~/projects/foo[1]/.claude/...).
Backups then accumulate forever, which is the thing KEEP_BACKUPS exists to stop, and the failure
is invisible because prune_backups is best-effort by design.

glob.glob(glob.escape(path) + ".context-guru-backup-*") fixes it.

[amiddavid]

Low, process rather than code: requireTool uses t.Skipf, so on a runner without python3 or
bash every test in this file skips and go test is green. ubuntu-latest has both today, so
this is latent — but the whole point of the package header is that these scripts get the coverage
their blast radius warrants, and a skip is indistinguishable from a pass in CI output.

For python3 and bash specifically I'd t.Fatalf instead: they are guaranteed on the CI image,
so an absence means the image changed and you want to hear about it. Keep t.Skipf for anything
genuinely optional.

[amiddavid]

Low, and unverified — please confirm before acting on it, I could not. On Linux, opening a
currently-executing binary with O_TRUNC fails with ETXTBSY, and coreutils install truncates
rather than replacing the inode. If that holds here, the upgrade path (CONTEXT_GURU_UPGRADE=1)
fails with install_failed_to_$DEST in exactly the situation upgrades happen in — a proxy that is
running, which with --idle-exit 24h is most of the time. macOS permits the write, so this would
not reproduce on a dev box.

I could not test it: no Go toolchain and no Linux here.

If it does hold, install -m 755 "$found" "$DEST/$BIN.new" && mv -f "$DEST/$BIN.new" "$DEST/$BIN"
avoids it — rename swaps the directory entry and leaves the running image alone — and is also
atomic for anyone starting the proxy concurrently.

[amiddavid]

Low — the comment above claims the starter "detaches the proxy from this hook's process group so
it survives the hook returning and is not killed with the session's process tree." setsid does
that; the macOS fallback nohup does not — it only makes the child ignore SIGHUP, leaving it in
the caller's process group. So on macOS, which is presumably where most of this was developed, a
signal delivered to the session's process group still reaches the proxy.

disown at line 93 does not help either — that is bash job-table bookkeeping, not a process-group
change.

In practice nohup + disown usually survives, so I would not change the mechanism on my say-so.
But the comment states a property the fallback does not provide, and the next person to read it
will trust it. Either say "setsid where available; nohup only blocks SIGHUP", or get the real
property portably with (trap '' HUP; exec "$BIN" ... &) in a subshell.

[amiddavid]

Nit — "two commands" sits directly above a three-command block, and docs/how-to/install-plugin.md
spells out why it is three ("The first two are once per machine. The third is once per repo").
marketplace.json compresses it further to "in one command."

Since the split is the genuinely reassuring part of the design — the machine-wide step and the
per-repo routing decision are separate on purpose, and the routing one is the one with blast
radius — I would say so rather than round the count: "two commands to install, one per repo to
route."

[amiddavid]

All thirteen. Two of them were blocking, both on the two surfaces I asked you to go deep on — and the fact that I named those surfaces as the highest-risk ones and had still not tested them is the honest summary of this round.

Pushed as 6577320. Every claim below is measured, before and after.

The 19s hook with a 10s timeout

Reproduced exactly, and it is worse than a slow hook: check-proxy.sh exists only for the case where a prompt produces nothing at all, and its explanation is the last thing it prints. So on precisely that path it was killed first and the user got the original silent hang — with a hook that looked like it had handled it.

Your diagnosis of the shared cause was right, and I confirmed the mechanism separately: the seq 1 60 + --max-time 2 loop is only ~15s when the port is refused. Against a socket that accepts and never answers I measure 2046ms per probe → ~122s against SessionStart's 60s, so the failure report at the bottom of start-proxy.sh never ran either. Your "a hung port is one of the likelier ways to reach that block" is the part that makes it a real defect rather than a slow path.

Wall-clock budget via CONTEXT_GURU_HEALTH_BUDGET (default 15s), check-proxy.sh asks for 5s, hooks.json raised to 30s. Measured after: 5s dead-proxy, 11.1s worst-shape, and the hung-port start path is 17s with the failure report printing.

I took one half of your fix and not the other. The note stays last. Printing first guarantees it survives a kill, but the common case here is a silent successful recovery after an idle-exit between two prompts, and a paragraph about a dead proxy on every one of those is noise on a working path — it also lands in the model's context for that turn. The budget is what makes the ordering safe. If you still think the guarantee beats the noise, say so and I will flip it; it is a judgement call, not a disagreement about the facts.

remove with no --url

Reproduced: result=removed, restored= empty, exit 0, corporate gateway gone.

Took your second option (missing --url means "remove only if is_ours") rather than making the flag required, because it fails toward leaving the user's configuration alone even when a caller forgets. Your framing is the part I want to keep on the record — the property lived in a prompt, in a file whose prose asserted it. That prose is corrected too, since it was actively misleading about where the safety came from.

The test asserts the file comes back byte-identical, not merely that a base URL is still there: a refusal that rewrites the file has already done the thing it refused.

Your two unverified items — both are false, and I tested them

You were right to flag rather than assert these. Go lives on a Linux box here, so I ran both:

• go install DOES create a missing GOBIN (Linux, Go 1.26.4). No mkdir needed on the fallback path.
• install -m 755 over a running binary SUCCEEDS on Linux — no ETXTBSY. coreutils install unlinks the destination first, which is why.

I still install-then-rename, but for the reason your false premise was sitting next to: that unlink is a window in which $DEST/$BIN does not exist, so a SessionStart hook firing in another project mid-upgrade reports "not on PATH". rename(2) closes it.

The on_path half of that finding was entirely real and is fixed — both paths call a shared report_path now.

One suggestion declined on its mechanics

O_EXCL on the fixed name .context-guru-tmp closes the mode window, but a leftover temp from a crash or a full disk then makes every later save fail until somebody deletes it by hand — trading a mode window for a permanent lockout of the file this script exists to edit. mkstemp gets the 0600-at-creation property with a unique name and no lockout.

The test you said was missing

Six properties for check-proxy.sh, and two details from your note that I would not have chosen unprompted:

• the timeout is read out of hooks.json, not retyped, so the pair cannot drift again — the defect was a mismatch between the two files;
• the timing assertion measures the accept-and-stall shape, not the refused-port one. My first version used a free port, measured 5s, and would have passed at almost any timeout — sensitive to nothing. On the worst shape it is 11.1s against a 15s bound, and a drop back to 10s fails clearly.

requireTool now fails rather than skips for python3/bash, per your last point.

Nine mutations, each proven to have landed before its result counted, each failing its named test, each file restored byte-identical, suite green after the sweep. Full table in the commit body; it includes reverting hooks.json to 10s and dropping the dashboard flags from the printed command.

One gap, stated rather than papered over

The 0600 temp-file window is fixed but not tested. It is a window between two syscalls, and a test asserting on it would be asserting on a race it cannot observe — you noted TestSettingsPreservesFileMode passes either way, and that is still true. Verified by reading.

Also worth recording: my first attempt at the timeout test failed for a reason that had nothing to do with the script — my assertion string spanned a hard line break in the heredoc. Matching on collapsed whitespace now, since that prose will be rewrapped again.

On the merge order

Unchanged and still the thing gating usefulness rather than correctness: release.yaml via workflow_dispatch, then v0.1.0, then this. Until a tag publishes assets, /context-guru:install ends at no_release_found — so the end-to-end path these fixes are on remains unexercised by anything except tests.

Ready for re-review.

[amiddavid]

Re-review of 6577320 — all thirteen verified fixed; two new findings, one of them mine to own

I re-ran the measurements rather than reading the diff. Everything I filed is genuinely fixed, and
the three places you did not do what I suggested are all better than what I suggested. Detail below,
then two new items — the first of which I should have caught in round one and did not.

The two blocking findings, measured before and after

Path	Before	After	Bound
check-proxy.sh, dead proxy	18s	5s	30s timeout
check-proxy.sh, accept-and-stall	—	5s	30s timeout
start-proxy.sh, accept-and-stall	~138s	16s	60s timeout

The diagnostic is delivered in every case now, including the stall shape. I measure the stall
shape at 5s rather than your 11.1s — probably harness differences, and in the safe direction either
way; the assertion has margin regardless.

settings.py remove verified on three shapes:

• no --url, foreign gateway, no record → result=conflict, exit 2, file byte-identical ✓
• no --url, our own recorded URL → still result=removed ✓ (the fix did not overshoot into
  making remove useless without the flag)
• --url mismatching a foreign URL → still result=conflict, exit 2 ✓

Your three pushbacks: all accepted, and one correction to my own record

1. My two unverified items were wrong, and you tested what I could not. go install creating a
missing GOBIN, and install not hitting ETXTBSY because coreutils unlinks first — I flagged
both as needing confirmation precisely because I had no Linux box and no toolchain, and you went and
got the answer. Thank you for testing rather than deferring. The install-then-mv you kept is
better justified than my version of it: the unlink window is real, rename(2) closes it, and the
comment in the script now records the corrected reasoning instead of my wrong premise, which is the
right thing to leave behind for the next reader.

2. Diagnostic staying last: you are right, and TestCheckHookRecoversSilently is what settles
it. My reasoning was "the kill-survival guarantee is unconditional, ordering makes it free." Yours
is that the guarantee is only worth anything on the failure path, while the common path after an
idle-exit is a silent successful recovery — and a dead-proxy paragraph on every one of those lands
in the model's context on a turn where nothing is wrong. That cost is certain and recurring; mine
was hypothetical once the budget exists. Budget first, ordering second is the correct dependency,
and the test now pins the silence so nobody "helpfully" moves the note up later. No flip needed.

3. mkstemp over O_EXCL: correct, and I missed the failure mode. A leftover
.context-guru-tmp from a crash or a full disk would have made every subsequent save fail
permanently — turning a brief mode window into a permanent lockout of the one file this script
exists to edit, recoverable only by hand. Trading a transient defect for a persistent one is
strictly worse. The except BaseException unlink is a good addition I had not asked for.

I also accept the untested-window gap as stated: a race between two syscalls is not honestly
assertable, and "verified by reading" is the right label rather than inventing a test that would
pass either way.

On the test I asked you to check

TestCheckHookFinishesInsideItsOwnTimeout is right, and the change you describe is the one that
matters: measuring the accept-and-stall shape instead of a free port is what makes it sensitive to
anything. Reading the bound from hooks.json via hookTimeout and asserting against limit/2
rather than limit is better than what I suggested — the half-margin means it fails on a real
regression instead of only on a loaded runner, and it cannot drift from the config. Asserting
--dashboard appears in the printed recovery command is a nice touch: it pins finding 5 in the same
place, so the 404 cannot come back through the text.

TestCheckHookIsSilentWhereRoutingIsNotConfigured covering 127.0.0.1:87871 as a prefix of 8787
pins the gate fix at the level it actually broke. That is the table-driven version I would have
written.

Nothing further on the other eleven — report_path shared by both install paths, glob.escape,
t.Fatalf for python3/bash, the nohup comment, and the "two commands to install, one per repo
to route" wording all check out.

Merge order

Agreed and unchanged: release.yaml via workflow_dispatch → v0.1.0 → this. I am not treating
"has never run end to end" as a finding, for the reason you gave — it is gated on a tag existing,
not on anything in this diff. Worth saying plainly in the PR when it merges, so the first person to
run /context-guru:install against a real release knows they are the first.

Two new comments below. The first is a genuine miss on my part from round one — same defect class
as #141's finding 1, which you noted is not hypothetical here. It is unchanged by this commit, so it
is not a regression; I simply did not see it. The second is a small test-hygiene item on new code.

[amiddavid]

Medium, and my miss from round one — the ownership check is printed after the kill it is
supposed to gate.

This is not a regression from 6577320; it has been here since 0abd995 and I did not catch it. Same
defect class as #141's finding 1, which is why I want it fixed rather than left as prose ordering.

Read the skill top to bottom, which is how it gets executed:

• lines 74-78 — if [ -n "$pid" ]; then kill "$pid" && rm -f "$PIDFILE"
• line 81 — "Before killing anything, confirm the PID is ours — the port may be held by
  something else entirely, and this step must not kill a stranger's process"
• line 85 — the ps -p "$pid" -o command= | grep -q context-guru-proxy check

The instruction says "before killing anything" and sits after the block that kills. An agent working
through this file in order has already sent the signal by the time it reads the guard, and the check
then reports on a process that is already dying. The safety property is stated but structurally
unreachable.

It matters most on the path that produces an untrustworthy PID in the first place. The pidfile is
ours by construction, but the fallback at lines 66-72 is lsof/ss on "whoever holds the port" —
which is explicitly there to cover a stale pidfile and a hand-started proxy, i.e. exactly
the cases where the PID may belong to something else. A recycled PID from a stale pidfile passes
kill -0 too.

The fix is to make it one unskippable block rather than two blocks and a warning, so the ordering
cannot be got wrong by reading:

if [ -z "$pid" ]; then
  echo "(nothing listening on ${PORT})"
elif ps -p "$pid" -o command= | grep -q context-guru-proxy; then
  kill "$pid" && rm -f "$PIDFILE"
else
  # The port is held by something that is not ours — a hand-started service, or a recycled PID
  # from a stale pidfile. Report it and stop; do not kill it.
  echo "NOT OURS — pid $pid holds ${PORT} and is not context-guru-proxy; left alone"
fi

Then keep the liveness re-check at lines 91-93 as it is, since "did the kill work" is a separate
question from "is it ours".

One consequence worth stating in the skill while you are there: if the check says NOT OURS, step 1
has already removed the routing, so the user is safe and unblocked — the leftover is a port
conflict to report, not something to escalate on. That is the reassurance that stops the next reader
reaching for kill -9.

[amiddavid]

Low, new-code hygiene: held is appended here on the listener goroutine and iterated by the
t.Cleanup closure at line 812, which runs on the test goroutine. That is an unsynchronised
read/write of the same slice, and make cover runs go test -race — so if a connection is being
accepted while cleanup runs, this is a race-detector failure rather than a test failure, which is a
confusing thing to land on.

Registering t.Cleanup from inside the goroutine is also a hazard in its own right: t.Cleanup
called after the test has finished panics, and nothing here orders the goroutine's registration
before the test's end.

Cleanup order makes it narrow rather than safe — the held cleanup is registered second so it runs
first (LIFO), while the goroutine is still blocked in Accept and can still append. In practice
curl is done by then, so I would not expect this to fire often; "not often" is the bad frequency
for a race in CI.

Simplest fix is to drop held entirely — closing the listener is what ends the goroutine, and the
accepted connections die with the process at test end:

t.Cleanup(func() { ln.Close() })
go func() {
    for {
        c, err := ln.Accept()
        if err != nil {
            return
        }
        defer c.Close() // held open for the life of the goroutine, answers nothing
    }
}()

If you want the connections explicitly closed, register the cleanup on the test goroutine before
starting the listener goroutine and guard the slice with a mutex. Either way the property the helper
exists for — accept, never answer — is unaffected.

Worth saying: stallingPort is the right helper to have added. The shape it produces is what turned
both timeout tests from assertions that would pass at any value into ones that catch the defect.

[amiddavid]

Correcting my own severity: this is not "low", it is the build-test failure on 6577320. It
fired on the first CI run, not rarely.

--- FAIL: TestCheckHookFinishesInsideItsOwnTimeout (11.14s)
FAIL  github.com/rossoctl/context-guru/context-guru-plugin  20.047s

Three WARNING: DATA RACE reports, all the same shape — Read at ... by goroutine 65 inside
testing.go:1317 (that is tRunner running the registered cleanups) against Previous write at ... by goroutine 66, the accept loop. The frames are plugin_test.go:812, :813, :817 and :821:
the held iteration and c.Close() in the cleanup, versus the append and Accept in the
goroutine. Run: https://github.com/rossoctl/context-guru/actions/runs/33815232946/job/100845795572

I said "not often is the bad frequency for a race in CI" and then underrated it anyway — I reasoned
that curl would be finished by cleanup time, but this test holds three probes against the
stalling port, and ln.Close() runs after the held cleanup (LIFO), so the accept loop is still
live and writing while the cleanup reads. Sorry for filing it as a nit; the fix in my comment above
is unchanged and is what unblocks CI.

Two things worth taking from the failure beyond the fix.

Your 11.1s was right and my 5s was the outlier — the run measures 11.14s, so that was Linux/CI
versus my macOS box, and your figure is the one to design the margin against. That also means the
timing assertion passed: 11.14s against a limit/2 bound of 15s. The defect I filed on
hooks.json is fixed and the test proves it on the platform that matters; the only thing red here
is the helper's bookkeeping. Good news for the substance, and an argument for the half-margin you
chose over my limit — at 30s the bound is 15s and 11.14s fits, but it is not roomy, so I would
resist any future trimming of that timeout.

Second: this is test-only code. No production path touches held, so nothing about the plugin's
behaviour is implicated — worth stating in the commit message that fixes it, so the next reader of
the history does not go hunting for a proxy defect behind a red build-test.

Everything else in the run is green (DCO, build, purego, trivy), and this is the only failing
test in the only failing package.

[amiddavid]

Both done, pushed as 82b8f6f. go test -race ./... and gofmt -l clean.

The race — yours to flag, mine to have caused

You were right to lead with it, and right that it is test-only: nothing in the proxy touches stallingPort. The commit says so explicitly, so a red build-test on 6577320 does not get read later as a proxy defect.

I had it fixed from the same CI log before your message arrived, and the diagnosis matches: t.Cleanup registered from inside the listener goroutine, so the cleanup closure read held on the test goroutine while the accept loop appended on another. Fixed by ownership rather than a mutex — the goroutine that appends is the only thing that reads, and it closes the connections in its own deferred func; the test goroutine only closes the listener, which is what makes Accept return.

Why I did not catch it is the part worth recording. I ran that suite repeatedly on a box without -race, where this is invisible — a real race with no symptom until the scheduler interleaves badly. CI runs -race through make cover, so CI was the first thing that could see it. I had also just finished reporting nine green mutations, which verified that my tests catch defects in the code under test and said nothing whatsoever about whether the test code was correct. New concurrent test code gets -race locally now.

On underrating it as "low hygiene": I would not spend much on that. You filed the right observation with a wrong severity, and the severity was recoverable in one CI run. Filing it at all is what made the fix a five-minute job.

uninstall killing before the check

Real, and the diagnosis is exact — the guard was decoration, because these files execute the way they read. Your point that it bites hardest on the lsof/ss fallback is the part that makes it a defect rather than untidiness: that fallback exists for a stale pidfile or a hand-started proxy, i.e. precisely when the PID may not be ours, and a recycled PID satisfies kill -0 perfectly well.

Collapsed into one if/elif/else as you suggested, with one addition you did not ask for: when the PID is not ours the pidfile survives. Removing it would strand a proxy of ours that is still running under a different pid — and "not ours" plus "delete our handle to it" is the worse of the two failure modes.

Given an executing test rather than better prose, because this class has now recurred twice in this PR's lineage and reading is what missed it both times. TestUninstallDoesNotSignalAProcessThatIsNotOurs extracts the fenced block straight out of the skill and runs it against a stubbed ps: a stranger's process must not be signalled and its pidfile must survive; ours must be signalled. Mutation-checked — with the two-block ordering restored, the block sends SIGTERM to /usr/bin/postgres.

That test has a process note of its own, and it is the same trap as the race: my first version stubbed kill as a file on PATH. kill is a bash builtin, so the stub was never consulted — the block's kill -0 probe failed for a pid that does not exist, it fell through to the socket-owner lookup, and the run exercised none of the branch under test. It failed loudly only because one of the two cases asserts a positive outcome (kill WAS invoked); the negative case alone would have passed for exactly the wrong reason. A shell function overrides the builtin.

Your measurements

Taken, and both recorded in the commit body rather than just agreed to here:

• 11.14s on Linux/CI is the number to design against, not my macOS reading or your 5s. Your table and the CI run agree.
• That leaves 11.14s against a limit/2 bound of 15s — real margin, not roomy. Noted in the commit that the 30s UserPromptSubmit timeout should not be trimmed later without re-measuring on Linux, since the next person to see "30s for a hook?" will be tempted.

Thanks for verifying the two blocking fixes by measurement rather than by reading the diff — the three remove shapes in particular, including that the fix did not overshoot into refusing our own recorded URL. That is the failure mode I would have shipped if I had only tested the refusal.

Merge order unchanged: release.yaml via workflow_dispatch, then v0.1.0, then this.

[amiddavid]

Re-review of 82b8f6f — both items fixed and independently verified; no blocking findings left

The uninstall ordering fix, verified by re-running it rather than reading it

I extracted the fenced block from uninstall/SKILL.md myself and ran it against a stubbed ps, in
both shapes and then with your old two-block ordering restored:

Block	ps reports	signalled	pidfile survives	said NOT OURS
82b8f6f	/usr/bin/postgres -D /var/lib/postgres	no	yes	yes
82b8f6f	context-guru-proxy --listen 127.0.0.1:8787	yes	no (removed, correctly)	no
mutation: old two-block order	/usr/bin/postgres …	yes	no	yes

So the mutation does signal postgres, and the fix does not. Your report matches exactly, and the
test is sensitive to the defect rather than to its surroundings.

The pidfile surviving on NOT OURS is a better call than what I proposed, and I had not thought it
through. I only said "do not remove it"; you gave the reason that makes it load-bearing — a proxy of
ours may still be running under a different pid, and "refuse to touch this process" plus "throw away
our own handle" combine into a proxy nobody can stop by the documented route. Deleting the handle is
the more expensive half of that pair.

skillBlock is the right shape for testing prose that gets executed, and it fails on zero or more
than one match rather than silently taking the first — which is the failure mode that would otherwise
make it test the wrong block after an edit. I confirmed exactly one of the three bash blocks in that
skill contains the needle.

The race, fixed by ownership

Correct, and better than the mutex I offered as the alternative: held is now touched only by the
listener goroutine and closed by its own defer, with t.Cleanup(ln.Close) registered on the test
goroutine as the thing that unblocks Accept. No shared state, so nothing to synchronise — and
t.Cleanup is no longer called from a non-test goroutine, which was the second hazard in the same
five lines. Noting in the commit that no production path touches stallingPort is the right thing to
leave in the history.

Agreed on the process point, and it is the more transferable lesson: a non-race run cannot see this
class at all, so -race locally before pushing is the cheap version of what CI told you.

The trap you flagged — I went looking, and there are exactly two instances left

Thank you for naming the shape; it made this a mechanical search rather than a guess. Your
kill-as-a-file version is a textbook vacuous pass, and the reason it surfaced is exactly what you
said: the positive row asserted a positive outcome, so the stub had to actually be consulted for
the test to go green. That is the property worth generalising.

By that standard the stub-based tests come out clean. TestInstallRefusesAnUnverifiedDownload is
refusal-only on its face, but it asserts checksum_unavailable — a string only reachable if the curl
stub successfully served the tarball first, since real curl would have produced download_failed
instead. So the stub is proven consulted by the assertion itself.
TestInstallReportsPATHFromTheSourceFallbackToo asserts built_from=source and covers both
on_path values, so the go stub is likewise pinned.

Two tests remain in the shape you told me to distrust, and I filed the detail on one of them below.
Neither is wrong today — both are saved by a positive control that lives in a different test — and
that is the fragility worth one line each.

CI

DCO, build, purego, trivy green on 82b8f6f; build-test was still running when I posted, and
I am watching it. Merge order unchanged and still the real gate: release.yaml via
workflow_dispatch → v0.1.0 → this.

Nothing here blocks. From my side this is ready once build-test is green.

[amiddavid]

Following up on the trap you flagged — this is one of the two tests still in that shape, and the
only reason I am filing it is that you asked me to look for it.

Every assertion here is an absence: exit 0, and empty output. Nothing in the test requires the
script to have done anything, so it cannot distinguish "the gate correctly declined" from "the
script exited before reaching the gate." Demonstrated by running its four rows against a
check-proxy.sh gutted to a bare exit 0:

all four rows pass: yes    (exit 0 + silence are the only assertions)

TestHookIsSilentAndInertWhereRoutingIsNotConfigured at line 291 is the same shape — the sentinel's
absence plus silence, with no in-test evidence the sentinel mechanism works at all.

Both are saved today, but from a distance: the positive control for this one is
TestCheckHookFinishesInsideItsOwnTimeout (routed → it speaks), and for line 291 it is
TestHookStartsTheProxyAndWaitsForHealthz (routed → the sentinel appears). That is real coverage, so
neither test is wrong. The fragility is that the control is somewhere else: a future change that
skips, renames or narrows the positive test takes the meaning out of the silence test without
touching it or making it fail, and a silence test that has quietly stopped proving anything is the
exact failure you just spent a round on.

Cheapest fix is to make the control local — one more row in each table asserting the routed case
does act:

{"routed to our port", "http://127.0.0.1:8787/anthropic", wantSilent: false},

with the assertion inverted for that row (non-empty output here; sentinel present at line 291). Then
the table itself proves the mechanism is live, and the silence rows mean something on their own.

Low priority and entirely your call — the coverage exists either way. It is the "would this still
fail if the thing under test were removed?" question, which is the same question your kill stub
answered the hard way.

[amiddavid]

build-test is red on 82b8f6f, and it is not this PR — it is a pre-existing main defect

The plugin package passed:

ok  github.com/rossoctl/context-guru/context-guru-plugin  21.137s

So the stallingPort race fix is confirmed green in CI, which was the open question from my last
review. The sole failure is elsewhere:

--- FAIL: TestArriveCancelsAndReports (0.03s)
    keepalive_test.go:413: refreshed = 0, want the ping's own cache_read of 48576
FAIL  github.com/rossoctl/context-guru/proxy  220.284s

This PR touches no file under proxy/ — git diff --name-only origin/main...HEAD matches nothing
for proxy/ or keepalive. And main is green on its last eight CI runs, so this does not
reproduce on main on demand.

It is a real synchronisation defect in the test, not an unexplained flake

I read the path rather than filing it as "timing-sensitive and probably fine". The test waits on one
signal and then asserts on state written by a different, later one:

• proxy/keepalive.go:899 — k.pings.Add(1), immediately after k.send(...) returns. This is the
  counter waitPings polls (keepalive_test.go: for ... if k.pings.Load() >= n { return }).
• proxy/keepalive.go:989 — e.refreshed = u.CacheRead, under k.mu, roughly ninety lines
  later in the same flow: after the error check, the dash.Event construction, model/provider/
  route resolution and ev.Price(...).

TestArriveCancelsAndReports calls waitPings(t, k, 1) and then immediately k.arrive(...), which
reads e.refreshed (line 548). So the wait returns as soon as the ping is sent and counted, while
the value being asserted is not written until the response has been priced. The window is
deterministic and always present; it only becomes visible when the writer goroutine is descheduled
inside it. refreshed = 0 is exactly the pre-write value.

That makes the assertion sound and the wait wrong: k.pings fires strictly before the state under
test exists.

Why it likely surfaced here rather than on main

Not a code dependency — a contention one. make cover runs go test -race ./..., which schedules
packages concurrently on a 2-core runner, and this PR adds a package that is deliberately
wall-clock-heavy: 21.137s, most of it the two budget tests holding a stalling socket for ~11s
each while curl waits out its timeouts. That is real added pressure on the same runner, and
widening an existing race's window is exactly what it would do. ci.yaml:81-89 already records this
class — a timing-sensitive test that "flaked twice on unrelated PRs, then passed on re-run and passes
3/3 locally".

So: contributing factor here, root cause on main.

Recommendation

Do not absorb this into #160. Per the convention for a main defect found on a feature branch,
it wants its own worktree, its own branch off main, and its own PR — otherwise a plugin PR carries
an unrelated proxy change and the history stops explaining itself.

The fix belongs on the test's wait, not on the production ordering (pings counting a sent ping
is correct, and refreshed genuinely cannot be known before the response is priced). A wait on the
state actually asserted, e.g. polling e.refreshed under k.mu — or a signal published after line
990 — closes it deterministically for every test in this file that follows waitPings with an
assertion on post-response state. Worth checking whether TestArriveCancelsAndReports is the only
one in that shape; a grep for other waitPings callers is the cheap version of that question.

A re-run will very likely go green and would let #160 merge on its own merits. I would still open the
main issue rather than let a re-run close the subject, since a re-run does not make the window
smaller — and #160 measurably makes it easier to hit from now on, so this will recur on unrelated PRs.

Nothing here changes my assessment of #160 itself: no blocking findings, ready once build-test is
green.

[amiddavid]

Addendum — I ran the grep I suggested, and the blast radius is exactly one test.

There are seven waitPings call sites in proxy/keepalive_test.go (179, 260, 278, 406, 441, 814,
942). Only two are followed by an arrive(), and only one of those asserts state from the late
post-response block:

• 406 — TestArriveCancelsAndReports: asserts refreshed, written at keepalive.go:989, after
  the response is priced. This is the racy one.
• 441: asserts strategyID, i.e. e.appliedStrategy — written at keepalive.go:661 when the
  entry is created, well before the sweep fires. Not affected.

The remaining five assert nothing from that block within their following lines.

So the fix is genuinely one test's wait, not a pattern to unpick across the file — which makes the
separate main PR small. Two shapes would both work: poll e.refreshed under k.mu instead of
k.pings, or publish a signal after keepalive.go:990 and wait on that. The second is preferable if
e.spent (written on the adjacent line, 988) ever grows an assertion in this position, since it
closes the window for the whole block rather than for one field.

Worth noting for whoever picks it up: -race will not catch this one. Both accesses are properly
mutex-guarded — the bug is ordering, not unsynchronised memory, so the only signal is the flake
itself. That is the opposite of the stallingPort race in this PR, which -race caught immediately
and a plain run could not see. Two adjacent failure modes with inverted detection stories, which is
worth a line in whatever fixes this.

[amiddavid]

Review complete — all findings resolved, CI green at eb9eac9

DCO · build · build-test (10m54s) · purego · trivy — all pass. The plugin package ran and
passed on its own line (ok github.com/rossoctl/context-guru/context-guru-plugin 22.142s), so the
suite is genuinely exercised rather than skipped.

Sixteen findings over four rounds, all closed: two blocking (the UserPromptSubmit hook unable to
finish inside its own 10s timeout; settings.py remove deleting a base URL that was not ours when
--url was omitted), four medium, the uninstall kill-before-ownership-check ordering, the
stallingPort data race, and the two silence tests whose positive control lived in another file. I
re-measured or re-ran each fix rather than reading the diff; the numbers are in the individual
reviews.

The build-test failure on 82b8f6f was triaged to #206 — a pre-existing main defect, not this PR —
and the re-run on the same commit going green is consistent with that: same code, red then green,
nothing changed in between. Worth keeping #206 open regardless, since a re-run does not shrink the
window and this PR's wall-clock-heavy test package makes it easier to hit from now on.

One correction on the merge state

The PR reports mergeable=MERGEABLE but mergeStateStatus=BLOCKED, and the block is not the
checks — it is reviewDecision=REVIEW_REQUIRED. Ruleset "main branch" (19779069) requires 1
approving review.

That gate cannot be cleared by an approval from this account: the reviewing credential and the PR
author are the same user, and GitHub refuses a self-approval. So there are two real routes, and it is
worth knowing which one is intended before the release sequence starts:

• an approving review from another account with access — OsherElhadad is already an always-bypass
  actor on that ruleset, so is the natural reviewer; or
• an admin bypass at merge time, which the author's own admin: true permission plus the
  RepositoryRole id=5 … mode=always bypass entry does allow.

I am not taking either action — the first is not available to me, and the second is a merge decision,
not a review one.

Merge order

Unchanged, and still the thing that makes this work rather than merely be correct: release.yaml via
workflow_dispatch → v0.1.0 → merge this. Until a tag publishes assets, /context-guru:install
ends at no_release_found, so the end-to-end path stays unexercised by anything but tests. That
remains the single largest untested surface here, and the PR description says so plainly, which is
the right place for it.

No blocking findings outstanding from this review.

[amiddavid]

Review of the install rework — 8 commits, eb9eac9..4cb2817

Reviewed at 4cb2817. This is a substantially different install flow, so I treated it as a new pass
rather than a re-check, and I spent the time where you asked: start-proxy.sh argument parsing,
settings.py, and the not-platform-specific claim. Four findings, two of which I would fix before
merge, both in the two files you named as thinnest and highest blast radius. Everything below was
measured by running the code, not read.

First, the claim you most wanted checked: nothing here is DAM-specific — it holds

I checked it three ways of my own choosing rather than repeating yours:

1. No platform literal in shipped code. Grepped scripts/, hooks/ and .claude-plugin/ for
   platform names, private hostnames, .corp, and bare IPv4 — nothing outside 127.0.0.1 and the
   gateway.corp.example placeholder in a comment.
2. A no-gateway install writes exactly one key. add --url with no --upstream/--bin produces
   env keys: ['ANTHROPIC_BASE_URL']. The two new keys are strictly opt-in.
3. The no-upstream path emits no upstream flag. With ANTHROPIC_UPSTREAM and
   CLAUDE_PLUGIN_OPTION_UPSTREAM both unset, the proxy's argv is
   --listen … --idle-exit=24h --dashboard --dashboard-db … and carries no --anthropic-upstream.

So the platform work is additive and the laptop path is unchanged. I did not re-verify a full fresh
install end to end — no release assets, same gate as before.

The install.sh rate-limit fix is the right shape: web redirect first, API only as a fallback, and
github_rate_limited distinguished from no_release_found. Falling back when -fI fails (a HEAD
that GitHub refuses) is handled, and the ''|releases|latest guard catches the no-releases redirect.
Conflating "rate limited" with "no release exists" was the worse half of that bug, and it is now
impossible to conflate.

The step-4-before-step-5 reordering is correct and the reasoning in the skill is the clearest
statement of it — env applying live is exactly why the installer could break its own session, and
"if it did not come up, stop and do not write the settings key" is the right gate.

On the vacuous-evidence shapes you hit three more times

All three of yours were in test code, and all three produced a confident wrong reading — a PATH stub
for a shell builtin, exec sleep making pgrep miss a live process, and reading an argv log before
the async write. Worth naming what they share: each broke the link between the observation and the
thing observed, while leaving the observation itself intact. The stub existed, the process existed,
the log existed. That is why they read as evidence. The cheap general defence is the one your kill
case demonstrated by accident — assert a positive outcome somewhere in the same test, so the
machinery has to actually work for the test to pass.

I found no instance of that shape in the new production code, and the two new tests
(TestChainingUpstreamSurvivesIntoLaterSessions, TestBinPathSurvivesAMachineWhereItIsNotOnPATH)
both assert positives and both check the uninstall-conservatism half.

Coverage gap, confirmed

Your self-assessment is accurate: no test passes --unrouted, --upstream or --bin to
start-proxy.sh. Grepping the suite, those strings appear only in unrelated assertions and in the
settings.py tests. Both findings below live exactly in that gap, and the settings.py ones survive
because the new tests exercise only the fresh-file added path.

[amiddavid]

Fix before merge — --upstream and --bin are silently dropped on two of cmd_add's three exit
paths, and one of them is the repair case that motivated the keys.

The new writes happen only after both early returns. Measured:

1. Already routed to the same URL — the re-run case. A user whose install died at the last step
(which is how all four DAM attempts ended), or whose install predates these keys, re-runs
/context-guru:install to repair it:

$ settings.py add --file s.json --url http://127.0.0.1:8787/anthropic \
      --upstream http://gw:4000 --bin /opt/cg/context-guru-proxy
result=unchanged
note=already routed to this proxy
exit=0
env keys after: ['ANTHROPIC_BASE_URL']

Success-shaped, exit 0, and neither key is written. There is no way to add the upstream to an
already-routed project through this script — re-running install, the obvious remedy, is a no-op. On a
gateway platform that leaves every later session's hook starting a proxy aimed at
api.anthropic.com, which is precisely the failure the ANTHROPIC_UPSTREAM key exists to prevent.

2. The repointed path. Our own URL on a different port:

result=repointed  previous=http://127.0.0.1:8787/anthropic
env keys after: ['ANTHROPIC_BASE_URL']

The routing moves, the chaining config does not follow it — so changing the configured port quietly
un-chains the proxy.

Both are silent, and the skill cannot detect either from the output (see my note on the emit line).

The unchanged branch is the one I would not leave: its whole meaning is "nothing to do here", and
with these arguments present that is now false. Suggested shape — compute the desired env once and
compare, rather than treating the base URL as the sole thing that can be out of date:

want = {KEY: args.url}
if args.upstream: want[UPSTREAM_KEY] = args.upstream
if args.bin:      want[BIN_KEY] = args.bin
if all(env.get(k) == v for k, v in want.items()):
    emit(result="unchanged", ...)
    return 0

and then let the write path below handle every remaining case, repointed included, so there is one
place that knows what a complete install looks like. That also makes result=unchanged mean
"complete", which is what the skill reads it as.

Worth a test on each: add --upstream over an already-routed file must land the key, and the same
over a port change must carry it across. The two new tests only cover the fresh-file added path,
which is why both of these got through.

[amiddavid]

Fix before merge — the argument loop has no *) branch, so a mistyped or valueless flag is
discarded in silence, and --upstream will swallow the next flag as its value.

You flagged this file as the weakest coverage; here is what running the combinations produces. In each
case the proxy is launched and reports success:

--upstream http://gw:4000        -> proxy got: --anthropic-upstream http://gw:4000   (correct)
--upsteam  http://gw:4000  TYPO  -> proxy got: (none)
--upstream          (no value)   -> proxy got: (none)
--upstream --bin /some/path      -> proxy got: --anthropic-upstream --bin

Nothing printed a warning in any of the three broken cases.

Rows 2 and 3 mean a proxy that forwards straight to api.anthropic.com on a platform whose
gateway holds the credential and rewrites model names — the exact configuration you found makes every
request fail, reached by a single transposed letter with no diagnostic. Row 4 is worse: --bin's value
is consumed as the upstream URL and --bin is lost, so the script both misconfigures the upstream
and falls back to resolving the binary by name, which is the failure --bin was added to fix. Two
defects from one typo.

This is also the one place where property 5 ("never fails the session") cuts the wrong way if applied
too literally: the fix is not to exit non-zero, it is to say something. There is already a log
breadcrumb for the declined-gate case, on exactly the reasoning that silence is indistinguishable from
"the script never ran" — the same argument applies here:

    --upstream|--bin)
      case "${2:-}" in
        ''|-*) note "ignoring $1: expected a value, got '${2:-}'" ;;
        *)     [ "$1" = --bin ] && BIN_ARG="$2" || UPSTREAM_ARG="$2"; shift ;;
      esac ;;
    *) note "ignoring unrecognised argument '$1'" ;;

Keep every exit at 0; just make the discard visible. A note reaches the installing session, where
the skill and the user can act on it, which is where these arguments come from.

Given the coverage gap, a small table-driven test over these four rows against a sentinel binary that
records its own argv would pin all of it — the harness in TestHookMakesTheProxyIdentifiable already
does exactly that capture.

[amiddavid]

Fix before merge — step 4 contradicts itself, and the losing half is the shape that got denied on a
hosted agent.

Line 190, in this same step:

Everything goes in as arguments, and nothing as an environment prefix. Two reasons, both learned
the hard way on a hosted agent…

Then lines 236-240:

CONTEXT_GURU_FORCE=1 asks for what it means and leaves the routing variable alone.

If step 3 found a gateway to chain behind, add ANTHROPIC_UPSTREAM=<their gateway> — or better,
tell the user to set the plugin's Upstream base URL option, which start-proxy.sh reads by
itself…

Three problems, in descending order:

1. ANTHROPIC_UPSTREAM=<their gateway> is an environment prefix — forbidden fifty lines above,
   ungrantable under prefix-matched Bash rules, and superseded by the --upstream argument this very
   commit added. An agent reading top-to-bottom hits the prohibition first and the instruction second,
   and the instruction is the actionable one.
2. CONTEXT_GURU_FORCE=1 is the superseded form of --unrouted. Still accepted for
   compatibility, per the script's comment — but recommending it here reintroduces the ungrantable
   invocation, and the paragraph presents it as the fix for the classifier denial when the actual fix
   was making it an argument.
3. "which start-proxy.sh reads by itself" is only true in the hook path. Finding 4 of this round
   is that CLAUDE_PLUGIN_OPTION_* does not reach a Bash tool call — and step 4 is the skill running
   the script as a Bash tool call. So in this step the option is precisely what the script cannot
   read, and calling it "better" points the reader at the thing that just failed on a hosted agent.

Deleting the paragraph is most of the fix, since --unrouted --upstream --bin twenty lines earlier
already says all of it correctly. If the plugin option is worth mentioning, it belongs in step 6 as
"this makes chaining apply in later sessions automatically, because hooks do see plugin options" —
framed as the hook-path benefit it is, and paired with the --upstream write which is what makes it
work in the session you are in.

This is the highest-value thing in this review relative to effort: three of your four DAM failures were
consent-gate denials caused by this exact shape, and the file an agent executes still recommends it.

[amiddavid]

Low, but it undercuts the skill's contract. Two reporting gaps now that add can write three keys:

1. Neither new key is reported. result=added names base_url, replaced and backup, so a
skill that (correctly) acts on key=value output rather than re-reading the file cannot confirm the
upstream or the bin path landed. Given the whole reason --upstream exists is that a missing key
fails silently in a later session, "did it get written" is exactly the fact worth emitting:

upstream=args.upstream or "", bin=args.bin or "",

2. other_env_keys now counts our own keys as the user's. It is
len([k for k in env if k != KEY]), and env includes the two keys we just added. Measured on a file
whose only pre-existing variable was THEIR_OWN:

other_env_keys=3        # actual user-owned keys: 1

cmd_show has the same expression, so /context-guru:status misreports it too. The count is there to
tell the skill how much of the user's own configuration is in play, and it now inflates by up to two on
every chained install. Excluding all three of our keys fixes both:

OURS = {KEY, UPSTREAM_KEY, BIN_KEY}
... len([k for k in env if k not in OURS])

[amiddavid]

Nit — two steps are both numbered ### 6. (this one and "Prove it, and only then say it worked" at
285), so the skill runs 1, 2, 3, 4, 5, 6, 6.

Trivial to fix and worth fixing anyway: this is a document whose reader executes it in order and
reports which step it is on, and a duplicate number makes "I finished step 6" ambiguous between
"verified it works" and "told the user what happens next" — which are not interchangeable, since one
is a gate and the other is a wrap-up.

[amiddavid]

Review of 762442f — all three fixed, verified by running them; two low findings left

I re-ran my own measurements against the new code and added a regression matrix, because cmd_add was
restructured and that is the highest-blast-radius function in the plugin.

The three fixes, confirmed

1. settings.py — the complete-desired-env approach works, and nothing regressed.

Case	result	env keys after
re-run with --upstream --bin (the repair case)	completed, added_keys=ANTHROPIC_UPSTREAM,CONTEXT_GURU_BIN	all three
re-run with nothing missing	unchanged	unchanged
port change with --upstream	repointed	chaining carried across
add over a stranger's gateway, no --force	conflict, exit 2	untouched
remove with no --url over a stranger's	conflict, exit 2	untouched
add where user owns one key	added, other_env_keys=1	correct

The last two matter most to me: those were round-one's blocking finding and the round-three fix, and
restructuring the function around desired did not disturb either. changed is computed before
env.update(desired), so added_keys reports what was actually missing rather than the whole set.

2. start-proxy.sh — all four space-separated shapes now report what they discarded, and the
shift-inside-the-branch fix is correct: --upstream --bin2 /x no longer swallows the following flag.

3. install/SKILL.md — the contradiction is gone, and so is the step-3 instance I had not spotted.
Grepping the whole skills tree for CONTEXT_GURU_FORCE=1, ANTHROPIC_UPSTREAM=<, "reads by itself",
and any env-prefixed script invocation returns nothing. Steps renumbered 1-7. Deleting rather than
qualifying was the right call.

On your shift bug

Worth saying plainly: you found that by running the fix rather than reading it, which is the whole
lesson of the last four rounds applied to your own work in the same session. That is the harder
direction to apply it in.

It also leads to the second finding below, which is the same class one level up — so the discipline is
working but has not yet reached the tests that certify it.

[amiddavid]

The row named for your shift bug passes against that bug. This is the vacuous-evidence class
again, and I would not have found it without your own description of what the buggy parser did.

The assertion is that stderr contains "needs a value". I reconstructed the pre-fix parser (shift
outside the accepted branch) and ran this exact row against it:

$ buggy.sh --upstream --bin /nope/x
ignoring --upstream: it needs a value, and got '--bin'
ignoring unrecognised argument '/nope/x'
  -> BIN_ARG='' UPSTREAM_ARG=''

does the buggy output contain "needs a value"?  YES

So the buggy parser satisfies both of this row's assertions — wantSaid matches, and wantUp is
correctly empty. The row is green either way, which means the defect it was added for is still
uncovered.

What actually separates the two versions is what happens to --bin:

• buggy: --bin is consumed as a rejected value, BIN_ARG stays empty, and /nope/x is reported as
  an unrecognised argument.
• fixed: --upstream is rejected alone, then --bin /nope/x parses normally and BIN_ARG=/nope/x.

Either of those is a cheap discriminator. The positive one is stronger, and it fits the rule you said
you are adopting — assert that the machinery worked, not only that it complained:

{"swallowed flag", []string{"--upstream", "--bin", "/nope/x"},
    "needs a value", "", /* wantBinHonoured: */ true},

asserting the run reports the binary /nope/x as missing (which only happens if --bin was honoured),
or negatively that the output does not contain unrecognised argument '/nope/x'.

The four other rows in this table are sound — good asserts a positive --anthropic-upstream value in
argv, which is exactly the control that makes the rest mean something.

[amiddavid]

Low, and the last hole in the parsing: the = forms bypass takes_value, so an empty value is
accepted in silence while the space-separated form reports it.

--upstream http://gw:4000   -> --anthropic-upstream http://gw:4000
--upstream                  -> SAID: ignoring --upstream: it needs a value…
--upstream=                 -> (none)          SAID: (nothing)

--bin= behaves the same way — BIN_ARG becomes empty and the script falls back to resolving the
binary by name, silently, which is the failure --bin exists to prevent.

The realistic trigger is not a user typing a bare --upstream=. It is the skill interpolating a value
that turns out to be empty: --upstream="$GATEWAY" with GATEWAY unset expands to exactly
--upstream=, and step 4 builds these command lines from values discovered in step 3. So this is the
one shape most likely to arrive from the install path itself, and it is the one the new validation does
not see.

One line each:

    --upstream=*) takes_value --upstream "${1#--upstream=}" && UPSTREAM_ARG="${1#--upstream=}" ;;
    --bin=*)      takes_value --bin      "${1#--bin=}"      && BIN_ARG="${1#--bin=}" ;;

takes_value also rejects only --*, so a single-dash -x would be accepted as a value; -* would
be more thorough, though I cannot construct a realistic caller that produces it.

Neither = form appears in TestStartProxyReportsArgumentsItCannotUse's table, which is why this
survived — two more rows cover it.

[amiddavid]

Review of 4b39f79 — both findings fixed, no findings left

The extracted test is right, and it asserts the discriminator that matters. Moving that case out of
the table into TestRejectingAValueDoesNotEatTheNextFlag was the better call than adding a column:
it needs the argv log, a free port, and four assertions, none of which the other four rows want.

What makes it sound, reading it:

• the positive is len(launched) != 0 — the fake proxy only writes that log if --bin was honoured;
• CONTEXT_GURU_BIN= is cleared, so the buggy path's fallback resolves the bare name and starts
  nothing, which is what makes the positive discriminate rather than merely pass;
• ANTHROPIC_UPSTREAM and CLAUDE_PLUGIN_OPTION_UPSTREAM are cleared too, so an ambient value cannot
  supply the upstream the test asserts is absent;
• the stray-argument check pins the buggy parser's other symptom, so both halves of the old
  behaviour are covered.

I could not run go test — there is no Go toolchain on this machine, as I have said each round — so I
verified the behaviour it asserts at the shell level instead, on a free port and in both directions:

fixed  parser, --upstream --bin <path>:  proxy launched from <path>, no --anthropic-upstream in argv,
                                         path NOT reported as a stray argument
buggy  parser, same arguments:           --bin eaten, BIN_ARG empty, path reported as
                                         "unrecognised argument '<path>'"

That is the separation the test keys on, so I am confident in it without having executed it. Your
mutation run is the stronger evidence and I am taking it as such.

Both = forms now go through takes_value, and --upstream= is in the table. The only asymmetry left
is cosmetic: --bin= has no row, though it takes the identical code path. Not worth a commit on its own.

One note from my own side, since it cuts the other way

My first pass at verifying this reported launched=no and I nearly filed it. The cause was my harness,
not your code: this machine has a real context-guru proxy answering on 8787, so a probe against the
default port exits early at "already up" and never launches anything. Which is worth saying because
your tests already guard against exactly that — every test that probes or launches uses freePort, and
only the gate-exit rows use the literal 8787, where the gate returns before any probe. The comment at
plugin_test.go:978 ("so a developer's real 8787 is never touched") is the reason a developer running
this plugin can still run its suite.

So the hermeticity question I went looking for was already answered in the code. Recording it because a
future reader may reach for 8787 in a new test and reintroduce it, and the failure would look like a
flake on one machine only.

State

All findings from five rounds are resolved. DCO, build, purego, trivy pass on 4b39f79;
build-test was still running when I posted. Nothing outstanding from this review.

The merge gate remains reviewDecision=REVIEW_REQUIRED, which no review from this account can clear —
the reviewing credential is the PR author, so GitHub declines a self-approval and all five of my reviews
land as COMMENT. That needs either an approval from another account with access or an admin bypass at
merge time, and it is the repository owner's call, not mine. Merge order unchanged: release.yaml via
workflow_dispatch → v0.1.0 → this.

[OsherElhadad]

First real end-to-end run: install, two live preset sessions, and one blocker

The PR body is honest that this has never run end to end. I ran it — real install, real
claude -p sessions through a real proxy, real numbers pulled from /stats and /api/config —
in an isolated worktree (4b39f791) and an isolated CLAUDE_CONFIG_DIR, never touching my own
~/.claude/settings.json (sha256sum identical before/after every step) or port 4000. Full
methodology and raw output: happy to paste more if useful, trimmed here for length.

Read + ran the existing tests

All four scripts, three SKILL.mds, hooks.json, both manifests, plugin_test.go, both docs
pages. Traced install/uninstall/SessionStart/UserPromptSubmit end to end.

$ go test -count=1 -v ./context-guru-plugin/...   ->  ok  ...  29.554s   (28 funcs / 52 incl. subtests actually ran)
$ go test -race -count=1 ./context-guru-plugin/... ->  ok  ...  28.577s

Revert-verification (mutated the SOURCE, reverted with git checkout --, worktree clean before/after):

Test	Mutation	Result
TestUninstallRefusesAForeignBaseURLEvenWithNoURLGiven	settings.py: reinstated the historical if args.url and current != args.url and ... (omitting --url used to skip the check)	FAIL, reproducing the described worst case exactly — a stranger's gateway.corp.example.com deleted, result=removed, exit 0
TestSettingsFollowsASymlink	save(): real = os.path.realpath(path) → real = path	FAIL, naming its own subject: symlink replaced by a regular file
TestHookNeverFailsTheSessionWhenTheBinaryIsMissing	start-proxy.sh:174: exit 0 → exit 1	FAIL: "exit 1 — the hook must never fail a session"
TestSettingsPreservesFileMode	save(): deleted the copymode/chmod block entirely	PASS — did not catch it. See minor finding below.

Real numbers, both presets

Proxy built from this worktree (make build). This environment already had ANTHROPIC_BASE_URL
pointed at a pre-configured upstream when the review started, which put the --anthropic-upstream
chaining path (install/SKILL.md §3 — "something else is already the gateway") in front of me
for real rather than as a scripted scenario, so I exercised it rather than working around it.
Bound to 4041/4042 only; no infrastructure details below beyond that.

house (4041) — two turns, one --resumed session, against a small git repo:

Turn 1 "add(3,4)?" -> "7"  (correct)   usage: cache_creation=27806 cache_read=24817
Turn 2 "multiply(3,4)?" -> "12" (correct) usage: cache_creation=51    cache_read=27806

Turn 2's cache_read == turn 1's cache_creation, exactly — genuine warm cache, not corruption.
/stats: requests:3, cachesplit:{mutated:3, verdict:"moved", acted:0, saved_tokens:0} —
exactly what status/SKILL.md says to expect (breakpoint relocation, not content removal).
/api/config: components:["extract"], matching the documented house pipeline.

housellm minus extract_llm_sweep (4042) — no plugin option exists to drop one component
out of a named preset, so this ran the proxy directly with housellm's own YAML from
config/config.go minus the sweep's pipeline entry and component block (posted on request). Two
turns against a 300-function/1199-line file:

Turn 1 "helper_150(10)?" -> "160" (correct)   usage: cache_creation=17641 cache_read=46303
Turn 2 "helper_299(1)?"  -> "300" (correct)   usage: cache_creation=58    cache_read=39086

/stats: llm_calls:0, extract_llm:{verdict:"skipped", gates:{cheap_model_price_unconfigured:1}}
— an honest zero, not a hidden one: no pricing table for claude-haiku-4-5 with source:incoming
in this deployment, so the economic gate declines rather than spending blind. /api/config
confirmed the loaded pipeline (["extract","extract_llm"], no sweep). Caveat, stated plainly:
with zero extract_llm calls either way and a two-turn session that never crossed the 5-minute
cache TTL, this comparison can't show the sweep's marginal yield — it shows the mechanism loading
and running correctly, not a yield delta. Would need a longer session or priced cheap-model config.

Aside confirmed along the way: v0.1.1 published 2026-09-04 with real assets for all 4
platforms + checksums.txt — install.sh downloaded and verified it against the live repo. The
"no release exists" premise in the PR body is now stale; only the bug below stopped a real install
session, not the missing release.

Blocker: the configured proxy port can never be honoured by /context-guru:install

Confirmed two ways — directly, and independently reproduced by an unmodified install session.

Direct proof, with port=4041 configured via claude plugin install --config port=4041:

$ claude -p 'Run exactly this Bash command and show the raw output, nothing else: env | grep -E "^CLAUDE_PLUGIN_OPTION_|^CLAUDE_PLUGIN_ROOT="'
'The command produced no output (no matching environment variables are set).'

Zero CLAUDE_PLUGIN_OPTION_* (and no CLAUDE_PLUGIN_ROOT) reach a Bash tool call — this extends
the PR's own documented finding (stated only for UPSTREAM, in start-proxy.sh:82-88) to
PORT. The system-init message gives the model the plugin's file path (which is why
${CLAUDE_PLUGIN_ROOT} substitution works when the model writes it out literally) but never
the configured option values — port/preset/idle_exit are nowhere in the model's context.

A real, otherwise-unmodified /context-guru:install session (told only "use port 4041") found and
reported this itself, unprompted:

"Blocked: starting the proxy. Two separate walls: 1. start-proxy.sh has no --port
argument — it reads the port only from CLAUDE_PLUGIN_OPTION_PORT (default 8787), and the
skill forbids env-prefixing (ungrantable, since Bash rules match by prefix). So the script can
only ever start a proxy on 8787 from here, while routing must point at 4041."

Why it's a blocker and not cosmetic:

1. start-proxy.sh has no --port/--listen-port flag; PORT comes only from
   CLAUDE_PLUGIN_OPTION_PORT (scripts/start-proxy.sh:30).
2. Unlike --upstream/--bin — discovered from other sources (install.sh's own stdout,
   printenv ANTHROPIC_BASE_URL) and passed as arguments a permission rule can cover — there is
   no alternate discovery path and no argument form for the port. install/SKILL.md:258 instructs
   exactly the read that cannot succeed: ${CLAUDE_PLUGIN_OPTION_PORT:-8787} inside a Bash-tool
   command.
3. Even the workaround (CLAUDE_PLUGIN_OPTION_PORT=4041 .../start-proxy.sh) is, by the plugin's
   own stated design principle for why --upstream/--bin/--unrouted are arguments and not
   env vars (start-proxy.sh:61-66): ungrantable — "Bash permission rules match by command
   PREFIX... an env-prefixed command is one nobody can approve." Every other option got this fix
   twice already; PORT was missed.
4. Consequence, not just inconvenience: if the install silently defaults to 8787 while the
   user configured a different port (which plugin.json's own description explicitly invites —
   "change it when something already holds 8787"), the routing key names 8787 while every LATER
   SessionStart/UserPromptSubmit hook correctly reads the configured port and self-gates on
   it. Mismatch → the hook treats the project as unrouted and does nothing. The one proxy that
   is running then has no auto-restart safety net at all — once it idle-exits, crashes, or
   the box reboots, nothing brings it back, silently, which is the exact failure mode (a hanging
   prompt with no error) this whole plugin exists to prevent.

Fix shape: give start-proxy.sh an explicit port argument (parallel to --upstream/--bin),
and have install/SKILL.md obtain the configured port some other way than a shell expansion a
Bash tool call can never see (asking the user directly is the cheapest fix).

Major: settings.py crashes with a raw traceback instead of its own result=error contract

The script's docstring promises "Output is one key=value line per fact on stdout" — true for
every failure path I could find except genuine OS-level I/O errors during backup()/save(),
which have no top-level handler:

$ chmod 0555 <dir>; settings.py add --file <dir>/settings.json --url ...
Traceback (most recent call last):
  ...
  File ".../settings.py", line 112, in backup
    fd = os.open(dest, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
PermissionError: [Errno 13] Permission denied: '.../settings.json.context-guru-backup-...'

Same shape (uncaught PermissionError) for a dangling symlink into an unwritable path
(save() → os.makedirs(os.path.dirname(os.path.realpath(path)))). Non-destructive in every
case tried — the original file was untouched both times — but it breaks the "read the
key=value lines, don't guess" contract the calling skill is told to rely on. Recommend wrapping
main()'s dispatch in a try/except OSError that emits result=error reason=os_error detail=...
and exits non-zero, matching the existing unparseable_json/env_not_an_object pattern.
(scripts/settings.py:112, :165)

Minor: TestSettingsPreservesFileMode doesn't test what it says

The fixture chmods the original to 0600 before the edit — the same value
tempfile.mkstemp() defaults to on its own — so deleting the copymode/chmod call entirely
still passes the test (confirmed above). Direct proof the removal is a real regression the test
just can't see:

$ chmod 0640 settings.json   # NOT 0600
$ python3 settings.py add ...   # mutated code, no copymode
$ stat -c%a settings.json
600   # silently narrowed from 640

Fix: start the fixture at 0640 (or anything ≠ 0600) so the assertion actually depends on
copymode. (plugin_test.go:544-562)

Confirmed working, with real evidence

• idle-exit floor, live: --idle-exit=1h on a fresh proxy refused to start with the exact
  documented message ("idle-exit 1h0m0s is below the floor of 5h33m20s..."); 24h started fine.
• UserPromptSubmit (check-proxy.sh) never blocks a session, tested against two shapes:
  unreachable port + missing binary (fast path, exit 0 immediately) and a pathological
  "accepts, never answers" listener (the real stallingPort shape from plugin_test.go) —
  11s real wall clock, exit 0, comfortably inside the 30s hook timeout, and produced the
  documented user-facing "your request will hang..." note both times.
• Uninstall's pidfile-based stop correctly found and killed the real proxy I started via
  start-proxy.sh, confirmed via /healthz afterward (connection refused).
• settings.py add/remove round-trip on a real file: wrote the key, took a timestamped
  backup, later removed it, correctly left an unrelated permissions block untouched and deleted
  env: {} once it was empty.
• A settings file chmod 0444'd by its owner is still overwritten by the atomic
  os.replace() (directory write permission governs rename, not the target's own mode bit) — but
  its 0444 mode is preserved on the result (copystat still runs). Not a bug — os.replace
  behaves this way by design — but worth knowing "read-only" doesn't mean "untouchable" here. Nit,
  no action needed.
• This box happens to already have a different, real context-guru-proxy on PATH at
  /usr/local/bin (the production hosted binary, root-owned, predates --version) —
  install.sh correctly discovered it, correctly decided to "upgrade" past it into $DEST without
  ever touching it, and ~/.local/bin (where the plugin installs) happens to precede
  /usr/local/bin in this account's PATH so the right one gets picked up. Purely an artifact of
  this box also hosting the production service under the same binary name — not a defect here —
  but on a machine with the opposite PATH order, command -v context-guru-proxy in the
  SessionStart hook would silently resolve to the wrong binary. --bin/CONTEXT_GURU_BIN
  already exists and is the robust answer; worth a one-line mention in the docs that it's not only
  for the "not on PATH" case.

Not verified

brew/macOS-specific paths (no macOS available here). The dollar-figure "cost" comparisons in
/stats beyond what's quoted above — this environment's traffic is not metered per-request, so
list-price $ figures don't correspond to a real bill here, exactly as status/SKILL.md warns.

Teardown

Real proxies killed by PID via the documented pidfile mechanism (never pkill -f pattern
matching); isolated CLAUDE_CONFIG_DIR and scratch projects removed; downloaded binary removed;
~/.local/state/context-guru (which a manual start-proxy.sh run wrote into the real state path)
removed. sha256sum /home/vpcuser/.claude/settings.json unchanged throughout —
ff1de4913bee48586aecfb1c80a6772f940d9b82eb0456f021023ecfaa1604c8. Worktree git status clean.
Port 4000 (this box's production service) never touched.