You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The tsx CLI (whether launched as bunx tsx, npx tsx or a global tsx) opens a Unix-socket IPC server ($TMPDIR/tsx-<uid>/<pid>.pipe) before it runs the script. In sandboxes that deny socket binding, everything a git hook launches that way exits with listen EPERM: the two lint-staged fences block every .ts/.js commit, and the pre-push validator blocks every push. node --import tsx runs the same script on Node through the tsx loader, which opens no socket.
The diff:
package.json: the lint-staged entries for script/deploy/safe/funnel-fence.ts and script/utils/node-runtime-fence.ts start with node --import tsx. They are still the third and fourth steps, after prettier --write and oxlint and before typecheck-files.sh. lint:funnel and lint:node-runtime are changed the same way and are still exactly the lint-staged command plus . and script tasks.
.husky/pre-push: validateScripts.ts launches with node --import tsx.
enforceProposalFunnel.yml, validateScripts.yml: a setup-node step (Node 22) before the fence runs. node --import needs Node ≥20.6, and on a runner with no Node, bun run puts its own node shim on PATH and would run the fence on Bun.
Each fence test file gets one test that spawns the lint-staged command with TMPDIR set to a path longer than a Unix socket path may be (104 bytes on macOS, 108 on Linux) and asserts a clean run. Any launcher that binds a socket fails it: with the entries switched back to bunx tsx, both tests fail with listen EINVAL.
.agents/rules/200-typescript.md: the launcher exception covers what git hooks run, and names the tsx CLI (not bunx) as what opens the socket.
How each criterion is met:
Order: the existing "runs after the fixers and before the type check" test checks it.
Lockstep: the existing lint:funnel / lint:node-runtime equality tests check it.
Runs without a bindable socket: the new tests check it.
Commit still refused on a violation: the existing "fires through the lint-staged entry" tests spawn the new command on a violating file and assert the refusal message.
Run locally:
bun test script/deploy/safe/propose-funnel-fence.test.ts script/utils/node-runtime-fence.test.ts script/utils/validateScriptsWorkflow.test.ts: all pass.
node --import tsx ./script/utils/validateScripts.ts --base origin/main --head HEAD with an unbindable TMPDIR: passes.
bash script/utils/typecheck-files.sh, bunx oxlint --type-aware, bunx prettier --check on the changed files, and actionlint on the two workflows: clean.
The pre-commit and pre-push hooks ran the new commands on the last push.
The other bunx tsx package scripts are left as they are, as the ticket specifies.
Checklist before requesting a review
I have performed a self-review of my code
This pull request is as small as possible and only tackles one problem
I have added tests that cover the functionality / test the bug
bunx tsx opens an IPC socket before running the script, so the
pre-commit fences die with listen EPERM wherever socket binding is
denied. node --import tsx runs the same script on Node without it.
Refs: EXSC-1219
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: bc22f158-7f71-4fc1-b6fe-9bb8732917fc
Commits
Reviewing files that changed from the base of the PR and between bd7e68b and 3816f27.
Files selected for processing: 4
.github/workflows/enforceProposalFunnel.yml
.github/workflows/validateScripts.yml
script/deploy/safe/propose-funnel-fence.test.ts
script/utils/node-runtime-fence.test.ts
No actionable comments were generated in the recent review.
Walkthrough
The lint fences and pre-push validator now use node --import tsx. Workflows set up Node 22. Integration tests run the configured commands with a long TMPDIR path.
The lint fences, staged commands, and pre-push validator use node --import tsx. Both workflows set up Node 22 before Bun.
Test commands with long TMPDIR script/deploy/safe/propose-funnel-fence.test.ts, script/utils/node-runtime-fence.test.ts
The tests run configured commands with a long TMPDIR path. They check for successful execution and, in the Node-runtime test, absence of a listen error.
lifinance/contracts#2584: Updates TypeScript runtime guidance, which this change further specifies for lint fences.
Merge Risk: Minimal · previous review e9262
The launchers have the required Node version and dependency. No merge-blocking issue was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description check
The description includes the Linear task, implementation rationale, detailed change summary, test coverage, validation results, and the required checklists. Reviewer-only checklist items are appropria…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Title check
The title "fix: launch git-hook TS scripts with node --import tsx" matches the main change. The PR switches the lint fences, the pre-push validator, and the related package scripts from bunx tsx to …
Full details: Description check
Explanation
The description includes the Linear task, implementation rationale, detailed change summary, test coverage, validation results, and the required checklists. Reviewer-only checklist items are appropriately left for reviewer completion.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (4 skipped: 4 unsupported.)
Full details: Title check
Explanation
The title "fix: launch git-hook TS scripts with node --import tsx" matches the main change. The PR switches the lint fences, the pre-push validator, and the related package scripts from bunx tsx to node --import tsx. The stated reason is to avoid the tsx CLI's Unix-socket listener, which can fail with listen EPERM in restricted sandboxes. The title is specific and concise, and it uses the conventional fix: prefix. It does not name the sandbox or socket cause, but it does not need to cover every detail.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
🧪 Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 472a4382-9790-4d32-8929-a97fbd3a6255
Commits
Reviewing files that changed from the base of the PR and between e668692 and e9262d8.
Files selected for processing: 6
.agents/rules/200-typescript.md
.github/workflows/enforceProposalFunnel.yml
.github/workflows/validateScripts.yml
.husky/pre-push
script/deploy/safe/propose-funnel-fence.test.ts
script/utils/node-runtime-fence.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
The independent review found 1 minor nit. None is fixed unless you pick it.
script/deploy/safe/propose-funnel-fence.test.ts:229 The diff adds a new test to each fence test file; the ticket only requires the existing tests to hold unedited. Existing tests are untouched, so this is a small scope expansion and a useful regression guard.
Proposed fix: Optional: keep, or drop if no test-file change is wanted.
Reply fix 1, 3 to have those fixed, fix all, or skip. Anything else you write is handled as ordinary review feedback.
The pre-push hook ran validateScripts.ts through the tsx CLI, which binds
an IPC socket under $TMPDIR, so pushes failed in sandboxes that deny
socket listen. The fence tests now spawn the staged command with a TMPDIR
too long for a socket path instead of pinning the command string, and
the two CI fence jobs set up Node 22 so `node --import` never runs on an
older runner Node or on Bun's node shim.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gvladika
changed the title
fix: launch the pre-commit lint fences with node --import tsx
fix: launch git-hook TS scripts with node --import tsx
Oct 28, 2026
Delta since the last approval (b1ad022 → 3816f27, 1 commit; 2 files): the two "no IPC socket" regression tests address the reviewer's nits. No source, workflow or hook file changed.
Resolution status of the prior review
The prior review had 0 findings. The two reviewer nits that dismissed an earlier approval are both fixed:
✅ propose-funnel-fence.test.ts and node-runtime-fence.test.ts: the TMPDIR segment is now 200 characters, so the test also fails on macOS with a recent Node (which binds socket paths up to about 250 bytes). The comment states the 108-byte Linux limit and the macOS behaviour.
✅ node-runtime-fence.test.ts now asserts the 1 file(s) checked, 0 refused line, as the funnel test does. It sets NODE_ENV=development for that child only, because bun test sets NODE_ENV=test and consola drops the success line under it. The shared spawnEnv is left alone.
What was verified
The 200-character directory is mkdtemp'd under tmpdir() and removed in finally; a 200-character component stays under the filesystem's 255-byte name limit.
The added assertion matches the fence's real output text and does not weaken the existing not.toContain('listen') check.
CI on the head passes (run-ts-tests, validate-scripts, enforce-proposal-funnel, CodeQL, Aikido). protect-critical-code is red; it is the maintainer-approval gate.
Tests only in this delta; the PR itself touches the git-hook fence launchers, two CI workflows and docs. No src/ change, so no audit entry. No sibling PRs.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One thing to sort out on rebase: #2607 moves every workflow to setup-node with .nvmrc (Node 26). This PR adds a node-version: 22 step in enforceProposalFunnel.yml and validateScripts.yml. If it lands second, those jobs run on Node 22 while engines says >=26. Suggest dropping the step here, or using node-version-file: .nvmrc.
Thanks — switched both Set up Node steps to node-version-file: .nvmrc in b1ad022 (kept the step, since the fence launches with node --import). Whichever of this and #2607 lands second should drop the duplicate Set up Node step in enforceProposalFunnel.yml / validateScripts.yml, keeping it before Set up Bun.
Lengthen the TMPDIR segment to 200 chars so a socket-binding launcher
fails on macOS (recent Node binds up to ~250 bytes there), and assert
the node-runtime fence's summary line so a silent exit 0 cannot pass.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…8) (#2645)
* test(fence): call withheldEnv in the no-IPC-socket fence test
#2618 renamed spawnEnv to withheldEnv while #2597 added a test calling
spawnEnv, so main fails with "spawnEnv is not defined".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(fence): build the no-IPC-socket child env with freshChildEnv
A copy of the test env carries the developer's NODE_OPTIONS into the
child, where --inspect would print to stderr and fail the assertion.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(fence): build fence-test child envs with freshChildEnv
Both fence suites now spawn their direct `node --import tsx` children
from freshChildEnv, and propose-funnel-fence drops its local spawnEnv
helper. The `bun lint:funnel` spawn keeps withheldEnv because bun
reloads `.env` for unset names. The node-runtime refusal case pins
NODE_ENV=test, which it previously inherited from `bun test`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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
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.
github-actions
Bot
added
requires-types
![lifi-qa-agent[bot]](https://avatars.githubusercontent.com/in/3411599?s=60&v=4){kind=small}
lifi-qa-agent
Bot
added
QA AI Approved
thesems
left a comment
Which Linear task belongs to this PR?
EXSC-1219
Why did I implement it this way?
Why it is implemented this way
The tsx CLI (whether launched as
bunx tsx,npx tsxor a globaltsx) opens a Unix-socket IPC server ($TMPDIR/tsx-<uid>/<pid>.pipe) before it runs the script. In sandboxes that deny socket binding, everything a git hook launches that way exits withlisten EPERM: the two lint-staged fences block every.ts/.jscommit, and the pre-push validator blocks every push.node --import tsxruns the same script on Node through the tsx loader, which opens no socket.The diff:
package.json: the lint-staged entries forscript/deploy/safe/funnel-fence.tsandscript/utils/node-runtime-fence.tsstart withnode --import tsx. They are still the third and fourth steps, afterprettier --writeandoxlintand beforetypecheck-files.sh.lint:funnelandlint:node-runtimeare changed the same way and are still exactly the lint-staged command plus.andscript tasks..husky/pre-push:validateScripts.tslaunches withnode --import tsx.enforceProposalFunnel.yml,validateScripts.yml: asetup-nodestep (Node 22) before the fence runs.node --importneeds Node ≥20.6, and on a runner with no Node,bun runputs its ownnodeshim on PATH and would run the fence on Bun.TMPDIRset to a path longer than a Unix socket path may be (104 bytes on macOS, 108 on Linux) and asserts a clean run. Any launcher that binds a socket fails it: with the entries switched back tobunx tsx, both tests fail withlisten EINVAL..agents/rules/200-typescript.md: the launcher exception covers what git hooks run, and names the tsx CLI (notbunx) as what opens the socket.How each criterion is met:
lint:funnel/lint:node-runtimeequality tests check it.Run locally:
bun test script/deploy/safe/propose-funnel-fence.test.ts script/utils/node-runtime-fence.test.ts script/utils/validateScriptsWorkflow.test.ts: all pass.node --import tsx ./script/utils/validateScripts.ts --base origin/main --head HEADwith an unbindableTMPDIR: passes.bash script/utils/typecheck-files.sh,bunx oxlint --type-aware,bunx prettier --checkon the changed files, andactionlinton the two workflows: clean.The other
bunx tsxpackage scripts are left as they are, as the ticket specifies.Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)
🤖 Generated with Claude Code