diff --git a/records/openclaw-openclaw/items/79550.md b/records/openclaw-openclaw/items/79550.md index 3ced8fe9f8..efaf4ea60a 100644 --- a/records/openclaw-openclaw/items/79550.md +++ b/records/openclaw-openclaw/items/79550.md @@ -1,5 +1,5 @@ --- -review_comment_synced_at: 2026-05-08T22:59:33.234Z +review_comment_synced_at: 2026-05-08T23:35:22.735Z number: 79550 repository: openclaw/openclaw type: pull_request @@ -7,13 +7,13 @@ title: "fix(sessions): report ACP-runtime metadata for ACP-keyed sessions" url: https://github.com/openclaw/openclaw/pull/79550 state_at_review: open item_created_at: 2026-05-08T22:51:00Z -item_updated_at: 2026-05-08T22:51:32Z +item_updated_at: 2026-05-08T23:28:09Z author: efpiva author_association: CONTRIBUTOR -labels: ["commands","agents","size: M","triage: needs-real-behavior-proof"] -reviewed_at: 2026-05-08T22:58:57.826Z -main_sha: 49e2f9133501f884068cd5ade85e8f8830b0ffcf -pull_head_sha: 952afa3114c79d636dd0bb607502d6872e0454ca +labels: ["gateway","commands","agents","size: M","triage: needs-real-behavior-proof"] +reviewed_at: 2026-05-08T23:34:42.815Z +main_sha: 7356517fbf537d89b8d47cdf7cad21d72c4c3340 +pull_head_sha: d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac latest_release: v2026.5.7 latest_release_sha: eeef4864494f859838fec1586bedbab1f8fa5702 fixed_release: unknown @@ -31,19 +31,19 @@ review_model: gpt-5.5 review_reasoning_effort: high review_sandbox: danger-full-access review_service_tier: default -review_prompt_chars: 57187 +review_prompt_chars: 76208 review_static_prompt_chars: 33412 -review_context_chars: 22531 +review_context_chars: 41552 review_schema_chars: 14081 review_additional_prompt_chars: 0 -review_context_elapsed_ms: 3987 -review_codex_elapsed_ms: 283369 +review_context_elapsed_ms: 4200 +review_codex_elapsed_ms: 205794 review_mode: propose review_status: complete local_checkout_access: verified -item_snapshot_hash: fd905f45c831480d508bf62706803f86ba1d008d1102da90f3bb1c7de83961f3 +item_snapshot_hash: 88f4dc405f8794b47d0f3025423de12bf8212cfa8c570d55cac89b0a20c75247 close_comment_sha256: none -review_comment_sha256: d27b3db53eca09139da70b84bb4ed099872f09e37db55b1a6bf5d073153ac5f2 +review_comment_sha256: 11ff6fe1c1b1d7ab915216fc633f91276e305188224544aee3ccef137625bd35 review_comment_id: 4410446961 review_comment_url: https://github.com/openclaw/openclaw/pull/79550#issuecomment-4410446961 decision: keep_open @@ -54,11 +54,11 @@ work_candidate: manual_review work_confidence: high work_priority: medium work_status: manual_review -work_reason_sha256: ff6c7a017af423086dcd4cd4566bc5b19f18b25e0b36ae81793ebf8690a1188e +work_reason_sha256: 451d8bf899e2cb4a35de2aa3f31f381ac0c52c31b6f01051d4642c9749d2322d work_prompt_sha256: none -work_cluster_refs: ["https://github.com/openclaw/openclaw/pull/79550","https://github.com/openclaw/openclaw/pull/54647"] +work_cluster_refs: [] work_validation: [] -work_likely_files: ["src/agents/agent-runtime-metadata.ts","src/commands/sessions.ts","src/gateway/session-utils.ts","src/gateway/server-methods/sessions.ts","src/config/sessions/types.ts"] +work_likely_files: [] item_category: bug reproduction_status: source_reproducible reproduction_confidence: high @@ -80,13 +80,13 @@ Author: efpiva Author association: CONTRIBUTOR -Labels: commands, agents, size: M, triage: needs-real-behavior-proof +Labels: gateway, commands, agents, size: M, triage: needs-real-behavior-proof Created at: May 8, 2026, 22:51 UTC -Updated at: May 8, 2026, 22:51 UTC +Updated at: May 8, 2026, 23:28 UTC -Reviewed against: [49e2f9133501](https://github.com/openclaw/openclaw/commit/49e2f9133501f884068cd5ade85e8f8830b0ffcf) +Reviewed against: [7356517fbf53](https://github.com/openclaw/openclaw/commit/7356517fbf537d89b8d47cdf7cad21d72c4c3340) Codex review: model gpt-5.5, reasoning high @@ -104,41 +104,41 @@ Action taken: kept_open ## Summary -Keep open: current main still lacks ACP session-key-aware runtime classification, but this PR is not merge-ready because it hard-codes `acpx` for every ACP key and has only test proof, not after-fix real behavior proof. +Keep open: the updated code direction addresses the earlier backend-hardcoding concern, but this external PR still lacks after-fix real behavior proof and the required changelog entry. ## What This Changes -The PR adds an ACP session-key overlay so session runtime metadata reports `acpx/session-key` for ACP-keyed sessions and adds a focused sessions regression test. +The PR adds an ACP session-key runtime overlay, propagates the new `session-key` source type, threads stored ACP backend metadata through session/status emitters, and adds focused sessions regression tests. ## Best Possible Solution -Report ACP runtime metadata from the stored or resolved ACP backend when available, keep the resolver/row contract explicit, and add coverage for both default `acpx` and a non-`acpx` backend. +Land the focused overlay after the contributor adds redacted after-fix `openclaw sessions --all-agents --json` proof and a single-line `CHANGELOG.md` entry for the sessions metadata fix. ## Reproduction Assessment -Yes. Source inspection shows current main routes ACP-keyed rows through `resolveModelAgentRuntimeMetadata` without ACP classification, and the PR body includes a deployed OpenClaw 2026.5.6 repro where 7/7 ACP sessions reported `auto/implicit`; I did not run tests because this review is read-only. +Yes, source inspection gives a high-confidence reproduction path: create or inspect an ACP-keyed session row and run `openclaw sessions --all-agents --json`; current main routes it through a resolver that does not classify ACP keys. I did not run the live command because this review is read-only. ## Solution Assessment -No. The session-key overlay is in the right area, but the key only identifies ACP family; the maintainable fix should use the persisted/resolved ACP backend instead of hard-coding `acpx`. +Yes, the updated overlay is the narrowest maintainable code shape: it keeps provider/model policy intact while using stored ACP backend metadata when available and `acpx` only as a fallback. Merge should still wait for real behavior proof and the changelog entry. ## Review Findings -Overall correctness: patch is incorrect +Overall correctness: patch is correct -Overall confidence: 0.87 +Overall confidence: 0.84 Full review comments: -- **[P2] Use the recorded ACP backend instead of hard-coding acpx:** `src/agents/acp-runtime-overlay.ts:20` - - body: ACP session keys only prove that a row is ACP-shaped; OpenClaw supports configurable ACP backends through `acp.backend`, binding/agent overrides, and persisted `entry.acp.backend`. With this overlay, any ACP session backed by a non-`acpx` registered backend will still serialize `agentRuntime.id: "acpx"`, so the sessions output remains wrong for supported configurations. Pass the stored/resolved backend into this classification path or apply the override where the session entry is available. - - confidence: 0.87 +- **[P3] Add the required changelog entry:** `src/agents/acp-runtime-overlay.ts:21-28` + - body: This PR changes user-visible `openclaw sessions` and Gateway session metadata, but `CHANGELOG.md` is not in the diff. Repo policy requires user-facing fixes to be recorded, so add a single-line entry under the active release before merge. + - confidence: 0.9 ## Security Review Status: cleared -Summary: The diff is limited to runtime metadata classification and tests, with no dependency, workflow, credential, or code-download surface added. +Summary: Cleared: the diff is limited to runtime metadata classification, type propagation, and tests, with no dependency, workflow, credential, or code-download surface added. Concerns: @@ -152,7 +152,7 @@ Evidence kind: none Needs contributor action: true -Summary: The PR has before-fix live output and test claims only; please add redacted after-fix terminal/log/screenshot/video proof showing the corrected real `openclaw sessions --all-agents --json` output, then update the PR body to trigger re-review or ask a maintainer to comment `@clawsweeper re-review`. +Summary: Needs after-fix real behavior proof: the PR has before-fix live output and after-fix test claims, but no redacted terminal/log/screenshot/recording showing corrected real session output. ## Work Candidate @@ -164,20 +164,15 @@ Priority: medium Status: manual_review -Reason: The remaining blockers are contributor after-fix real behavior proof and a reviewer decision on backend-accurate ACP runtime metadata, not a safe standalone repair job. +Reason: Contributor/maintainer action is needed for after-fix real behavior proof, and a small changelog addition remains; automation cannot supply the contributor's runtime evidence. Cluster refs: -- https://github.com/openclaw/openclaw/pull/79550 -- https://github.com/openclaw/openclaw/pull/54647 +- none Likely files: -- src/agents/agent-runtime-metadata.ts -- src/commands/sessions.ts -- src/gateway/session-utils.ts -- src/gateway/server-methods/sessions.ts -- src/config/sessions/types.ts +- none Validation: @@ -185,47 +180,60 @@ Validation: ## Evidence -- **Current main ignores ACP classification in the runtime resolver:** `resolveModelAgentRuntimeMetadata` accepts `sessionKey` but returns only the model/provider runtime policy result, so ACP-keyed rows can fall through to `auto`, `codex`, or another implicit/configured runtime instead of ACP metadata. - - file: [src/agents/agent-runtime-metadata.ts:21](https://github.com/openclaw/openclaw/blob/49e2f9133501f884068cd5ade85e8f8830b0ffcf/src/agents/agent-runtime-metadata.ts#L21) - - command: `nl -ba src/agents/agent-runtime-metadata.ts | sed -n '1,220p'` - - sha: [49e2f9133501](https://github.com/openclaw/openclaw/commit/49e2f9133501f884068cd5ade85e8f8830b0ffcf) -- **Session list rows pass the key into the resolver:** The CLI sessions listing computes `agentRuntime` through `resolveModelAgentRuntimeMetadata` with `sessionKey: row.key`, so a resolver-level overlay would affect this production output path. - - file: [src/commands/sessions.ts:265](https://github.com/openclaw/openclaw/blob/49e2f9133501f884068cd5ade85e8f8830b0ffcf/src/commands/sessions.ts#L265) - - command: `nl -ba src/commands/sessions.ts | sed -n '220,310p'` - - sha: [49e2f9133501](https://github.com/openclaw/openclaw/commit/49e2f9133501f884068cd5ade85e8f8830b0ffcf) -- **ACP key detection proves family but not backend:** `isAcpSessionKey` checks for `acp:` session-key shape; separately, OpenClaw config and persisted session metadata carry a concrete ACP backend id, so the key alone does not prove the backend is `acpx`. - - file: [src/sessions/session-key-utils.ts:86](https://github.com/openclaw/openclaw/blob/49e2f9133501f884068cd5ade85e8f8830b0ffcf/src/sessions/session-key-utils.ts#L86) - - command: `nl -ba src/sessions/session-key-utils.ts | sed -n '1,260p'; nl -ba src/config/types.acp.ts | sed -n '1,130p'; nl -ba src/config/sessions/types.ts | sed -n '1,120p'` - - sha: [49e2f9133501](https://github.com/openclaw/openclaw/commit/49e2f9133501f884068cd5ade85e8f8830b0ffcf) -- **PR diff hard-codes the ACP backend:** The proposed overlay returns `{ id: "acpx", source: "session-key" }` whenever the key is ACP-shaped, without consulting `entry.acp.backend`, per-binding backend overrides, or `acp.backend`. - - file: [src/agents/acp-runtime-overlay.ts:20](https://github.com/openclaw/openclaw/blob/952afa3114c79d636dd0bb607502d6872e0454ca/src/agents/acp-runtime-overlay.ts#L20) - - command: `curl -L --fail --silent https://patch-diff.githubusercontent.com/raw/openclaw/openclaw/pull/79550.diff` - - sha: [952afa3114c7](https://github.com/openclaw/openclaw/commit/952afa3114c79d636dd0bb607502d6872e0454ca) -- **Real behavior proof gate is not satisfied:** The PR body includes a before-fix deployed-container repro and unit/adjacent test commands, but no after-fix real CLI, Gateway, container, log, terminal, screenshot, or recording proof showing the corrected `openclaw sessions --all-agents --json` output. - - command: `curl -L --fail --silent https://api.github.com/repos/openclaw/openclaw/pulls/79550` - - sha: [952afa3114c7](https://github.com/openclaw/openclaw/commit/952afa3114c79d636dd0bb607502d6872e0454ca) -- **Relevant history for current behavior:** Merged PR https://github.com/openclaw/openclaw/pull/79238 / commit 02fe0d8978dbf5b7acc4529d505b31f044fdf481 moved session runtime reporting to model/provider-scoped runtime policy and left `resolveAgentRuntimeMetadata` as the automatic stub referenced by this PR. - - file: [src/gateway/session-utils.ts:1733](https://github.com/openclaw/openclaw/blob/02fe0d8978dbf5b7acc4529d505b31f044fdf481/src/gateway/session-utils.ts#L1733) - - command: `curl -L --fail --silent https://api.github.com/repos/openclaw/openclaw/pulls/79238` +- **Current main resolver has no ACP session-key classification:** `resolveModelAgentRuntimeMetadata` accepts `sessionKey` but only returns the provider/model runtime policy result, so ACP-keyed rows are not classified as ACP on current main. + - file: [src/agents/agent-runtime-metadata.ts:21](https://github.com/openclaw/openclaw/blob/7356517fbf537d89b8d47cdf7cad21d72c4c3340/src/agents/agent-runtime-metadata.ts#L21) + - command: `nl -ba src/agents/agent-runtime-metadata.ts | sed -n '1,120p'` + - sha: [7356517fbf53](https://github.com/openclaw/openclaw/commit/7356517fbf537d89b8d47cdf7cad21d72c4c3340) +- **CLI sessions production path sends the key into that resolver:** `openclaw sessions --json` builds each row through `resolveModelAgentRuntimeMetadata({ ..., sessionKey: row.key })`, matching the reported surface. + - file: [src/commands/sessions.ts:265](https://github.com/openclaw/openclaw/blob/7356517fbf537d89b8d47cdf7cad21d72c4c3340/src/commands/sessions.ts#L265) + - command: `nl -ba src/commands/sessions.ts | sed -n '230,310p'` + - sha: [7356517fbf53](https://github.com/openclaw/openclaw/commit/7356517fbf537d89b8d47cdf7cad21d72c4c3340) +- **ACP metadata records a backend id:** Session ACP metadata has a required `backend` field, and ACP docs describe `runtime.acp.backend` / `bindings[].acp.backend` overrides, so using `entry.acp.backend` is the right correction to the earlier hard-coded `acpx` approach. + - file: [src/config/sessions/types.ts:41](https://github.com/openclaw/openclaw/blob/7356517fbf537d89b8d47cdf7cad21d72c4c3340/src/config/sessions/types.ts#L41) + - command: `nl -ba src/config/sessions/types.ts | sed -n '35,78p'; sed -n '330,390p' docs/tools/acp-agents.md` + - sha: [7356517fbf53](https://github.com/openclaw/openclaw/commit/7356517fbf537d89b8d47cdf7cad21d72c4c3340) +- **PR head uses stored ACP backend when available:** At head `d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac`, the overlay returns `acpBackend` for ACP-keyed sessions and falls back to `acpx`; CLI/Gateway/status call sites with a session entry pass `entry?.acp?.backend`. + - file: [src/agents/acp-runtime-overlay.ts:21](https://github.com/openclaw/openclaw/blob/d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac/src/agents/acp-runtime-overlay.ts#L21) + - command: `curl -L --fail --silent https://raw.githubusercontent.com/openclaw/openclaw/d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac/src/agents/acp-runtime-overlay.ts | nl -ba | sed -n '1,120p'` + - sha: [d4a6eb0b925c](https://github.com/openclaw/openclaw/commit/d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac) +- **Real behavior proof is still mock-only after the fix:** The PR body/comments include before-fix live output and after-fix unit/adjacent test claims, but no after-fix terminal/log/screenshot/video proof showing real `openclaw sessions --all-agents --json` output with corrected ACP runtime metadata. + - command: `Review provided PR body and comments for after-fix runtime evidence` + - sha: [d4a6eb0b925c](https://github.com/openclaw/openclaw/commit/d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac) +- **No changelog entry in PR file list:** The changed files list contains runtime metadata code and tests but no `CHANGELOG.md`, even though this is a user-facing sessions metadata fix. + - command: `Inspect provided pullFiles list and PR diff for CHANGELOG.md` + - sha: [d4a6eb0b925c](https://github.com/openclaw/openclaw/commit/d4a6eb0b925ca0674f20f693e5f2e52a94ce57ac) +- **Relevant runtime-policy provenance:** Merged PR https://github.com/openclaw/openclaw/pull/79238 / commit `02fe0d8978dbf5b7acc4529d505b31f044fdf481` moved runtime reporting toward provider/model-scoped policy and left ACP session-key classification out of the resolver. + - file: [src/agents/agent-runtime-metadata.ts:21](https://github.com/openclaw/openclaw/blob/02fe0d8978dbf5b7acc4529d505b31f044fdf481/src/agents/agent-runtime-metadata.ts#L21) + - command: `curl -L --fail --silent https://api.github.com/repos/openclaw/openclaw/commits/02fe0d8978dbf5b7acc4529d505b31f044fdf481` - sha: [02fe0d8978db](https://github.com/openclaw/openclaw/commit/02fe0d8978dbf5b7acc4529d505b31f044fdf481) ## Likely Related People - **pashpashpash:** introduced behavior - - reason: Authored and merged the runtime-policy refactor in commit 02fe0d8978dbf5b7acc4529d505b31f044fdf481, which switched session/runtime reporting to model/provider-scoped resolution and is explicitly referenced by this PR as the current baseline. + - reason: Authored and merged the provider/model-scoped runtime routing refactor in https://github.com/openclaw/openclaw/pull/79238, which current main uses for session runtime metadata and which this PR explicitly builds on. - confidence: high - commits: 02fe0d8978dbf5b7acc4529d505b31f044fdf481 - - files: src/agents/agent-runtime-metadata.ts, src/gateway/session-utils.ts, src/gateway/server-methods/sessions.ts, src/shared/session-types.ts -- **Peter Steinberger:** recent maintainer - - reason: Current-main blame for the central resolver and sessions surfaces points at commit 672426eb50475acc832d03a1386aa3336a662bb4, which materialized the current versions of the affected files in this checkout. + - files: src/agents/agent-runtime-metadata.ts, src/commands/sessions.ts, src/gateway/session-utils.ts +- **vincentkoc:** adjacent owner + - reason: Authored the recent sessions-table runtime visibility change, making them relevant for `openclaw sessions` runtime metadata output and changelog expectations. - confidence: medium - - commits: 672426eb50475acc832d03a1386aa3336a662bb4 - - files: src/agents/agent-runtime-metadata.ts, src/commands/sessions.ts, src/gateway/session-utils.ts, src/gateway/server-methods/sessions.ts + - commits: c874c0863ada70748abf85e075eb879a2b2e0d8c + - files: src/commands/sessions.ts, src/commands/sessions.test.ts, CHANGELOG.md +- **obviyus:** recent maintainer + - reason: Recently maintained Gateway session list model normalization, an adjacent path to the Gateway session row metadata touched by this PR. + - confidence: medium + - commits: 6be5422fd64072a09020d5297d7d683811a4ec5b + - files: src/gateway/session-utils.ts +- **steipete:** recent maintainer + - reason: Local blame and GitHub history show recent maintenance/refactor work across the agent/runtime and ACP control-plane files involved in this session metadata path. + - confidence: medium + - commits: e428a2dfe2a3c9174e6bf1b5bafd92e2cbb095cd, 20ed5974957f9b04da2b2eb104efc9d36df5db83 + - files: src/agents/agent-runtime-metadata.ts, src/acp/control-plane/manager.core.ts ## Risks / Open Questions -- The PR has no after-fix real behavior proof from a real CLI/Gateway/container setup. -- The proposed overlay can misreport ACP sessions that use a configured or persisted backend other than `acpx`. +- After-fix real behavior proof is still absent; tests alone do not show a real session row now reports the corrected runtime. +- The required changelog entry for this user-facing sessions metadata fix is missing. ## Close Comment @@ -233,19 +241,19 @@ _No close comment posted._ ## GitHub Snapshot -- comments: 0 -- timeline events: 7 -- related items: 2 -- PR files: 5 -- PR commits: 3 +- comments: 2 +- timeline events: 11 +- related items: 3 +- PR files: 9 +- PR commits: 4 ## Review Telemetry -- prompt chars: 57187 +- prompt chars: 76208 - static prompt chars: 33412 -- context chars: 22531 +- context chars: 41552 - schema chars: 14081 - additional prompt chars: 0 -- context collection ms: 3987 -- Codex review ms: 283369 +- context collection ms: 4200 +- Codex review ms: 205794 \ No newline at end of file