refactor: clean up ACP package metadata and helpers by steipete · Pull Request #88659 · openclaw/openclaw

steipete · 2026-05-31T14:22:03Z

Summary

derive ACP core package aliases, d.ts boundary paths, and tsdown entries from the ACP package export map instead of copied subpath lists
split ACP manager background-task and timeout handling into focused modules
split ACP translator session presentation/config snapshot helpers into a focused module
preserve packaged @openclaw/acp-core/* plugin aliases when source package metadata is unavailable by deriving from shipped dist/acp-core/**/*.js

Verification

pnpm test src/plugins/sdk-alias.test.ts src/plugins/plugin-sdk-native-resolver.test.ts test/vitest-scoped-config.test.ts src/plugins/contracts/extension-package-project-boundaries.test.ts src/acp/policy.test.ts src/acp/control-plane/manager.test.ts src/acp/translator.lifecycle.test.ts src/acp/translator.session-lineage-meta.test.ts src/acp/translator.session-rate-limit.test.ts src/acp/translator.set-session-mode.test.ts src/acp/translator.event-ledger.test.ts src/acp/translator.permission-relay.test.ts src/acp/translator.cancel-scoping.test.ts src/acp/translator.stop-reason.test.ts src/acp/translator.error-kind.test.ts src/acp/translator.prompt-prefix.test.ts -- --reporter=verbose
pnpm build:plugin-sdk:dts
pnpm --filter @openclaw/acp-core build
pnpm build
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main -> clean, no accepted/actionable findings

pnpm check:changed was attempted but blocked before repo checks by local Crabbox 0.21.0; current Testbox path requires Crabbox >=0.22.0. Focused local proof above passed, and PR CI should cover the broad lanes.

Real behavior proof

Behavior addressed: ACP refactor cleanup keeps ACP core package alias/boundary generation aligned with exported package surfaces while splitting manager/translator helpers without behavior changes.
Real environment tested: local macOS checkout, Node/pnpm repo wrappers.
Exact steps or command run after this patch: focused ACP/plugin alias tests, build:plugin-sdk:dts, @openclaw/acp-core package build, full pnpm build, and branch autoreview.
Evidence after fix: all listed commands passed; autoreview clean after fixing the packaged ACP alias fallback finding.
Observed result after fix: ACP manager and translator tests continue to pass, package d.ts generation passes, full build passes, and packaged ACP aliases are covered by a regression test without source package metadata.
What was not tested: remote CI/Testbox via pnpm check:changed locally because the installed Crabbox binary is too old for current Testbox sync.

clawsweeper · 2026-05-31T14:24:04Z

Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 10:42 AM ET / 14:42 UTC.

Summary
The PR derives ACP core alias, d.ts boundary, Vitest, native resolver, and tsdown metadata from ACP package exports while splitting ACP manager and translator helper code into focused modules.

PR surface: Source +113, Tests +18, Config +27, Other +55. Total +213 across 16 files.

Reproducibility: not applicable. This PR is a refactor/package cleanup, not a bug report. The relevant verification is package/build/runtime proof rather than a failing reproduction path.

Review metrics: 1 noteworthy metric.

ACP package contract surfaces: 1 export map drives 5 alias/build surfaces. The diff routes extension d.ts paths, Vitest aliases, native resolver aliases, plugin loader aliases, and tsdown ACP entries through package export metadata, so maintainers should review it as package-compatibility work.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Let checks-node-agentic-control-plane-runtime finish green before merge.
Have a maintainer explicitly review the ACP export-map package compatibility surface.

Risk before merge

[P1] ACP core subpath alias, d.ts boundary, Vitest alias, native resolver, and tsdown entry derivation now depend on the ACP package export map; an export-map mismatch could break package or plugin resolution.
[P1] At review time, checks-node-agentic-control-plane-runtime was still in progress for the current head, so merge should wait for that shard or equivalent runtime proof.

Maintainer options:

Finish package and runtime proof (recommended)
Let the remaining control-plane runtime shard finish and have a maintainer review the export-map-derived package surfaces before merge.
Accept existing focused proof
Maintainers can intentionally land with the local builds and currently green package-boundary/build-artifact CI if they are comfortable owning the package-compatibility risk.
Narrow the refactor if confidence drops
If the remaining check fails or package behavior is unclear, pause this broad cleanup and split the package derivation from the ACP helper moves.

Next step before merge

