feat(bin): open local review files in tmux and wezterm surfaces#105
Open
MerpGoaterman wants to merge 6 commits into
Open
feat(bin): open local review files in tmux and wezterm surfaces#105MerpGoaterman wants to merge 6 commits into
MerpGoaterman wants to merge 6 commits into
Conversation
4 tasks
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.
Intent
The developer was building firstmate tooling so that local report and file references become reviewable in the captain's terminal UI instead of being dead, unclickable paths in chat. They wanted a new helper script (bin/fm-open-file.sh) plus updated AGENTS.md/README/docs guidance covering when to use tmux, WezTerm, or Lavish surfaces, with rules for renderer preference, safe window naming, path rejection, preserving orchestrator focus, and concise output. This was a relaunch continuing existing branch fm/tmux-file-open-p2, and the key correction was that multiple review links should open as a clickable list in a single real WezTerm tab (not tmux windows), while single targets open directly in the appropriate surface and tmux is reserved for explicit requests or terminal-text review. They also instructed the agent to verify with shell syntax checks and a test on an existing report, commit on that branch, and then push the change through the no-mistakes gate, including a workaround of deleting and recreating the gate ref and reconfiguring the fork as the push target after an upstream 403 push failure.
What Changed
bin/fm-open-file.sh, a helper that turns local report/file paths into reviewable terminal surfaces: a single Markdown/text target opens directly in a tmux review tab, multiple targets open as a clickable OSC-8file://link list in one real WezTerm tab, and--lavishroutes a single HTML file to Lavish. It validates paths (rejecting missing, directory, out-of-home, and bare invocations unless--allow-outside), uses safe collision-suffixed window names, and restores the orchestrator's focus after opening.AGENTS.md,README.md,docs/scripts.md, andCONTRIBUTING.mdto document when to use the tmux, WezTerm, or Lavish surface for captain-facing artifacts and to register the new script and its test.tests/fm-open-file.test.sh(4 cases) covering surface selection and path rejection; fixed a temp link-list leak on the WezTerm spawn-failure path and resolved shellcheck SC2209/SC2012 warnings.Risk Assessment
✅ Low: A well-bounded new helper script plus matching docs and hermetic tests; the only prior finding (temp-file leak on WezTerm spawn failure) is correctly fixed with a parent-side EXIT trap, and no further substantiated issues remain.
Testing
Baseline
bash tests/fm-open-file.test.shpasses 4/4. Beyond the stubbed unit tests I ran the helper end-to-end against real tmux and a real generated link list: a single report opened in a focus-preserving, collision-safe tmux tab and rendered in-pane (via less, since glow/bat are absent); three review targets produced one WezTerm tab whose link list contains a real OSC-8 file:// hyperlink per target with clean relative-path labels, satisfying the core correction that multiple links open as a clickable list in a single WezTerm tab; and a CLI transcript confirms missing/directory/out-of-home/bare/bad-lavish inputs are rejected while --allow-outside is the explicit opt-in. No file content leaked into any printed line. Because this is a terminal-hyperlink surface, the reviewer-visible artifact is the captured OSC-8 link-list bytes plus the tmux pane capture rather than a GUI screenshot (a headless WezTerm GUI tab cannot be screenshotted here). Transient fixtures and the temporary tmux session were removed; the worktree is clean.Evidence: Generated WezTerm clickable link list (raw OSC-8 hyperlink bytes via cat -v)
Firstmate review links 1. ^[]8;;file:///.../data/fix-login-k3/report.md^[\data/fix-login-k3/report.md^[]8;;^[ 2. ^[]8;;file:///.../.lavish/plan.html^[.lavish/plan.html^[]8;;^[ 3. ^[]8;;file:///.../data/audit-x9/report.md^[\data/audit-x9/report.md^[]8;;^[ Press Enter to close this tab.Evidence: Raw .ansi link list (terminal renders these as clickable file:// links)
Evidence: Single report rendered in the tmux review tab
# Login fix report Reproduced the 500 on empty password. Root cause in auth.go:42. /private/tmp/.../report.md (END)Evidence: Path-rejection / safety CLI transcript
missing path -> exit 1; directory -> exit 1; outside-home -> exit 1; bare -> usage exit 2; --lavish non-HTML -> exit 1; --lavish multi-file -> exit 1; --allow-outside outside.md -> opened in tmux tab file-outside, exit 0Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-open-file.sh:232- In the multi-path WezTerm branch, the temp link-list file is created at line 232 but its only cleanup is thetrap '... EXIT'that runs inside the spawned WezTerm shell. If spawn_wezterm_tab fails (wezterm not on PATH at line 193, orwezterm cli spawnfails at line 194), the function callsexit 1in the parent before that shell ever starts, leaking the mktemp file. Add a parent-side cleanup (e.g. a parent EXIT trap that rm's $list_file, or rm on the spawn error path) so the file is removed when the tab never opens.🔧 Fix: fix temp link-list leak on WezTerm spawn failure
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-open-file.test.sh(4/4 pass)Real tmux:bin/fm-open-file.sh data/fix-login-k3/report.mdtwice -> tabs report-fix and report-fix-2, focus restoredtmux capture-pane -t firstmate:report-fix-> report rendered in the review tabMulti-target runbin/fm-open-file.sh report.md plan.html report.md-> single WezTerm tab 'links-fix' with one OSC-8 file:// hyperlink per target (inspected generated link-list bytes)Rejection transcript: missing path, directory, out-of-home, bare invocation,--lavishnon-HTML,--lavishmulti-file all exit non-zero;--allow-outsideopens an explicit outside file✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.