fix: persist transcript for ACP agent run outputs by robo7 · Pull Request #38614 · openclaw/openclaw

robo7 · 2026-03-07T05:49:30Z

Summary

Fix ACP agent run sessions so successful one-shot ACP turns persist a session transcript that chat.history / sessions_history can read.

This patch does two small things in the ACP branch of agentCommandInternal:

persist sessionFile early for ACP child sessions
append the final visible assistant text to the session transcript after acpManager.runTurn(...) completes

Without this, ACP child sessions can have metadata in sessions.json but no transcript .jsonl, so sessions_history comes back empty even when the run was accepted and produced output.

Fixes/relates to #38565.

What changed

src/commands/agent.ts
- in the ACP-ready branch:
  - call resolveAndPersistSessionFile(...) before runTurn
  - call appendAssistantMessageToSessionTranscript(...) with the finalized visible text after runTurn

Validation

node --check src/commands/agent.ts
pnpm exec vitest run src/commands/agent.acp.test.ts src/commands/agent.test.ts
- passed: 41/41 tests

Notes

I intentionally kept this patch minimal to validate the root cause first.
A fuller follow-up could decide whether ACP should also mirror the user prompt and/or unify transcript persistence more centrally for ACP flows.

greptile-apps · 2026-03-07T05:53:02Z

Greptile Summary

This PR fixes ACP (Agent Communication Protocol) child session turns so that a successful one-shot run persists both the session metadata in sessions.json and the final assistant text in a .jsonl transcript file, enabling sessions_history / chat.history to return meaningful results.

The two-part fix in agentCommandInternal is minimal and correctly sequenced: resolveAndPersistSessionFile is called early (before runTurn) to guarantee a valid sessionFile path is on disk before the transcript writer tries to look it up, and appendAssistantMessageToSessionTranscript is called after runTurn completes to append the finalized visible text.

Key observations:

The unhandled return value of appendAssistantMessageToSessionTranscript (which can return { ok: false, reason }) is consistent with every other call site in the codebase (outbound-send-service.ts, deliver.ts) — transcript writes are treated as best-effort.
resolveAndPersistSessionFile is invoked twice per turn (once in agent.ts, once inside appendAssistantMessageToSessionTranscript); this is harmless because the second call finds sessionFile already set, but it is redundant.
The "end" lifecycle event is emitted before the transcript write completes; any listener that reads the transcript in response to this event would observe a momentarily empty transcript. Moving the appendAssistantMessageToSessionTranscript call before the "end" event emission would close this ordering window.
There are no new tests that assert the .jsonl file is written, which the PR description acknowledges as intentional — coverage is deferred to a follow-up.

Confidence Score: 4/5

Safe to merge; the changes are narrowly scoped to the ACP path and introduce no correctness regressions.
The core logic is correct and follows the existing patterns for session-file persistence and transcript appending. The two style concerns (lifecycle-event ordering and double resolveAndPersistSessionFile) do not affect current runtime behavior. The missing transcript-write test coverage is acknowledged and explicitly deferred.
No files require special attention beyond the noted ordering consideration in src/commands/agent.ts.

Comments Outside Diff (1)

src/commands/agent.ts, line 668-685 (link)

Transcript appended after "end" lifecycle event

The "end" lifecycle event is emitted at line 668–675, and only then is appendAssistantMessageToSessionTranscript awaited. Any in-process listener that reacts to "end" by immediately reading the .jsonl transcript (e.g. to forward it to another service) would observe a state where the metadata in sessions.json is updated but the final assistant turn is not yet persisted. The write is guaranteed to complete before deliverAgentCommandResult, but not before the event is dispatched.

Consider moving the appendAssistantMessageToSessionTranscript call before the emitAgentEvent "end" emission so the transcript is always complete when downstream consumers receive the signal:

const finalVisibleText = visibleTextAccumulator.finalize();
if (finalVisibleText && sessionKey) {
  await appendAssistantMessageToSessionTranscript({
    agentId: sessionAgentId,
    sessionKey,
    text: finalVisibleText,
    storePath,
  });
}

