chore(ci): ignore generated sidecar bundles in edge checks - #5926
Open
JacobisEpic wants to merge 3 commits into
Open
chore(ci): ignore generated sidecar bundles in edge checks#5926JacobisEpic wants to merge 3 commits into
JacobisEpic wants to merge 3 commits into
Conversation
|
@JacobisEpic is attempting to deploy a commit to the World Monitor Team on Vercel. A member of the Team first needs to authorize it. |
…its CLI (koala73#5926) Review findings on this PR. The approach verified clean under scrutiny -- selection is an exact set-equality match with the old pre-push rules, the pathspec cannot match apiv2/, out-names do not collide, and nothing under api/ is generated at deploy time -- so everything below is about the gate's own entry path and test surface. - tests/edge-functions.test.mjs registered one it() PER DISCOVERED FILE for both api-wide guards, with no non-empty floor. `git ls-files` exits 0 with empty output on a non-matching pathspec, so empty discovery emitted zero assertions and the suite went green while checking nothing -- a `node:` import smuggled into an edge function would have shipped unnoticed. The pre-PR readdirSync threw instead, so this was a fail-open the PR introduced. Adds a discovery floor mirroring the checker's own zero-entry guard. - The entry guard used `import.meta.url === pathToFileURL(path.resolve(argv[1]))`, which does not resolve symlinks while Node sets import.meta.url to the realpath. For an absolute symlinked argv[1] that returns false, main() never runs, and this merge-blocking gate exits 0 having bundled nothing. Not reachable from the current relative-path call sites, but the identical fail-open already shipped here once (koala73#4246). Switches to the repo's existing realpath-safe isMainModule(), as 7 other scripts already do. - Nothing executed the CLI. The suite called the exported functions directly, so main(), the entry guard, --list, and the exit code -- everything CI and .husky/pre-push actually invoke -- were untested, and the wiring tests only assert the command line appears in the workflow/hook text. Adds five subprocess tests, including one that invokes through a symlink. - isolatedGitEnv() was duplicated in the test file and had already drifted (the copy nulls GIT_CONFIG_GLOBAL/SYSTEM; the shipped gate does not). Exports the real one and layers the fixture-only overrides on top. - Factors the underscore rule behind one predicate, documenting why the two selectors apply it at different depths (each mirrors the surface it replaced). - AGENTS.md:209 still documented the edge check trigger as api/|server/| src/generated/ after the hook gained the checker-script path. Verified: edge-function-bundles 9 pass (was 4), edge-functions 241 (was 240), prepush-attest 35, ci-workflow-coverage 16, biome clean, and the real checker still passes on this repo at 48 tracked entrypoints. Three mutants confirmed killed -- reverting to the naive entry guard, gutting main() to a no-op, and making discovery return empty. Claude-Session: https://claude.ai/code/session_01GwG8L6jjf8wxjbUqAr23m5
Resolves the conflict blocking this PR. One real conflict, in tests/ci-workflow-coverage.test.mts: both sides appended a test at the same point in the same describe. They are unrelated -- this branch's "shares tracked edge bundle discovery with pre-push" and main's "routes Tauri config edits into the job that runs the one-binary gate (koala73#5908)" -- so both are kept. The other three files (.github/workflows/test.yml, .husky/pre-push, tests/edge-functions.test.mjs) auto-merged. Checked the merged result rather than trusting the auto-merge: the CI step and the pre-push branch both still invoke the shared checker, and no working-tree `find api/` or readdirSync discovery survived anywhere. Verified on the merged tree: edge-function-bundles 9 pass, edge-functions 250, prepush-attest 35, ci-workflow-coverage 19 (both conflicting tests present and passing), and the real checker passes at 50 tracked entrypoints -- up from 48, main having added two api/ entries. Claude-Session: https://claude.ai/code/session_01GwG8L6jjf8wxjbUqAr23m5
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
2 participants
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
Fixes #5920.
This PR centralizes edge-function candidate discovery in
scripts/check-edge-function-bundles.mjsand makes both pre-push and CI use that checker.The checker uses Git's tracked API inventory, which represents the files that can ship in a commit, rather than arbitrary working-tree JavaScript left by local desktop builds.
The sidecar builder, output locations, route table, packaging, and runtime behavior remain unchanged.
Type of change
Affected areas
/api/*)Reproduction and root cause
The exact pre-push esbuild command selected 48 candidates and passed before sidecar generation.
Running
node scripts/build-sidecar-handlers.mjsproduced 34 ignored Node bundles with this signature:The same pre-push command then selected 82 files and failed with 36 browser-bundling errors, including unresolved
node:cryptoimports from the generated bundles.The recursive API source invariant test and the CI edge loop also failed when run after generation.
Deleting only the 34 verified ignored bundles restored the original 48-candidate pass.
CI previously avoided the failure only because
npm run test:dataand the inline edge loop ran before the sidecar handler build in theunitjob.Filtering contract
The canonical selector now uses
git ls-files -z -- api, sorts deterministically, and applies the existing edge-entry rules to tracked paths.Tracked files are the correct deployment contract because an untracked file cannot reach CI or Vercel through a push.
This cannot hide a legitimate committed edge function: tracked JavaScript remains selected even when it has a same-basename TypeScript sibling, and a tracked similarly named
api/v2/shipping/[rpc].jsfixture remains selected.The current repository has zero tracked JavaScript/TypeScript sibling pairs under
api/.The checker passes path arrays directly to esbuild, assigns extension-specific output names so legitimate JS/TS siblings cannot collide, cleans temporary output on success and failure, and fails if zero candidates are selected.
Pre-push, CI, and
tests/edge-functions.test.mjsshare this tracked inventory.Regression coverage
tests/edge-function-bundles.test.mjscreates isolated temporary Git repositories and verifies:[rpc].jsentry is not hidden;Workflow and pre-push wiring tests verify both surfaces call the shared checker and no working-tree
find apidiscovery remains.Mutation results
api/domain/v1/[rpc].jsmade the candidate assertion fail and the real checker fail onnode:crypto.api/health.jsmade both the exact candidate assertion and real-checker inclusion assertion fail.Both mutations were restored before final validation.
Validation
node scripts/check-edge-function-bundles.mjspassed with 48 tracked entrypoints.node:cryptoimport in trackedapi/health.jsmade the checker exit 1, then the file was restored to its original SHA-256.node --test tests/edge-function-bundles.test.mjspassed 4/4.node --test tests/edge-functions.test.mjspassed 240/240 with generated residue present.node --test tests/prepush-attest.test.mjspassed 35/35.node --import tsx --test tests/ci-workflow-coverage.test.mtspassed 16/16.timeoutshim.npm run test:sidecarpassed 306/306.node scripts/build-sidecar-sebuf.mjscorrectly reported the historical gateway as removed.node scripts/build-sidecar-handlers.mjsbuilt all 34 domain handlers.npm run typecheckpassed.npm run typecheck:apipassed.npm run lintexited 0; changed files are Biome-clean, and the repository-wide command reported only existing warnings outside this patch.bash -n .husky/pre-pushpassed.git diff --checkandgit diff --cached --checkpassed..husky/pre-pushexecutable passed all scoped gates and 319 changed/wiring tests on the committed tree.Maintainer-style adversarial review
Checklist
api/rss-proxy.jsallowlist (if adding feeds) - N/Anpm run typecheck)Documentation Alignment Checklist
N/A.
This PR does not publish or change documentation claims, API contracts, generated docs, examples, Redis keys, CII, CRI, news, digest, or briefing behavior.
Screenshots
N/A.
This is a tooling-only change with no user-interface impact.