Skip to content

Support standard ACP agents with explicit session guarantees - #1085

Merged
taras merged 13 commits into
mainfrom
codex/standard-acp-1081
Nov 3, 2026
Merged

Support standard ACP agents with explicit session guarantees#1085
taras merged 13 commits into
mainfrom
codex/standard-acp-1081

Conversation

@taras

@taras taras commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Why

An ordinary ACP peer can return text and end_turn while XMD rejects its first turn for lacking XMD's private acceptance marker. Standard ACPv1 agents need to work without claiming stronger session evidence.

Closes #1081. Fixes #1087. Fixes #1089. Predecessor #1079 is merged at bd17b463. The #1081 implementation range is bd17b463..2dde263a; its original frozen base has the identical file tree. A signed ancestry merge preserves the entire tested source tree.

What changes

Before, every new ACP-first session required the private marker, and workflow mapping required a native identity. After, ordinary peers can stream and complete turns; declared-strong Codex/Claude adapters retain their acceptance barrier. Qualification reports basic interaction, acceptance, native identity, durability and cleanup separately.

Workflows retain the ACP loading target under the distinct acpx.acpSessionId kind when no native identity exists. Reattachment compares kind and value. A used held conversation whose provider storage disappeared refuses before creating a replacement.

How it works

standard peer -> ordinary ACP creation -> configured/permission-routed turn -> actual events
strong adapter -> pending occupancy -> acceptance marker -> saved identity/mapping -> events
used history + missing provider record -> setup refusal before backend creation

The existing adapter descriptor declares strength. The existing provider, workflow mapping and private Core request/setup owners enforce the distinction. No new registry, storage format or generic recovery framework is added.

Review guide

Start with packages/acp/src/native-launch.ts and provider.ts, then retained identity projection in session-store.ts and workflow mapping in packages/cli/src/workflow-agent.ts. Review the private continuation requirement in Core's conversation/request/setup modules and the qualification report consumers.

Look carefully at pending strong records supplying no retained assertion, ACP IDs never becoming native IDs, and refusal before replacement when used history survives storage loss.

What must stay true

  • Strong construction remains marker-gated; missing evidence cannot select the standard path.
  • Used or uncertain conversations are never silently replaced or automatically resent. Existing load/resume support controls reopening; the limited never-prompted -32603 fallback stays.
  • Refused attempts preserve identity/configuration and prior messages. Local failed-attempt/lifecycle accounting may advance.
  • Permissions, tool ceilings, FIFO, Stop and owned cleanup keep their existing owners. Local cancellation is not evidence of remote termination.

How to verify it

Focused Deno groups passed on the final candidate: F 21 tests / 99 steps, A 21 / 126, B 17 / 143. Documentation validation passed. The tests cover standard first/sequential turns, strong refusal/promotion, permissions and cancellation, replay, exact loading and failed reopening, lost provider storage, workflow identity precedence/kind mismatch, and independent qualification evidence. Vendor/native-launch checks pass.

The Architect and Planner passed the feature and both original Fable findings; the independent reviewer passed 1e2edbfd. Both observations are fixed: failed partial output prevents conversation replacement, and identity selection uses ordinary narrowing. The original Planner and Fable reviewer both passed the affected journal-observation correction and integration proof on exact 2dde263a. The full Fable review was not repeated. Required CI run2375 passes on this exact head. The delivery workflow also verifies successful CI for main’s exact current commit after #1079’s merge. The human marked the PR ready. Its separate merge decision remains. The first CI attempt hit the existing adapter-confirmation flake #1088; its first output and unchanged-boundary evidence are recorded there. That cause is not fixed by this PR, and the issue remains open after the one failed-CI retry.

The five files affected by the earlier CI fixture repair pass locally: Deno 36 suites / 141 steps, Bun 1.4.0 141 tests, and Node 22 141 tests, with final R2 reruns bound separately. The target-compatible fixture lookup passes the exact Node typecheck and four focused IN5 cases.