emitAgentEvent({
  runId,
  stream: "lifecycle",
  data: {
    phase: "end",
    endedAt: Date.now(),
  },
});

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/commands/agent.ts
Line: 668-685

Comment:
**Transcript appended after "end" lifecycle event**

The `"end"` lifecycle event is emitted at line 668–675, and only _then_ is `appendAssistantMessageToSessionTranscript` awaited. Any in-process listener that reacts to `"end"` by immediately reading the `.jsonl` transcript (e.g. to forward it to another service) would observe a state where the metadata in `sessions.json` is updated but the final assistant turn is not yet persisted. The write is guaranteed to complete before `deliverAgentCommandResult`, but not before the event is dispatched.

Consider moving the `appendAssistantMessageToSessionTranscript` call _before_ the `emitAgentEvent` "end" emission so the transcript is always complete when downstream consumers receive the signal:

```ts
const finalVisibleText = visibleTextAccumulator.finalize();
if (finalVisibleText && sessionKey) {
  await appendAssistantMessageToSessionTranscript({
    agentId: sessionAgentId,
    sessionKey,
    text: finalVisibleText,
    storePath,
  });
}

emitAgentEvent({
  runId,
  stream: "lifecycle",
  data: {
    phase: "end",
    endedAt: Date.now(),
  },
});
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: b581951}

src/commands/agent.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b581951553

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/commands/agent.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8771733d40

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-07T06:05:09Z

src/commands/agent.ts

+        await appendAssistantMessageToSessionTranscript({
+          agentId: sessionAgentId,
+          sessionKey,
+          text: finalVisibleText,
+          storePath,
+        });


Avoid failing ACP turn when transcript mirror write fails

appendAssistantMessageToSessionTranscript(...) is awaited directly here, but that helper can still throw during file I/O (for example in ensureSessionHeader/SessionManager.appendMessage) and this call sits before the lifecycle end emission. In environments with transient or read-only session storage, a successful acpManager.runTurn(...) will be reported as a failed command and never emit the normal end event, which is a behavioral regression introduced by the new mirror step.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5521654987

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/commands/agent.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00ebc0232c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/commands/agent.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f44a2fe81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/commands/agent.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a6efd2c3a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T02:59:13Z

.github/workflows/ci.yml

+          if [ "$CI_GITHUB_REF" = "refs/heads/main" ]; then
+            echo "Skipping detect-secrets on main until the allowlist cleanup lands."
+            exit 0
+          fi


Run detect-secrets on main pushes

This new guard exits the Detect secrets step for every refs/heads/main run, and this workflow only triggers pushes on main, so pre-commit run --all-files detect-secrets no longer executes for merged or direct hotfix pushes. In that scenario, only the private-key hook still runs, which regresses coverage for other secret types and can let leaked credentials reach main undetected.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e1802d2f3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T05:45:27Z

src/agents/acp-spawn.ts

+      const fallbackSessionFile = !sessionEntry?.sessionFile
+        ? resolveSessionTranscriptPath(sessionId, targetAgentId, threadIdFromSessionKey)
+        : undefined;
+      const resolvedSessionFile = await resolveAndPersistSessionFile({


Delay session-file persistence until thread binding resolves

For mode="session" spawns, this writes a non-threaded sessionFile before the child thread is bound. Later in the same function we call resolveAndPersistSessionFile again with a thread-specific fallback, but resolveAndPersistSessionFile only applies fallbackSessionFile when sessionEntry.sessionFile is missing, so the second call cannot switch to the -topic-<thread> path. That means thread-bound ACP sessions keep using the generic transcript file despite successful binding.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07f22e896e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T06:27:48Z

src/commands/agent.ts

+                opts.threadId ?? threadIdFromSessionKey,
+              )
+            : undefined;
+          const resolvedSessionFile = await resolveAndPersistSessionFile({


Treat ACP transcript-path persistence as best-effort

This new pre-turn resolveAndPersistSessionFile(...) call is awaited before acpManager.runTurn(...), so any filesystem/session-store write failure (for example transient sessions.json I/O errors from updateSessionStore) now turns an otherwise runnable ACP request into ACP_TURN_FAILED. Because this persistence step is only metadata/transcript setup, hard-failing the turn here is a behavioral regression from the prior ACP path where the turn could still run.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-08T06:27:48Z

src/agents/acp-spawn.ts

+      const fallbackSessionFile = !sessionEntry?.sessionFile
+        ? resolveSessionTranscriptPath(sessionId, targetAgentId, threadIdFromSessionKey)
+        : undefined;
+      const resolvedSessionFile = await resolveAndPersistSessionFile({


Avoid aborting ACP spawn on transcript metadata write errors

The new resolveAndPersistSessionFile(...) call is on the main spawn path and is not isolated from the control flow, so if session-file persistence throws (e.g., transient write failure while updating the session store), spawnAcpDirect now returns status: "error" before runtime initialization. This makes ACP spawn success depend on non-essential transcript metadata I/O, which is a reliability regression for spawn requests.

Useful? React with 👍 / 👎.

robo7 · 2026-03-09T08:01:20Z

Looks like this was superseded by #40137, which closed issue #38565 with a more complete fix (including broader transcript/session persistence coverage and related follow-up pieces).

Closing this PR to avoid duplicate review work. Thanks!

openclaw-barnacle bot added commands Command implementations size: XS labels Mar 7, 2026

greptile-apps bot reviewed Mar 7, 2026

View reviewed changes

src/commands/agent.ts Outdated Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Mar 7, 2026

View reviewed changes

src/commands/agent.ts Outdated Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Mar 7, 2026

View reviewed changes

src/commands/agent.ts Outdated Show resolved Hide resolved

openclaw-barnacle bot added size: S and removed size: XS labels Mar 7, 2026

chatgpt-codex-connector bot reviewed Mar 7, 2026

View reviewed changes

src/commands/agent.ts Outdated Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Mar 7, 2026

View reviewed changes

src/commands/agent.ts Outdated Show resolved Hide resolved

robo7 force-pushed the fix/acp-claude-transcript-persistence branch from 6e0406b to 7a6efd2 Compare March 8, 2026 02:53

chatgpt-codex-connector bot reviewed Mar 8, 2026

View reviewed changes

openclaw-barnacle bot added agents Agent runtime and tooling size: M and removed size: S labels Mar 8, 2026

chatgpt-codex-connector bot reviewed Mar 8, 2026

View reviewed changes

robo7 and others added 9 commits March 8, 2026 14:19

fix: persist transcript for ACP agent run outputs

3052774

fix: append ACP transcript before end lifecycle event

5ca4b53

fix: canonicalize ACP session key for transcript persistence

7555250

fix: pass canonical ACP session key to transcript path persist

92fcfac

fix: guard ACP session path options for transcript persistence

ae46f81

ci: ensure base commit before secrets diff

842dc05

fix: clean up ACP policy failure path and secrets base fetch

203c511

fix: normalize ACP transcript output and harden secrets workflow

c0974cd

fix(acp): initialize child session transcript path during spawn

07f22e8

robo7 force-pushed the fix/acp-claude-transcript-persistence branch from 3e1802d to 07f22e8 Compare March 8, 2026 06:20

chatgpt-codex-connector bot reviewed Mar 8, 2026

View reviewed changes

fix(acp): make transcript mirror writes best-effort

0d37c1b

mbelinky mentioned this pull request Mar 8, 2026

fix(acp): persist spawned child session history #40137

Merged

11 tasks

robo7 closed this Mar 9, 2026

Uh oh!

Conversation

robo7 commented Mar 7, 2026

Summary

What changed

Validation

Notes

Uh oh!

greptile-apps bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

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 Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

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 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

robo7 commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps bot commented Mar 7, 2026 •

edited

Loading