Skip to content

fix(acp): keep idle permissions on their session policy - #30316

Open
njfife wants to merge 1061 commits into
pingdotgg:fix/issue-26852from
njfife:fix/acp-idle-policy-ownership
Open

njfife wants to merge 1061 commits into
pingdotgg:fix/issue-26852from
njfife:fix/acp-idle-policy-ownership

Conversation

@njfife

@njfife njfife commented Nov 3, 2026 •

Copy link
Copy Markdown

Problem

Stacked follow-up to #26870, targeting its fix/issue-26852 branch. During session activation, its idle permission fallback can answer a callback from the previous session using the incoming session's Full access policy. An explicit tighter policy on same-session resume is also ignored, and a failed activation can leave the provisional policy installed.

Change

Bind the existing fallback policy to one native session ID before activation and decline foreign idle requests. Restore both values when activation fails; retain the incoming binding once the new session has been adopted, including if later model configuration fails. Refresh explicit same-session policies and preserve the current binding when the policy is omitted.

Extend the existing resume regression across both activation entry points, success, real load failure and post-adoption configuration failure. Add a real ACP v1 replay table covering model-configuration confirmations and initial open. Existing mock/runtime/replay helpers are reused.

This preserves #26870's permission behavior: eligible Full access requests allow; Supervised/Auto-accept edits decline or cancel pre-turn confirmations. No interactive approval UI, model defaults or provider-specific bypass is added. The refreshed shared policy also feeds the existing terminal/file and work-state policy checks.

Scope and approval

Depends on #26870; this does not replace or duplicate its original fix for #26852. Please review only the follow-up on top of #26870: commit a3ecf3c (adapter and its test) and Maria’s one-file conflict fix 0b950cf. The narrow obvious-bug exception is the intended route: a callback must use its owning session's policy, an explicit policy must take effect, and a failed activation must restore the previous binding. No broader product-direction approval is claimed.

Maintainer update: Maria merged current main and resolved the permission-handler conflict at 0b950cf24793, preserving idle-session handling, policy ownership and Kiro tool-kind inference. This includes the already-merged usage-test clock fix #28603. The PR still targets #26870; until that base is updated, GitHub’s comparison also includes the merged upstream history (2,340 files at this head). The combined ACP diff against merged main 15707a84b03e is three files and includes #26870’s still-unmerged fallback, mock fixture and tests. This maintenance update is not claimed as approval to merge the PR.

Verification

Historical baseline: before the main merge, the six ownership/refresh/recovery cases fail on parent 43b7a7b0069a. Both the six-case table and five-case v1 replay table at current head 0b950cf24793 are byte-identical to their pre-merge text at a3ecf3c. Current validation (local commands use Node 26.12.1):

  • vp test run apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts -t 'v1 set_model permissions|binds idle permissions': 11 pass.
  • vp run check:changed 15707a84b03e27bb257d60e027812ebf94bbda25: pass, including all 101 adapter tests (102 tests total), 14 affected package typechecks, formatting, lint, both Knip checks and GraphQL generation checks.
  • GitHub CI passes on 0b950cf24793, including Test Server 7 and the aggregate Check.
  • The formerly failing usage-window case passes independently: vp test run apps/server/src/usage/UsageService.test.ts -t 'reports saved usage of a removed directory only for windows it reaches'. The 102-test changed-file check covers the ACP-specific diff against merged main; full CI covers the larger comparison against the older prerequisite branch.
  • Historical runtime capture from the actual v1 replay at a3ecf3c: session/set_model → nested session/request_permission → allow-once in Full access; supervised replies reject-once or cancelled. Recovery and ownership assertions use actual runtime-handler responses. The peer's success/refusal is scripted.

Additional integration evidence comes from the equivalent fix on newer main-based build 9629001, not the current main-merged head: packaged startup/render checks passed on Linux x64, Windows x64 and macOS arm64; all 99 adapter cases passed natively on Windows and Mac. A user testing that Mac build with real Hermes 0.22.5 reported successful basic chat and model switching. The exact context count, large-context confirmation frame and post-switch follow-up response were not independently captured, so this is not claimed as a complete real-agent reproduction of the >100,000-token case.

Interactive supervised pre-turn approval remains unimplemented. A refused switch during resume can still enter the pre-existing fresh-native-session recovery. Other client/connection paths and native packaging of the current main-merged head were not rerun. The temporary SSH workspace bridge used for integration testing is not part of the patch.

Original follow-up written with GPT-6.1 Sol via the Codex harness in T3 Code. The prerequisite #26870 is Julius Marminge's work and credits Claude Opus 5.5 via Claude Code. Maria's maintenance update credits gpt-6.1-sol in her linked comment; its harness is not named there.

iXab3r and others added 30 commits November 1, 2026 21:53
…ut (pingdotgg#28491)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
… chat (pingdotgg#28563)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… chat timeline (pingdotgg#28564)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…#28582)

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
… runs in (pingdotgg#28592)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…run (pingdotgg#28610)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ry remote (pingdotgg#22119)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: GPT-6.1 Sol <noreply@openai.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…#23136)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…gdotgg#23168)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…otgg#23176)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…#23241)

Co-authored-by: GPT-6.1 Sol <noreply@openai.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…ngdotgg#23242)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…pingdotgg#23270)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
)

Co-authored-by: GPT-6.1 Sol <noreply@openai.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…otgg#23633)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…gdotgg#24485)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
…ingdotgg#24494)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…le (pingdotgg#24664)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
…pingdotgg#24665)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…pingdotgg#24673)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…pingdotgg#24684)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Yash-Singh1 and others added 15 commits November 3, 2026 11:47
…ingdotgg#28684)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
…squash-merged (pingdotgg#29919)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ointed parent run (pingdotgg#25662)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pingdotgg#29580)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
… the project folder (pingdotgg#29908)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Maria <maria-rcks@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…oading it (pingdotgg#29893)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Nov 3, 2026
saphid and others added 4 commits November 3, 2026 13:20
juliusmarminge and others added 4 commits November 3, 2026 11:46
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

merged current main without rebasing and resolved the permission-handler conflict by retaining nullable idle-turn lookup, session policy ownership, and kiro's tool-kind inference.

verified head 0b950cf247932afd63eea2f16bde5c378cd8ea40 through blacksmith: vp run check:changed against main passed formatting, lint, both knip checks, graphql generation, 14 affected package typechecks, and 102 tests including all 101 acp adapter tests. the exact previously failing usage-service test also passed after the main merge. two independent reviews found no remaining blocker or should-fix findings.

real-provider client interaction and native packaging were not rerun. acp subprocess replay and handler-level ownership/recovery checks passed. the implementation remains three files relative to current main; this pr still targets its prerequisite branch.

@github-actions github-actions Bot added 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. size:XXL 1,000+ changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Nov 3, 2026

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

📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.