Skip to content

feat: edit and fork historical user messages - #72

Merged
auchan merged 7 commits into
mainfrom
feat/issue-70-edit-and-fork-user-messages
Sep 6, 2026
Merged

auchan merged 7 commits into
mainfrom
feat/issue-70-edit-and-fork-user-messages

Conversation

@pi-claw-agent

@pi-claw-agent pi-claw-agent Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Historical user messages now expose Edit and Fork actions on hover. Edit opens an inline editor; saving it rewrites the conversation by branching everything before that message (keeping prior entry ids so the prefix cache is reused), sending the edited text as the fresh user message, and retiring/deleting the superseded session, so the AI reply continues from the new text. Fork creates an independent session at that message via the existing createBranchedSession primitive, names it <fork: Original Title>, and leaves the original session untouched. Actions are hidden while a run is in progress.

Validation

  • bun install --frozen-lockfile
  • bun run check-types
  • bun run lint
  • bun run compile-tests
  • bun esbuild.js
  • bun esbuild.webview.js
  • xvfb-run -a bun x vscode-test (205 passing)
  • git diff --check

Closes #70

@pi-claw-agent pi-claw-agent Bot added the agent:reviewing Independent Agent review is in progress label Sep 5, 2026
@pi-claw-agent

pi-claw-agent Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Independent Agent review: changes requested

Review round 1/2 for feba7a71d388.

The feature architecture (SDK branch-at-predecessor plus fresh prompt for edit, existing branch primitive plus fork label for fork) matches the issue, and the pure helpers are reasonable, but two defects make the destructive flow unsafe: Edit/Fork buttons stay clickable while a run is in progress (render-time-only hiding, no extension-side guard) and the rewrite can delete a session file that is still being appended; and an aborted or failed rewrite discards the new branch while leaking an orphan session file, with the destructive core carrying no focused tests.

Findings

  • blocking: Edit/Fork stay active during a run and can delete a session file that is still streaming — src/webview/handlers/index.ts:626
    • Evidence: Buttons are gated only at creation time (`if (data.entryId && !state.isStreaming)`), and handleAgentStart() never hides or disables existing .user-actions. Any user message rendered while idle keeps working Edit/Fork buttons once a later agent run starts. The commands pi-on-code.editHistoryMessage / forkHistoryMessage and editHistoryMessageFromEntry() perform no sw.isStreaming check and no abort; the edit path branches from the on-disk file and later calls deleteSessionFileIfPresent(sourcePath) on the superseded session while that session's PiService may still be appending entries. Reproduction: idle session with a few turns; send a new prompt; during the run click Edit on an earlier user message and save. This contradicts the PR claim that actions are hidden while a run is in progress.
    • Recommended fix: Hide or disable the actions on agent-start (restore on agent-end), and enforce a runtime guard in both commands (refuse, or abort+confirm, when sw.isStreaming). Only retire and delete the superseded session after confirming no agent is running on it.
  • blocking: Abort or failure mid-rewrite discards the edited branch and leaks an orphan session file — src/extension.ts:491
    • Evidence: editHistoryMessageFromEntry() awaits replacement.piService.sendPrompt(text); pi-service sendPrompt awaits session.prompt, which resolves when the whole run finishes. If the user stops the run or the API fails after content already streamed into the replacement, the catch block disposes and removes the replacement (throwing "Rewrite ready but could not start the reply"), so the edited session the user was watching disappears even though a reply did start. The forkedPath file is not deleted in that path (webviewPanel.onDispose is nulled before dispose), so the partial rewritten session is left on disk and later appears under Past Sessions as a stray duplicate. The whole destructive rewrite core (branch, prompt ordering, retire/delete, rollback, streaming guard) has no focused test: user-message-branch.test.ts covers only pure helpers plus source-lock regexes.
    • Recommended fix: Commit the edit once the new reply starts: on run abort/error keep the replacement session (original deletion may still proceed or be retried) and only roll back + delete forkedPath when no content was produced. Add focused tests for the rollback/abort path, the delete ordering, and the streaming guard so the data-mutating flow is credible.

Reviewer checks

  • Inspected git diff main...feba7a7 and traced edit/fork entry points (webview buttons, panel callbacks, commands, branch-at-predecessor, sendPrompt await semantics, original-session retirement/deletion) against PiService prompt/entry-id behavior.
  • bun run compile-tests
  • ./node_modules/.bin/mocha --ui tdd out/test/user-message-branch.test.js (7 passing)
  • ./node_modules/.bin/eslint src/test/user-message-branch.test.ts src/user-message-branch.ts
  • bun run check-types
  • bun esbuild.webview.js
  • git diff --check main...feba7a7

Generated by the independent sandboxed Reviewer Agent. The PR still requires human review and merge.

@pi-claw-agent

pi-claw-agent Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Agent review repair 1

Addressed both blocking findings. (1) Edit/Fork buttons are now disabled on agent-start and re-enabled on agent-end (plus the existing creation-time gate), and both commands plus the edit helper refuse while sw.isStreaming; the superseded session is re-checked as idle before its file is deleted so a streaming file is never removed. (2) The rewrite now commits once the replacement reply actually starts: sendPrompt is observed via waitForReplyStart, a started (or cleanly settled) run keeps the replacement session on later abort/error, and rollback — which disposes the replacement and deletes the forked branch file — happens only when no run ever started; init failure also cleans up the branch file. The destructive flow decisions are covered by focused edit-flow tests (streaming guard, commit-on-start, settle-without-start rollback, stalled-prompt rollback, started-then-fails keep) plus extended wiring assertions.

Independent Agent review will run on the updated commit. The PR still requires human review and merge.

@pi-claw-agent

pi-claw-agent Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Independent Agent review: approved

Review round 2/2 for f555be49cc88.

The round-2 patch addresses both round-1 blockers: Edit/Fork are disabled at agent-start and re-enabled at agent-end on top of the creation-time gate, both commands and the edit helper refuse while sw.isStreaming, and the superseded session is re-checked idle immediately before its file is deleted; the rewrite commits as soon as the replacement reply starts (waitForReplyStart), rolling back and deleting the branch file only when no run ever started, with init-failure cleanup and keep-on-later-failure behavior. The destructive-flow decisions are extracted into tested pure helpers with focused tests, and all validation passes.

Reviewer checks

  • Inspected git diff main...f555be4 and the follow-up commit delta, tracing the streaming guard, commit-on-start observation loop, rollback cleanup, superseded-file delete re-check, and later-error reporting paths.
  • bun run compile-tests
  • ./node_modules/.bin/mocha --ui tdd out/test/edit-flow.test.js out/test/user-message-branch.test.js (12 passing)
  • ./node_modules/.bin/eslint src/test/edit-flow.test.ts src/test/user-message-branch.test.ts src/edit-flow.ts src/user-message-branch.ts
  • bun run check-types
  • bun esbuild.webview.js
  • git diff --check main...f555be4

Generated by the independent sandboxed Reviewer Agent. The PR still requires human review and merge.

@pi-claw-agent pi-claw-agent Bot added agent:pending-approval Independent Agent review passed; awaiting human approval and removed agent:reviewing Independent Agent review is in progress labels Sep 5, 2026
@auchan
auchan merged commit c446e93 into main Sep 6, 2026
1 check passed
@auchan
auchan deleted the feat/issue-70-edit-and-fork-user-messages branch September 6, 2026 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:pending-approval Independent Agent review passed; awaiting human approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

功能:允许用户修改任意用户消息

1 participant