Skip to content

Clean up retired protocol-handler files, add tests and docs - #1

Merged
inth3shadows merged 9 commits into
masterfrom
claude-proceed
Jul 2, 2026
Merged

Clean up retired protocol-handler files, add tests and docs#1
inth3shadows merged 9 commits into
masterfrom
claude-proceed

Conversation

@inth3shadows

Copy link
Copy Markdown
Owner

Summary

  • Removed 5 leftover files from the retired terminal protocol-handler design (handler/, hook/, install/, test/sample.jsonl) that the README already called retired but were never deleted.
  • render/callgraph.js now exports buildDot/isTestRef so they're testable; test/run.js is a real suite (package.json's test script previously pointed at a file that never existed).
  • Added TECHNICAL.md and USAGE.md, cross-linked from README.

Test plan

  • npm test — 7/7 assertions pass

handler/, hook/, install/, and test/sample.jsonl were leftovers from
the terminal protocol-handler design the README already calls retired.
render/callgraph.js now exports buildDot/isTestRef so test/run.js can
exercise them directly (package.json's test script previously pointed
at a test/run.js that never existed).
Required before first push per doc-check hook.
buildDot(symbol, callers, callees) threw when called without callers/
callees now that it's a public export (previously safe only because
main() always passed || []). test/run.js aborted the whole suite
uncaught on the first failing assertion, silently skipping later
tests; each test body is now isolated in try/catch with a pass/fail
summary and non-zero exit on failure.
Removing install/install.ps1 and install/install.sh in the prior
cleanup dropped the only shipped uninstall path for anyone who had
already run them (HKCU registry key, Claude Code Stop hook entry,
~/.codeshot dir, and on macOS a Launch Services app registration).
These two scripts extract just the cleanup logic so those installs
can still be reversed; README links to them from the Status section.
runCodegraph() called JSON.parse on the CLI's stdout unconditionally,
but codegraph exits 0 with a plain info line (not JSON) for cases like
an unrecognized symbol — e.g. 'Symbol "X" not found'. That produced a
raw SyntaxError stack trace instead of the clean message USAGE.md
already promises for this exact scenario. Now surfaces codegraph's
actual output as a readable error and exits 1.
--path/--out consumed the next token unconditionally as their value.
`codeshot Sym --path --out out.png` (missing --path's value) set
repoPath='--out' and silently dropped the user's --out request instead
of erroring. Now validates the next token exists and isn't itself a
recognized flag before consuming it.
The default --out path and the temp .dot path both interpolated the
raw symbol name into a filename. A symbol like "Foo/Bar" produced
/tmp/callgraph-Foo/Bar-<ts>.dot, whose parent directory doesn't exist,
crashing with ENOENT. Filenames now use a sanitized copy of the
symbol (path separators and other filesystem-unsafe characters
replaced with '_'); the raw symbol is still used for the codegraph
query and the rendered graph label.
Install command, prerequisites, and the shell-out/graphviz rationale
were each copy-pasted verbatim into two or three docs. Each is now
stated once in README.md (the entry point) and linked from the
others. Also dropped TECHNICAL.md's "Deployment" section, whose only
content was "there is no deployment" restating the install command —
folded its one real fact (no service/rollback) into Maintenance
Commands, and trimmed External Dependencies' call-sequence prose
that duplicated the ASCII pipeline diagram above it.
isTestRef matched the raw substring "test" anywhere in a node's name
or filePath, misclassifying production code like AttestationService
or src/contest.js as test-only, while missing genuine test code under
a spec/ layout. Now checks word-boundary Test/Spec prefixes/suffixes
in the (camelCase-aware) name, plus test/tests/__tests__/spec
directory segments and .test./.spec. filenames in the path. Added
regression tests pinning the previously-misfiring cases.
@inth3shadows
inth3shadows merged commit d184dbe into master Jul 2, 2026
1 check passed
@inth3shadows
inth3shadows deleted the claude-proceed branch July 2, 2026 20:26
inth3shadows added a commit that referenced this pull request Jul 19, 2026
## What

The sharpest new-user papercut: point codeshot at a repo that codegraph
has never indexed, and you got a confusing raw wrapper —

```
codeshot: Command failed: codegraph query --path /repo --json --limit 1 -- hello
✗ CodeGraph not initialized in /repo
```

Now codeshot recognizes that state and hands back the exact fix:

```
codeshot: codegraph has no index for '/repo' yet — build one first with
'codegraph init /repo', then rerun. (codeshot reads codegraph's index; it doesn't create it.)
```

This is the sibling of the existing `matchSymbolNotFound` clean-message
handling, and it closes the #1 adoption-friction gap vs rivals that
parse source directly (madge/pyan/go-callvis need no index).

## The subtlety (found by running it, not reasoning)

The naive fix — check the message in `parseCodegraphOutput` — **doesn't
work**: an unindexed repo makes `codegraph query` exit **non-zero**, so
`execFile` *throws* before any stdout is parsed. The clean message has
to be caught at the **throw path** in `runCodegraph`, which recognizes
the not-initialized text (`matchNotInitialized`) and re-throws anything
else so a genuine codegraph failure isn't misreported as a missing
index. Covers both symbol and `--architecture` mode via the shared choke
point.

## Explicitly *not* doing

No auto-install of codegraph, no auto-`codegraph init`. Installing
software / building a heavy persistent index is codegraph's call, not a
diagram tool's — same detect-and-instruct stance as the existing
`requireOnPath`. (An opt-in `--init` flag is a possible *future*
candidate, never implicit.)

## Verified

- Live end-to-end against a real unindexed tmp repo → clean message,
exit 1, both modes.
- Connected symbols on codeshot's own indexed repo → no false trigger.
- **88 tests pass** (was 85): unit tests for `matchNotInitialized` /
`argRepoPath`, plus a guarded CLI test that runs the real binary against
a fresh unindexed tmp dir and asserts the clean message (not the raw
error).
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