Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions desktop/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 10 additions & 12 deletions desktop/frontend/src/components/Composer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3827,7 +3827,7 @@ export function Composer({
</button>
</div>
</AnchoredPopover>
{!heroMode && <AnchoredPopover
<AnchoredPopover
open={intentMenuOpen}
closing={intentMenuClosing}
anchorRef={intentMenuAnchorRef}
Expand Down Expand Up @@ -3946,8 +3946,8 @@ export function Composer({
</div>
)}
</div>
</AnchoredPopover>}
{!heroMode && <AnchoredPopover
</AnchoredPopover>
<AnchoredPopover
open={profileMenuOpen}
closing={profileMenuClosing}
anchorRef={profileMenuAnchorRef}
Expand Down Expand Up @@ -3987,7 +3987,7 @@ export function Composer({
</button>
))}
</div>
</AnchoredPopover>}
</AnchoredPopover>
<AnchoredPopover
open={moreMenuOpen && !disabled && !running}
closing={moreMenuClosing}
Expand Down Expand Up @@ -4549,8 +4549,10 @@ export function Composer({
</Tooltip>
</div>
)}
{!heroMode && (
<div className="composer-meta__control composer-meta__control--intent">
{/* 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. */}
<div className="composer-meta__control composer-meta__control--intent">
<Tooltip label={taskModeTooltipLabel} disabled={intentMenuOpen || intentMenuClosing || creationChrome}>
<button
ref={intentMenuAnchorRef}
Expand All @@ -4571,9 +4573,7 @@ export function Composer({
</button>
</Tooltip>
</div>
)}
{!heroMode && (
<div className="composer-meta__control composer-meta__control--profile">
<div className="composer-meta__control composer-meta__control--profile">
<Tooltip label={runtimeProfileTooltipLabel} disabled={profileMenuOpen || profileMenuClosing || creationChrome}>
<button
ref={profileMenuAnchorRef}
Expand All @@ -4597,9 +4597,7 @@ export function Composer({
</button>
</Tooltip>
</div>
)}
{!heroMode && (
<div className="composer-meta__control composer-meta__control--approval">
<div className="composer-meta__control composer-meta__control--approval">
{/* 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
Expand Down
17 changes: 14 additions & 3 deletions desktop/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 23 additions & 0 deletions desktop/terminal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion internal/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
13 changes: 9 additions & 4 deletions internal/agent/canonical_todo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion internal/agent/complete_step_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
19 changes: 17 additions & 2 deletions internal/agent/final_readiness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"}}
Expand Down
74 changes: 74 additions & 0 deletions internal/permission/bash_approval.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,87 @@ 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
}
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
Expand Down
22 changes: 22 additions & 0 deletions internal/permission/bash_approval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions internal/permission/bash_decompose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
Loading