Skip to content

docs: the SDK's CLI compatibility job is opt-in; smoke goldens stop pinning the tool version - #491

Merged
bomly-guy merged 2 commits into
mainfrom
claude/cli-compat-opt-in
Oct 4, 2026
Merged

bomly-guy merged 2 commits into
mainfrom
claude/cli-compat-opt-in

Conversation

@bomly-guy

@bomly-guy bomly-guy commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Follows bomly-dev/bomly-sdk#107.

  • Docs. bomly-sdk#107 made its CLI compatibility job run only on SDK pull requests labelled cli-compat. The full CLI suite took 435s, against 26s for the SDK's own tests. This PR corrects the sentence in AGENTS.md that said the job runs on every SDK pull request.
  • Smoke fix (348f856).
    • The approval-triggered smoke run failed in every slice that diffs a scan golden: the goldens pinned run.tool.version at 0.26.0, and the build now stamps 0.27.0.
    • The normalizer now scrubs run.tool.version like the run's id and timestamps, so a release can no longer break the goldens. The 48 affected goldens carry the placeholder.
    • Locally, the SBOM and lite smoke cases pass. The full matrix runs again on re-approval.

🤖 Generated with Claude Code

bomly-dev/bomly-sdk#107 runs it only with the cli-compat label.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: bomly-dev/bomly-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 11989450-b5fb-4c0d-92f6-cad280d20035
📥 Commits

Reviewing files that changed from the base of the PR and between 66a2b6e and e161b78.

📒 Files selected for processing (1)
  • AGENTS.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

AGENTS.md now documents that local SDK commands use the checked-out SDK for builds, tests, and plugin fixture compilation. It also states that the CI CLI compatibility job runs when an SDK pull request has the cli-compat label.

Changes

SDK and CLI compatibility instructions

Layer / File(s) Summary
Local SDK and CI compatibility instructions
AGENTS.md
The instructions describe local SDK use for builds, tests, and plugin fixture compilation. They state that the CI compatibility job requires the cli-compat label.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to e161b

The documented local commands and label-gated compatibility job match the inspected behavior; no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the opt-in CLI compatibility job, which is the main change. The claim about smoke goldens is not supported by the provided change summary.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Bomly Diff Summary

Compared 66a2b6e26676effb44f843d9563658770f0fae69 to 348f856510fae62f39b6d50b7bc46520d98f77ad.

Overview

Status Manifests Dependencies Findings Duration
✅ Pass +0 / ~0 / -0 0 added / 0 version changed / 0 detail changes / 0 removed 0 introduced / 0 persisted / 0 resolved 1m 53s

Dependency Changes

✅ No dependency changes.

Vulnerabilities

✅ No vulnerability changes.

License Changes

✅ No license changes.

Project Posture

✅ No project posture changes (--matchers +scorecard was not selected).

Policy Findings

✅ No policy differences were identified.

@bomly-guy bomly-guy left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Ahmed3lmallah
Ahmed3lmallah previously approved these changes Oct 4, 2026
run.tool.version is stamped from the latest release tag, so the v0.27.0
release made every scan golden that pinned 0.26.0 fail. The normalizer now
treats it like the run's id and timestamps, and the 48 goldens carry the
placeholder instead.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@bomly-guy bomly-guy changed the title docs: the SDK's CLI compatibility job is opt-in docs: the SDK's CLI compatibility job is opt-in; smoke goldens stop pinning the tool version Oct 4, 2026
@bomly-guy
bomly-guy merged commit 991c9bb into main Oct 4, 2026
40 checks passed
@bomly-guy
bomly-guy deleted the claude/cli-compat-opt-in branch October 4, 2026 01:33
bomly-guy added a commit that referenced this pull request Oct 4, 2026
The catalog stored a SHA-256 for every fixture and expected-result file a
claim names. Those files are regenerated routinely, so every golden update
left the catalog stale, and the `catalog-valid` gate then refused the next
release until someone rewrote the checksums.

The previous commit on this branch did that rewrite. This one removes the
reason for it.

A file in this repository is already content-addressed by git: a release tag
and a path identify exact bytes, and the report records the commit. The stored
checksum was a second, hand-kept copy of what git knows. It could not catch
anything git does not, and the only thing it could do was fall behind.

