feat(tui): toggle YOLO mode from the TUI with ctrl+y - #1078
Conversation
YOLO mode could only be turned on at startup, via `--yolo` or `ALTIMATE_CLI_YOLO`. Add a `ctrl+y` toggle so it can be switched on and off mid-session. - Enabling asks for confirmation; disabling is immediate, so turning a dangerous mode off is never harder than leaving it on. - Scoped to the current session, and to subagents spawned from it. A subagent runs in its own child session, so its permission requests carry the child id; lookups normalize to the root of the parent chain. - In-memory only. YOLO never survives a restart, so resuming an old session cannot silently put you back into it. - An explicit toggle beats the `--yolo` default in both directions, so a session started with `--yolo` can still be switched off. - The confirmation says what stops asking AND what stays blocked. Deny rules are refused by `Permission.ask` before any event reaches the TUI, so DROP DATABASE / DROP SCHEMA / TRUNCATE cannot be auto-approved by this mode. The footer indicator previously read `Flag.ALTIMATE_CLI_YOLO` directly. That is a `process.env` getter, not a signal, so it never re-rendered on change; it is now reactive and per-session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a session-scoped YOLO toggle with ChangesSession-scoped YOLO mode
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TUI
participant Sync
participant PermissionAPI
User->>TUI: Press ctrl+y
TUI->>TUI: Show DialogYoloConfirm
User->>TUI: Confirm enablement
TUI->>Sync: Set session YOLO state
PermissionAPI->>Sync: Submit permission request
Sync->>Sync: Evaluate yoloEnabled
Sync->>PermissionAPI: Auto-approve eligible request
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Security and correctness fixes from review of the initial implementation.
Scoping (critical): a welcome-screen choice no longer acts as the fallback
for every session without an explicit override. `session.new` navigates to
home while the previous session keeps streaming, so pressing ctrl+y there
would start auto-approving for that still-running session. The pending
choice is now display-only until adopted, and adoption moved from a route
effect to the session-creation path — the route effect also fired when
resuming an existing conversation, silently enabling YOLO on it.
Fail closed on an unresolvable session. Root resolution reads the TUI
session store, which can lag a child's first permission request. Resolving
an unhydrated child to itself missed the root's override, so a session the
user had explicitly turned OFF could still auto-approve under `--yolo`.
`resolveRoot` now returns undefined for an unknown session, an unknown
ancestor, or an over-deep chain, and callers treat that as "do not approve".
Auto-approve failures are no longer silent. The handler does not enqueue the
request before replying, so a lost reply left the server-side Deferred
unresolved and the agent hanging with nothing on screen. The generated SDK
returns `{ error }` rather than throwing unless `throwOnError` is set, so
the previous `.catch()` could never fire. Now opts in and falls back to the
normal prompt.
Enabling YOLO while a prompt is already on screen now clears it. Previously
the most natural moment to press ctrl+y — blocked awaiting approval — showed
"YOLO ON" beside a still-blocked agent.
Also: re-check the route before applying a confirmation (quick-switch stays
live during a modal), drop overrides on session.deleted, and say
"this conversation and any subagents it spawns" rather than the contradictory
"this session only" two lines above "applies to subagents too".
Testing: the real-binary journeys are gated on OPENCODE_TEST_CLI, which CI
deliberately does not set, so none of them gated the merge. Adds in-process
sync tests covering auto-approve, reply failure, subagent inheritance,
fail-closed, pending isolation and flush-on-enable.
Fixing the harness to run those uncovered a missing ExitProvider in
sync-fixture: every test in it failed with "Exit context must be used within
a context provider". That one-line fix takes the TUI suite from 9 failures
to 1 by repairing 8 pre-existing reds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Review flagged that `ctrl+y` is already bound to `copy-selection` in the opentui console overlay (`consoleOptions` in app.tsx) and that the runtime behaviour was undefined and untested. A static keybind-config check cannot answer it — the console binding does not live in TuiKeybind.Definitions — so this drives the real binary and records what actually happens. Measured: with the console open, the yolo binding takes precedence and the confirmation dialog still appears. YOLO is never enabled without it, so the dangerous case (a bypass switched on by a keystroke aimed at the console) does not occur. Closing the console leaves the shortcut working normally. Pinned as assertions rather than logged, so changing the precedence has to be a deliberate decision. The practical cost is that console copy-selection is shadowed while this binding is registered — acceptable given the console ships unbound (`app_console: "none"`) and must be deliberately bound to open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/tui/src/context/sync.tsx (1)
829-838: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign
forgetwith the root-session key and reuse it in thesession.deletedhandler.
setandadoptwrite the override under the ROOT session id.forgetdeletes under the rawsessionIDit receives. A caller that passes a child id removes nothing.The
session.deletedhandler at lines 419-424 performs the same deletion inline instead of callingforget. That leaves two copies of the same logic that can drift.Resolve the root in
forget, then call it from thesession.deletedhandler.♻️ Proposed refactor
// Drop state for a session that no longer exists. forget(sessionID: string) { - if (store.yolo[sessionID] === undefined) return + const root = rootSessionID(sessionID) ?? sessionID + if (store.yolo[root] === undefined) return setStore( "yolo", produce((draft) => { - delete draft[sessionID] + delete draft[root] }), ) },Then replace the inline deletion in the
session.deletedhandler:- setStore( - "yolo", - produce((draft) => { - delete draft[event.properties.info.id] - }), - ) + result.yolo.forget(event.properties.info.id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tui/src/context/sync.tsx` around lines 829 - 838, Update forget to resolve the root session ID before checking or deleting the yolo state, matching the key used by set and adopt. Replace the inline deletion in the session.deleted handler with a call to forget, preserving the existing cleanup behavior.packages/tui/test/cli/cmd/tui/yolo-sync.test.tsx (1)
247-250: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the flag inside
withFlag.
withFlagsetsprocess.env[FLAG]and relies on theafterEachhook of anotherdescribeblock to restore it. Atry/finallyinside the helper makes the teardown local and keeps the helper safe if the hook changes.♻️ Proposed refactor
async function withFlag(fn: () => Promise<void>) { + const previous = process.env[FLAG] process.env[FLAG] = "true" - await fn() + try { + await fn() + } finally { + if (previous === undefined) delete process.env[FLAG] + else process.env[FLAG] = previous + } }As per coding guidelines: "Tests using global
mock.module, dispatchers, or similar shared state must provide teardown and isolation safe for parallelbun testexecution."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tui/test/cli/cmd/tui/yolo-sync.test.tsx` around lines 247 - 250, Update the withFlag helper to capture the existing process.env[FLAG] value, set the flag for fn, and restore the captured value in a try/finally block, including correctly removing the variable when it was initially unset; do not rely on an external afterEach hook for cleanup.Source: Coding guidelines
packages/opencode/test/tui-journeys/yolo-console.test.ts (1)
48-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace the fixed sleep before the confirmation assertion with
waitFor.
Bun.sleep(2000)fixes the wait at two seconds. Under CI load the dialog can render later, and line 59 then fails for a timing reason rather than a behavior reason.tui.waitForis already used in this file and polls until the predicate holds. Keep a short settle sleep only for the negative assertion.♻️ Proposed refactor
await openConsole(tui) tui.send("C-y") - await Bun.sleep(2000) - const after = tui.snapshot() + await tui.waitFor((plain) => CONFIRM.test(plain), 20_000) + const after = tui.snapshot() ... - expect(CONFIRM.test(after)).toBe(true) expect(ENABLED.test(after)).toBe(false)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/tui-journeys/yolo-console.test.ts` around lines 48 - 60, Replace the fixed Bun.sleep(2000) in the yolo binding test before the confirmation assertions with tui.waitFor, polling until the confirmation dialog state is observed. Retain only a short settle sleep for the negative ENABLED assertion, and preserve the existing CONFIRM and ENABLED expectations.packages/tui/test/yolo-keybind.test.tsx (1)
61-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuarantee renderer teardown for every outcome.
testRenderis awaited outside thetry. If it rejects,app.renderer.destroy()never runs and the renderer plus its keymap layers stay alive for the rest of thebun testprocess. Register the teardown so it also runs on the failure path.♻️ Proposed refactor
- const app = await testRender(() => <Harness />) - try { - return captured.sequence - } finally { - app.renderer.destroy() - } + let app: Awaited<ReturnType<typeof testRender>> | undefined + try { + app = await testRender(() => <Harness />) + return captured.sequence + } finally { + app?.renderer.destroy() + }As per coding guidelines: "Tests using global
mock.module, dispatchers, or similar shared state must provide teardown and isolation safe for parallelbun testexecution."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tui/test/yolo-keybind.test.tsx` around lines 61 - 67, Move the testRender await into the try/finally scope so renderer teardown is registered before any render failure can occur. Preserve returning captured.sequence on success, and ensure the available app renderer is destroyed whenever rendering completes or rejects.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tui/src/app.tsx`:
- Around line 906-907: Update the YOLO enable path around sync.yolo.set in the
session UI to pass project.workspace.current() as the workspace argument,
matching permission.asked, flushPendingPermissions, and the existing session
prompt reply behavior.
In `@packages/tui/src/component/dialog-yolo-confirm.tsx`:
- Around line 53-67: Add evt.stopPropagation() to the y and n branches in the
dialog key handler, alongside evt.preventDefault(), before calling run(true) or
run(false), matching the existing return branch so confirm keys cannot reach the
underlying prompt textarea.
In `@packages/tui/src/context/sync.tsx`:
- Around line 291-316: Track in-flight request IDs for auto-approval and have
autoApprove skip any request already being processed, including duplicate calls
from flushPendingPermissions or repeated yolo enablement. Add the request ID
before awaiting sdk.client.permission.reply, and remove it in a finally block so
cleanup occurs on success or failure; only enqueuePermission for genuine reply
failures.
In `@packages/tui/test/cli/cmd/tui/yolo-sync.test.tsx`:
- Around line 210-221: Update the “enabling yolo clears a prompt that is already
on screen” test to emit the permission.replied event after the reply is
recorded, then assert that sync.data.permission[ROOT] is empty. Keep the
existing reply-count assertion and use the fixture’s established event-emission
flow.
---
Nitpick comments:
In `@packages/opencode/test/tui-journeys/yolo-console.test.ts`:
- Around line 48-60: Replace the fixed Bun.sleep(2000) in the yolo binding test
before the confirmation assertions with tui.waitFor, polling until the
confirmation dialog state is observed. Retain only a short settle sleep for the
negative ENABLED assertion, and preserve the existing CONFIRM and ENABLED
expectations.
In `@packages/tui/src/context/sync.tsx`:
- Around line 829-838: Update forget to resolve the root session ID before
checking or deleting the yolo state, matching the key used by set and adopt.
Replace the inline deletion in the session.deleted handler with a call to
forget, preserving the existing cleanup behavior.
In `@packages/tui/test/cli/cmd/tui/yolo-sync.test.tsx`:
- Around line 247-250: Update the withFlag helper to capture the existing
process.env[FLAG] value, set the flag for fn, and restore the captured value in
a try/finally block, including correctly removing the variable when it was
initially unset; do not rely on an external afterEach hook for cleanup.
In `@packages/tui/test/yolo-keybind.test.tsx`:
- Around line 61-67: Move the testRender await into the try/finally scope so
renderer teardown is registered before any render failure can occur. Preserve
returning captured.sequence on success, and ensure the available app renderer is
destroyed whenever rendering completes or rejects.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c24601d-81d9-4875-a319-902fbec61bbb
📒 Files selected for processing (15)
packages/opencode/test/tui-journeys/yolo-console.test.tspackages/opencode/test/tui-journeys/yolo-effect.test.tspackages/opencode/test/tui-journeys/yolo-subagent.test.tspackages/opencode/test/tui-journeys/yolo.test.tspackages/tui/src/app.tsxpackages/tui/src/component/dialog-yolo-confirm.tsxpackages/tui/src/component/prompt/index.tsxpackages/tui/src/config/keybind.tspackages/tui/src/context/sync.tsxpackages/tui/src/routes/session/footer.tsxpackages/tui/src/util/yolo.tspackages/tui/test/cli/cmd/tui/sync-fixture.tsxpackages/tui/test/cli/cmd/tui/yolo-sync.test.tsxpackages/tui/test/util/yolo.test.tspackages/tui/test/yolo-keybind.test.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tui/src/routes/session/footer.tsx">
<violation number="1" location="packages/tui/src/routes/session/footer.tsx:26">
P1: The YOLO footer indicator will never appear because this `Footer` component is not mounted anywhere in the TUI. Wiring the indicator into the currently rendered session/app footer (or mounting this component) is needed for the advertised reactive status display to work.</violation>
</file>
<file name="packages/tui/src/component/dialog-yolo-confirm.tsx">
<violation number="1" location="packages/tui/src/component/dialog-yolo-confirm.tsx:29">
P2: A rapid repeated keypress or click can invoke `onChoose(true)` more than once before this dialog unmounts, producing duplicate YOLO state writes and warning toasts. A one-shot `chosen` guard set before `dialog.clear()` would make the confirmation idempotent, matching the existing confirmation dialogs.</violation>
</file>
<file name="packages/tui/src/context/sync.tsx">
<violation number="1" location="packages/tui/src/context/sync.tsx:350">
P1: YOLO remains disabled for newly spawned subagents until their session happens to enter the TUI session list, so child-agent permission prompts can block an otherwise enabled root session. Handling `session.created` in the sync store (or otherwise hydrating the child/root chain before this check) would preserve the advertised subagent inheritance.</violation>
</file>
<file name="packages/opencode/test/tui-journeys/yolo-console.test.ts">
<violation number="1" location="packages/opencode/test/tui-journeys/yolo-console.test.ts:49">
P2: The dialog assertion relies on a fixed `Bun.sleep(2000)` after ctrl+y, then snapshots and asserts directly, rather than `waitFor`. On a slower machine or a cold real-binary start the confirmation can take longer than 2s, making the journey fail on the first run and depend on its single retry; it also diverges from the repo's documented 'prefer waitFor' journey rule and from every other yolo test that gates on the dialog. Use `const { plain: after } = await tui.waitFor((plain) => CONFIRM.test(plain), 20_000)` and keep only the `ENABLED` negative assert on it.</violation>
</file>
<file name="packages/tui/src/app.tsx">
<violation number="1" location="packages/tui/src/app.tsx:906">
P1: Enabling YOLO while a permission prompt is already pending in a non-default workspace cannot reliably clear that prompt because the flush reply is sent without the active workspace and may be routed to the wrong instance. Passing the current workspace preserves the documented behavior of auto-answering prompts already on screen.</violation>
</file>
<file name="packages/opencode/test/tui-journeys/yolo-subagent.test.ts">
<violation number="1" location="packages/opencode/test/tui-journeys/yolo-subagent.test.ts:93">
P2: The control test blocks on an arbitrary 25s fixed sleep instead of waiting for a terminal condition. Per the journey flake policy (avoid hard sleeps; keep each test ≤30s), this adds a large fixed delay to every run and lets a slow subagent schedule pass/fail nondeterministically. Wait for a concrete signal — e.g. `tui.waitFor` on the bash permission-ask dialog appearing (proving the subagent reached the gate), then assert the marker still does not exist — instead of sleeping.</violation>
</file>
<file name="packages/opencode/test/tui-journeys/yolo-effect.test.ts">
<violation number="1" location="packages/opencode/test/tui-journeys/yolo-effect.test.ts:71">
P2: The negative tests (yolo-off control, guardrail, and off-again) assert `exists(marker) === false` only after a hard `Bun.sleep(12_000/15_000)`, with no positive signal that the ask-gated bash call was actually dispatched and blocked. If the mock's scripted tool never reaches the app within the window (slow boot, or prompt not rendering reliably — which the file's own header notes under this harness), the marker is simply never created and the assertion passes vacuously, falsely validating the permission/guardrail claim. The guardrail test's `chained-ran` control mitigates this for that one case, but the two off-controls have no such guard. Confirm a pending-permission/block state (e.g., wait for the ask request to appear in the UI/session rather than a bare sleep) before asserting non-existence, or at least assert the mock actually issued the bash tool call.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // altimate_change start - yolo mode visual indicator, now per-session and reactive. | ||
| // Previously read Flag.ALTIMATE_CLI_YOLO directly, which is a plain process.env getter | ||
| // and therefore never re-rendered when the mode changed. | ||
| const yolo = createMemo(() => sync.yolo.enabled(route.data.type === "session" ? route.data.sessionID : undefined)) |
There was a problem hiding this comment.
P1: The YOLO footer indicator will never appear because this Footer component is not mounted anywhere in the TUI. Wiring the indicator into the currently rendered session/app footer (or mounting this component) is needed for the advertised reactive status display to work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/routes/session/footer.tsx, line 26:
<comment>The YOLO footer indicator will never appear because this `Footer` component is not mounted anywhere in the TUI. Wiring the indicator into the currently rendered session/app footer (or mounting this component) is needed for the advertised reactive status display to work.</comment>
<file context>
@@ -23,6 +20,11 @@ export function Footer() {
+ // altimate_change start - yolo mode visual indicator, now per-session and reactive.
+ // Previously read Flag.ALTIMATE_CLI_YOLO directly, which is a plain process.env getter
+ // and therefore never re-rendered when the mode changed.
+ const yolo = createMemo(() => sync.yolo.enabled(route.data.type === "session" ? route.data.sessionID : undefined))
+ // altimate_change end
const directory = useDirectory()
</file context>
There was a problem hiding this comment.
Verified and correct — routes/session/footer.tsx is imported nowhere (its sibling subagent-footer.tsx is). So the badge there could never render, and my claim about fixing its reactivity was wrong. Reverted that edit in 5a0e259 and left the dead component alone. The indicator users actually see is the chat-panel hint in component/prompt/index.tsx, which the real-binary journeys assert.
| // evaluates the ruleset first and returns DeniedError without emitting any | ||
| // event for a "deny" match, so configured guardrails (DROP DATABASE, DROP | ||
| // SCHEMA, TRUNCATE) never reach this handler and cannot be auto-approved. | ||
| if (yoloEnabled(request.sessionID)) { |
There was a problem hiding this comment.
P1: YOLO remains disabled for newly spawned subagents until their session happens to enter the TUI session list, so child-agent permission prompts can block an otherwise enabled root session. Handling session.created in the sync store (or otherwise hydrating the child/root chain before this check) would preserve the advertised subagent inheritance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/context/sync.tsx, line 350:
<comment>YOLO remains disabled for newly spawned subagents until their session happens to enter the TUI session list, so child-agent permission prompts can block an otherwise enabled root session. Handling `session.created` in the sync store (or otherwise hydrating the child/root chain before this check) would preserve the advertised subagent inheritance.</comment>
<file context>
@@ -237,40 +338,21 @@ export const {
+ // evaluates the ruleset first and returns DeniedError without emitting any
+ // event for a "deny" match, so configured guardrails (DROP DATABASE, DROP
+ // SCHEMA, TRUNCATE) never reach this handler and cannot be auto-approved.
+ if (yoloEnabled(request.sessionID)) {
+ void autoApprove(request, workspace)
break
</file context>
There was a problem hiding this comment.
Deliberate trade-off rather than an oversight, but worth stating explicitly. Failing closed on an unhydrated child costs a prompt; failing open lets a child inherit --yolo past an explicit parent-off, which is a bypass. I chose the former. In practice the task tool creates the child session before the subagent prompts, and the real-binary journey yolo-subagent.test.ts confirms a subagent command runs with YOLO on and does not with it off. Hydrating from session.created would close the remaining race — happy to add it if you think the extra prompt under load is worth it.
| }) | ||
| return | ||
| } | ||
| sync.yolo.set(sessionID, true) |
There was a problem hiding this comment.
P1: Enabling YOLO while a permission prompt is already pending in a non-default workspace cannot reliably clear that prompt because the flush reply is sent without the active workspace and may be routed to the wrong instance. Passing the current workspace preserves the documented behavior of auto-answering prompts already on screen.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/app.tsx, line 906:
<comment>Enabling YOLO while a permission prompt is already pending in a non-default workspace cannot reliably clear that prompt because the flush reply is sent without the active workspace and may be routed to the wrong instance. Passing the current workspace preserves the documented behavior of auto-answering prompts already on screen.</comment>
<file context>
@@ -865,6 +871,46 @@ function App(props: { onSnapshot?: () => Promise<string[]>; pluginHost: TuiPlugi
+ })
+ return
+ }
+ sync.yolo.set(sessionID, true)
+ toast.show({ message: "YOLO mode on for this session", variant: "warning" })
+ }}
</file context>
| sync.yolo.set(sessionID, true) | |
| sync.yolo.set(sessionID, true, project.workspace.current()) |
There was a problem hiding this comment.
Fixed in e8e42c7 — same root cause as the CodeRabbit thread above. Handled inside sync (reading project.workspace.current()) rather than at the call site.
|
|
||
| onMount(() => dialog.setSize("large")) | ||
|
|
||
| function run(enable: boolean) { |
There was a problem hiding this comment.
P2: A rapid repeated keypress or click can invoke onChoose(true) more than once before this dialog unmounts, producing duplicate YOLO state writes and warning toasts. A one-shot chosen guard set before dialog.clear() would make the confirmation idempotent, matching the existing confirmation dialogs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/component/dialog-yolo-confirm.tsx, line 29:
<comment>A rapid repeated keypress or click can invoke `onChoose(true)` more than once before this dialog unmounts, producing duplicate YOLO state writes and warning toasts. A one-shot `chosen` guard set before `dialog.clear()` would make the confirmation idempotent, matching the existing confirmation dialogs.</comment>
<file context>
@@ -0,0 +1,142 @@
+
+ onMount(() => dialog.setSize("large"))
+
+ function run(enable: boolean) {
+ dialog.clear()
+ props.onChoose(enable)
</file context>
There was a problem hiding this comment.
Fair. Low impact — the dialog clears before dispatching and the toast is idempotent in effect — so I have left it for now rather than grow this diff further. Noting it as a known nit.
| const marker = path.join(ctx.workspace, "subagent-without-yolo.txt") | ||
| await booted(tui) | ||
| await scriptSubagentBash(tui, marker) | ||
| await Bun.sleep(25_000) |
There was a problem hiding this comment.
P2: The control test blocks on an arbitrary 25s fixed sleep instead of waiting for a terminal condition. Per the journey flake policy (avoid hard sleeps; keep each test ≤30s), this adds a large fixed delay to every run and lets a slow subagent schedule pass/fail nondeterministically. Wait for a concrete signal — e.g. tui.waitFor on the bash permission-ask dialog appearing (proving the subagent reached the gate), then assert the marker still does not exist — instead of sleeping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/test/tui-journeys/yolo-subagent.test.ts, line 93:
<comment>The control test blocks on an arbitrary 25s fixed sleep instead of waiting for a terminal condition. Per the journey flake policy (avoid hard sleeps; keep each test ≤30s), this adds a large fixed delay to every run and lets a slow subagent schedule pass/fail nondeterministically. Wait for a concrete signal — e.g. `tui.waitFor` on the bash permission-ask dialog appearing (proving the subagent reached the gate), then assert the marker still does not exist — instead of sleeping.</comment>
<file context>
@@ -0,0 +1,101 @@
+ const marker = path.join(ctx.workspace, "subagent-without-yolo.txt")
+ await booted(tui)
+ await scriptSubagentBash(tui, marker)
+ await Bun.sleep(25_000)
+ expect(await exists(marker)).toBe(false)
+ },
</file context>
There was a problem hiding this comment.
Same shape as the console note — it is a negative control, so there is no terminal condition to wait for. The paired positive test (yolo on: subagent command runs) is the discriminator: it waits on a real signal, so the control only has to show the same flow does not complete without YOLO. Agreed on the wall-clock cost; happy to trim the 25s if you have a bound you trust.
| }) | ||
| await tui.ctx.llm.text("done") | ||
| await submitPrompt(tui, "create the marker") | ||
| await Bun.sleep(12_000) |
There was a problem hiding this comment.
P2: The negative tests (yolo-off control, guardrail, and off-again) assert exists(marker) === false only after a hard Bun.sleep(12_000/15_000), with no positive signal that the ask-gated bash call was actually dispatched and blocked. If the mock's scripted tool never reaches the app within the window (slow boot, or prompt not rendering reliably — which the file's own header notes under this harness), the marker is simply never created and the assertion passes vacuously, falsely validating the permission/guardrail claim. The guardrail test's chained-ran control mitigates this for that one case, but the two off-controls have no such guard. Confirm a pending-permission/block state (e.g., wait for the ask request to appear in the UI/session rather than a bare sleep) before asserting non-existence, or at least assert the mock actually issued the bash tool call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/test/tui-journeys/yolo-effect.test.ts, line 71:
<comment>The negative tests (yolo-off control, guardrail, and off-again) assert `exists(marker) === false` only after a hard `Bun.sleep(12_000/15_000)`, with no positive signal that the ask-gated bash call was actually dispatched and blocked. If the mock's scripted tool never reaches the app within the window (slow boot, or prompt not rendering reliably — which the file's own header notes under this harness), the marker is simply never created and the assertion passes vacuously, falsely validating the permission/guardrail claim. The guardrail test's `chained-ran` control mitigates this for that one case, but the two off-controls have no such guard. Confirm a pending-permission/block state (e.g., wait for the ask request to appear in the UI/session rather than a bare sleep) before asserting non-existence, or at least assert the mock actually issued the bash tool call.</comment>
<file context>
@@ -0,0 +1,202 @@
+ })
+ await tui.ctx.llm.text("done")
+ await submitPrompt(tui, "create the marker")
+ await Bun.sleep(12_000)
+ expect(await exists(marker)).toBe(false)
+ },
</file context>
There was a problem hiding this comment.
The positive signal exists in the paired tests — yolo on: an ask-gated command runs waits for the marker to appear, and the guardrail control runs the identical ;-chained command with the DDL swapped for echo and asserts it DOES run. So a silently-undispatched tool call would fail those, not pass. The negative cases still use a sleep because there is no event to wait on; open to a better bound.
Marker Guard failed in strict mode on four upstream-shared files. Each had a single-line `// altimate_change - ...` comment, which the guard does not count — it looks for a `altimate_change start` … `altimate_change end` block so the enclosed lines are protected when upstream changes are merged in. No behaviour change; comments only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Two defects introduced by the previous review round, found by CodeRabbit. The workspace was never sent on the flush path. `set()` took an optional workspace argument, but the app.tsx caller did not pass one, so replies from flushPendingPermissions went out with `workspace: undefined` while the session permission UI sends `project.workspace.current()` on every reply. Fixed at the source instead of at the call site — sync already has the project context, and threading the value through each caller is how it went missing in the first place. Replies could be sent twice for one request. flushPendingPermissions does not remove requests from the store; removal waits for the server's `permission.replied` event. Toggling yolo off and on again, or two set() calls, therefore replied to an already-settled id. The server rejects the second reply and autoApprove treated that rejection as a lost reply, putting a prompt back on screen for a request that was already answered. Now guarded by an in-flight set for concurrent duplicates plus optimistic removal on success for sequential ones — the event handler's removal stays idempotent. Adds a regression test for the double-enable path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…agation Two remaining review points. The test named "enabling yolo clears a prompt that is already on screen" only asserted that a reply was sent — never that the prompt left the screen, which is the behaviour in its name. It now asserts the pending list empties. This works without emitting `permission.replied` because autoApprove removes the request optimistically on success. Adds `stopPropagation()` to the dialog's `y` and `n` branches, matching the `return` branch. Two reviewers disagreed on whether this is load-bearing: the dialog blurs the focused renderable on open and refocuses in a later tick, so a stray character cannot reach the prompt today. It is free consistency that keeps that true if the ordering ever changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tui/src/context/sync.tsx">
<violation number="1" location="packages/tui/src/context/sync.tsx:321">
P2: The new in-flight guard (`autoApproving.has(id)`) is only cleared in `finally`, and `permission.reply` has no timeout. If a single reply call ever hangs without settling, that request id stays in the Set permanently, so every later toggle/flush returns early and the request is never re-sent or re-prompted — the agent can remain blocked with nothing on screen. Consider bounding the reply with a timeout/abort so the guard always releases and the existing fail-loudly fallback (enqueuePermission) can still run.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // UI passes `project.workspace.current()` on every reply, and threading it through | ||
| // each call site is exactly how it went missing on the flush path. | ||
| async function autoApprove(request: PermissionRequest, workspace?: string) { | ||
| if (autoApproving.has(request.id)) return |
There was a problem hiding this comment.
P2: The new in-flight guard (autoApproving.has(id)) is only cleared in finally, and permission.reply has no timeout. If a single reply call ever hangs without settling, that request id stays in the Set permanently, so every later toggle/flush returns early and the request is never re-sent or re-prompted — the agent can remain blocked with nothing on screen. Consider bounding the reply with a timeout/abort so the guard always releases and the existing fail-loudly fallback (enqueuePermission) can still run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tui/src/context/sync.tsx, line 321:
<comment>The new in-flight guard (`autoApproving.has(id)`) is only cleared in `finally`, and `permission.reply` has no timeout. If a single reply call ever hangs without settling, that request id stays in the Set permanently, so every later toggle/flush returns early and the request is never re-sent or re-prompted — the agent can remain blocked with nothing on screen. Consider bounding the reply with a timeout/abort so the guard always releases and the existing fail-loudly fallback (enqueuePermission) can still run.</comment>
<file context>
@@ -282,26 +282,63 @@ export const {
+ // UI passes `project.workspace.current()` on every reply, and threading it through
+ // each call site is exactly how it went missing on the flush path.
async function autoApprove(request: PermissionRequest, workspace?: string) {
+ if (autoApproving.has(request.id)) return
+ autoApproving.add(request.id)
try {
</file context>
There was a problem hiding this comment.
Real but narrow: finally only runs when the promise settles, so a reply that never settles would pin that id in the Set. The SDK has no per-call timeout today, so a fix means adding one rather than changing this guard. Given the failure mode is "yolo stops auto-approving for one request id" (fails closed, not open), I have left it. Happy to add an AbortSignal timeout if you would prefer.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of commit Resolved by this commit:
Files Reviewed (5 files)
Previous Review Summary (commit 860cab1)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 860cab1)Status: No Issues Found | Recommendation: Merge Incremental review of commit Resolved by this commit:
Files Reviewed (5 files)
Reviewed by glm-5.2 · Input: 43.5K · Output: 7K · Cached: 292.2K Review guidance: REVIEW.md from base branch |
…removal Follow-up review round. resolveRoot treated a cyclic parent chain as a valid root, returning whichever node the walk stopped on. That node could carry an override or inherit `--yolo`, so a malformed chain could auto-approve — the same fail-open shape the unknown-session guard already prevents. Cycles now return undefined. The unit test had encoded the old behaviour as expected; corrected, and extended to assert a cyclic chain never auto-approves. Reverts the change to routes/session/footer.tsx. That component is imported nowhere (its sibling subagent-footer.tsx is), so the indicator there can never render and the earlier claim about fixing its reactivity was wrong. The visible indicator users get is the chat-panel hint in component/prompt, which the journeys assert. Leaving the dead component untouched. Removes `sync.yolo.forget()` — added but never called; session cleanup already happens in the session.deleted handler. `permission.replied` now shares removePermission with the auto-approve path instead of repeating the search/splice inline. The double-enable regression test was passing vacuously: with an instant mock the store is cleared before the second set() runs, so the second flush was a no-op and the in-flight guard was never exercised. The reply is now held open so the second toggle lands while the first is genuinely in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Marker Guard strict mode again — the permission.replied handler now delegates to removePermission, and that replacement line needs start/end markers like the rest of the fork's edits to upstream-shared files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tui/src/routes/session/footer.tsx">
<violation number="1">
P1: The YOLO footer badge is no longer reactive or per-session. Reading Flag.ALTIMATE_CLI_YOLO directly only reflects the launch-time --yolo env flag and never re-renders when the user toggles YOLO in-session with Ctrl+Y, so the badge will not appear/disappear as the mode changes — regressing the PR's stated reactive footer badge goal. Restore the reactive memo, e.g. `const yolo = createMemo(() => sync.yolo.enabled(route.data.type === "session" ? route.data.sessionID : undefined))` and use `<Show when={yolo()}>`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Issue for this PR
Closes #1079
Type of change
What does this PR do?
YOLO mode already exists, but you can only switch it on at launch (
--yolo/ALTIMATE_CLI_YOLO). If you're already in a conversation and get tired ofapproving every step, there's no way to turn it on. This adds
ctrl+y.Enabling asks first. Disabling doesn't — turning a dangerous mode off shouldn't
be harder than leaving it on.
Three things that weren't obvious while building it:
its permission requests arrive tagged with the child's id, not the one you
can see. Without normalizing to the root of the parent chain, you'd switch
YOLO on and it would still stop and ask the moment the agent delegated
anything.
packages/tui/src/util/yolo.tsdoes that walk.Flag.ALTIMATE_CLI_YOLO, which is aprocess.envgetter rather than asignal, so it only ever reflected the startup value. It's reactive now.
old session tomorrow would silently put you back in YOLO.
An explicit toggle also beats the
--yolodefault in both directions, so asession started with
--yolocan still be switched off.The confirmation deliberately says what stays blocked, not just what stops
asking. YOLO can only auto-answer prompts the server chose to raise — deny
rules are refused in
Permission.askbefore any event reaches the TUI, soDROP DATABASE/DROP SCHEMA/TRUNCATEcan't be auto-approved by it. Thereal exposure is the "ask"-level rules:
rm -rf, force pushes, reading.env.Saying "the agent can now do anything" would have been misleading in both
directions.
How did you verify your code works?
bun testinpackages/tui, plus the real-binary tmux journeys(
packages/opencode/test/tui-journeys), which drive the compiled CLI and assertwhat's actually on screen.
12 new journeys. The ones worth calling out assert whether a command actually
executed, not what the dialog says —
journeys.test.tsalready documents thatthe permission prompt doesn't render reliably under the mock model, and "did the
tool run?" is the property that matters for a permission bypass anyway:
bashcommand does not runthe identical
;-chained shape with the DDL swapped forecho, so the deniedpattern is the only difference between the blocked and passing case
the session table to confirm a real child session was involved rather than the
work quietly happening in the parent
Two bugs the journeys caught that the unit tests were happy with: the shortcut
didn't exist before your first message (registered in the session route, which
isn't mounted on the welcome screen), and enabling it there showed no change
because both indicators hard-coded "off" when there was no session yet.
Rebased on
main(b18bbf3c46) and re-ran everything after, since #1067 touchedthe same welcome screen. Full journey suite 22 pass / 3 pre-existing todo / 0
fail. TUI unit suite matches the
mainbaseline exactly (9 pre-existingfailures, unchanged).
Screenshots / recordings
Captured from the journey suite. Hint on the chat panel, off then on:
The confirmation:
Defaults to No.
ctrl+yagain turns it off with no prompt.Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01964Prd1Sz5JwWZmNTrFdiU
Summary by CodeRabbit
New Features
Ctrl+Ytoggle support.Bug Fixes
Tests