From 0b08801e1b12bfb2d0a718a6569a83e47facbc1f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 02:07:48 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat(goal-loop):=20a=20per-agent=20Goal=20L?= =?UTF-8?q?oop=20MCP=20command,=20and=20the=20overlay=20chord=20off=20?= =?UTF-8?q?=E2=8C=98=E2=87=A7Y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #1006: every sibling built-in MCP domain (TLDR, Goal) has a per-agent enable command, but goal_loop had only its Settings row. The new enable-goal-loop-mcp mirrors enable-goal-mcp: it toggles the per-agent override and reloads the agent. It is covered by the real-hook per-agent override and reset test, listed in the feature reference, and counted in the catalog (124, and 43 approved additions). - #1007: goal-loop-preview moves from ⌘⇧Y, which macOS reserves for the system New Sticky Note service and check:keybindings cannot see, to ⌘⇧G. That chord is beside the Goal peek (⌘G) and was freed when the unified stage retired Global Dispatch. The router tests use the new chord and assert the old one does nothing, and the Settings text is updated. check:keybindings is OK. Closes #1006 Closes #1007 Co-Authored-By: Claude Opus 5 (1M context) --- .../features/command-keybindings/defaults.ts | 6 +++- .../features/command-palette/catalog.test.ts | 25 +++++++------ .../features/goal-loop/controlReference.ts | 4 +-- .../goalLoop.router.renderer.test.tsx | 16 ++++++--- .../features/settings/lib/settingsRegistry.ts | 2 +- .../workspace/commands/sessionCommands.ts | 36 +++++++++++++++++++ .../actions/mcpPreferences.renderer.test.tsx | 2 ++ 7 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/renderer/src/features/command-keybindings/defaults.ts b/src/renderer/src/features/command-keybindings/defaults.ts index 5e2371a8..b2468b13 100644 --- a/src/renderer/src/features/command-keybindings/defaults.ts +++ b/src/renderer/src/features/command-keybindings/defaults.ts @@ -323,7 +323,11 @@ export function buildDefaultKeybindings(): CommandBindingDefault[] { { commandId: 'toggle-reader-mode', bindings: ['Alt+R'], context: 'global' }, { commandId: 'tldr-preview', bindings: ['Cmd+L'], context: 'global' }, { commandId: 'goal-preview', bindings: ['Cmd+G'], context: 'global' }, - { commandId: 'goal-loop-preview', bindings: ['Cmd+Shift+Y'], context: 'global' }, + // Not Cmd+Shift+Y (#1007): macOS reserves ⇧⌘Y for the system "New + // Sticky Note" service, which check:keybindings cannot see. ⌘⇧G sits + // beside the Goal peek (⌘G), and the unified stage (#992) freed it when + // Global Dispatch was retired. + { commandId: 'goal-loop-preview', bindings: ['Cmd+Shift+G'], context: 'global' }, { commandId: 'toggle-spotlight', bindings: ['Alt+S'], context: 'global' }, // ⌥F, leaving ⌥⇧F open for Auto-follow ALL Visible Agents — the same // soft/heavy pairing, and the command that OWNS the effective state when diff --git a/src/renderer/src/features/command-palette/catalog.test.ts b/src/renderer/src/features/command-palette/catalog.test.ts index 9d35b54d..ef42b88f 100644 --- a/src/renderer/src/features/command-palette/catalog.test.ts +++ b/src/renderer/src/features/command-palette/catalog.test.ts @@ -132,6 +132,7 @@ const BASELINE_COMMAND_IDS: readonly string[] = [ 'enable-root-agent-code-management', 'enable-tldr-mcp', 'enable-goal-mcp', + 'enable-goal-loop-mcp', 'enable-workflow-mcp', 'reload-agent', 'soft-reload-agent', @@ -233,12 +234,12 @@ const NAVIGATION_COMMAND_GROUP: readonly string[] = [ const ids = (): string[] => builtInCommandCatalog.map(c => c.id) describe('built-in command catalog — baseline characterization', () => { - it('contains exactly the 123 governed commands in registration order', () => { + it('contains exactly the 124 governed commands in registration order', () => { // Order matters: this is the palette's empty-query browse order. expect(ids()).toEqual([...BASELINE_COMMAND_IDS]) }) - it('has exactly 123 commands', () => { + it('has exactly 124 commands', () => { // Stated separately from the order assertion because this number is the // thing that moves, and a bare count failure is a clearer signal than a // 99-line array diff. @@ -260,11 +261,12 @@ describe('built-in command catalog — baseline characterization', () => { // −normalize×3 → 114 with stage 3a: −tiled-tabs, −bury/revive/kill-buried, // −attach×2, −detach → 115 with Clear Lane (#992 stage 4) → 119 with the // lane keyboard grammar (#992 stage 5) → 123 once main's Goal Loop preview - // and stop (#1001) and the two generated Grok splits (#844) merged in. + // and stop (#1001) and the two generated Grok splits (#844) merged in → 124 + // with Goal Loop MCP (#1006). // Each step of that arithmetic was a deliberate edit to this line, which is the entire point of pinning it. (The two test // titles above had drifted to "115" while this line said 116; they now // track it again.) - expect(builtInCommandCatalog).toHaveLength(123) + expect(builtInCommandCatalog).toHaveLength(124) }) it('reports no structural defects', () => { @@ -298,13 +300,14 @@ describe('generated per-provider split commands', () => { }) it('accounts for the difference between literal and total command count', () => { - // 123 total - 6 generated = 117 literal `id:` fields across the command + // 124 total - 6 generated = 118 literal `id:` fields across the command // modules. At the original baseline this read 102 - 4 = 98; it moved down by // the five retirements, then back up by the nine additions, Grid Dispatch's // six row commands, New Window, and the later additions recorded in the // count test above (through the lane keyboard grammar, #992 stage 5, and - // Goal Loop, #1001). Grok (#844) grew only the GENERATED term, 4 → 6. - expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(117) + // Goal Loop, #1001, and Goal Loop MCP, #1006). Grok (#844) grew only the + // GENERATED term, 4 → 6. + expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(118) }) it('emits both directions for every non-default provider', () => { @@ -411,7 +414,7 @@ describe('governance targets', () => { }) it('lands on the arithmetic the plan predicted', () => { - // 102 baseline - 21 retirements + 42 additions = 123, checked against the + // 102 baseline - 21 retirements + 43 additions = 124, checked against the // real catalog rather than trusted as prose. (5 governance retirements + // 16 unified-layout retirements, all recorded in RETIRED_COMMAND_IDS.) // @@ -441,9 +444,9 @@ describe('governance targets', () => { // `agent-analytics.open` (#964), `goal-loop-preview` and `goal-loop-stop` // (#1001), `grok-vertical` and `grok-horizontal` (#844, generated from // AGENT_PROVIDER_KINDS), `clear-focused-lane` (#992 stage 4), and the four - // lane-grammar commands (#992 stage 5). - expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 42).toBe(102) - expect(builtInCommandCatalog).toHaveLength(123) + // lane-grammar commands (#992 stage 5), and `enable-goal-loop-mcp` (#1006). + expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 43).toBe(102) + expect(builtInCommandCatalog).toHaveLength(124) }) }) diff --git a/src/renderer/src/features/goal-loop/controlReference.ts b/src/renderer/src/features/goal-loop/controlReference.ts index 6ecab826..80fd0217 100644 --- a/src/renderer/src/features/goal-loop/controlReference.ts +++ b/src/renderer/src/features/goal-loop/controlReference.ts @@ -9,7 +9,7 @@ export const controlReference = [{ title: 'Harness-owned goal loops', purpose: 'Keep an agent working toward a goal across turn boundaries until it reports the goal complete, without the user re-prompting.', ui: 'An always-on per-pane status strip while a loop exists, a latched Goal Loop overlay with pause, resume, raise-cap and stop controls (Close leaves it), Dismiss on an ended loop to clear its strip, and a Stop Goal Loop session command.', - prerequisites: 'Enable Goal Loop MCP for the agent that should run loops (off by default). The agent starts the loop itself with goal_loop_start when asked; users never write the continuation prompt.', + prerequisites: 'Enable Goal Loop MCP for the agent that should run loops (off by default; per agent with the Goal Loop MCP command, or for all agents in Settings). The agent starts the loop itself with goal_loop_start when asked; users never write the continuation prompt.', workflow: [ 'Enable Goal Loop MCP for the agent; it reloads with the loop tools.', 'Ask the agent to run a loop on an outcome; it calls goal_loop_start with the goal and its own continuation prompt.', @@ -19,5 +19,5 @@ export const controlReference = [{ ], outcome: 'The pane strip reads e.g. “Goal loop · active · iteration 3/25 · ” with Pause and Stop, and the latched overlay centers the full state with all controls.', cautions: 'Loops consume model calls on every continuation; the budget pauses rather than kills, and Raise cap re-arms it knowingly. A paused(loop) after a restart or provider switch stays paused until resumed. complete(done) is the agent\'s own claim of completion, not a verified fact; the summary states what it believes finished.', - commandIds: ['goal-loop-preview', 'goal-loop-stop'], + commandIds: ['goal-loop-preview', 'goal-loop-stop', 'enable-goal-loop-mcp'], }] satisfies FeatureReference[] diff --git a/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx b/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx index e639ccfd..3b5b7f7c 100644 --- a/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx +++ b/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx @@ -78,11 +78,11 @@ describe('goal loop overlay keyboard dismissal', () => { keyDown({ key: 'Escape' }) expect(screen.queryByRole('dialog')).toBeNull() }) - it('the Cmd+Shift+Y toggle chord dismisses the latched overlay', async () => { + it('the Cmd+Shift+G toggle chord dismisses the latched overlay', async () => { toggleGoalLoop() render(<>) expect(await screen.findByRole('dialog')).toBeTruthy() - keyDown({ key: 'y', code: 'KeyY', metaKey: true, shiftKey: true }) + keyDown({ key: 'g', code: 'KeyG', metaKey: true, shiftKey: true }) expect(screen.queryByRole('dialog')).toBeNull() }) }) @@ -158,7 +158,7 @@ describe('goal loop command with no loop on the session (#1021)', () => { // press re-ran the command and re-armed an invisible latch. render() runGoalLoopCommand() - keyDown({ key: 'y', code: 'KeyY', metaKey: true, shiftKey: true }) + keyDown({ key: 'g', code: 'KeyG', metaKey: true, shiftKey: true }) expect(useGoalLoopView.getState().latched).toBe(false) expect(harness.appState.requestCommandInvocation).not.toHaveBeenCalled() }) @@ -180,8 +180,8 @@ describe('goal loop command with no loop on the session (#1021)', () => { }) it('a rebound goal-loop-preview chord still dismisses the overlay', async () => { - // #1007 plans to move the default chord off Cmd+Shift+Y. Before this fix - // the dismissal was a hardcoded Meta+Shift+KeyY check, so any rebind would + // #1007 moved the default chord off Cmd+Shift+Y. Before #1021's fix the + // dismissal was a hardcoded Meta+Shift+KeyY check, so any rebind would // have left Escape as the only keyboard exit. harness.appState = { ...harness.appState, settings: { agentViewMode: 'agent', commandKeybindingOverrides: { 'goal-loop-preview': ['Cmd+Ctrl+J'] } } } render(<>) @@ -190,4 +190,10 @@ describe('goal loop command with no loop on the session (#1021)', () => { keyDown({ key: 'j', code: 'KeyJ', metaKey: true, ctrlKey: true }) expect(screen.queryByRole('dialog')).toBeNull() }) + + it('the old Cmd+Shift+Y no longer opens the goal loop (#1007: macOS New Sticky Note)', () => { + render() + keyDown({ key: 'y', code: 'KeyY', metaKey: true, shiftKey: true }) + expect(harness.appState.requestCommandInvocation).not.toHaveBeenCalledWith('goal-loop-preview', expect.anything()) + }) }) diff --git a/src/renderer/src/features/settings/lib/settingsRegistry.ts b/src/renderer/src/features/settings/lib/settingsRegistry.ts index 5d934a30..036718a8 100644 --- a/src/renderer/src/features/settings/lib/settingsRegistry.ts +++ b/src/renderer/src/features/settings/lib/settingsRegistry.ts @@ -747,7 +747,7 @@ export function getSettingsRegistry( category: 'agents', title: 'Goal Loop MCP', description: - 'Let agents run harness-owned goal loops that keep re-prompting until the goal is complete, with a control strip and Cmd+Shift+Y overlay. Off by default. Applies to new agents and existing agents on their next reload. Per-agent overrides take precedence; Use Global MCP Settings clears them.', + 'Let agents run harness-owned goal loops that keep re-prompting until the goal is complete, with a control strip and Cmd+Shift+G overlay. Off by default. Applies to new agents and existing agents on their next reload. Per-agent overrides take precedence; Use Global MCP Settings clears them.', keywords: ['mcp', 'goal', 'loop', 'persistence', 'autonomous', 'default', 'reload', 'existing agents'], metadata: { scope: 'app', apply: 'new-session', storage: 'settings' }, control: { diff --git a/src/renderer/src/features/workspace/commands/sessionCommands.ts b/src/renderer/src/features/workspace/commands/sessionCommands.ts index 83deafbb..f711c9d3 100644 --- a/src/renderer/src/features/workspace/commands/sessionCommands.ts +++ b/src/renderer/src/features/workspace/commands/sessionCommands.ts @@ -775,6 +775,42 @@ export const sessionCommands: CommandDef[] = [ }) }, }, + { + // #1006: the per-agent toggle every sibling domain has (TLDR, Goal). + // goal_loop already had the Settings row and the per-session override + // plumbing, but no command, so "turn the loop on for just this agent" + // meant a trip through Settings and a reload by hand. + id: 'enable-goal-loop-mcp', + category: 'session', + surface: 'session', + title: 'Goal Loop MCP', + description: '**What it does:** Reloads the focused agent with goal-loop tools on or off.\n\n**Use when:** You want this agent to be able to run a harness-owned goal loop that keeps re-prompting it until the goal is done.\n\n**Notes:** The agent starts a loop itself when you ask it to (goal_loop_start). Every continuation is a model call, and the loop pauses at its budget. The Goal Loop command shows and controls a running loop.', + keywords: ['goal', 'loop', 'autonomous', 'persistence', 'keep going', 'mcp'], + when: ({ workspace }) => { + return targetSupportsBuiltInMcpDomain(workspace, 'goal_loop') + }, + getState: ctx => builtInMcpDomainState(ctx, 'goal_loop'), + run: async ({ workspace, ui }) => { + const sessionId = commandTargetSessionId(workspace) + if (!sessionId) return + const meta = workspace.state.sessions[sessionId] + const kind = meta?.kind ?? DEFAULT_PROVIDER + // Provider policy is repeated at the mutation boundary: visibility is + // advisory and the command stays reachable from keybindings and control. + if ( + !isAgentProviderKind(kind) || + !providerSupportsBuiltInMcpDomain(kind, 'goal_loop') || + !meta + ) return + + ui.closePalette() + const enable = !meta.builtInMcpDomains?.includes('goal_loop') + await reloadSessionWithBuiltInMcpChoice(workspace, sessionId, 'goal_loop', enable, { + reloaded: enable ? 'Reloaded with Goal Loop MCP' : 'Reloaded without Goal Loop MCP', + failed: 'Goal Loop MCP reload failed', + }) + }, + }, { id: 'enable-workflow-mcp', category: 'session', diff --git a/src/renderer/src/workspace/hook/actions/mcpPreferences.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/mcpPreferences.renderer.test.tsx index 5136b1ce..3da81802 100644 --- a/src/renderer/src/workspace/hook/actions/mcpPreferences.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/mcpPreferences.renderer.test.tsx @@ -75,6 +75,8 @@ describe('global MCP preferences at actual provider replacement', () => { it.each([ { domain: 'tldr', commandId: 'enable-tldr-mcp' }, { domain: 'goal', commandId: 'enable-goal-mcp' }, + // #1006: the sibling command goal_loop lacked. + { domain: 'goal_loop', commandId: 'enable-goal-loop-mcp' }, ] as const)('preserves a per-agent $domain off override and lets the reset command restore inheritance', async ({ domain, commandId }) => { const h = setup({ builtInMcpDomains: [domain], builtInMcpOverrides: {} }) h.refs.defaultBuiltInMcpDomainsRef.current = [domain] From aa57a8cbac04be162c3d608aca7b01e722f97984 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:25:21 -0700 Subject: [PATCH 2/3] fix(goal-loop): the editor keeps Cmd+Shift+G, and turning the tools off ends the loop Review of #1045 (CHANGES REQUESTED): 1. Blocking: Cmd+Shift+G is Monaco's Find Previous, exactly as Cmd+G beside it is Find Next. Review reproduced the collision: pressing it in the global editor ran Find Previous AND latched the goal-loop overlay, and the latch gate then swallowed every keystroke until Escape. check:keybindings passed only because Monaco's find chords were not in the reservation table. - The router now yields the chord whenever editor chrome owns the target, the same rule Cmd+L and Cmd+G already follow, returning without preventDefault so Monaco's own handler runs. - The stale-latch branch yields too: it still drops the latch, but consuming the key there cost one find per stale latch. - reservations.ts records Monaco's Find Previous (verified in monaco-editor findController.js) with an approved overlap, so the checker stops offering the chord as free. 14 reserved interactions, 9 approved overlaps, check:keybindings OK. 2. Turning Goal Loop MCP off now ends a running loop. The loop is harness-owned and survives the reload, but the reloaded agent has no goal_loop_complete: it could never report success, and every continuation would run to the cap. The stop names the session the loop is filed under, before the replacement exists, and a failure to stop never blocks the reload. 3. The stale WHY naming #1007 as future work, the Settings text and the defaults comment are updated. Tests: the router suite gains the positive chord case, an editor-owned case and a stale-latch editor case; the last two fail with the yield reverted. The MCP-preferences suite pins the loop stop, and that the enable direction never touches the loop. Co-Authored-By: Claude Opus 5 (1M context) --- .../features/command-keybindings/defaults.ts | 6 +++ .../command-keybindings/reservations.ts | 10 +++++ .../goalLoop.router.renderer.test.tsx | 44 ++++++++++++++++++- .../features/settings/lib/settingsRegistry.ts | 2 +- .../workspace/commands/sessionCommands.ts | 16 ++++++- .../actions/mcpPreferences.renderer.test.tsx | 19 +++++++- .../src/workspace/tile-tree/useKeybinds.ts | 29 ++++++++++-- 7 files changed, 117 insertions(+), 9 deletions(-) diff --git a/src/renderer/src/features/command-keybindings/defaults.ts b/src/renderer/src/features/command-keybindings/defaults.ts index b2468b13..72e99fc4 100644 --- a/src/renderer/src/features/command-keybindings/defaults.ts +++ b/src/renderer/src/features/command-keybindings/defaults.ts @@ -327,6 +327,12 @@ export function buildDefaultKeybindings(): CommandBindingDefault[] { // Sticky Note" service, which check:keybindings cannot see. ⌘⇧G sits // beside the Goal peek (⌘G), and the unified stage (#992) freed it when // Global Dispatch was retired. + // + // ⌘⇧G is ALSO Monaco's Find Previous, exactly as ⌘G beside it is Monaco's + // Find Next (#1045 review reproduced the collision). Both are approved + // overlaps: the router yields the chord whenever editor chrome owns the + // target, so only one owner is ever live. reservations.ts records + // Monaco's claim so the checker stops offering the chord as free. { commandId: 'goal-loop-preview', bindings: ['Cmd+Shift+G'], context: 'global' }, { commandId: 'toggle-spotlight', bindings: ['Alt+S'], context: 'global' }, // ⌥F, leaving ⌥⇧F open for Auto-follow ALL Visible Agents — the same diff --git a/src/renderer/src/features/command-keybindings/reservations.ts b/src/renderer/src/features/command-keybindings/reservations.ts index 68f9c98a..484e0d60 100644 --- a/src/renderer/src/features/command-keybindings/reservations.ts +++ b/src/renderer/src/features/command-keybindings/reservations.ts @@ -264,6 +264,15 @@ export const RESERVED_INTERACTIONS: readonly ReservedInteraction[] = [ }, { bindings: ['Cmd+L'], context: 'editor', owner: 'Editor Select Line' }, { bindings: ['Cmd+G'], context: 'editor', owner: 'Editor Find Next' }, + // Monaco's Find Previous, verified in + // node_modules/monaco-editor/esm/vs/editor/contrib/find/browser/findController.js + // (PreviousMatchFindAction, mac: CtrlCmd|Shift|KeyG) rather than from memory. + // Missing until #1007 moved goal-loop-preview onto this chord: the app-side + // checker passed because nothing in this table claimed it, and review found + // the router was latching the goal-loop overlay on top of Monaco's own + // Find Previous. This is the second time the "incomplete list" failure in + // the header has happened, and both times a reservation was the fix. + { bindings: ['Cmd+Shift+G'], context: 'editor', owner: 'Editor Find Previous' }, ] export type BindingOwnerRef = { @@ -294,6 +303,7 @@ const APPROVED_OVERLAPS: ReadonlyArray<{ }> = [ { binding: 'Cmd+L', owners: ['tldr-preview', 'Editor Select Line'], reason: 'The TLDR hold handler explicitly yields while editor chrome owns the input target; Monaco keeps its native Select Line command and TLDR operates only in the agent workspace.' }, { binding: 'Cmd+G', owners: ['goal-preview', 'Editor Find Next'], reason: 'Goal shares the TLDR hold handler, which yields while editor chrome owns the input target; Monaco keeps its native Find Next and Goal operates only in the agent workspace.' }, + { binding: 'Cmd+Shift+G', owners: ['goal-loop-preview', 'Editor Find Previous'], reason: 'The router yields the chord while editor chrome owns the input target, the same rule Cmd+L and Cmd+G already follow; Monaco keeps its native Find Previous and the goal loop overlay opens only in the agent workspace.' }, { binding: 'Cmd+W', owners: ['close-pane', 'Editor-native close file and indentation', 'Native application menu'], diff --git a/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx b/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx index 3b5b7f7c..38449337 100644 --- a/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx +++ b/src/renderer/src/features/goal-loop/goalLoop.router.renderer.test.tsx @@ -48,8 +48,17 @@ function workspace(): Workspace { } function Harness({ model }: { model: Workspace }) { useKeybinds(model) - // A composer-like focus target, as in the tldr router harness. - return + return ( + <> + {/* A composer-like focus target, as in the tldr router harness. */} + + {/* Editor chrome, with the marker the router reads to decide that Monaco + owns the target. The real global editor stamps it the same way. */} +
+