fix(ci): unbreak self-test on quirk-def file + auto-track registry count#2
Merged
Merged
Conversation
Two long-standing red CI jobs on main, both rooted in the testkit exercising its own detection surface: * `composite action self-test`: bitbox-audit found legitimate E1 (non-ASCII) hits in `go/bitbox/simulator/scenarios.go` — that file IS the umlaut-reject scenario, so the matches are test data, not regressions. Marked with `audit-skip-file`, the same mechanism already used by `go/core/guards/*.go`. * `TypeScript unit tests`: hardcoded `expect(Registry.length).toBe(30)` drifted when A4 was added (registry has 31 quirks). Switched to a self-deriving assertion against `quirks.json`, mirroring how the Go side tests for a non-empty registry instead of a fixed count. Closes #1.
… SHA
The self-test passed `${{ github.sha }}` as testkit-ref to `go install`.
For PR events that resolves to the synthetic merge commit, which is
not in `refs/heads/*` and therefore unreachable to the Go module
resolver — every PR self-test exited with `unknown revision <sha>`.
Introduce a `local` sentinel in the action's testkit-ref input that
builds the CLI from the checked-out source. The self-test workflow
opts in; consumer repos keep using their pinned tag.
joshuakrueger-dfx
added a commit
to joshuakrueger-dfx/bitbox-testkit-1
that referenced
this pull request
May 17, 2026
- Drop the JSON \u-escape workaround in scenarios.go; the audit-skip-file marker TaprootFreak added in PR DFXswiss#2 is the right per-file opt-out for intentional non-ASCII test fixtures, and matches the pattern already used in core/guards/*.go. - Backfill CHANGELOG entries for v0.4.5 (Go module rename) and v0.4.6 (auto-tag + auto-release-pr + audit-skip-file). v0.5.0 entry now points at the DFXswiss release URL and references test.yaml (not test.yml). - ONBOARDING simulator example and ts/src/index.ts JSDoc now reference DFXswiss/bitbox-testkit consistently (ts/package.json was already at @DFXswiss).
5 tasks
TaprootFreak
added a commit
that referenced
this pull request
May 18, 2026
…nal-commits auto-tag (#5) * feat(simulator): add BTC + root-fingerprint + legacy-polygon-sign scenarios - BtcXpubZpubMainnet: BIP-84 native segwit zpub shape - BtcAddressP2WPKHMainnet: bech32 bc1q P2WPKH derivation - BtcAddressP2TRTaproot: bech32m bc1p P2TR derivation - BtcSignMessageMainnet: 64-byte sig + 65-byte electrum envelope - RootFingerprintDeterministic: pins 4c00739d (upstream fixture seed) - EthSignLegacyPolygonMultiByteV: actually exercises CC-5 v-byte path (the existing chainId=137 address probe never did — addresses don't depend on chainId) Plus simulator.Connect helper (extracted from cmd/bitbox-simulator-check) so the integration test, CLI, and any future consumer share the exact Noise XX + channel-hash-verify bring-up. Integration test now runs the full BaselineScenarios set on every push, surfacing any firmware drift or scenario regression at testkit CI time instead of consumer time. Fake TS proxy: add clearCalls, ignore symbol-keyed lookups, return undefined for then/catch/finally so awaiting the proxy does not infect chains as thenable. quirks.test.ts now reads quirks.json directly to stay self-consistent across releases instead of needing a hardcoded count bump every time. * fix(simulator): encode umlaut KYC payload as JSON \u-escapes The umlaut-rejection scenario's payload had three literal non-ASCII bytes (ü, ß, ü) in a raw-string Go const, which the audit's quirk-E1 regex flagged as a critical finding when self-auditing the testkit. Encoding them as JSON ü / ß keeps the SOURCE pure ASCII while the JSON parser inside the BitBox SDK still resolves them to the exact same UTF-8 bytes a literal "ü" would produce — the scenario still exercises the firmware reject path. Removes 3 false-positive critical findings from the testkit's own action-selftest job, and from any consumer who ever decides to point their bitbox-audit at the testkit source tree. * v0.5.0 prep: matrix-mode CLI + CI matrix + CHANGELOG backfill + ONBOARDING - bitbox-simulator-check gains --firmware <name|all>; LaunchVersion + ErrSimulatorNotFound let any caller pin a specific embedded build. - New CI job go-simulator-matrix drives the 14-scenario baseline against all 8 embedded firmwares (v9.19.0 → v9.26.1) in parallel on every push. Catches regressions that only surface on older firmwares still in the production tail — BitBox02 only auto-updates when the user opens the BitBoxApp. - bitbox-simulator composite action exposes firmware: input; slash template parses firmware=X and ref=Y modifiers + 'fail' shorthand. - Composite action defaults: bitbox-audit testkit-ref v0.2.0 → v0.5.0, bitbox-simulator v0.4.2 → v0.5.0. Workflow-templates bumped to match. - CHANGELOG backfilled for every version between v0.3.1 and v0.4.4 (previously only the v0.1.0/v0.2.0/v0.3.0/v0.3.1 entries existed) and the new v0.5.0 entry. - ONBOARDING gains a §6 simulator section covering the 14 baseline scenarios, matrix mode, slash trigger, and what the simulator validates vs. doesn't (transport still needs a real device). * align v0.5.0 with develop conventions - Drop the JSON \u-escape workaround in scenarios.go; the audit-skip-file marker TaprootFreak added in PR #2 is the right per-file opt-out for intentional non-ASCII test fixtures, and matches the pattern already used in core/guards/*.go. - Backfill CHANGELOG entries for v0.4.5 (Go module rename) and v0.4.6 (auto-tag + auto-release-pr + audit-skip-file). v0.5.0 entry now points at the DFXswiss release URL and references test.yaml (not test.yml). - ONBOARDING simulator example and ts/src/index.ts JSDoc now reference DFXswiss/bitbox-testkit consistently (ts/package.json was already at @DFXswiss). * feat(release): conventional-commits-aware auto-tag Replaces the hardcoded PATCH+1 logic in .github/workflows/auto-tag.yaml with a small testable Go tool at go/cmd/release-version. The tool reads every commit subject + body between the last release tag and HEAD, parses them as Conventional Commits 1.0, and picks the highest bump: feat! / <type>! / BREAKING CHANGE: footer -> MAJOR feat: -> MINOR fix:, perf:, refactor:, revert: -> PATCH chore:, ci:, docs:, test:, style:, build: -> PATCH non-conventional subjects -> PATCH + warning A single feat! anywhere in the range promotes the whole release to a major bump; a single feat: promotes to minor. The aggregator is pure: 31 table-driven tests in main_test.go lock every classification arm + the SemVer math + the report shape consumers parse. The auto-tag workflow now surfaces the per-commit breakdown as a CI group so reviewers can see exactly which commit voted which way, and short-circuits cleanly (exit code 4) when the range is empty. CONTRIBUTING.md "Releases" rewritten with the new policy: a commit-message -> bump table, the local preview command, and the manual-release escape hatch for hotfixes. Practical effect for v0.5.0: the feat(simulator): commit in this PR will cause the auto-tagger to emit v0.5.0 (not v0.4.7) when the develop -> main release PR merges, with no manual tag intervention. * maintainer-edit: fix broken CHANGELOG links + atomic dual-tag push CHANGELOG had 13 release links pointing at github.com/joshuakrueger-dfx/bitbox-testkit/releases/tag/vX.Y.Z, but that account no longer hosts releases — every linked page 404s. The v0.4.5 entry also pointed at DFXswiss for a release that doesn't exist yet. All historical release links now point at DFXswiss/bitbox-testkit consistently; the actual GitHub-Release backfill for v0.3.2 → v0.4.5 is a separate maintenance task and doesn't gate the v0.5.0 cut. auto-tag.yaml now uses `git push --atomic` for the vX.Y.Z + go/vX.Y.Z pair. Without it, a partial push (server-side ref protection trip, network blip on the second ref) could leave the repo with one tag present and the other missing — and the next auto-tag run would fail the "tag exists" check while consumers' `go install ...@vX.Y.Z` would still 404 on the missing submodule tag. The --atomic flag tells the server to apply both updates as a single transaction or neither. --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
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
1 participant
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
Resolves both red jobs in the
testworkflow onmain(#1):composite action self-test (no consumer)— bitbox-audit found legitimate E1 hits insidego/bitbox/simulator/scenarios.go. That file is the umlaut-reject scenario (quirk E1), so the umlauts are test data, not production strings. Marked withaudit-skip-file, the same per-file opt-out the audit-runner already uses forgo/core/guards/*.go.TypeScript unit tests—expect(Registry.length).toBe(30)drifted when A4 was added (registry is at 31). Swapped for a self-deriving assertion againstquirks.json, mirroring how the Go side already tests for a non-empty registry instead of a hardcoded count.Why this design
audit-skip-fileoverfail-on-findings: false: keeping the self-test in blocking mode is the point — if someone later writes a real violation in the testkit's own code, CI should catch it. The marker is per-file and the mechanism already exists.toBe(31): avoids the same drift next time a quirk lands.Local validation
go vet ./...+go test -race -timeout 60s ./..../scripts/sync-quirks.sh --check(cd ts && npx tsc --noEmit)(cd ts && npm test)bitbox-audit --repo .. --format json.github/**/*.y*ml)Out of scope
No CHANGELOG bump — this is a CI fix, not a behavior change. Action defaults and pinned tags untouched.