Skip to content

fix: preserve resolved prompt permissions - #503

Open
kocaemre wants to merge 1 commit into
agentclientprotocol:mainfrom
kocaemre:fix/preserve-resolved-prompt-permissions
Open

kocaemre wants to merge 1 commit into
agentclientprotocol:mainfrom
kocaemre:fix/preserve-resolved-prompt-permissions

Conversation

@kocaemre

Copy link
Copy Markdown

Summary

  • Preserve the Codex app-server's resolved default approval/reviewer/sandbox settings in session metadata.
  • Use those resolved permissions for default Agent prompts instead of reapplying the hardcoded default Agent preset.
  • Keep explicit non-default mode behavior unchanged, and continue adding ACP additionalDirectories to workspace-write roots.
  • Add regression coverage for configured writable roots/network access and explicit mode overrides.

Closes #477

Test Plan

  • npm run typecheck
  • npx vitest run src/__tests__/CodexACPAgent/CodexAcpClient.test.ts --testNamePattern 'preserves resolved default prompt permissions|uses explicit non-default mode prompt permissions|applies ACP additional directories'
  • npx vitest run src/__tests__/CodexACPAgent/CodexAcpClient.test.ts
  • git diff --check

Note: the full targeted test file prints existing stderr log lines from auth/rate-limit negative-path tests, but the Vitest run exited successfully with 107/107 tests passing.

@kocaemre
kocaemre force-pushed the fix/preserve-resolved-prompt-permissions branch from 01d6ed3 to 8b229be Compare October 1, 2026 18:38
@kocaemre

kocaemre commented Oct 1, 2026

Copy link
Copy Markdown
Author

Rebased this PR onto current main and resolved the prompt-permission conflicts against the current session resume/load flow.

Verification:

  • RED proof: temporarily restored production files to upstream/main while keeping the regression tests; npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t 'resolved default prompt permissions|explicit non-default mode prompt permissions' failed because default turns lost the resolved reviewer/sandbox/network/writable root values.
  • GREEN: npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t 'resolved default prompt permissions|explicit non-default mode prompt permissions' — 2 passed, 112 skipped.
  • GREEN broader file: npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts — 114 passed.
  • npm run typecheck — passed.
  • npm run build — passed.
  • git diff --check upstream/main..HEAD — clean.

Head: 8b229be5d8615a22268a1fd3370e5193c4ca3887.

Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com>
@kocaemre
kocaemre force-pushed the fix/preserve-resolved-prompt-permissions branch from 8b229be to 038cf41 Compare October 2, 2026 20:41
@kocaemre

kocaemre commented Oct 2, 2026

Copy link
Copy Markdown
Author

Rebased this onto current main (ca1d971) after it became conflicting again and kept the existing prompt-permission replay behavior while preserving the current MCP/session-failure/auth-status code paths.

Verification on head 038cf410:

  • npm ci
  • npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t 'preserves resolved prompt permissions|prompt permissions' — 2 passed, 113 skipped
  • npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts — 115 passed
  • npm run typecheck
  • npm run build
  • git diff --check upstream/main..HEAD

gh pr view 503 now reports OPEN / MERGEABLE / UNSTABLE; files are still limited to the prompt-permission patch and its tests.

This branch has not been deployed

No deployments
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.

Default Agent prompts overwrite configured writable roots and sandbox permissions

1 participant