[P2] Protected maintainer label plus ACP package/subpath compatibility surface require maintainer review; no narrow automated repair is identified.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes internal TypeScript/scripts/package resolution logic without new dependencies, lockfile changes, workflow permission changes, or secret handling changes.

Review details

Best possible solution:

Land after maintainer review confirms the export-map-derived ACP package surfaces preserve compatibility and all required head checks are green.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR is a refactor/package cleanup, not a bug report. The relevant verification is package/build/runtime proof rather than a failing reproduction path.

Is this the best way to solve the issue?

Yes, with maintainer review: deriving ACP package surfaces from package exports is the maintainable source-of-truth direction. Because it changes resolver and build-boundary behavior, the safer merge path is to wait for full head CI/runtime proof and explicit package-compatibility review.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against d689893a6fa8.

Label changes

Label changes:

add P2: This is normal-priority cleanup, but it touches ACP package/plugin compatibility surfaces with limited but real blast radius.
add merge-risk: 🚨 compatibility: Merging changes how ACP core subpaths are generated and resolved for plugin/package build and runtime paths, which can break existing package consumers if the mapping is wrong.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes the required real behavior proof fields with exact local commands and observed results, and the current head has a successful Real behavior proof check.

Label justifications:

P2: This is normal-priority cleanup, but it touches ACP package/plugin compatibility surfaces with limited but real blast radius.
merge-risk: 🚨 compatibility: Merging changes how ACP core subpaths are generated and resolved for plugin/package build and runtime paths, which can break existing package consumers if the mapping is wrong.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes the required real behavior proof fields with exact local commands and observed results, and the current head has a successful Real behavior proof check.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes the required real behavior proof fields with exact local commands and observed results, and the current head has a successful Real behavior proof check.

Evidence reviewed

PR surface:

Source +113, Tests +18, Config +27, Other +55. Total +213 across 16 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	10	899	786	+113
Tests	2	32	14	+18
Docs	0	0	0	0
Config	1	27	0	+27
Generated	0	0	0	0
Other	3	95	40	+55
Total	16	1053	840	+213

What I checked:

Protected review state: The provided live PR context includes the protected maintainer label, which requires explicit maintainer handling for this PR. (352058e3aa20)
ACP package alias derivation: The PR head adds export-map based ACP package alias entry discovery and uses it when building plugin loader aliases. (src/plugins/sdk-alias.ts:1084, 352058e3aa20)
ACP build entry derivation: The PR head changes tsdown ACP core entries to derive source paths from packages/acp-core/package.json exports. (tsdown.config.ts:460, 352058e3aa20)
ACP helper split: The PR head moves turn timeout cleanup into a focused helper module while preserving the same timeout, cancel, close, and cache-clear flow from the original manager implementation. (src/acp/control-plane/manager.turn-timeout.ts:13, 352058e3aa20)
PR validation state: GitHub check-runs at the current head show package boundary, build-artifacts, dependency, lint, type, security, and real behavior proof checks succeeded; checks-node-agentic-control-plane-runtime was still in progress at review time. (352058e3aa20)
Feature history: The same ACP package and resolver surfaces were recently expanded by merged PR refactor: expand acp core package #88618, which touched ACP package exports, generated d.ts paths, sdk-alias, native resolver, translator, manager, and tsdown config. (packages/acp-core/package.json:1, 7dea2837565e)

Likely related people:

