Skip to content

Fix Copilot allow all mode fallback#1240

Open
theontho wants to merge 2 commits into
getpaseo:mainfrom
theontho:fix/copilot-allow-all-fallback
Open

Fix Copilot allow all mode fallback#1240
theontho wants to merge 2 commits into
getpaseo:mainfrom
theontho:fix/copilot-allow-all-fallback

Conversation

@theontho
Copy link
Copy Markdown

Summary

  • fall back to Copilot's /allow-all slash command when allow_all config fails before the permission service is ready
  • keep Paseo mode/config state in sync after the fallback
  • add regression coverage for the Copilot startup failure path

Fixes #1239

Test plan

  • npm run format:files -- packages/server/src/server/agent/providers/copilot-acp-agent.ts packages/server/src/server/agent/providers/acp-agent.test.ts
  • npx vitest run packages/server/src/server/agent/providers/acp-agent.test.ts --bail=1
  • npm run typecheck:server
  • npm run lint -- packages/server/src/server/agent/providers/copilot-acp-agent.ts packages/server/src/server/agent/providers/acp-agent.test.ts
  • manual desktop dev validation against isolated daemon

When Copilot rejects the allow_all ACP config because its permission service is not ready, fall back to Copilot's /allow-all command and keep Paseo's mode state in sync.

Fixes getpaseo#1239

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR fixes a Copilot startup race where setSessionConfigOption(allow_all=on) fails with "Permission service is unavailable" before the permission service is ready, causing Allow All mode to silently not apply. The fix falls back to sending Copilot's /allow-all slash command directly, with state kept in sync on the Paseo side via a local configOptions update.

  • copilot-acp-agent.ts: writeCopilotProviderMode wraps the config write in try/catch; on the specific permission-service error it sends a connection.prompt("/allow-all") fallback, guarded against concurrent foreground turns and non-end_turn stop reasons (which surface the original error).
  • acp-agent.ts: ACPProviderModeWriterContext gains hasActiveTurn and suppressUserEcho so the fallback can suppress the ACP echo and avoid unsafe concurrent prompts.
  • acp-agent.test.ts: Three new regression tests cover the success path, the concurrent-turn guard, and the non-end_turn error-surfacing path.

Confidence Score: 5/5

Safe to merge — the fallback is narrowly scoped to a specific error during initialization, well-guarded against concurrent turns, and covered by three new regression tests.

The fallback only fires when setSessionConfigOption throws the exact 'Permission service is unavailable' message, no active foreground turn exists, and Copilot confirms success via an end_turn stop reason. All three decision branches are exercised by the new tests, and Paseo-side state is correctly updated in each path.

No files require special attention.

Important Files Changed

Filename Overview
packages/server/src/server/agent/providers/copilot-acp-agent.ts Core logic change: wraps setSessionConfigOption in try/catch, adds /allow-all fallback with guards for concurrent turns and non-end_turn stop reasons. New helpers are straightforward and correct.
packages/server/src/server/agent/providers/acp-agent.ts Adds hasActiveTurn and suppressUserEcho to ACPProviderModeWriterContext; wired up correctly from existing session fields.
packages/server/src/server/agent/providers/acp-agent.test.ts Adds prompt mock to shared harness and three new regression tests covering all new branches.

Sequence Diagram

sequenceDiagram
    participant P as Paseo (applyConfiguredOverrides)
    participant C as Copilot (ACP)

    P->>C: "setSessionConfigOption(allow_all=on)"
    alt Success
        C-->>P: configOptions[]
        P->>P: update currentMode + configOptions
    else PermissionServiceUnavailable error
        C-->>P: throws -32603 error
        alt "activeForegroundTurnId !== null"
            P->>P: log warn, re-throw
        else no active turn
            P->>P: suppressUserEcho(fallbackMessageId)
            P->>C: prompt("/allow-all")
            C-->>P: "PromptResponse {stopReason}"
            alt "stopReason === end_turn"
                P->>P: setCopilotAllowAllConfigValue(on) locally
                P->>P: "update currentMode = ALLOW_ALL"
            else other stopReason
                P->>P: log warn, re-throw original error
            end
        end
    end
Loading

Reviews (2): Last reviewed commit: "Address PR review for Copilot allow-all ..." | Re-trigger Greptile

Comment thread packages/server/src/server/agent/providers/copilot-acp-agent.ts Outdated
- Refuse the /allow-all fallback while a foreground turn is active to
  avoid issuing a concurrent prompt on the same ACP session; surface the
  original error instead.
- Suppress the user echo for the synthesized /allow-all message so the
  slash command does not appear in the visible thread.
- Inspect the prompt response and surface the original error if the
  fallback turn ends with a non-end_turn stop reason rather than
  optimistically marking allow_all as on.
- Extend ACPProviderModeWriterContext with hasActiveTurn and a
  suppressUserEcho hook to wire the above without leaking session
  internals into providers.
- Add regression tests for both the active-turn refusal and the
  non-end_turn stop reason paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: copilot in allow all mode gives an error

1 participant