Skip to content

Audit fixes: close two confident-false-green paths and 13 related defects - #116

Open
inth3shadows wants to merge 17 commits into
mainfrom
claude/awesome-heisenberg-5gjvbr
Open

inth3shadows wants to merge 17 commits into
mainfrom
claude/awesome-heisenberg-5gjvbr

Conversation

@inth3shadows

Copy link
Copy Markdown
Owner

Summary

This PR fixes the findings from a module-by-module audit of testgraph (2026-10-08). For every finding below, two separate agents either reproduced it independently or both found it. Each fix is its own commit with a regression test, and each test was confirmed to fail on the old code.

Critical: the tool printed a green it could not back up

  • C1, select. The diff parser only recognised +++ b/… headers. When the user's git config set diff.noprefix=true or color.diff=always, or a changed file's name had a non-ASCII character, a space or a quote, that file was silently dropped. The output was then "journeys to test: NONE (no product-behavior change detected)". Git's output format is now pinned regardless of user config (--no-color --src-prefix=a/ --dst-prefix=b/ -c core.quotePath=false --no-ext-diff --no-textconv). Quoted paths are decoded. Every diff --git block now resets the parser state, so a header it can't read no longer gives its hunks to the previous file. A block with no readable header is reported as unparseable instead of being dropped. A mode-only change (chmod) is ignored.
  • C2, verify. It exited 0 ("every selected journey ran and passed") in four cases: a journey whose only test was skipped or xfailed inside its body; a declared test that failed before reaching an entry; a test that errored in setup; and a test whose JUnit verdict didn't match its trace (that journey was also missing from the report). Now a journey counts as verified only if it is credited pass. Any failing declared test fails every journey it declares. Skipped or verdict-less journeys exit 3, and are reported on their own ! ran, but not a pass line. An abnormal pytest exit can no longer produce 0.

High

  • M2. base/head were passed to git as possible options. Through the MCP tool, --output=… emptied a file chosen by the caller and still returned a clean NONE. Now --end-of-options is passed, revisions starting with - are refused before any git call, and ledger.resolve_commit refuses them too.
  • H1. verify used strict_registry=False, a mode TECHNICAL.md reserves for analysing history. A registered journey that could never be selected became a warning under exit 0. It is now refused (exit 2).
  • M5. The pre-push hook hard-defaulted a new branch's base to main. In a master repo, a multi-commit first push was diffed over its tip only, with no warning. It now tries wt.base, then origin/HEAD, main, master and the origin/ variants.
  • H4. _join_key split on :: before splitting off [params], unlike pytest. A test like test_p[a::b] lost its verdict, which fed C2.
  • H5. An unapproved proj.draft.json sorted before the approved proj.json and replaced it. An approved registry now wins within a directory; a lone draft still runs, loudly, as before.
  • H6. propose crashed when one file had two decorated Python handlers with the same name. They are now merged, as TypeScript hits already were.

Medium

  • H2. export defaulted to honeyslate's registry and a personal path. It now finds the registry by target the same way select does, and refuses when there is none. --repo defaults to . in both commands.
  • H3. The hook depended on GNU timeout, which stock macOS doesn't have, and threw away stderr, so the hook silently did nothing there. It now uses a portable POSIX-sh watchdog that prints a heartbeat every 5 s and kills the run at the limit, and it shows the first stderr line when the selector crashes. It still always exits 0. Ctrl-C skips the step that is running.
  • M1. resolve_symbol matched with LIKE '%suffix': no directory boundary, case-insensitive, and _/% acting as wildcards. That inflated the integrity spot-checks. Matching now respects directory boundaries and case.
  • M4. CI ran on 3.11 only. It now runs a 3.11–3.13 matrix, plus a job with pytest installed. That job has nothing pytest-specific to run yet; CI: add an end-to-end verify test that drives real pytest + tgtrace #114 tracks the end-to-end test it needs.
  • M6. sqlite connections are now closed deterministically, which makes the MCP docstring's claim true.
  • M10. On 3.11 the tracer missed worker threads; it now also calls threading.setprofile.
  • M11. Skill docs: the registry path is now under .testgraph/journeys/, the skill also lists untracked new files, and the "NONE is trustworthy" rule now includes the unapproved-registry case.

Not in this PR

Testing

  • python3 -m unittest discover -s tests: 477 tests OK on Python 3.11, 3.12 and 3.13, and in a venv with pytest installed (422 tests before this PR).
  • New tests/test_pre_push.py runs the real hook script under sh, dash and bash, with no timeout on PATH.
  • No release label is set, so merging does not publish.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa


Generated by Claude Code

claude added 17 commits October 8, 2026 13:34
changed_ranges now pins git's output (--no-color, --src/--dst-prefix,
core.quotePath=false, no ext-diff/textconv, --end-of-options), so
diff.noprefix and color.diff=always no longer make every file vanish.

_parse_unified_diff resets its per-file state on every `diff --git` line,
strips git's trailing TAB on names with spaces, decodes C-quoted paths
(new _unquote_git_path), and reports a product-looking block with no
readable '+++' header in whole_files ("unparseable diff header") instead
of dropping it or attributing its hunks to the previous file. Empty
new/deleted files (no header, no content) are exempt.

Each case previously gave status OK, journeys [], unknown_because [].

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
… M2)