steipete: Authored and merged the recent ACP core package expansion that touched the same ACP package exports, alias, d.ts boundary, tsdown, manager, and translator surfaces; this PR also comes from the same account. (role: recent area contributor; confidence: high; commits: 7dea2837565e; files: packages/acp-core/package.json, src/plugins/sdk-alias.ts, src/plugins/plugin-sdk-native-resolver.ts)
openperf: Authored the recently merged ACP task/runtime fix that changed src/acp/control-plane/manager.core.ts and src/acp/control-plane/manager.test.ts, which are adjacent to the manager helper split in this PR. (role: recent adjacent contributor; confidence: medium; commits: 02c7b5b82fc5; files: src/acp/control-plane/manager.core.ts, src/acp/control-plane/manager.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-05-31T14:53:09Z

Pre-merge verification for head 61f47c9961e440ced613dccc68eef1d105faab41:

Behavior addressed: ACP package metadata/alias maps now derive from @openclaw/acp-core exports, ACP manager/translator helpers are split out, and the long gateway control-plane runtime CI bucket is split into smaller shards while preserving the existing runtime check name.
Real environment tested: local OpenClaw checkout on Node/pnpm plus GitHub Actions PR CI.
Exact steps or command run after this patch:

pnpm check:test-types
pnpm test src/agents/acp-spawn.test.ts -- --reporter=verbose
pnpm test test/scripts/ci-node-test-plan.test.ts -- --reporter=verbose
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
GitHub Actions CI run 26715757298 on PR head 61f47c9961e440ced613dccc68eef1d105faab41
Evidence after fix: local test-type lane passed, focused ACP spawn test passed 62 tests, CI plan test passed 14 tests, autoreview reported no accepted/actionable findings, and required GitHub checks are green.
Observed result after fix: PR merge state is clean; the previous long checks-node-agentic-control-plane-runtime lane no longer remains the last running shard after the split.
What was not tested: no live ACP provider session was started; this refactor keeps runtime behavior covered by existing focused/unit lanes.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61f47c9961

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-31T14:53:52Z

+function buildAcpCoreDistEntries(): Record<string, string> {
+  return buildPackageDistEntriesFromExports("acp-core");


Keep ACP error-format in the packaged dist

When building from a clean tree, buildAcpCoreDistEntries() now mirrors only packages/acp-core/package.json exports, so error-format is no longer emitted into the root dist/acp-core output. However packages/acp-core/src/runtime/errors.ts imports ../error-format.js, and the package's own build script still lists src/error-format.ts, so the emitted runtime/errors module needs that sibling artifact; otherwise built imports of @openclaw/acp-core/runtime/errors (for example through src/acp/runtime/errors.ts) will fail at runtime with a missing ../error-format.js. Please keep error-format as an emitted entry or bundle it into runtime/errors.

Useful? React with 👍 / 👎.

…n-rotation-current * origin/main: (52 commits) fix(agents): prevent embedded runtime shadowing fix(outbound): route source replies through configured channels refactor(cron): split tool and doctor repair helpers perf: reduce tui refresh work feat: default exec shell snapshots fix(ui): keep chat usable during session loading fix(cron): guard flat atMs canonicalization refactor(cron): keep runtime on canonical sqlite rows fix(codex): restore bounded recovery continuity refactor: clean up ACP package metadata and helpers (#88659) fix(discord): ping mention-bearing final replies fix(telegram): preserve usage footer for tool-only replies fix(agents): avoid alias setup load for matching refs chore(ui): translate thinking default label fix(agents): preserve runtime tools in lean mode (#88381) fix(messages): use best-effort for implicit tool-only source replies (#84232) docs: raise bulk PR close threshold feat: add exec shell snapshot cache fix: use typed tui empty session defaults perf: speed up tui session refresh ... # Conflicts: # src/tui/tui-command-handlers.test.ts

* refactor: derive acp core package subpath maps * refactor: split acp manager task and timeout helpers * refactor: split acp translator presentation helpers * fix: keep packaged acp core plugin aliases * ci: split gateway control plane runtime shard

openclaw-barnacle Bot added scripts Repository scripts extensions: xai size: XL maintainer Maintainer-authored PR labels May 31, 2026

clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 31, 2026

steipete added 4 commits May 31, 2026 10:30

refactor: derive acp core package subpath maps

386f4a0

refactor: split acp manager task and timeout helpers

f837dbe

refactor: split acp translator presentation helpers

c82d654

fix: keep packaged acp core plugin aliases

352058e

steipete force-pushed the refactor/acp-cleanup-core-metadata branch from 281752f to 352058e Compare May 31, 2026 14:35

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 31, 2026

ci: split gateway control plane runtime shard

61f47c9

steipete merged commit 7b78941 into main May 31, 2026
146 checks passed

steipete deleted the refactor/acp-cleanup-core-metadata branch May 31, 2026 14:53

chatgpt-codex-connector Bot reviewed May 31, 2026

View reviewed changes

github-actions Bot mentioned this pull request May 31, 2026

📡 Upstream Digest — 2026-05-31 15:13 UTC curtismercier/openclaw-mods#990

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: clean up ACP package metadata and helpers#88659

refactor: clean up ACP package metadata and helpers#88659
steipete merged 5 commits into
mainfrom
refactor/acp-cleanup-core-metadata

steipete commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026 •

edited

Loading

Uh oh!

steipete commented May 31, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		function buildAcpCoreDistEntries(): Record<string, string> {
		return buildPackageDistEntriesFromExports("acp-core");

Uh oh!

Conversation

steipete commented May 31, 2026

Summary

Verification

Real behavior proof

Uh oh!

clawsweeper Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented May 31, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 31, 2026 •

edited

Loading