fix(sessions): preserve image attachment summaries on resume

[lhk0504]

Summary

Image-only ACP and exec turns were saved as empty user messages, so loading or resuming a session erased any indication that the user had attached an image. This change records the existing TUI-compatible attachments: {images, documents} counts and restores a short marker such as [1 image] beside the original text.

• ACP uses the same marker for in-memory history, restarted load/resume context, and session/load transcript replay.
• Exec records counts from stream-json and --image input and renders them in resumed context instead of exposing raw count values through the generic payload summary.
• Session events retain the original text and attachment counts only. Historical image data is neither stored nor sent back to the model, following the approved scope.
• Older text-only events remain readable; malformed counts are ignored and oversized counts use the TUI's existing bound.

For an image-only turn, resumed ACP context now contains User: [1 image] instead of User: .

Linked issue

Fixes #1135

Validation

Validated on Linux with the Go version declared in go.mod:

• make fmt-check
• make build-all (go build ./...)
• go vet ./...
• go test ./...
• go test ./internal/acp ./internal/cli ./internal/sessions -race -count=1
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke
• make vulncheck — no vulnerabilities found
• git diff HEAD --check

The full test suite passed in an isolated network namespace with ordinary user permissions and temporary user directories. On the host, two existing OAuth tests cannot bind their fixed port 1455 because another service already holds it; the isolated run avoids that conflict without changing those tests or stopping the service.

make lint-static reports four existing advisory findings in untouched files: QF1001 in internal/installtest/workflow_permissions_test.go:20, and QF1008 in internal/proxydial/proxydial.go:67,72 and internal/tools/web_fetch.go:315. No finding points to this change.

Regression evidence

The regression tests were run before the fix. The ACP image-only case failed with:

stored user event = {"content":"","role":"user"}, want original text and attachments.images=1
seeded prompt missing attachment marker "[1 image]"

The exec case also failed because its resumed prompt contained message: user with no image marker. Both paths pass with the fix. Coverage includes same-process follow-up, fresh-agent/store load and resume, text plus multiple images, non-vision model handling, combined stream/file input, absence of persisted or reattached image data, legacy events, malformed counts, and failed turns committing no history.

Checklist

• The linked issue already has the issue-approved label.
• go build ./..., go vet ./..., and go test ./... pass locally, with the test environment described above.
• gofmt clean.
• Tests added/updated for the change and run under -race where relevant.
• UI screenshots/recording: not applicable to this protocol/persistence fix; the transcript text change is illustrated above and tested through ACP replay.

Summary by CodeRabbit

• New Features
  • Session history now shows image and document attachment markers alongside user messages, including after a session is restored.
  • Exec session records include attachment counts without storing image data. Resumed prompts show historical attachment markers without reattaching those files.
  • Image attachment counts are recorded even when the selected model cannot process images.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Twigpine/zero/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: ea6e8541-0d09-4082-88b4-529226cd680e

📥 Commits

Reviewing files that changed from the base of the PR and between 8c4e759 and 51bb11f.

📒 Files selected for processing: 8

• internal/acp/agent.go
• internal/acp/agent_test.go
• internal/acp/image_history_test.go
• internal/cli/exec.go
• internal/cli/exec_streamjson_images_test.go
• internal/sessions/exec_session.go
• internal/sessions/message_attachments.go
• internal/sessions/message_attachments_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

ACP and exec session events now record image attachment counts without storing image data. Restored user messages include attachment markers in conversation context. Tests cover live prompts, persistence, follow-up prompts, and session load or resume.

Changes

Image attachment history

Layer / File(s)	Summary
Attachment count contract and context formatting internal/sessions/message_attachments.go, internal/sessions/exec_session.go, internal/sessions/message_attachments_test.go	MessageAttachments validates and bounds image and document counts, and renders attachment markers. FormatExecPrompt includes those markers in message context. Tests cover valid, missing, malformed, and bounded attachment counts.
ACP event recording and restoration internal/acp/agent.go, internal/acp/agent_test.go, internal/acp/image_history_test.go	ACP user events record image counts. Restored and in-memory user history includes attachment markers. Assistant events retain the prior payload shape. Tests cover image history across prompts, persistence, load, and resume.
Exec event recording and resume tests internal/cli/exec.go, internal/cli/exec_streamjson_images_test.go	Exec records supplied image counts before the vision-support check. Tests cover stored attachment counts and resumed prompt markers for stream-JSON image input.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: gnanam1990, vasanthdev2004, pierrunoyt

