Skip to content

feat(session): add Auto to permission controls - #413

Merged
zfy0701 merged 3 commits into
mainfrom
codex/session-auto-permission
Aug 2, 2026
Merged

feat(session): add Auto to permission controls#413
zfy0701 merged 3 commits into
mainfrom
codex/session-auto-permission

Conversation

@zfy0701

@zfy0701 zfy0701 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a single Auto permission preset to Home and session composers
  • expose the same preset in Slack Session Options and Telegram/Discord /permission controls
  • persist the session preset while decomposing Auto to ACP mode=agent plus _approvals_reviewer=auto_review
  • reset the reviewer to user when leaving Auto, before widening to another permission mode

This follows up #410 so Agent configuration and every session-level control now use the same merged permission UI.

Validation

  • daemon, web, protocol, and control-plane typechecks
  • focused daemon tests: ACP config/host, Slack render, chat commands, and WebChat runtime-authority paths
  • focused Web permission-control tests
  • protocol WebChat tests
  • daemon and Web production builds
  • repository pre-push lint

Created by Codex . GPT-5.6 Sol

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the Auto-permission-preset composition across daemon/protocol/web. Logic is sound: the synthetic agent:auto-review value never leaks onto the ACP wire (always decomposed to mode+_approvals_reviewer before set_config_option), and reviewer-disable-before-mode-widen ordering correctly avoids a transient Auto+full-access window. Test coverage is solid across all four packages.

Two small non-blocking notes:

  • AcpHost.setSessionPermissionPreset returns modeApplied || reviewerApplied — if the mode-set call fails but the preceding reviewer-reset succeeded, this returns true even though the requested mode never took effect. No current caller inspects the bool (daemon.ts discards it), so no live impact, but worth tightening if a future caller starts relying on it.
  • Every non-Auto permission change on a reviewer-capable runtime unconditionally re-sends approvals_reviewer=user, even when already user — a redundant ACP round-trip on plain mode switches (harmless/idempotent, just extra chatter).

Nothing blocking; approving.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This revision only touches packages/daemon/test/daemon-commands.test.ts: wrapping the full access and ask for approval permission-mode assertions in vi.waitFor (since the host apply is a fire-and-forget async chain, asserting immediately after onInbound was racy), and clearing the setSessionPermissionMode/setSessionApprovalsReviewer mocks before the Auto-preset step so that assertion can't be trivially satisfied by a leftover call from the prior step. Good test-robustness fix, no logic changes. My prior approval of the permission-preset composition logic (unchanged since the last revision) stands — approving.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test

Copy link
Copy Markdown
Contributor

Changes requested: one permission-ordering gap remains.

setSessionPermissionPreset() safely clears Auto-review before widening a warm session, but applySessionConfig() still applies mode before _approvals_reviewer during session/new/session/load. Restoring Full Access + user review over a loaded Auto session therefore temporarily creates Full Access + Auto-review.

Please use the same conditional ordering for new/load configuration and add a load regression test.

Validated on 04002e28: all four typechecks, focused daemon tests (222), focused web tests (17), protocol tests (240), and git diff --check passed. Compatibility concerns are non-blocking.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one permission-ordering gap.

setSessionPermissionPreset() safely clears Auto-review before switching a warm session to an ordinary (especially wider) mode, but restored sessions do not use that ordering. applySessionConfig() still applies configured mode before _approvals_reviewer. If a session is loaded with mode=agent and auto_review, then the Agent configuration has changed to Full Access + user review, the load path first applies Full Access while Auto-review is still active and only clears the reviewer afterward. That recreates the exact widened intermediate state this change is intended to prevent. Please make the load preference pipeline use the same conditional ordering—user reviewer before an ordinary/wider mode, auto_review only after Agent mode—and add a load regression test.

Validation on head 04002e28: all GitHub CI checks are green; daemon, web, protocol, and control-plane typechecks passed locally; focused daemon suites passed (222 tests); focused web suites passed (17 tests); protocol passed (240 tests); git diff --check passed. Compatibility and migration concerns were treated as non-blocking.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

const reviewerAvailable = this.approvalsReviewerOptions(sessionId) !== null
// Disable Auto-review before changing to an ordinary mode (especially Full
// Access); enable it only after the requested Agent mode is in force.
let reviewerApplied = false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ordering only protects live preset changes. applySessionConfig() still orders mode before _approvals_reviewer for session/load, so loading a session that currently has Auto and restoring Full Access + user review first creates Full Access + Auto-review, then clears it. Route configured permissions through the same conditional ordering and cover the load case.

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This revision extends the "disable Auto-review before widening, enable it only after Agent mode is set" ordering (already correct in the live setSessionPermissionPreset mid-session switch reviewed earlier) to the initial config-preference application in AcpHost used on session/new/session/load. Previously that path always applied mode before _approvals_reviewer regardless of direction, which could fail against a runtime that rejects widening to full-access while auto-review is still active on a restored session — exactly the scenario the new fake-acp-agent.mjs behavior (AC_REJECT_AUTO_FULL_ACCESS) and the new acp-host.test.ts case simulate and verify. The conditional ordering is symmetric with the existing mid-session logic, and the undefined/no-reviewer-preference case (the common path for non-Codex runtimes) is unaffected since the array position doesn't matter when the reviewer entry is a no-op.

No issues found — approving.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on a8f089cf.

The restored-session ordering issue is fixed: applySessionConfig() now clears auto_review before applying an ordinary/wider mode and enables it only after Agent mode, and the new load regression makes the fake adapter reject the previously unsafe Full Access + Auto sequence. The broader composite-preset handling across daemon, chat controls, protocol metadata, and Web surfaces remains coherent.

Validation: focused daemon suites passed (223 tests), focused Web suites passed (17 tests), protocol passed (240 tests), daemon typecheck passed, and git diff --check passed. GitHub Build and Check are green; the remaining CI jobs were still running when reviewed.

Non-blocking migration warning: older Agent configs that omit approvalsReviewer do not explicitly coalesce that value to user inside the load preference pipeline, so a restored prior Auto value can survive until the daemon's per-turn configured-setting reconciliation. Per the active-development guidance, this compatibility edge does not block approval.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@zfy0701
zfy0701 merged commit 45b9b42 into main Aug 2, 2026
9 checks passed
@zfy0701
zfy0701 deleted the codex/session-auto-permission branch August 2, 2026 09:21
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.

1 participant