Require ACP workspace conformance before execution#93
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughAgent manifests now require ChangesACP CWD conformance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Daemon as runOneDaemonIteration
participant Client as DaemonClient
participant Executor
Daemon->>Client: markRunning with executorCapability
Daemon->>Executor: canRun
Executor-->>Daemon: readiness result
Daemon->>Client: completeRun with needs_human on rejection
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a workspace conformance check for ACP agents, requiring them to declare workspace.sessionCwd: "required" in their manifest to attest that their file tools honor the ACP session cwd. Without this declaration, agents are treated as unverified, and the runtime will fail closed. The changes span documentation, schema validation, executor logic, and doctor checks, along with extensive test coverage. The reviewer suggested adding a fallback default value when mapping repository.defaultExecutor in the doctor checks to prevent undefined from being added to the executor IDs set.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
| } | ||
|
|
||
| const repositoryExecutorIds = new Set(input.config.repositories.map((repository) => repository.defaultExecutor)); |
There was a problem hiding this comment.
If repository.defaultExecutor is optional or undefined in the raw configuration, map could return undefined, adding undefined to the repositoryExecutorIds Set. To prevent any potential runtime type mismatches or unexpected Set behavior, consider adding a fallback default value (e.g., "echo") during the mapping.
Suggested change
| const repositoryExecutorIds = new Set(input.config.repositories.map((repository) => repository.defaultExecutor)); | |
| const repositoryExecutorIds = new Set(input.config.repositories.map((repository) => repository.defaultExecutor ?? "echo")); |
mingyooagi
force-pushed
the
codex/acp-workspace-conformance
branch
from
July 14, 2026 10:31
6830719 to
0a8d72e
Compare
Generic ACP previously advertised worktree isolation whenever the host created a worktree, even though ACP only forwards a session cwd. Require a strict manifest attestation during schema and config parsing, remove the deferred-readiness compatibility path, and preserve capability evidence in doctor and Attempt audit surfaces. Constraint: ACP v1 transports session cwd but does not attest how an agent real file tools resolve workspace paths Rejected: Accept old manifests and defer rejection to executor readiness | a security contract should fail at manifest parsing instead of preserving an ambiguous compatibility state Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not declare ACP session-cwd conformance until real file tools pass both worktree and scratch tests Tested: pnpm build; pnpm lint; pnpm typecheck; pnpm test (119 files, 1531 tests); pnpm release:check; 7 governance matrix cases; privacy redaction scan; git diff --check; standards and spec review Not-tested: Live third-party ACP agent real-tool conformance
mingyooagi
force-pushed
the
codex/acp-workspace-conformance
branch
from
July 14, 2026 11:55
0a8d72e to
cd45bdb
Compare
mingyooagi
marked this pull request as ready for review
July 14, 2026 12:05
mingyooagi
added a commit
that referenced
this pull request
Jul 15, 2026
* Gate OpenClaw before admitting it to the generic ACP path OpenClaw 2026.7.1 initializes through the generic ACP host and passes real worktree, scratch, and disposable-session checks. A hardened cancellation canary proves that the in-flight shell can outlive ACP and Gateway cancellation, so the live registry records the blocker and no manifest example ships. Constraint: ACP Agent manifests require the sessionCwd attestation before executor construction Rejected: Publish an OpenClaw manifest after the partial pass | cancellation still permits a late external effect Rejected: Add an OpenClaw-specific executor | the integration boundary remains the generic ACP host Confidence: high Scope-risk: narrow Reversibility: clean Directive: Do not publish the OpenClaw manifest or weaken the cancellation canary until the late completion marker no longer appears Tested: typecheck; lint; 1534 unit and integration tests; build; 7-case governance matrix; privacy scan; publication set; two real OpenClaw 2026.7.1 Gateway runs Not-tested: Remote authenticated Gateway Related: #93 * Keep the OpenClaw gate reliable under real setup noise The conformance harness now tolerates diagnostic lines before the CLI version, preserves leak detection when the configured default workspace does not exist yet, and guarantees fixture cleanup when setup fails after temporary-root creation. Constraint: The gate must stay fail closed for provider behavior while remaining reusable in clean environments. Rejected: Accept the review suggestions verbatim | unanchored version matching and an unnormalized workspace fallback would weaken the evidence boundary. Confidence: high Scope-risk: narrow Directive: Do not turn missing-workspace handling into automatic workspace creation or provider admission. Tested: targeted OpenClaw harness tests; typecheck; lint; 120 test files and 1536 tests Not-tested: live OpenClaw cancellation, which remains the recorded upstream provider failure. * Keep conformance failures bounded and non-destructive The live gate now enforces private report permissions on overwrite, bounds synchronous provider probes, authorizes cleanup only after marker absence is established, and settles the ACP run when cancellation-marker observation fails. Constraint: A failing provider gate must not hang indefinitely, weaken evidence privacy, delete pre-existing files, or leave an executor running during fixture cleanup. Rejected: Treat the bot findings as optional hardening | each finding reproduces a real failure or safety boundary in the current harness. Confidence: high Scope-risk: narrow Directive: Keep cleanup ownership explicit and settle every started executor before removing its workspace. Tested: 8 targeted harness tests; typecheck; lint; 120 test files and 1539 tests Not-tested: live OpenClaw cancellation remains the recorded upstream provider failure.
This was referenced Jul 15, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
roles.agent.workspace.sessionCwd: "required"attestation for every ACP Agent manifestValidation
corepack pnpm buildcorepack pnpm lintcorepack pnpm typecheckcorepack pnpm test— 119 files, 1531 tests passedcorepack pnpm release:check— packed-install CLI check passedcorepack pnpm smoke:governance -- --all— 7 cases passedcorepack pnpm smoke:privacygit diff --checkNotes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation