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

@kocaemre kocaemre commented Sep 12, 2026 •

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

  • RED (original test-first cycle): with production files restored to upstream, the new regression failed because default turns lost resolved reviewer/sandbox/network/writable-root values.
  • npm ci
  • npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t 'preserves resolved prompt permissions|prompt permissions' — 2 passed, 120 skipped.
  • npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts — 122 passed.
  • npm run typecheck
  • npm run build
  • git diff --check upstream/main..HEAD

Rebased onto main at 202e66e; conflicts resolved while preserving upstream skipped-MCP-server metadata and app-server-loss handling. #561 overlaps the explicit network flag but not the broader resolved default permission set here; older #368 overlaps roots/network and merits maintainer scope review.

@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.

@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.

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

Copy link
Copy Markdown
Author

Rebased onto main at 202e66e and preserved the upstream skipped-MCP-server metadata and app-server-loss cleanup while resolving four conflicts. Current head: d97576069915e90f6e8d489994664edfe49c5b5f (DCO signed-off).

Verification: npm ci; npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts -t "preserves resolved prompt permissions|prompt permissions" (2 passed); npm run test -- src/__tests__/CodexACPAgent/CodexAcpClient.test.ts (122 passed); npm run typecheck; npm run build; git diff --check upstream/main..HEAD. The existing regression tests previously failed when production files were restored to upstream (resolved reviewer/sandbox/network/writable roots lost) and pass on this head.

I also checked #561: it carries the explicit workspace network flag only. This PR preserves the full resolved default permission set (including reviewer, approval policy, sandbox, and writable roots) across new/resumed/forked sessions, while explicit non-default modes remain unchanged. I did not modify #561.

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