Skip to content

fix(link): preserve literal SSH include names on Bun 1.4.4 - #7629

Merged
lidge-jun merged 17 commits into
lidge-jun:devfrom
luvs01:fix/bun144-ssh-include-globs
Oct 26, 2026
Merged

lidge-jun merged 17 commits into
lidge-jun:devfrom
luvs01:fix/bun144-ssh-include-globs

Conversation

@luvs01

@luvs01 luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Preserve OpenSSH Include filename matching when Bun interprets parentheses or braces as glob extensions. For example, Include conf.d/*(1).conf must discover backup(1).conf rather than select 1.conf.

Quote opening parentheses and braces outside complete bracket classes while preserving escapes, bracket syntax, and unmatched suffixes. Path resolution, sorting, recursion limits, and conditional-Include handling remain unchanged. This is a literal-punctuation compatibility fix, not complete OpenSSH glob equivalence or SSH path isolation.

This PR is stacked on #7706, which fixes the port-fixture assertion that failed in this PR's earlier CI. That is a validation prerequisite, not a production dependency of SSH matching. After the parent lands, retarget this PR to dev.

Verification

Candidate fa01399bec1ebcad7b0031db076ad3e71384f520 normally merges the prior head with parent c530d3a7bcc8502bdd10f6a74e3ff93b739bad49, which includes dev 323a5e11. The parent-relative change is exactly four SSH source/test/documentation files (+83/-5), preserving their prior blobs. Current security dependencies and runtime/test-runner Bun 1.4.4 are retained.

  • Official Windows Bun 1.4.4+38afee6c8, installed TypeScript 7.0.2: reference TypeScript, structure, generated skill-surface consistency, scoped privacy/file-size, and diff checks passed on this candidate. All recorded native exits were 0, output completed, and temporary environments were cleaned.
  • The SSH test, implementation, argument helper, preload, runner, and dependency inputs match the earlier executed head 00b6c0bc. Its 20 passed / 1 existing Windows skip / 26 assertions are reused source-unit evidence, not a new test run.
  • The skipped POSIX escaped-parenthesis case remains unverified by the Windows result. New exact-head hosted/native CI is still required; earlier CI does not qualify this merge.
  • No live SSH connection, account, trust, global setting, or timeout/assertion was changed. Maintainer review remains required before landing.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • Bug Fixes
    • SSH configuration discovery now handles literal parentheses and braces in included filenames correctly, including when the include pattern contains wildcards.
    • Include paths now work more reliably with native Windows path separators while preserving POSIX escape behavior.
    • Discovery remains limited to top-level Include directives; includes inside Host or Match blocks are not used.
  • Documentation
    • Updated the remote-link guide with examples of include patterns containing literal parentheses and braces.

luvs01 and others added 5 commits October 23, 2026 21:25
Quote literal parentheses and braces before passing OpenSSH Include patterns
to Bun.Glob, preserving escapes and complete bracket classes. Add filesystem
regressions for Bun 1.4.4 syntax and malformed bracket handling.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 23, 2026
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 00b6c0b and f9594c0.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c32aec6 and 00b6c0b.

Files selected for processing: 11
  • .github/actions/setup-project-bun/action.yml
  • .github/workflows/ci.yml
  • CONTRIBUTING.md
  • Dockerfile
  • docs-site/src/content/docs/contributing.md
  • package.json
  • structure/runtime.md
  • tests/ci-workflows/ci-scope-gaps.test.ts
  • tests/ci-workflows/install-scripts.test.ts
  • tests/claude-integration/claude-picker-upload.test.ts
  • tests/test-layout-tooling.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

SSH Include discovery normalizes patterns before scanning and documents top-level Include handling. Bun test-runner defaults and validation guidance now use version 1.4.4. Tests also cover Bun runner selection and early TLS responses during buffered uploads.

Changes: SSH Include glob matching

Layer / File(s) Summary
Normalize and verify Include patterns
src/link/ssh-config.ts, tests/clients/link-ssh-config.test.ts, docs-site/src/content/docs/guides/remote-link.md, structure/remote-link.md
defaultGlob normalizes native path separators and makes parentheses and braces literal before passing patterns to Bun’s glob parser. Tests cover parentheses, braces, character classes, ordinary wildcards, and POSIX escaped parentheses. Documentation describes top-level Include discovery and pattern handling.

Bun role version and validation

Layer / File(s) Summary
Update Bun role defaults and guidance
package.json, structure/runtime.md, CONTRIBUTING.md, docs-site/src/content/docs/contributing.md, Dockerfile, .github/actions/setup-project-bun/action.yml, .github/workflows/ci.yml, tests/ci-workflows/*
The test-runner version changes from 1.4.3 to 1.4.4. Guidance and comments describe independent runtime and test-runner validation and the requirement for a successful full cross-platform lane=all run on the candidate head.
Verify test-runner selection
tests/test-layout-tooling.test.ts
Tests cover matching and differing caller versions, an incorrect override, version probing, and launching isolate tests with the pinned executable.

Early TLS response test

Layer / File(s) Summary
Exercise early TLS responses
tests/claude-integration/claude-picker-upload.test.ts
The fixture can send an early response over TLS and records whether the buffered request was complete and the response was flushed. The buffered upload test asserts that the response arrived before request completion and flushed.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: Minimal · up to 00b6c

No actionable issue is established for this change. Complete the documented candidate-head cross-platform validation before promotion.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. (7 skipped: 7… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: preserving literal SSH Include filename matching on Bun 1.4.4. It is concise and specific.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner

Independent source review (gpt-6.1-sol, dispatched by the triage manager, 2026-10-24) at head 0dd45828bd:

VERDICT: MUST-FIX
LANE_ALL_REQUIRED: yes

Reviewed head: 0dd45828bdf6ee3aaade2518cbd8c33577bf5ef5.

  • P2 — Missing native-platform verification — src/link/ssh-config.ts:155–156 changes Windows separator handling before glob evaluation. The only exact-head CI run passed Linux checks but skipped Windows/macOS; no exact-head dispatch exists. Fix: complete ci.yml lane=all on this head before integration, as required by .github/workflows/ci.yml:1000–1008. Linux evidence cannot establish Windows path correctness. Verification gap confirmed; no runtime failure demonstrated.

No additional source defects found across all four changed files. Regression fixtures exercise real discovery with positive and negative assertions. Existing layout registration is intact; layout.json remains 1,999 lines, and the updated structure document is 162/600. PR template is complete; unresolved review threads: 0. No credential, trust, or permission boundary regression identified.

Branch is 4 commits behind dev ceef4889e4666dc0492a375d272eafa5b785361e; read-only merge analysis found no conflicts. Maintainer approval/integration remains pending.

NOT RUN: tests, typecheck, builds, validation gates, or workflow dispatches, per read-only scope.

This is a read-only source review; merging still requires the gates named above (exact-head CI, lane=all where marked, and maintainer acceptance where required).

luvs01 commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

The current Ubuntu test failure is in the unchanged port-reclaim fixture, rather than the SSH Include change. The fixture can select the test process's own PID and also uses a real 200 ms clock despite mocking its OS operations.

I submitted the isolated test-only correction in #7706, on the Bun 1.4.4 test-runner baseline. The identical source unit passed eight focused cases plus a self-PID control on official Bun 1.4.4 for Windows; the candidate also passed typecheck. The CI process PID was not captured, so this does not establish which fixture hazard caused the recorded failure. I have not retried the failed run or marked this PR's CI as resolved.

@luvs01
luvs01 changed the base branch from dev to codex/port-reclaim-fixture-clock-bun144-20261025 October 25, 2026 05:20
@lidge-jun
lidge-jun changed the base branch from codex/port-reclaim-fixture-clock-bun144-20261025 to dev October 26, 2026 04:35
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer dev integration record (release 2.93 dev-merge loop), per the MAINTAINERS.md maintainer-integration exception for dev. This is maintainer integration, not an approving review.

  • Stack: parent test(server): isolate port reclaim fixture identity and clock #7706 landed as d056f84; this PR was retargeted to dev before the parent branch was deleted, then origin/dev was merged into the branch with a normal push (head f9594c0). Net diff vs dev: src/link/ssh-config.ts, tests/clients/link-ssh-config.test.ts, structure/remote-link.md, docs-site remote-link guide.
  • Required CI on the exact head: PR CI run 40269960441 (ci, gates, test 1/4–4/4: success), enforce-target 40269957400: success, hygiene 40269957388: success.
  • Windows-sensitive: Cross-platform CI lane=all dispatched on the exact head (pushed to ci/pr-7629-windows): run 40271190751 — windows 1/9–9/9, macos 1/3–3/3, macos control, macos widget + bundle and test 1/4–4/4 all success (27 success / 6 skipped; only the summary job was still queued at merge).
  • Independent technical review (separate agent): 0 blocking source findings; the 15 new fixtures pass and the pre-fix implementation fails 8 of them; no new traversal or trust-boundary regression.
  • Maintainer recheck on the head merged onto current origin/dev: link-ssh-config, layout and ratchet tests 31 pass / 1 skip / 0 fail (pinned Bun 1.4.4); typecheck exit 0; structure:check pass.

@lidge-jun
lidge-jun merged commit 8dd330a into lidge-jun:dev Oct 26, 2026
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants