PR 3a — audit binary substrate + extract/status subcommands#193
Merged
Conversation
This was referenced May 30, 2026
Owner
Author
Code reviewFound 4 issues:
code-audit-pipeline/internal/discovery/discovery.go Lines 49 to 63 in 4d385f0
code-audit-pipeline/internal/cli/status.go Lines 84 to 92 in 4d385f0
code-audit-pipeline/internal/cli/extract.go Lines 106 to 126 in 4d385f0
|
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
… drop ErrNoCommand Four code-review findings on PR 3a (#193): internal/discovery/discovery.go: candidate ordering put $AUDIT_HOME before cwd-relative, contradicting ADR-0006's documented "explicit flag, cwd-relative, then $AUDIT_HOME, then fallback" chain. A contributor with AUDIT_HOME set (which `audit init` plants in shell profiles) could never pick up local edits in ./pipeline/queries. Swapped the append order and added two tests (TestQueriesCwdBeatsAuditHome, TestExtractorsCwdBeatsAuditHome) locking in the precedence. internal/cli/status.go: a registered catalog whose file is missing on disk printed "MISSING" but did not set staleAny=true, so `audit status` exited 0 — directly contradicting its own doc comment ("exit 1 if any catalog is...not fresh"). Set staleAny=true in the !r.Exists branch. internal/cli/extract.go: when one [[command]] block in a multi-command manifest succeeded but a later one failed, the function returned 1 without calling cache.Save(), leaving meta.json out of sync with the catalog files on disk. Now: capture firstErr, break the loop, save the cache if any command succeeded, then propagate the error. Multi-command manifests (typescript has type-catalog + function-catalog) get an accurate meta.json even on partial failure. internal/extractor/runner.go: removed the exported ErrNoCommand sentinel — it had no consumer; the CLI checks `ran == 0` directly. Removing it also dropped the now-unused `errors` import.
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
Follow-up to PR 3a (#193). Implements the gojq evaluator and the `audit query` handler. internal/engine adds a gojq wrapper supporting -L, --arg, --argjson, --slurpfile, -n, -r, $ENV.NAME, plus a `#! engine: jq` shell-out fallback per ADR-0005. Library resolution works against both filesystem-rooted and embed.FS-rooted query directories. internal/cli/query.go parses each query's front-matter to validate --arg / --argjson and wires the catalog declarations into positional input + slurpfile bindings. Covers all seven distinct catalog combinations produced by PR 2's front-matter (including the two-of-same-kind cross-catalog-name-collisions case). cmd/audit/main.go re-enables the `query` case that PR 3a stubbed out, and cmd/audit/audit_test.go ships an integration test exercising extract → query → status end-to-end on a synthetic file-hashes fixture. go.mod pins itchyny/gojq v0.12.19 — the version verified parity-clean in #188.
First slice of the audit binary per tracker #177's PR 3 plan. Implements the substrate parsers (front-matter for .jq files, manifest.toml for extractor dirs, catalog envelope reader), per-repo .audit/ cache management per ADR-0001 amended by ADR-0007, lookup-order discovery for queries and extractors per ADR-0006, and subprocess invocation of extractors via each manifest's [[command]] blocks. Subcommands shipped: extract, status. The query subcommand stubs out with a message pointing at the follow-up PR (gojq engine). go.mod pins BurntSushi/toml v1.4.0. gojq lands in the follow-up. CI adds a `go` job running vet + test + build on the embedded queries (populated by `go generate ./...`). See plans/pr3-binary-skeleton.md for the full design; the split rationale is at the top of that file.
…st helpers Hook-flagged fixes: - ci.yml: my copy from a pre-rebase worktree clobbered the v6/v5 action pins from 841a99b. Restore actions/checkout@v6, actions/setup-node@v6, actions/cache@v5, and use them on the new go job too. - internal/cli/extract.go + status.go: discovery cwd was the user's audit-root (where catalogs live), but extractors/queries live next to the audit-pipeline source. Use the binary's actual os.Getwd() instead, with HomeDir set on the first call rather than a fallback retry. - internal/extractor/runner.go: drop hard-coded AUDIT_VERSION=skeleton env. No extractor reads it yet — add it back when a need lands. - internal/extractor/runner_test.go: replace lookNode/exec_LookPath/lookPath wrappers with a direct exec.LookPath("node") call. The "import cycle" comment was wrong; there's no cycle.
… drop ErrNoCommand Four code-review findings on PR 3a (#193): internal/discovery/discovery.go: candidate ordering put $AUDIT_HOME before cwd-relative, contradicting ADR-0006's documented "explicit flag, cwd-relative, then $AUDIT_HOME, then fallback" chain. A contributor with AUDIT_HOME set (which `audit init` plants in shell profiles) could never pick up local edits in ./pipeline/queries. Swapped the append order and added two tests (TestQueriesCwdBeatsAuditHome, TestExtractorsCwdBeatsAuditHome) locking in the precedence. internal/cli/status.go: a registered catalog whose file is missing on disk printed "MISSING" but did not set staleAny=true, so `audit status` exited 0 — directly contradicting its own doc comment ("exit 1 if any catalog is...not fresh"). Set staleAny=true in the !r.Exists branch. internal/cli/extract.go: when one [[command]] block in a multi-command manifest succeeded but a later one failed, the function returned 1 without calling cache.Save(), leaving meta.json out of sync with the catalog files on disk. Now: capture firstErr, break the loop, save the cache if any command succeeded, then propagate the error. Multi-command manifests (typescript has type-catalog + function-catalog) get an accurate meta.json even on partial failure. internal/extractor/runner.go: removed the exported ErrNoCommand sentinel — it had no consumer; the CLI checks `ran == 0` directly. Removing it also dropped the now-unused `errors` import.
Commit 22f00f0 added `{ flag = "--include-imports", when = "include_imports_enabled" }` to the TypeScript manifest after this branch's substrate commit. The manifest parser rejected the unknown `when` value, breaking `go test ./...`. Adds `include_imports_enabled` to the validWhen allowlist, plumbs IncludeImports through Args/whenSatisfied/recordArg, and wires `--include-imports` into `audit extract`.
jakebromberg
force-pushed
the
feat/pr3a-binary-substrate
branch
from
May 30, 2026 18:55
91f51c6 to
5e8a596
Compare
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
Follow-up to PR 3a (#193). Implements the gojq evaluator and the `audit query` handler. internal/engine adds a gojq wrapper supporting -L, --arg, --argjson, --slurpfile, -n, -r, $ENV.NAME, plus a `#! engine: jq` shell-out fallback per ADR-0005. Library resolution works against both filesystem-rooted and embed.FS-rooted query directories. internal/cli/query.go parses each query's front-matter to validate --arg / --argjson and wires the catalog declarations into positional input + slurpfile bindings. Covers all seven distinct catalog combinations produced by PR 2's front-matter (including the two-of-same-kind cross-catalog-name-collisions case). cmd/audit/main.go re-enables the `query` case that PR 3a stubbed out, and cmd/audit/audit_test.go ships an integration test exercising extract → query → status end-to-end on a synthetic file-hashes fixture. go.mod pins itchyny/gojq v0.12.19 — the version verified parity-clean in #188.
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
… drop ErrNoCommand Four code-review findings on PR 3a (#193): internal/discovery/discovery.go: candidate ordering put $AUDIT_HOME before cwd-relative, contradicting ADR-0006's documented "explicit flag, cwd-relative, then $AUDIT_HOME, then fallback" chain. A contributor with AUDIT_HOME set (which `audit init` plants in shell profiles) could never pick up local edits in ./pipeline/queries. Swapped the append order and added two tests (TestQueriesCwdBeatsAuditHome, TestExtractorsCwdBeatsAuditHome) locking in the precedence. internal/cli/status.go: a registered catalog whose file is missing on disk printed "MISSING" but did not set staleAny=true, so `audit status` exited 0 — directly contradicting its own doc comment ("exit 1 if any catalog is...not fresh"). Set staleAny=true in the !r.Exists branch. internal/cli/extract.go: when one [[command]] block in a multi-command manifest succeeded but a later one failed, the function returned 1 without calling cache.Save(), leaving meta.json out of sync with the catalog files on disk. Now: capture firstErr, break the loop, save the cache if any command succeeded, then propagate the error. Multi-command manifests (typescript has type-catalog + function-catalog) get an accurate meta.json even on partial failure. internal/extractor/runner.go: removed the exported ErrNoCommand sentinel — it had no consumer; the CLI checks `ran == 0` directly. Removing it also dropped the now-unused `errors` import.
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
Follow-up to PR 3a (#193). Implements the gojq evaluator and the `audit query` handler. internal/engine adds a gojq wrapper supporting -L, --arg, --argjson, --slurpfile, -n, -r, $ENV.NAME, plus a `#! engine: jq` shell-out fallback per ADR-0005. Library resolution works against both filesystem-rooted and embed.FS-rooted query directories. internal/cli/query.go parses each query's front-matter to validate --arg / --argjson and wires the catalog declarations into positional input + slurpfile bindings. Covers all seven distinct catalog combinations produced by PR 2's front-matter (including the two-of-same-kind cross-catalog-name-collisions case). cmd/audit/main.go re-enables the `query` case that PR 3a stubbed out, and cmd/audit/audit_test.go ships an integration test exercising extract → query → status end-to-end on a synthetic file-hashes fixture. go.mod pins itchyny/gojq v0.12.19 — the version verified parity-clean in #188.
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
Follow-up to PR 3a (#193). Implements the gojq evaluator and the `audit query` handler. internal/engine adds a gojq wrapper supporting -L, --arg, --argjson, --slurpfile, -n, -r, $ENV.NAME, plus a `#! engine: jq` shell-out fallback per ADR-0005. Library resolution works against both filesystem-rooted and embed.FS-rooted query directories. internal/cli/query.go parses each query's front-matter to validate --arg / --argjson and wires the catalog declarations into positional input + slurpfile bindings. Covers all seven distinct catalog combinations produced by PR 2's front-matter (including the two-of-same-kind cross-catalog-name-collisions case). cmd/audit/main.go re-enables the `query` case that PR 3a stubbed out, and cmd/audit/audit_test.go ships an integration test exercising extract → query → status end-to-end on a synthetic file-hashes fixture. go.mod pins itchyny/gojq v0.12.19 — the version verified parity-clean in #188.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First slice of the audit binary per tracker #177's PR 3 plan. Ships the substrate (front-matter / manifest / catalog-envelope parsers),
.audit/cache, discovery, extractor subprocess runner, and theextract+statussubcommands. Thequerysubcommand stubs out with a pointer to the follow-up PR.What this PR delivers
audit extract <name>— runs every[[command]]block inextractors/<name>/manifest.tomlas a subprocess; materializes catalogs into.audit/catalogs/<file>; updates.audit/meta.json; auto-bootstraps.gitignore.audit status— prints.audit/state, resolved queries/extractors source paths, source-mtime staleness, and root-vs-cwd warnings.audit query <name>— stub pointing at the follow-up PR.github.com/jakebromberg/code-audit-pipeline). PinnedBurntSushi/toml v1.4.0.gojqlands in the follow-up.go generate ./...copiespipeline/queries/*.jqintocmd/audit/queries/so the//go:embeddirective ships the canonical query corpus alongside the binary.gojob (vet + test + build).Architecture (per the ADRs)
.audit/meta.jsonschema withaudit_version,last_touched_at,root,catalogs[<kind>](path, source_sha, envelope_summary, cli_args).#! key: valuefront-matter on.jqfiles; TOMLmanifest.tomlon extractor dirs.os/exec.--flag→$AUDIT_HOME→ cwd-relative → embedded queries /~/.config/audit/extractors).meta.json.envelope_summaryis a derived cache; catalog file is authoritative; refreshed on everystatus/queryif the file's sha256 differs.Diff size note
~2.8k lines including ~490 lines of plan doc and ~800 lines of Go tests. Production Go is ~1100 lines across 12 files. Over the 1000-line soft limit, but coherent as one reviewable unit: every package is exercised by
extract+status. The follow-up PR (engine +query) is sized at ~800 lines.Test plan
go vet ./...passesgo test ./...passes (with-race)go build ./cmd/auditproduces a binaryaudit extract file-hashes --root <tmp> --audit-root <tmp>produces.audit/catalogs/file-hashes.jsonand a populatedmeta.jsonaudit status --root <tmp>reports cached catalogs; when invoked from the audit-pipeline checkout, it resolves queries+extractors via cwd-relative discovery and exits 0audit query <anything>exits 2 with the follow-up-PR messageCloses #191