`base`/`head` reach git as arguments and, via testgraph_impact, come from
the model. `--output=<path>` made `git diff` truncate an arbitrary file
and return an empty diff, i.e. a clean NONE. select() now raises
ValueError for a revision starting with '-' (mcp.call_tool already turns
that into an isError result), ledger.resolve_commit returns None for one,
and changed_ranges passes --end-of-options (added in the C1 commit).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…follow-up)

A `diff --git` block with `old mode`/`new mode` lines and no '+++' header is a
pure chmod: it changes no content, so reporting it as "unparseable diff header"
needlessly degraded recall. Genuinely unreadable headers still degrade.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
… H4)

pytest's mangle_test_address splits off the bracketed parameter suffix first;
a param id may contain '::' (test_p[a::b]). Splitting on '::' first shredded
the key so the test lost its JUnit verdict. Mirror pytest's order.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…(audit C2)

validate: a declared test whose JUnit verdict is fail/error marks every
journey it declares fail, whether or not its trace validated or exists.
exit_code: a selected journey is verified only if credited pass; skip-only,
verdict-less and uncovered journeys exit 3, as does an abnormal pytest exit
(not 0/5) that would otherwise be OK. run/_summary/render gain `incomplete`
("ran, but not a pass: J2 (skipped)"). Fixed the stale EXIT_NOTHING_TO_RUN
name in the exit_code docstring. No existing test expectation changed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…it H1)

verify.run passed strict_registry=False, a mode TECHNICAL.md reserves for
history analysis, so a journey whose entries resolve to nothing was a warning
and tests were chosen from a registry that could never select it. Use the
default (strict): it BLOCKs and the run is refused (exit 2).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…(audit M5)

A master-default repo's first push found no `main`, fell through to the parent
commit and diffed only the tip of a multi-commit branch, with no warning. Base
candidates are now wt.base, origin/HEAD, main, master, origin/main, origin/master;
the first that resolves to a commit is merge-based. Dash-leading values are
skipped so a config value can never become a git option. Every `|| true` kept.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…e-push (audit H3)

Without GNU `timeout` (macOS) both bounded calls failed with "command not found",
the error was swallowed, and the hook silently did nothing. tg_run_bounded is a
POSIX sh watchdog: background child, 1 s polling, a stderr heartbeat every 5 s
("still running (<n>s) -- Ctrl-C skips it"), TERM then KILL at the limit, always
returns 0. Limits stay 60 s (codegraph sync) and 20 s (selector); overridable with
TESTGRAPH_HOOK_SYNC_TIMEOUT / TESTGRAPH_HOOK_TIMEOUT / TESTGRAPH_HOOK_HEARTBEAT.
INT skips the running step and TERM ends the hook; both kill the child, and an
EXIT trap removes the temp file. The selector's stderr goes to a temp file whose
first line is shown on non-zero exit instead of being discarded. All `|| true`
guards are kept, so the hook still never fails a push.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…y (audit H5)

sorted() puts proj.draft.json before proj.json, so resolve_for_repo's
first-match scan returned the draft and shadowed the approved registry.
Within one directory a non-draft match now wins; a draft is still returned
when it is the only match. Directory order is unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…t (audit H6)

Two decorated module-level `def handler()` in one file produced two hits
for one (file, symbol); assign_ids keys on that pair, so propose died with
"journey ids are not unique". Python hits now go through the same
_merge_duplicate_hits the TypeScript scan uses. Chosen over keying by
(file, line) because the registry entry is the symbol name (the only thing
that resolves in the index), so a second per-line journey would resolve to
the same nodes and just duplicate the journey. One journey per symbol keeps
every route label from both definitions.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
… (audit H2)

export's --registry defaulted to journeys/honeyslate.json for every repo, so
exporting any other project mapped it against the wrong journeys. It now
resolves via registry.resolve_for_repo and refuses with exit 2 when none
matches. The refusal text moved to registry.not_found_message, shared with
select.

--repo defaults changed from a personal honeyslate checkout path to "." for
both export and select. No existing test or doc relied on the old default
(README/USAGE already pass --repo explicitly), so nothing needed updating.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…iteral _ % (audit M1)

LIKE '%suffix' matched mid-name, ignored case and treated _ and % as
wildcards, so app/config.py resolved to legacy/oldapp/config.py. LIKE is now
an escaped prefilter and the exact test (path == suffix or endswith "/"+suffix)
runs in Python.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…t_adapter (audit M6)

Each call opened a connection and never closed it; the MCP server is
long-lived and its docstring claims they are closed. Bodies after connect are
now wrapped in try/finally. The dash-revision ValueError in select still fires
before any connect.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…audit M10)

sys.setprofile only traces the calling thread, so on Python 3.11 (no
sys.monitoring) a journey entry running in a thread was never recorded.
_start_profile/_stop_profile now also set threading.setprofile. The plugin
stays import-free of testgraph.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
… M4)

The suite ran on 3.11 only, though the tracer has per-version backends, and
never with pytest importable. Action pins are unchanged. tests/test_workflows.py
guards the matrix, the pytest job and the sha pins.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…oval condition (audit M11)

testgraph-propose pointed at journeys/<target>.draft.json (the package
directory) instead of <repo>/.testgraph/journeys/, and propose --out help said
the same. testgraph-verify step 1 now also lists untracked new files, and the
NONE rule requires an approved registry. tests/test_skill_contract.py pins all
three.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
…3 follow-up)

communicate() flushed a stdin the test had already closed and raised
ValueError on 3.11 and 3.12. Found by running the suite on all three
interpreters.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gsh2XjQprosodq2YbgyFQa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants