trunk-merge/pr-845/86eb4d27-07c3-4a5a-84a2-d53b4f95f273 - #852
Closed
trunk-io[bot] wants to merge 3 commits into
Closed
trunk-merge/pr-845/86eb4d27-07c3-4a5a-84a2-d53b4f95f273#852trunk-io[bot] wants to merge 3 commits into
trunk-io[bot] wants to merge 3 commits into
Conversation
The gate ran both linters via an inline moon command: command: 'bash -c "squawk …; rc=$?; sqruff …; rc2=$?; exit $(( rc | rc2 ))"' moon wraps every task command in its own bash -c "<command>". The command is itself bash -c "…$?…$(( rc | rc2 ))…" in double quotes, so the nested quotes collide: the outer (moon) shell expands $?, $rc, $rc2 and $(( rc | rc2 )) — all unset, so 0 — before the inner shell runs. The inner shell received a literal '…; rc=0; …; rc2=0; exit 0' and ran fail-OPEN: it printed every finding then unconditionally exited 0. The 'Fail-closed' comment was false; the gate never failed on anything. It went unnoticed because 0001_init.sql genuinely passes both linters. 0002_rls.sql (RIG-3106, PR #830) is the first migration to actually trip findings, and the gate swallowed them, reporting the moon (nix) job green. Rewrite the gate as a bun/TypeScript CLI matching the sibling inline-sql-gate: the exit-code combination is now real code (rule://scripts-ts-over-bash + the no-bash-gate CI task forbid this logic in bash) and unit-tested in index.test.ts, red-green on the exact regression (squawk clean + sqruff finds -> exit 1). moon.yml moves from language:nix/ci-group.nix to language:typescript/ci-group.bun; both linters are on PATH on every moon leg (ci.yml phase-two puts the devenv nixpkgs tools on PATH per running leg). Also drop the CP02 exclusion from .sqruff — it existed solely to suppress a false positive on 'USING GIN' — and write 0001_init.sql:311 as canonical lowercase 'USING gin', so all capitalisation rules CP01-CP05 stay live. Verified: gate passes on clean 0001, fails (exit 1) with 0002's findings present; 12 unit tests pass; typecheck + biome clean. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…3208) Address review findings on PR #845 (additive; the core fail-closed fix is unchanged): - Bun.spawn throws synchronously when a linter binary is missing (ENOENT), so a spawn failure escaped runLinter as an unhandled rejection (exit 1 + stack trace) instead of the documented code 2. Extract the spawn closure into an exported makeSpawnLinter() and wrap it in try/catch that maps a throw to a clean code-2 result, so the code-2-dominates safety path is real and the second linter still runs if the first is missing. - Root resolution fell back to an empty string when git reported no toplevel (which is exactly the case in a jj workspace, whose .git lives in the colocated clone). Fall back to process.cwd() instead of "" (moon runs the gate with runFromWorkspaceRoot:true, so cwd is the repo root in CI). - Guard the per-linter err() emission so a clean run no longer prints blank lines ahead of the OK verdict. - Tests: drive the real makeSpawnLinter for the empty-glob and missing-binary code-2 paths (previously only the pure combine was covered), and assert runOnce surfaces BOTH linters' outputs and emits no blank noise on a clean run. Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Compass engineering docs preview: https://trunk-merge-pr-845-86eb4d27.compass-eng-docs.pages.dev Deployed from |
trunk-io
Bot
deleted the
trunk-merge/pr-845/86eb4d27-07c3-4a5a-84a2-d53b4f95f273
branch
September 3, 2026 04:35
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.
This pull request was created and is being managed by Trunk Merge.
This pull request is based on the main branch at SHA 569da8b4e1e3cfdd60b557ad88249373c4c6c6df.
See more details here.
When CI completes, this pull request will be closed automatically.
Pull Requests Being Tested
This pull request is testing the changes from pull request 845.