diff --git a/desktop/app.go b/desktop/app.go index 74655ed93c..c1c8c940dc 100644 --- a/desktop/app.go +++ b/desktop/app.go @@ -9848,11 +9848,13 @@ var ( // it only for the duration of a metadata rewrite (sub-millisecond); a // concurrent tab build that races that probe must not surface a spurious // "already open in another Reasonix window" error for a lease that is - // genuinely free once the probe releases it. A lease held by another - // window or process stays held for its whole lifetime, so the bounded - // retry still fails fast there. - sessionLeaseContentionRetryInterval = 50 * time.Millisecond - sessionLeaseContentionRetryAttempts = 2 + // genuinely free once the probe releases it. Multi-project tab switching + // (#7732, #7592) can stack several short probes, so the budget is longer + // than a single sub-ms rewrite. A lease held by another window or process + // stays held for its whole lifetime, so the bounded retry still fails + // fast there. + sessionLeaseContentionRetryInterval = 40 * time.Millisecond + sessionLeaseContentionRetryAttempts = 8 ) // withSessionLeaseContentionRetry retries acquire while it fails with diff --git a/desktop/frontend/src/components/Composer.tsx b/desktop/frontend/src/components/Composer.tsx index 8d81a691dc..03a0b01414 100644 --- a/desktop/frontend/src/components/Composer.tsx +++ b/desktop/frontend/src/components/Composer.tsx @@ -3827,7 +3827,7 @@ export function Composer({ - {!heroMode && )} - } - {!heroMode && + ))} - } + )} - {!heroMode && ( -
+ {/* Task / profile / approval stay available in the empty-session hero + so users can pick Plan, Goal, Standard, or delivery profile before + the first message (#7731). Content + remains non-hero only. */} +
- )} - {!heroMode && ( -
+
- )} - {!heroMode && ( -
+
{/* A pending tool approval disables the composer, but the approval bar stays usable so mode changes remain possible mid-prompt; the approval card explains that the pending request still needs diff --git a/desktop/terminal.go b/desktop/terminal.go index 8920cbab15..989aa06a0d 100644 --- a/desktop/terminal.go +++ b/desktop/terminal.go @@ -253,12 +253,20 @@ func (a *App) terminalTargetForTab(tabID string, requireWritable bool) (terminal if tabID == "" { tabID = activeID } - if tabID == "" || tabID != activeID { + if tabID == "" { + a.mu.RUnlock() + return terminalTarget{}, errTerminalStaleTab + } + // Mutations stay tied to the active tab so a background tab cannot drive + // PTY input. Listing/snapshot must work for any open tab: the frontend + // switches optimistically and re-syncs before SetActiveTab returns, and + // requiring active-only made restored tabs look "closed" (#7744). + if requireWritable && tabID != activeID { a.mu.RUnlock() return terminalTarget{}, errTerminalStaleTab } tab := a.tabByIDLocked(tabID) - if tab == nil { + if tab == nil || tab.removed { a.mu.RUnlock() return terminalTarget{}, errTerminalStaleTab } @@ -288,7 +296,10 @@ func (a *App) terminalTargetForTab(tabID string, requireWritable bool) (terminal func (a *App) revalidateTerminalTarget(target terminalTarget, requireWritable bool) error { a.mu.RLock() tab := a.tabByIDLocked(target.tabID) - valid := tab != nil && a.activeTabID == target.tabID + valid := tab != nil && !tab.removed + if requireWritable { + valid = valid && a.activeTabID == target.tabID + } readOnly := valid && tab.ReadOnly root := "" if valid { diff --git a/desktop/terminal_test.go b/desktop/terminal_test.go index aef1b92964..20fd4873f3 100644 --- a/desktop/terminal_test.go +++ b/desktop/terminal_test.go @@ -176,6 +176,29 @@ func TestTerminalTargetScopesSessionsToTheChatTab(t *testing.T) { } } +func TestTerminalWorkspaceListsInactiveTabSessions(t *testing.T) { + // Frontend switches optimistically and re-syncs before SetActiveTab returns. + // Listing must work for the previous tab so open terminals do not flash as + // "closed" during multi-session switches (#7744). + app := NewApp() + root := t.TempDir() + app.tabs["one"] = &WorkspaceTab{ID: "one", Scope: "project", WorkspaceRoot: root} + app.tabs["two"] = &WorkspaceTab{ID: "two", Scope: "project", WorkspaceRoot: root} + app.tabOrder = []string{"one", "two"} + app.activeTabID = "one" + + if _, err := app.TerminalWorkspaceForTab("one"); err != nil { + t.Fatalf("active list: %v", err) + } + app.activeTabID = "two" + if _, err := app.TerminalWorkspaceForTab("one"); err != nil { + t.Fatalf("inactive list after switch: %v", err) + } + if _, err := app.CreateTerminalForTab("one", ".", "default"); !errors.Is(err, errTerminalStaleTab) { + t.Fatalf("inactive create error = %v, want errTerminalStaleTab", err) + } +} + func TestEmptyTerminalWorkspaceViewSerializesArrays(t *testing.T) { view := emptyTerminalWorkspaceView() raw, err := json.Marshal(view) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index bf55fda04a..fb610d89c6 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1606,8 +1606,13 @@ func (a *Agent) finalReadinessCheckFor() finalReadinessCheck { return out } { + // Latest-turn incomplete todos still block a writer turn's final answer. + // Delivery also falls back to canonical todos so open items cannot be + // abandoned across turns. Balanced/full deliberately skip that fallback: + // stale open todos were re-emitting "delivery checks incomplete" on every + // later turn even when the user was not in delivery mode (#7694, #7634). incomplete, hasTodos := a.evidence.IncompleteLatestTodos() - if !hasTodos && a.evidence.HasAnySuccessfulReceipt() { + if a.deliveryProfile && !hasTodos && a.evidence.HasAnySuccessfulReceipt() { incomplete, hasTodos = a.incompleteCanonicalTodos() } if hasTodos && len(incomplete) > 0 && a.evidence.HasSuccessfulTodoProgressReceipt() { diff --git a/internal/agent/canonical_todo_test.go b/internal/agent/canonical_todo_test.go index 96246cd923..23d59be605 100644 --- a/internal/agent/canonical_todo_test.go +++ b/internal/agent/canonical_todo_test.go @@ -14,20 +14,25 @@ func TestFinalReadinessFallsBackToCanonicalTodos(t *testing.T) { open := []evidence.TodoItem{{Content: "push", Status: "completed"}, {Content: "rebase", Status: "pending"}} // Turn did work (a successful bash) but issued no todo_write this turn, so the - // per-turn ledger has no list — the canonical state must still gate. - a := &Agent{evidence: readinessLedger(ran), todoState: open} + // per-turn ledger has no list — delivery falls back to the canonical state. + a := &Agent{evidence: readinessLedger(ran), todoState: open, deliveryProfile: true} if got := a.ReadinessResult(); !strings.Contains(got.Reason, "incomplete") { t.Fatalf("cross-turn gate = %q, want it to report incomplete canonical todos", got.Reason) } + // Balanced/full must not fall back to stale canonical todos. + balanced := &Agent{evidence: readinessLedger(ran), todoState: open} + if got := balanced.ReadinessResult(); !got.Ready { + t.Fatalf("balanced mode gated on canonical todos: %+v", got) + } // A turn that touched nothing (pure Q&A) must never gate on stale canonical state. - idle := &Agent{evidence: evidence.NewLedger(), todoState: open} + idle := &Agent{evidence: evidence.NewLedger(), todoState: open, deliveryProfile: true} if got := idle.ReadinessResult(); !got.Ready { t.Fatalf("no-work turn gated on canonical todos: %+v", got) } // All canonical items completed → no gate even after work. - done := &Agent{evidence: readinessLedger(ran), todoState: []evidence.TodoItem{{Content: "push", Status: "completed"}}} + done := &Agent{evidence: readinessLedger(ran), todoState: []evidence.TodoItem{{Content: "push", Status: "completed"}}, deliveryProfile: true} if got := done.ReadinessResult(); !got.Ready { t.Fatalf("completed canonical todos still gated: %+v", got) } diff --git a/internal/agent/complete_step_e2e_test.go b/internal/agent/complete_step_e2e_test.go index a66b5890c0..94d32fbe72 100644 --- a/internal/agent/complete_step_e2e_test.go +++ b/internal/agent/complete_step_e2e_test.go @@ -158,7 +158,9 @@ func TestE2ECrossTurnCanonicalGateBlocksThenClears(t *testing.T) { Arguments: `{"step":"beta","result":"done","evidence":[{"kind":"manual","summary":"verified by inspection"}]}`}}}, testutil.Turn{Text: "all done now"}, ) - a := New(mp, evidenceRegistry(), sess, Options{}, event.Discard) + // Cross-turn canonical fallback is delivery-only: balanced/full must not + // re-block every later turn on stale open todos (#7694). + a := New(mp, evidenceRegistry(), sess, Options{DeliveryProfile: true}, event.Discard) a.SetSession(sess) // rebuilds canonical {alpha in_progress, beta pending} firstErr := a.Run(context.Background(), "finish up") diff --git a/internal/agent/final_readiness_test.go b/internal/agent/final_readiness_test.go index 9d6c2a84ad..52d622fea7 100644 --- a/internal/agent/final_readiness_test.go +++ b/internal/agent/final_readiness_test.go @@ -41,8 +41,8 @@ func TestFinalReadinessFailureBranches(t *testing.T) { {"writer without checks or todo never gates", nil, readinessLedger(writer), true, ""}, {"missing project check after writer is reported", []instruction.VerifyCheck{check}, readinessLedger(checkAfter, writer), false, "go test ./..."}, {"project check run after writer satisfies", []instruction.VerifyCheck{check}, readinessLedger(writer, checkAfter), true, ""}, - {"todo writer without complete_step is reported", nil, readinessLedger(writer, todo), false, "incomplete items"}, - {"complete_step without final todo update is reported", nil, readinessLedger(writer, todo, completeAfter), false, "latest successful todo_write"}, + {"todo writer without complete_step is reported under delivery", nil, readinessLedger(writer, todo), false, "incomplete items"}, + {"complete_step without final todo update is reported under delivery", nil, readinessLedger(writer, todo, completeAfter), false, "latest successful todo_write"}, {"todo writer with complete_step and completed todo satisfies", nil, readinessLedger(writer, todo, completeAfter, doneTodo), true, ""}, } for _, tc := range cases { @@ -102,6 +102,21 @@ func TestFinalReadinessCheckAuditsIncompleteTodos(t *testing.T) { } } +func TestFinalReadinessDoesNotFallBackToCanonicalTodosOutsideDelivery(t *testing.T) { + // Stale open todos from earlier turns must not poison balanced/full sessions. + ran := evidence.Receipt{ToolName: "bash", Success: true, Command: "echo hi"} + open := []evidence.TodoItem{{Content: "leftover", Status: "pending"}} + a := &Agent{evidence: readinessLedger(ran), todoState: open} + if got := a.ReadinessResult(); !got.Ready { + t.Fatalf("ReadinessResult() = %+v, want ready without delivery canonical fallback", got) + } + // Delivery still falls back to the canonical list. + delivery := &Agent{evidence: readinessLedger(ran), todoState: open, deliveryProfile: true} + if got := delivery.ReadinessResult(); !strings.Contains(got.Reason, "incomplete") { + t.Fatalf("delivery ReadinessResult() = %+v, want incomplete canonical todos", got) + } +} + func TestFinalReadinessAllowsFinalAfterLoopGuardedToolBlocker(t *testing.T) { todo := evidence.Receipt{ToolName: "todo_write", Success: true, Todos: []evidence.TodoItem{{Content: "edit", Status: "in_progress"}}} writer := evidence.Receipt{ToolName: "write_file", Success: true, Write: true, Paths: []string{"a.go"}} diff --git a/internal/permission/bash_approval.go b/internal/permission/bash_approval.go index 01419c0f72..08914b3b65 100644 --- a/internal/permission/bash_approval.go +++ b/internal/permission/bash_approval.go @@ -60,6 +60,11 @@ func classifyBashSegmentApproval(subject string) bashApprovalClass { if len(features.CommandPrefix) > 0 && isIndirectExecution(features.CommandPrefix) { return bashApprovalRequireHuman } + // Destructive git/file forms are exact-looking shell but must not auto-run + // under Auto: users report silent `git checkout` discarding work (#7784). + if bashSubjectIsHighRiskMutation(subject) { + return bashApprovalRequireHuman + } if features.Expansion || features.Assignment || features.Redirection || shellparse.ContainsUnquotedGlob(subject) || hasEnvWrapperAssignment(features.CommandPrefix) { return bashApprovalExactOnly @@ -67,6 +72,75 @@ func classifyBashSegmentApproval(subject string) bashApprovalClass { return bashApprovalReusable } +// bashSubjectIsHighRiskMutation mirrors the recovery high-risk git classifier +// without importing recovery (that package depends on agent → config → +// permission). Keep destructive worktree/index rewrites behind a human prompt +// even when the shell shape is "exact-only reusable" under Auto. +func bashSubjectIsHighRiskMutation(subject string) bool { + features, ok := shellparse.AnalyzeApprovalFeatures(subject) + if !ok || len(features.CommandPrefix) == 0 { + return false + } + fields := features.CommandPrefix + // Strip env-style wrappers so `env git checkout` still matches. + for len(fields) > 0 { + base := executableBase(fields[0]) + switch base { + case "env": + fields = fields[1:] + for len(fields) > 0 && isEnvironmentAssignment(fields[0]) { + fields = fields[1:] + } + continue + case "command", "builtin", "exec", "nohup", "sudo": + fields = fields[1:] + for len(fields) > 0 && strings.HasPrefix(fields[0], "-") { + fields = fields[1:] + } + continue + } + break + } + if len(fields) == 0 || executableBase(fields[0]) != "git" { + return false + } + args := fields[1:] + if containsFoldedArg(args, "push", "clean", "prune", "filter-branch", "filter-repo") { + return true + } + if containsFoldedArg(args, "reset") && containsFoldedArg(args, "--hard", "--merge", "--keep") { + return true + } + if containsFoldedArg(args, "checkout") { + return true + } + if containsFoldedArg(args, "switch") && containsFoldedArg(args, "--discard-changes") { + return true + } + if containsFoldedArg(args, "restore") && (!containsFoldedArg(args, "--staged") || containsFoldedArg(args, "--worktree")) { + return true + } + if containsFoldedArg(args, "branch") && containsFoldedArg(args, "-d", "--delete", "-f", "--force") { + return true + } + if containsFoldedArg(args, "stash") && containsFoldedArg(args, "clear", "drop") { + return true + } + return false +} + +func containsFoldedArg(args []string, needles ...string) bool { + for _, arg := range args { + low := strings.ToLower(strings.TrimSpace(arg)) + for _, needle := range needles { + if low == strings.ToLower(needle) { + return true + } + } + } + return false +} + func isIndirectExecution(fields []string) bool { if len(fields) == 0 { return true diff --git a/internal/permission/bash_approval_test.go b/internal/permission/bash_approval_test.go index 0ec20c05a4..8de251dbc8 100644 --- a/internal/permission/bash_approval_test.go +++ b/internal/permission/bash_approval_test.go @@ -136,6 +136,28 @@ func TestPolicyDynamicBashShapesRequireExplicitApproval(t *testing.T) { } } +func TestHighRiskGitBashRequiresHumanEvenInAuto(t *testing.T) { + // Exact-looking destructive git must not auto-run under Auto (#7784). + p := New("allow", []string{"Bash"}, nil, nil) + for _, command := range []string{ + "git checkout main", + "git checkout -f HEAD", + "git checkout -- src/main.go", + "git reset --hard HEAD", + "git push origin main", + "git clean -fd", + "env git checkout feature", + } { + if got := p.DecideSubject("bash", false, command); got != Ask { + t.Errorf("DecideSubject(%q) = %v, want Ask under Auto", command, got) + } + } + // Non-destructive git remains reusable under Auto. + if got := p.DecideSubject("bash", false, "git status --short"); got != Allow { + t.Fatalf("DecideSubject(git status) = %v, want Allow", got) + } +} + func TestPolicyExactOnlyBashUsesFallbackWithoutReusableAllow(t *testing.T) { for _, command := range []string{ "git diff $REV", diff --git a/internal/permission/bash_decompose_test.go b/internal/permission/bash_decompose_test.go index 9aa662cfc4..e2cca9cd36 100644 --- a/internal/permission/bash_decompose_test.go +++ b/internal/permission/bash_decompose_test.go @@ -179,9 +179,9 @@ func TestPolicyDecideCompoundBash(t *testing.T) { want Decision }{ { - name: "compound of atomic-allowed segments passes", + name: "compound with high-risk git push requires human", subject: `git add . && git commit -m "wip" && git push`, - want: Allow, + want: Ask, }, { name: "one uncovered segment turns into Ask", @@ -231,9 +231,9 @@ func TestPolicyDecideCompoundBash(t *testing.T) { want: Ask, }, { - name: "atomic subject with matching prefix rule still allows", + name: "atomic high-risk git push requires human even with prefix rule", subject: "git push origin main", - want: Allow, + want: Ask, }, } for _, tt := range cases {