Skip to content

doctor: fix false blockers on mixed attribution, make --redact work, quiet the noise - #353

Merged
zzet merged 7 commits into
mainfrom
fix/doctor-mixed-attribution-and-redact
Jul 26, 2026
Merged

doctor: fix false blockers on mixed attribution, make --redact work, quiet the noise#353
zzet merged 7 commits into
mainfrom
fix/doctor-mixed-attribution-and-redact

Conversation

@zzet

@zzet zzet commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Follow-up to #351 and #352, from the first real run of gortex doctor on a machine that had just upgraded. Four fixes, one of them a correctness bug that made doctor accuse a healthy install.

1. False blockers on mixed attribution (critical)

The attribution guard from #352 was all-or-nothing: the union stood in only while no row carried an agent. But a log is not attributed or unattributed — it is mixed, for as long as the window still reaches back past the upgrade. One attributed row flipped the summary to strict mode and hid every older row behind it.

PreToolUse fires constantly and attributed itself within minutes. SessionStart fires once per session and had not run again yet. On a working install:

✗ BLOCKER SessionStart is configured but never ran, while other hooks did (not once, ever).
✗ BLOCKER UserPromptSubmit is configured but never ran, while other hooks did (not once, ever).
✗ BLOCKER PostToolUse is configured but never ran, while other hooks did (not once, ever).

…against 17,317 rows in the same window saying otherwise, and beside a last seen of never for an event that had last run twenty minutes earlier. The guard meant to prevent that accusation is what produced it — the tests covered the two clean states and never the transition every machine passes through.

Unattributed rows are now rolled up per event and held out of every agent's counts rather than shared into all of them:

agent runs unattributed runs verdict
> 0 any normal analysis
0 0 genuine silence → blocker
0 > 0 cannot tell → withheld, reported as INFO

The table gains an unattrib column so a zero explains itself, and dates the last run from unattributed rows when that is all there is — never beside evidence of 48 invocations is a contradiction, not a diagnosis.

The regression test reproduces the reported machine state and fails against the previous rule with all three false blockers verbatim.

2. --redact was a no-op

It promised to hash repo paths and branch names, and rewrote only Adoption fields the human report never renders — while printing every absolute path, repo name and account name included, in both sections. Paths under the repo root now render as <repo>/… and under home as ~/…, installed before anything renders so the static report, the runtime report, the paths quoted inside findings, and --json all agree. Branch names are not paths and can carry a ticket or customer name, so they are hashed.

3. ✗ on optional files read as an accusation

Every per-repo file gortex init had not written was marked ✗, implying work the user has to do. It is not: a machine configured by gortex install needs no repo-local config, and most repos never get any.

  ✓ present   · absent (optional — repo-local config)   ! needs attention

  [✓ installed] [✓ gortex files]  claude-code
      · <repo>/.mcp.json (optional — `gortex init` would create)
      ✓ <repo>/.claude/settings.local.json (26774 bytes)

A stale MCP stanza previously shared the ✓ of a healthy file with the explanation buried in a trailing note. It is a real gap and now reads as one (!). The header's any-file-present becomes gortex files, which is what it always meant.

4. Uninstalled adapters buried the machine

All twenty adapters were listed with full planned-file sets regardless of installation — one uninstalled agent contributed twenty-one SKILL.md paths for writes that will never happen. Adapters that are neither installed nor holding Gortex files now collapse to a name list; the section drops from 113 lines to 56.

An absent agent that still holds Gortex files is kept and labelled leftover config — that is a finding, and a blanket "hide undetected" filter would have swallowed it.

--all restores the full listing. --json stays complete either way: a machine consumer has no scrolling problem, so filtering there would only lose data.

Tests

Seven new cases. The load-bearing ones:

  • mixed attribution does not accuse — reproduces the reported state; verified it fails against the old rule with the three false blockers verbatim.
  • attributed silence still blocks — the guard must not swallow the finding it exists to protect.
  • wholly ambiguous window reports uncertainty — INFO, not a blocker, not silence.
  • ✗ reserved for real gaps — absent-optional vs present vs outdated vs unreadable.
  • collapse + --all round trip, and leftover config survives the collapse.