The J2 live-journal observer now parses only newline-terminated records while a writer is active. A deterministic split-append regression fails before the correction and passes after; a malformed complete record still fails. Focused Node J2/TL10, journal controls, strict retained-history refusal and typecheck pass at signed 68c905ce; the integrated 2dde263a has exactly the same source tree. This changes one test file and preserves the three-retained-prompt assertion. No deadline increases, runtime exclusions or production lifecycle changes are introduced by these CI repairs. No additional live Kimi/Codex qualification was run.

Scope

Included: standard ACP interaction, truthful capability evidence, existing-owner continuation protection, affected regressions and current specifications. ACP remains on v1. Native CLI handoff still requires asserted native identity; a peer without loading support cannot reopen a retained conversation.

Generated or mechanical changes

The required PROVENANCE.md wording edit updates only its SHA-256 entry in the vendor manifest. Runtime/adapter bytes, other entries and integrity assertions remain unchanged.

Scope confirmation

  • Every changed file supports the purpose above.
  • Unrelated cleanup is excluded.
  • Mechanical changes are identified.
  • The description identifies the implementation range, predecessor and actual focused evidence.

@github-actions

github-actions Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

PR #1085: Support standard ACP agents with explicit session guarantees

36 files, +1227 / -214

Scope

🔴 PR has 1441 lines changed. Split into focused PRs.

🟡 1441 lines changed. PRs under 400 receive more thorough review.

🟡 36 files changed. Are all changes related?

🟡 Changes span 9 directories.

Structural

Oxlint structural signals:

  • no-unused-vars ×5: packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, packages/cli/tests/support/plan-harness.ts, packages/acp/src/provider.ts
  • no-empty-function ×2: packages/acp/src/provider.ts
  • no-unnecessary-type-assertion ×2: packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts

Slop

✅ Slop indicators look low.

Static Analysis

Oxlint: 57 diagnostics across 6 files (12 rules)
Density: 0.046 violations/added-line

consistent-function-scoping (15): packages/acp/src/provider.ts
no-shadow (8): packages/core/src/agent/conversation.ts, packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, packages/acp/src/provider.ts
no-floating-promises (7): packages/core/src/agent/conversation.ts, examples/agent-certification/observe.ts, packages/acp/src/provider.ts (+1)
unbound-method (6): packages/cli/tests/support/plan-harness.ts, packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, packages/acp/src/provider.ts
no-unused-vars (5): packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, packages/cli/tests/support/plan-harness.ts, packages/acp/src/provider.ts
no-useless-spread (4): packages/acp/src/provider.ts
no-unsafe-type-assertion (4): packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts
no-array-sort (2): packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, packages/acp/src/native-launch.ts
no-empty-function (2): packages/acp/src/provider.ts
no-unnecessary-type-assertion (2): packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts
no-array-reverse (1): packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts
no-unnecessary-template-expression (1): packages/acp/src/provider.ts

Correctness

FILE: packages/acp/src/provider.ts, PATTERN: no-unused-vars, CONCERN: Unused variables in test setup, QUESTION: Are these variables required for test execution or can they be removed?
FILE: packages/acp/src/provider.ts, PATTERN: no-empty-function, CONCERN: Empty functions may indicate placeholder code, QUESTION: Are these functions intended for future implementation or can they be eliminated?
FILE: packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, PATTERN: no-unused-vars, CONCERN: Unused variables in test scenarios, QUESTION: Are these variables necessary for test coverage or redundant?
FILE: packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, PATTERN: no-unsafe-type-assertion, CONCERN: Unsafe type assertions may hide missing properties, QUESTION: Can these assertions be replaced with safer alternatives?
FILE: packages/acp/src/provider.ts, PATTERN: signal cluster, CONCERN: Multiple Oxlint violations in single file, QUESTION: Are these violations indicative of unreviewed generated code?
FILE: packages/cli/tests/fixtures/repl/issue-883/recovery-phase.ts, PATTERN: signal cluster, CONCERN: Multiple Oxlint violations in single file, QUESTION: Are these violations indicative of unreviewed generated code?

@taras

taras commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Independent Fable review of 6f7a0d868e5499699ff23086de071ce544ba34ed covered all six recorded acceptance clauses and returned PASS with two nonblocking observations.

  1. A failed turn that had streamed partial text could be treated as unused when provider storage disappeared. The correction makes retained partial output evidence of use, preserving refusal before replacement and failed-before-answer retry controls.
  2. Workflow identity selection retained an unreachable empty-string fallback. The correction removes it and documents the existing fail-closed identity-change window before the first prompt.

