fix(ssh): record the archive lock owner's real PID

[Gigioxx]

Problem

The SSH launch script embeds the remote runner through applyScriptPlaceholders, which used String.replaceAll with a string replacement. In a replacement string, $$ means a literal $, so the runner's archive lock line printf '%s\n' "$$" > "$T3_LOCK/pid" reached the remote host as printf '%s\n' "$".

The lock then records $ as its owner. A second launch waiting on the lock runs kill -0 "$", which always fails, so it treats the owner as dead, deletes the lock, and installs the same release archive at the same time as the first launch. The deployed runner also never matches buildRemoteT3RunnerScript, which breaks any test that compares them.

Change

applyScriptPlaceholders passes a replacer function, so $ patterns in the value stay literal. The $$ in the launch script's own RUNNER_NEXT name was never affected, since templates are not replacement values.

Scope and approval

Small, focused fix for an obvious bug: the archive lock is meant to serialize concurrent installs and currently cannot, because the owner PID it records is not a PID. One line of production code plus a regression test. Found while rebasing #10951, whose reconnect fixture depends on the deployed runner matching buildRemoteT3RunnerScript; this was split out so that PR stays on one problem.

Verification

• New test embeds the runner in the launch script byte for byte in packages/ssh/src/tunnel.test.ts: fails on main (the launch script does not contain the runner), passes with the fix.
• vp test run src/ in packages/ssh: 5 files, 36 tests passed.
• vp fmt --check and vp lint on the two changed files: clean.
• Not checked: a live SSH host. No UI change.

Made with Claude Opus 5.5 in Claude Code.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a narrowly scoped, well-tested fix for incorrect PID interpolation in SSH archive locking. Because it changes security-sensitive SSH remote-execution code, the change requires human review despite its small size.

Notes:

• Diff unchanged. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 9e08b061-1353-419f-b037-d2540e28eafd

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc99e1 and ae64a68.

📒 Files selected for processing (2)

• packages/ssh/src/tunnel.test.ts
• packages/ssh/src/tunnel.ts

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

---

📝 Walkthrough

Walkthrough

applyScriptPlaceholders now inserts replacement values literally. A test verifies that the launch script includes the archive runner byte for byte.

Changes

Tunnel script placeholder handling

Layer / File(s)	Summary
Literal replacement and verification packages/ssh/src/tunnel.ts, packages/ssh/src/tunnel.test.ts	applyScriptPlaceholders inserts replacement values without interpreting $ sequences. The test checks that the launch script contains the archive runner byte for byte.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to ae64a

The change preserves dollar-sign sequences in generated tunnel scripts and adds a regression check. No concrete issue currently blocks merging.

Architecture Summary

Architecture risk: 🔵 Low · up to ae64a

The change affects 1 system.

Changed systems: packages/ssh

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — packages/ssh (library) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in packages/ssh/src/tunnel.test.ts: Adds a test that generates the archive runner, checks it contains the lock-owner PID publication command, and verifies the launch script includes the runner byte for byte.
• observed — Modified behavior in packages/ssh/src/tunnel.ts: applyScriptPlaceholders now inserts each replacement value literally; previously, $ patterns in values were interpreted by String.replace.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description includes all required sections. It clearly explains the lock-owner PID problem, the replacer-function fix, the focused scope, regression test, test results, formatting and lint checks,…
Title check	✅ Passed	The title is concise, specific, and accurately summarizes the main SSH fix: recording the archive lock owner's real PID.
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.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

Dismissing prior approval to re-evaluate ae64a68