It could also not be kept in step by checking earlier. `make test` already
verified the checksums on every pull request. #491 changed goldens and #398
added the catalog; each passed CI on its own, they touched different files so
they merged cleanly, and `main` failed the gate. A refresh step in `Update
Smoke Goldens` only helps when that workflow is what changes a golden.

So the catalog names files by path only, `catalog-validate --refresh` and its
step in the goldens workflow are gone, and that workflow no longer needs Go.
What is checked is what can actually go wrong: a claim naming a file that was
renamed or deleted fails `make assurance-catalog` and
`TestRepositoryCatalogIsValid` in `make test`, and both refuse a run that
checked no files at all (ADR-0044).

Still pinned, because this repository's history cannot address them: a git
input by revision and a container by digest.

The catalog and report keep their schema versions. No report has been
published, the site does not read the removed field, and the catalog has no
reader outside this repository. ADR-0047 carries a dated amendment.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
bomly-guy added a commit that referenced this pull request Oct 4, 2026
…sum (#492)

* fix(assurance): refresh the catalog checksums #491 invalidated

#491 rewrote smoke goldens so they stop pinning the tool version, and landed
on main between the last sync of #398 and its merge. The two touched different
files, so they merged cleanly, but the catalog's claims pin the checksum of the
golden that backs each of them, and fifteen of those now named a file that had
changed underneath them.

The first `Release prerequisites` run on main caught it: `catalog-valid` failed
on `finding-baseline-workflow.golden.json`. That is the gate working as
designed, and it would have refused the next tag.

Refreshed with `catalog-validate --refresh`; no claim changed, only the
recorded checksums.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* fix(assurance): a claim names its files by path, with no stored checksum

The catalog stored a SHA-256 for every fixture and expected-result file a
claim names. Those files are regenerated routinely, so every golden update
left the catalog stale, and the `catalog-valid` gate then refused the next
release until someone rewrote the checksums.

The previous commit on this branch did that rewrite. This one removes the
reason for it.

A file in this repository is already content-addressed by git: a release tag
and a path identify exact bytes, and the report records the commit. The stored
checksum was a second, hand-kept copy of what git knows. It could not catch
anything git does not, and the only thing it could do was fall behind.

It could also not be kept in step by checking earlier. `make test` already
verified the checksums on every pull request. #491 changed goldens and #398
added the catalog; each passed CI on its own, they touched different files so
they merged cleanly, and `main` failed the gate. A refresh step in `Update
Smoke Goldens` only helps when that workflow is what changes a golden.

So the catalog names files by path only, `catalog-validate --refresh` and its
step in the goldens workflow are gone, and that workflow no longer needs Go.
What is checked is what can actually go wrong: a claim naming a file that was
renamed or deleted fails `make assurance-catalog` and
`TestRepositoryCatalogIsValid` in `make test`, and both refuse a run that
checked no files at all (ADR-0044).

Still pinned, because this repository's history cannot address them: a git
input by revision and a container by digest.

The catalog and report keep their schema versions. No report has been
published, the site does not read the removed field, and the catalog has no
reader outside this repository. ADR-0047 carries a dated amendment.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* fix(assurance): report goldens are checked out byte for byte

The first prerequisites run failed TestReportGoldens on Windows only. The
goldens are compared byte for byte, and a Windows checkout rewrites text
files to CRLF; converting a golden to CRLF locally reproduces the same
"golden differs" failure.

The assurance testdata is marked -text so it is checked out exactly as
committed on every platform. Scoped to this directory on purpose: the rest of
the repository's testdata has passed on Windows as it is, and changing how it
is checked out is not this change's business.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* fix: three failures the unit suite had only on Windows

Nothing ran the unit suite on Windows except the portable stability workflow,
which was manual and last ran in July. Making it a release gate ran it again,
and three tests had gone red there in the meantime.

- SwiftPM published its evidence patterns backslash-separated on Windows. The
  candidate list was built with filepath.Join and is used both to read files
  and as the detector's evidence patterns, so the support matrix and `bomly
  plugin list` said different things depending on the machine. The list is
  slash-separated now; every read already joins an entry onto a directory with
  filepath.Join, which converts the separators for the host.
- The SBOM detector test looked for a file path inside an error that quotes
  it. Quoting doubles a Windows path's backslashes, so the bare path was not a
  substring there. It looks for the quoted path.
- The file-cache contract test asserted Unix permission bits. Windows has
  none: Go reports 0777 for every directory whatever mode it was created with.
  The mode assertions are Unix-only; the containment half of the contract is
  still checked everywhere.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* perf(assurance): the release stage fuzzes four targets at a time

Thirty fuzz targets ran one after another at 45 seconds each, plus a build and
a baseline pass per target. That is over half an hour, and it made fuzzing the
slowest job in the release prerequisites stage by a wide margin.

`scripts/run-fuzz.sh` takes FUZZ_JOBS and runs that many targets at once. Each
target is its own `go test` process, so they parallelize cleanly: workers are
shared out evenly instead of every fuzzer claiming every CPU, each target's
output is printed whole rather than interleaved, and every target is still
recorded. The default stays one at a time, so `make fuzz` and the nightly run
are unchanged.

The prerequisites stage asks for four at a time and thirty seconds each. Run
locally with a three-second budget, all thirty targets finish in 77 seconds.
A failing target still fails the run and is recorded in both modes.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* docs(assurance): say when a schema version starts to bind, and guard it

Review pointed out that removing the per-file checksum from report claims
without raising the report's schema version contradicts the rule this
framework wrote down for itself: only an optional addition may keep a version.

The rule was stated without its starting point. A version names a shape
someone already holds, and for the report that begins with the first report
committed to docs/assurance/reports -- from then on bomly.dev has mirrored a
document of that shape. No report has been published, the site never read the
removed field, and its sync does not validate it, so there was no reader to
break. Raising the version would have made the first report ever published a
"v2" beside a "v1" that never existed.

The developer guide now says that in so many words, and the ADR amendment
records it. More usefully, TestPublishedReportsParse makes it mechanical:
every committed report is read with the current parser, which rejects unknown
fields, so once a report exists a change that strands it fails in `make test`.
Checked by mutation -- a report in the current shape passes, one carrying the
removed field fails, and an index naming a missing report fails.

Also marks the sentence in ADR-0047 that the amendment supersedes, so a reader
does not have to reconcile the two.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* test(assurance): published reports must round-trip byte for byte

The guard added in the previous commit had two holes, both found in review.

It parsed only the reports the index lists, so a report file the index did not
mention was never read. And it relied on the strict parser alone, which
rejects a removed or renamed field but not a newly required one: such a field
decodes silently as its zero value, so an incompatible addition under the same
schema version passed.

Both close on one property. Every file in docs/assurance/reports must parse
under the current schema and encode back to exactly the bytes it was read
from. A removal or rename fails the parser. A newly required field appears in
the re-encoding and the bytes differ. An added optional field is omitted when
empty, so it passes -- which is precisely the one change allowed to keep a
version. The check asks the encoder instead of listing fields, so it cannot
fall behind the struct. The directory and the index must also agree in both
directions.

The same property is asserted on the fixture reports, so it is exercised
before the first report is published.

Checked by mutation: a current-shape report passes; a field added without
omitempty fails; the same field with omitempty passes; an unlisted report
fails, and is still read when its shape is broken; a listed report that is
missing fails.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

* test(assurance): remove the published-report shape guard

This pull request fixes a stale-checksum failure. In its first review round a
reviewer noted that dropping a report field without raising the schema version
contradicted the written rule, so a guard was added to hold that rule
mechanically. Every round since has been about the guard:

- round two: it skipped report files the index did not list, and it let a
  newly required field through, because such a field decodes as a zero value;
- round three: it did not cover the index, and its byte comparison would fail
  on a Windows checkout, which rewrites line endings.

The last one is the reason to stop. That failure would land in the portable
suite, which gates a release, as soon as the first real report was committed.
A guard that byte-compares published data had become a release risk in its
own right, and it was being refined in place of the change it was attached to.

So it is removed, and the reasoning is recorded where the next person will
look: beside TestSchemaVersionsArePinned and in the developer guide. The rule
itself is unchanged and is still stated there, including when a version starts
to bind. It is held by the pinned-version test, by the report goldens, where
any change of shape shows up as a diff in review, and by the reviewer. A
mechanical guard, if wanted, is its own change, and should compare the
report's JSON schema against the one recorded at the first published release
rather than committed bytes.

Co-Authored-By: Claude Fable 5.1 <[email protected]>

---------

Co-authored-by: Claude Fable 5.1 <[email protected]>
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