Merge Risk: ⚪ Minimal · up to 51bb1

Attachment summaries appear preserved across session resume without storing image data. No actionable issue is established for this change.

Security architecture risk: ⚪ Minimal · up to 51bb1

The reviewed change preserves attachment counts without saving historical image data. Restored counts become bounded text markers, and no material new security exposure was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The new metadata follows the selected session's existing history flow, including exec resume and fork context. Its reviewed downstream exposure is attachment-count text in restored history, not additional image access or tool authority.

Trust Boundaries and Controls

• observed — Persisted counts pass through a decoder that ignores invalid values and a formatter that independently clamps counts. Only fixed image/document labels are generated, and the new restoration projection applies to user messages.

Resilience and Maintainability Implications

• observed — Attachment counts remain inside their existing message event, avoiding a separate metadata commit or cleanup step. The unchanged store writes batches under a session lock and syncs the log before metadata; this provides batch ordering, not a new crash-atomic transaction or global prompt-execution lock.
• observed — Exec continues recording the user event before execution and records interruption, provider failure or completion afterward. Recording failures remain latched and reported once, rather than allowing later appends to proceed after a failed prerequisite.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 8 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving image attachment summaries when sessions resume.
Linked Issues check		Issue #1135 is open and directly linked. The PR records count-only attachment metadata for ACP and exec user events, restores markers such as [1 image] in ACP history, session replay, and resumed co…
Out of Scope Changes check		The changed production files support issue #1135: ACP history handling, exec recording and resume formatting, and shared attachment-count formatting. The added tests verify those behaviors. No unrelat…

Full details: Linked Issues check

Explanation

Issue #1135 is open and directly linked. The PR records count-only attachment metadata for ACP and exec user events, restores markers such as [1 image] in ACP history, session replay, and resumed context, and preserves text-only compatibility. MessageAttachments ignores malformed counts and caps valid counts at 64. The implementation does not persist or resend image data. The added ACP and exec tests cover image-only input, resumed sessions, replay, legacy events, malformed counts, model capability handling, and failed turns.

Full details: Out of Scope Changes check

Explanation

The changed production files support issue #1135: ACP history handling, exec recording and resume formatting, and shared attachment-count formatting. The added tests verify those behaviors. No unrelated production feature or unrelated test change is identified in the supplied whole-PR summary.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[Vasanthdev2004]

Thanks, this is the shape from the issue, and the tests hold up: taking out any one piece (the ACP event's attachments, the marker on load, the marker in live history, exec's attachments, the exec resume projection, the fractional-count check or the 64 bound) fails one of them.

One thing differs from the TUI, though. The TUI zeroes the image count when the model can't take images: model.go says "the sent user row must not claim the image was included", and the count it persists is that same value. Here ACP and exec take the count before the vision gate, so the ignored-by-model cases store images: 1 and resume as User: [1 image], for a turn where the model never got the image. In ACP the live turn at least carried the "ignoring 1 prompt image(s)" note, but that isn't persisted, so after a restart nothing says the image was dropped. The TUI reads the same field back through attachmentSummaryFromPayload, so it should mean the same thing whichever surface wrote it.

Could you take the count after the vision gate in both runTurn and runExec, and flip the two "ignored by model" cases to expect no marker? With exec's count moved below the gate, TestRunExecImageAttachmentHistory already fails on that case, so the test is in the right place.

The red Windows smoke job was TestACPSetModeUpdatesSession timing out at 5 s in set_config_option, which this PR doesn't touch. It passes 20 times in a row here and passed when I re-ran the job, so CI is green now. Otherwise internal/acp and internal/sessions pass under -race on Windows, internal/cli and internal/tui pass, and go vet is clean for linux, darwin and windows.

[kevincodex1]

please rebase to main branch and fix conficts