go build ./..., go vet ./..., go test ./cmd/gortex ./internal/doctor ./internal/hooks green.

zzet added 4 commits July 26, 2026 10:38
Two bugs the first real run on a mixed machine exposed.

The attribution guard was all-or-nothing: the union stood in only while
NO row carried an agent. But a log is not attributed or unattributed, it
is mixed — for as long as the window still reaches back past the upgrade
that introduced the field. One attributed row flipped the whole summary
to strict mode and hid every older row behind it. PreToolUse fires
constantly and attributes itself within minutes; SessionStart fires once
per session and had not run again yet. So a healthy install reported
three blockers — SessionStart, UserPromptSubmit and PostToolUse "never
ran, not once, ever" — against 17,317 rows in the same window saying
otherwise. The guard meant to prevent exactly that accusation was what
produced it.

Unattributed rows are now rolled up per event and held out of every
agent's counts rather than shared into all of them. Silence is claimed
only when an agent has no runs of an event AND nothing unattributed
could account for it; when something could, doctor says it cannot tell.
The table grows an unattrib column so a zero explains itself, and dates
the last run from the unattributed rows when that is all there is —
"never" next to evidence of 48 invocations is a contradiction, not a
diagnosis.

--redact promised to hash repo paths and branch names and rewrote only
Adoption fields the human report never rendered, while printing every
absolute path — repo name and account name included — in both sections.
It now rewrites paths under the repo root to <repo>/… and under home to
~/…, installed before anything renders so the static report, the runtime
report, the paths quoted inside findings, and --json all agree. Branch
names, which are not paths and can carry a ticket or customer name, are
hashed.

The regression test reproduces the reported machine state and fails
against the previous rule with the three false blockers verbatim.
The adapter section listed all twenty adapters with their full planned
file sets, whether or not the agent was installed. One uninstalled agent
contributed twenty-one SKILL.md paths describing writes that will never
happen, and the handful of lines describing the actual machine were
buried in the middle of them.

Adapters that are neither installed nor holding Gortex files now
collapse to a name list. An absent agent that still has Gortex files is
kept and labelled instead: that is leftover config, which is a finding
rather than noise. --all restores the full listing, and --json stays
complete either way, since a machine consumer has no scrolling problem.
Every per-repo file `gortex init` has not written was marked ✗, which
read as a fault and implied work the user had to do. It is not: a
machine configured by `gortex install` needs no repo-local config, and
most repos never get any. The section that exists to say "here is what
your machine looks like" was instead listing chores.

Three markers now, and the difference between the first two is the
point:

  ✓  present
  ·  absent — optional repo-local config, nothing to do
  !  needs attention: present but outdated, or unreadable

A stale MCP stanza used to share the ✓ of a healthy file with the
explanation buried in a trailing note; it is a real gap and now reads as
one. The header's "any-file-present" becomes "gortex files", which is
what it always meant.
Config shapes drift between releases — an MCP stanza gains a field, a
hook command changes, an instructions block is rewritten. The only thing
that noticed was doctor, which told the user to go run `gortex install`.
That hands back a chore at the wrong moment: upgrading is exactly when
the configs should be brought current, and the user already asked for
the new version by running the command.

`gortex upgrade --run` now re-applies agent config after the binary
lands, and the closing advice no longer names a step it just performed.
--no-migrate opts out, and says so rather than skipping silently.

Two constraints shape it.

The refresh must run from the NEW binary. This process is the old one —
the package manager replaced the file underneath it — so re-applying
adapters in-process would faithfully write the shapes the upgrade is
meant to migrate away from. It re-execs, resolving the binary through
PATH first because `go install` can land it somewhere other than where
the old one ran from.

It deliberately goes through `gortex install` rather than a quieter
bespoke migrator. Re-applying can rewrite a hook definition, and Codex
records trust against each hook's hash: a changed hook is skipped until
re-approved in /hooks. The install summary already carries that notice,
and silently re-trusting is not something a migrator should invent.

The refresh is best-effort — the binary has already been replaced by
then, so a refresh that cannot run is a warning with a manual command,
never a failed upgrade.
@zzet

