Repository navigation
fix(t3): defer browser setup when the CLI lacks the command - #3071
Conversation
backup-check.sh landed with its spec in cliproxyapi_backup_spec.sh but without a coverage entry, so the shell-test coverage check fails on main and on every open PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setupT3Browser activation runs `t3 browser setup`. The npm latest
release (0.0.45) has no browser subcommand and rejects the call
("Unexpected positional argument: \"setup\""), so `make switch` fails on
every fresh install. This has kept the E2E and Docker checks red on main.
Probe the CLI help and defer to the connect timer, as for a missing CLI.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning This organization's free trial has ended, so Indent couldn't start this review. Manage billing to resume reviews on this repository.
Select any unchecked box below to run or rerun that agent. Pending (1)
Full resultsIndent Review Agent
|
…fix/t3-browser-setup-capability
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
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 2 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: deferring browser setup when the T3 CLI lacks the browser command. Description check The description directly explains the browser-command probe, the activation failure it prevents, the retry behavior, and the added test coverage. 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.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 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
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.
A rabbit checks the browser command,
Then lets setup wait when it is not at hand.
The shell script list grows by one,
And coverage checks its work is done.
I nibble a carrot and hop away.
Comment @coderabbitai help to get the list of available commands.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix correctly defers t3 browser setup when the installed CLI lacks the browser subcommand. The approach of probing t3 --help output is sound since the npm latest (0.0.45) doesn't have the command while nightly (0.0.46-nightly) does. The connect timer will retry after t3 update moves to a runtime that includes it.
Changes look good:
- setup-browser.sh: Added capability check that exits 0 cleanly when
browsersubcommand is missing, with a clear log message. - t3_browser_setup_spec.sh: New test case covers the missing-command scenario via
MOCK_NO_BROWSER=1. - coverage_spec.sh: Added missing coverage entry for
cliproxyapi/scripts/backup-check.sh(unrelated but needed for test suite to pass).
No issues found. Tests pass (11 examples, 0 failures) and shellcheck is clean.
free ｜ 𝕏
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/services/t3-connect/setup-browser.sh">
<violation number="1" location="home-manager/services/t3-connect/setup-browser.sh:15">
P2: `|| true` hides every failure from the CLI help probe, so a missing or broken active runtime is misreported as “no browser command” and setup is skipped silently. Preserve a nonzero help status; only defer after successful help output lacks `browser`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| # The npm `latest` release predates the `browser` subcommand that nightly | ||
| # runtimes ship; it rejects `browser setup` and would fail activation. The | ||
| # connect timer repeats setup once `t3 update` moves to a runtime that has it. | ||
| help="$(env "T3CODE_HOME=$base" "${T3_BROWSER_CLI:?}" --help 2>&1 || true)" |
There was a problem hiding this comment.
P2: || true hides every failure from the CLI help probe, so a missing or broken active runtime is misreported as “no browser command” and setup is skipped silently. Preserve a nonzero help status; only defer after successful help output lacks browser.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At home-manager/services/t3-connect/setup-browser.sh, line 15:
<comment>`|| true` hides every failure from the CLI help probe, so a missing or broken active runtime is misreported as “no browser command” and setup is skipped silently. Preserve a nonzero help status; only defer after successful help output lacks `browser`.</comment>
<file context>
@@ -9,6 +9,15 @@ if [ ! -f "$base/runtime/service-state.json" ] && [ ! -x "${T3_REAL_CLI:-$HOME/.
+# The npm `latest` release predates the `browser` subcommand that nightly
+# runtimes ship; it rejects `browser setup` and would fail activation. The
+# connect timer repeats setup once `t3 update` moves to a runtime that has it.
+help="$(env "T3CODE_HOME=$base" "${T3_BROWSER_CLI:?}" --help 2>&1 || true)"
+if ! grep -qE '^ +browser ' <<<"$help"; then
+ echo "T3 CLI has no browser command; deferring browser setup to the connect timer."
</file context>
| help="$(env "T3CODE_HOME=$base" "${T3_BROWSER_CLI:?}" --help 2>&1 || true)" | |
| if ! help="$(env "T3CODE_HOME=$base" "${T3_BROWSER_CLI:?}" --help 2>&1)"; then | |
| printf '%s\n' "$help" >&2 | |
| exit 1 | |
| fi |
Summary
setup-browser.shcheckst3 --helpfor abrowsersubcommand. If it is missing, the script defers to the connect timer, the same way it does when the CLI is not installed yet.browsercommand.Why
Since #3055, E2E (Ubuntu) and both Docker builds have failed on main at the
setupT3Browseractivation:package.jsonpinst3: ^0.0.44, which resolves to npmlatest(0.0.45). That release has nobrowsersubcommand, so it readsbrowseras the cwd argument and rejectssetup. Hosts on the nightly runtime (matic: 0.0.46-nightly) listbrowserand keep running setup.Since
e2e-checkanddocker-checkare required, every open PR is blocked, including #3067-#3070.decided: probe the help output rather than pin a nightly in package.json - the npm channel is a separate choice, and the connect timer already re-runs setup after
t3 updateTest plan
shellspec spec/t3_browser_setup_spec.sh(11 examples, 0 failures)shellcheckon the changed script and spect3 --helpmatches the probe (browser Manage T3's headless browser on this host.)🤖 Generated with Claude Code
Summary by cubic
Defers
t3 browser setupwhen the installed CLI lacks thebrowsersubcommand, so fresh installs no longer fail on npm'slatestrelease (0.0.45).t3 --helpand exits early if thebrowsercommand is missing; the connect timer retries setup oncet3 updatemoves to a runtime that has it.browsercommand.Bug Fixes
home-manager/services/cliproxyapi/scripts/backup-check.shso the shell-test coverage check passes.Written for commit a3c1df4. Summary will update on new commits.