Both observations are integrated locally in signed commit c089945eb72e3cd90cefe5894b9a12d43b712652. Affected Architect and Planner reviews passed; focused tests passed 20 tests/74 steps and documentation validation passed. That correction will be pushed with the ongoing CI repair. The published head remains 6f7a0d86.

This records the completed full review and its dispositions. CI has demonstrated additional Plan/REPL fixture and Bun cancellation failures; those remain under repair. A focused independent closeout must cover the final corrected commit. This is not a merge-readiness claim.

@taras taras mentioned this pull request Nov 3, 2026
Closed
@taras

taras commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Independent Fable affected-source closeout: PASS, exact signed f687518a419eff4a7ef30532a9a74aebc819b972, all six original acceptance clauses. This reuses the earlier full review of unchanged code and inspects the source corrections; it is not a repeated full review.

  • Both previous observations are fixed: retained partial output prevents replacement after failed turns, and workflow identity selection uses ordinary narrowing with the existing mapping refusal window documented.
  • The eight-file CI correction preserves discriminating assertions. Fixtures declare intended strength and actual retained backend state. The Stop fixture signals cancellable readiness; the test observes real local handle closure while keeping backend cleanup unverified.
  • Nonblocking coverage note: the corrected R2n fixture no longer covers the later post-turn native-identity divergence / multiple-native-conversation refusal branches. Those production branches remain; a later direct Core stub regression could cover them. This is recorded as follow-up coverage, not an open delivery blocker.

Architect/Planner feedback and focused Deno/Bun/Node evidence are retained. The corrected head is pushed to this PR; new CI is pending. The cancellation readiness race and first failures are tracked in #1087. Neither a merge nor readiness is claimed here; #1079 remains a separate predecessor delivery.

@taras

taras commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Final exact-head Fable closeout: PASS at signed 1e2edbfd992d051cc212df74408d8015b77a449b, all six original clauses. The only change since the previous pass is a target-compatible fixture lookup: a reversed copy plus the same pure predicate selects the same last record without mutating the original list. Node's exact typecheck failed before and passes after; all four focused IN5 tests pass on Node 22. No additional Planner/Architect review was selected for this mechanical change.

Prior review dispositions and the nonblocking coverage note remain as posted. The final head is pushed; required CI remains pending. #1079 is still a separate predecessor; no merge is claimed.

@taras

taras commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Final affected-source Fable closeout: PASS, signed 2dde263a6e0da30ca2934f09b78d79f9173e8129, all six original clauses. The reviewer reused its prior full assessment and inspected the J2 correction plus ancestry proof; the full feature review was not repeated.

  • The test-local live journal observer consumes only newline-terminated records. A deterministic split-append test fails before the correction and passes after. Every terminated nonempty record still undergoes strict parsing; the malformed-complete control rejects corruption. Existing J2/TL10 and three-retained-prompt assertions remain.
  • The separate strict retained-history control and Node typecheck pass. Receipts retain their actual tested 68c905ce revision; final source-tree equality establishes their applicability to this head. The original Linux CI record bytes are unknown: the regression establishes the mechanism, without inventing the lost record.
  • The ancestry merge is content-neutral. Merged Add reusable local ACP agent qualification #1079 and the frozen base have the same tree; the new merge preserves the entire tested source tree, has the intended parents, verifies its signature and leaves a clean checkout. No incoming content was lost.

The original Planner also passed affected acceptance on this exact head. Prior nonblocking coverage/runtime notes remain recorded. The approved fourth correction is complete; new exact-head CI run2375 is pending. #1087 and #1089 retain first failure/regression evidence. No merge or CI pass is claimed here.

@taras
taras marked this pull request as ready for review November 3, 2026 12:47
@taras taras mentioned this pull request Nov 3, 2026
Closed
@taras
taras merged commit 47adc1f into main Nov 3, 2026
112 of 114 checks passed
@taras
taras deleted the codex/standard-acp-1081 branch November 3, 2026 13:09
This was referenced Nov 3, 2026
Merged
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@taras