zzet commented Jul 26, 2026

Copy link
Copy Markdown
Owner Author

Added: gortex upgrade now does the migration itself

2f707b71. The [outdated stanza: run gortex install to migrate] remedy was handing back a chore at the wrong moment — upgrading is exactly when configs should be brought current, and the user already asked for the new version.

gortex upgrade --run now re-applies agent config after the binary lands, and the closing advice no longer names a step it just performed:

Next:
  gortex index .    # reindex so the graph picks up extractor changes

versus a dry run, which still owes it:

After upgrading:
  gortex install    # refresh agent config for the new version
  gortex index .    # reindex so the graph picks up extractor changes

--no-migrate opts out, and says so rather than skipping silently. Doctor's remedy now reads [outdated stanza: gortex upgrade --run migrates it, or gortex install].

Two constraints worth flagging in review

It must re-exec the new binary. The running process is the old one — the package manager replaced the file underneath it — so re-applying adapters in-process would faithfully write the shapes the upgrade exists to migrate away from. It resolves through PATH first, because go install can land the new binary somewhere other than where the old one ran from; re-execing the stale path would be the same bug wearing a different hat.

It goes through gortex install, not a quieter bespoke migrator. Re-applying can rewrite a hook definition, and Codex records trust against each hook's hash — a changed hook is skipped until re-approved in /hooks. That is the failure #348 was about, and an upgrade that silently reintroduced it would be worse than the chore it replaced. The install summary already carries the trust notice, so routing through it means the migration cannot quietly un-trust anything.

The refresh is best-effort: the binary has already been replaced by then, so a refresh that cannot run is a warning with a manual command, never a failed upgrade.

Tests

  • re-exec target is absolute and runs a non-interactive install — the sequencing assertion.
  • a failed refresh warns with a manual command and does not fail the upgrade.
  • follow-up advice lists the config step only when it was not performed.

go build ./..., go vet ./..., go test ./cmd/gortex — 732 tests green.

zzet added 3 commits July 26, 2026 13:11
`gortex update` was an unknown-command error, though the command's own
summary line has always read "Update gortex to the latest release". The
two read as synonyms and now are: without the alias the post-upgrade
config refresh would simply never run for anyone whose habit is
`update`.

Splits the post-upgrade decision into postUpgradeSteps so the wiring the
feature hangs on — refresh by default, skip visibly under --no-migrate,
and report which happened so the closing advice does not name a step
already performed — is testable without a real install method, a real
release check, or a real package manager. Drops a helper that was
written and never called.
On a machine running Claude Code and Codex in the same repo, 3,224 Codex
calls were booked against a Claude model and no OpenAI model was ever
recorded at all. Two independent faults compounded.

The hint is keyed by working directory, and only one writer existed:
Claude Code's hook, which always tags its hint "claude-code". The reader
took the model without ever comparing the hint's agent to the session's
client. Two agents sharing a repo share a cwd, so Codex read whatever
Claude last announced. That is worse than missing attribution — it
prices a call at another vendor's rates and reports it as fact.

The reader now adopts a model only when the hint's agent is consistent
with the session's client. An unknown on either side still matches: a
hint with no recorded client predates the field, and rejecting it would
drop attribution that used to work. Two known names that disagree are
proof the hint belongs elsewhere. The comparison normalises the two
spellings of one harness — the hook knows "codex", the MCP session
reports "codex-mcp-client".

Codex sends the active model slug on every hook event and nothing was
reading it, which is why the ledger held no OpenAI models. It is now
captured on the session- and turn-scoped events; PreToolUse is excluded
deliberately, since it fires hundreds of times a session and would turn
a twelve-hour hint into a per-tool-call disk write for no freshness.

Existing rows keep their bad attribution — the ledger is an append-only
record of what was believed at the time. `gortex savings --reset` clears
it for anyone who would rather start clean.
Two helpers were written for one call site and only one was ever used,
which golangci-lint's unused check caught. The surviving call site also
reads better in the positive: "skip when it ran, or when we cannot tell"
rather than "skip when not silent".
@zzet
zzet merged commit 2500c6a into main Jul 26, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant