Skip to content

CI: fail build when audit-harness citation markers leak into compiled output#1678

Open
marcin-kordas-hoc wants to merge 4 commits into
developfrom
feat/ci-audit-strip-check
Open

CI: fail build when audit-harness citation markers leak into compiled output#1678
marcin-kordas-hoc wants to merge 4 commits into
developfrom
feat/ci-audit-strip-check

Conversation

@marcin-kordas-hoc
Copy link
Copy Markdown
Collaborator

@marcin-kordas-hoc marcin-kordas-hoc commented May 25, 2026

Summary

Adds a defensive post-build scan to .github/workflows/build.yml that fails the workflow if internal audit-harness markers leak into compiled JS output.

The markers ([V<n>] citation tags and §Sources footers) are an internal convention used in spec drafts and agent prompts. They must never appear in shipped JS — if they do, something slipped from a comment/string literal in source into bundled output.

What changed

  • .github/workflows/build.yml: new Verify no audit-harness markers leaked into build output step, placed immediately after the existing Build (npm run bundle-all) step.
  • Greps dist/, commonjs/, es/ for the patterns and exits 1 with the offending path and line number on hit.
  • Skips gracefully if an output directory is absent so unrelated build failures aren't masked.

Patterns

Pattern Catches
\[V[0-9]+\] Citation markers like [V1], [V12]
§[[:space:]]*Sources Section heading §Sources (or § Sources)

Why

Tiny, self-contained guardrail. No new dependency on external repos or npm packages — just a few lines of bash with grep -rnE. Fires on the same matrix as the existing build (Node 20/22/24 across Linux/Windows/macOS), but since the step uses shell: bash it runs identically everywhere.

Test plan

  • Manually verified locally: synthetic dist/foo.js containing [V3] and §Sources triggers exit 1 with line-numbered output
  • Manually verified locally: clean JS in all three dirs exits 0
  • Manually verified locally: no output dirs present exits 0 (skip path)
  • YAML parse check passed
  • CI run on this PR is green against the real build output (no markers present today)

Note

Low Risk
Changes only CI workflow and bash verification scripts; no runtime library or auth/data paths are modified.

Overview
Adds a post-build CI gate so internal audit-harness tokens ([V<n>] citations and §Sources footers) cannot ship in compiled artifacts.

After npm run bundle-all, Verify no audit-harness markers leaked into build output runs scripts/marker-scan.sh over dist, commonjs, and es, failing the job when grep finds those patterns (including in *.js.map via sourcesContent). A follow-up step runs scripts/test-marker-scan.sh on one matrix slice (Node 22 + npm ci) so the live scan and synthetic fixtures stay aligned.

scripts/marker-scan.sh centralizes the scan (skips missing dirs, distinguishes grep “no match” vs I/O errors). scripts/test-marker-scan.sh asserts clean vs dirty fixtures across webpack dist, source maps, CommonJS, and ESM outputs.

Reviewed by Cursor Bugbot for commit b9a7ba2. Bugbot is set up for automated code reviews on this repo. Configure here.

… output

Adds a post-build scan step to `.github/workflows/build.yml` that
greps `dist/`, `commonjs/`, and `es/` for two internal-only marker
patterns:

- `\[V[0-9]+\]` — audit-harness citation markers used in spec drafts
- `§[[:space:]]*Sources` — section heading used in audit-harness footers

Both are conventions from the audit-harness tooling and belong in
internal docs/prompts only. If they ever appear in compiled JS it
means a comment or string literal slipped through from a spec draft
into shipped output — the scan fails the workflow with the offending
file path and line number.

The step runs after `npm run bundle-all` (which produces the three
output directories) and skips gracefully if a directory is missing,
so unrelated build failures aren't masked by this guardrail.

Manual verification:
- Synthesized `dist/foo.js` containing both markers — grep matched
  both lines and exited 1 with a clear message.
- Repeated with clean JS — grep exited 0.
- Repeated with no output dirs — step exited 0 (skip path).
@netlify
Copy link
Copy Markdown

netlify Bot commented May 25, 2026

Deploy Preview for hyperformula-docs ready!

Name Link
🔨 Latest commit 803f11a
🔍 Latest deploy log https://app.netlify.com/projects/hyperformula-docs/deploys/6a1406fc85be2a000849e0c8
😎 Deploy Preview https://deploy-preview-1678--hyperformula-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 25, 2026

Deploy Preview for hyperformula-dev-docs ready!

Name Link
🔨 Latest commit b9a7ba2
🔍 Latest deploy log https://app.netlify.com/projects/hyperformula-dev-docs/deploys/6a150b708772160008fdf0f5
😎 Deploy Preview https://deploy-preview-1678--hyperformula-dev-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread .github/workflows/build.yml Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

Performance comparison of head (b9a7ba2) vs base (508d78f)

                                     testName |   base |   head |  change
-------------------------------------------------------------------------
                                      Sheet A | 501.49 | 492.41 |  -1.81%
                                      Sheet B |  160.1 | 157.15 |  -1.84%
                                      Sheet T | 142.18 | 138.73 |  -2.43%
                                Column ranges |  531.9 |  527.8 |  -0.77%
Sheet A:  change value, add/remove row/column |  15.36 |  17.28 | +12.50%
 Sheet B: change value, add/remove row/column | 129.85 | 138.66 |  +6.78%
                   Column ranges - add column | 155.55 |  160.5 |  +3.18%
                Column ranges - without batch | 485.33 | 492.48 |  +1.47%
                        Column ranges - batch | 121.27 | 126.24 |  +4.10%

Bugbot review #3296952334 flagged that the `if grep ...` form treats grep's
exit code 2 (scan/IO error) identically to exit code 1 (no matches) — so a
permission or read error on dist/, commonjs/, or es/ would silently green-
light the step. Split the rc into 0/1/other and fail the step explicitly on
any non-zero, non-1 result.
Validates the build.yml marker-scan step against synthetic fixtures: clean
build, marker in dist/*.js, marker in dist/*.js.map (sourcesContent), marker
in commonjs/*.js, marker in es/*.mjs. Wired as a single self-test step in
build.yml that runs once per OS (node 22, ci install).

Empirically confirmed (probed by planting a marker in src/index.ts and
running `npm run bundle-all`) that source comments survive into:
  - commonjs/index.js and es/index.mjs (babel preserves comments)
  - dist/hyperformula{,.full}.js (webpack development build preserves comments)
  - dist/hyperformula.js.map (`sourcesContent` embeds full original source)
All three surfaces are inside the existing `grep -rn dist commonjs es` scope,
so the scan already covers source-maps. The new self-test pins this behavior
so a future bundler/comment-stripping change cannot silently erode coverage.
@marcin-kordas-hoc
Copy link
Copy Markdown
Collaborator Author

Tier-2 hardening: integration test for marker scan + source-map coverage

Added scripts/test-marker-scan.sh (5 synthetic fixture cases) and wired it into build.yml as a single self-test step (node 22, ci install, runs once per OS, <1s).

Empirical answer to the SFDIPOT P0 question — do source-maps carry source comments?

Yes. Probed by planting // [V99] test marker — empirical sourcemap probe in src/index.ts and running npm run bundle-all. The marker survived into THREE distinct surfaces:

File Hit count
dist/hyperformula.js 3
dist/hyperformula.full.js 3
dist/hyperformula.js.map 1 (inside sourcesContent)
commonjs/index.js 3
es/index.mjs 3

The existing grep -rn dist commonjs es step already catches all three because .map files are plain JSON and grep treats them as text. So the scope was already correct — but it was untested. This PR pins that coverage with a self-test that asserts each surface (clean / dist-js / dist-map-sourcesContent / commonjs / es) reaches the expected branch of the scan logic.

Also added an inline comment block in build.yml documenting the three surfaces, so future bundler/comment-stripping changes can't silently erode coverage without someone reading the rationale.

Verification (local):

  • scripts/test-marker-scan.sh — 5 passed, 0 failed
  • npm run bundle-all end-to-end — completes, publish-check OK
  • Real-build scan on clean source — rc=1 (no markers)

New head SHA: 108396574

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1083965. Configure here.

Comment thread .github/workflows/build.yml
…test cannot drift

The verify step in build.yml previously inlined the audit-marker grep logic
while scripts/test-marker-scan.sh kept its own duplicate copy. A workflow-only
edit could silently desynchronize the live scan from the self-test fixtures
that are supposed to guard it.

Move the scan into scripts/marker-scan.sh as a single parameterized entry
point (accepts paths as $@, exit 0=clean, 1=dirty, 2+=error). The workflow
step now invokes `bash scripts/marker-scan.sh dist commonjs es`, and the
self-test drives the SAME script against synthetic fixture roots.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (508d78f) to head (b9a7ba2).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1678   +/-   ##
========================================
  Coverage    97.16%   97.16%           
========================================
  Files          175      175           
  Lines        15319    15319           
  Branches      3356     3356           
========================================
  Hits         14884    14884           
  Misses         427      427           
  Partials         8        8           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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