fix(sessions): preserve image attachment summaries on resume

[lhk0504]

Summary

Image-only ACP and exec turns sent to vision-capable models were saved as empty user messages, so loading or resuming a session erased any indication that the turn included 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.
• ACP and exec count images after the vision-capability check, matching the TUI. Images dropped for a non-vision model produce no saved image count or historical image marker. Exec combines stream-json and --image input before this check.
• 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 after rebasing onto 2ffe183 on Linux with the Go version declared in go.mod:

• make fmt-check
• make build-all (go build ./...)
• go vet ./...
• go test -p 1 ./... (full suite)
• go test ./internal/acp ./internal/cli ./internal/sessions ./internal/workspaceindex -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 with packages run serially, 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. An initial parallel run also hit TestBridgeBundleUploadRoundTrip while a test-created worktree HEAD was changing; that case passed 10 consecutive isolated runs, and serial package execution avoids this interaction. The test implementations were not changed for either environment issue.

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.

The revised non-vision cases were also run before moving the counts after the capability check. They failed with attachments.images=1 where zero was expected, and the exec resume assertion reported resume marker "[1 image]" present = true, want false. The same cases now pass, covering both the stored event and restored history/replay.

Review follow-up

• Rebased onto upstream main at 2ffe183, retaining the upstream module rename to github.com/Twigpine/zero and the workspace-index safety changes.
• The new upstream workspaceindex/gitignore.go and its test still imported github.com/Gitlawb/zero/internal/gitsafe, which prevented the rebased tree from compiling. This PR adapts those two import paths to the current module without changing workspace-index behavior.
• Moved ACP and exec counts below their vision gates and changed the two ignored-image cases to assert zero saved images and no historical marker, addressing the requested TUI semantics.

Checklist

• The linked issue already has the issue-approved label.
• go build ./..., go vet ./..., and the full test suite (go test -p 1 ./...) 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 includes image and document attachment summaries with user messages, including when sessions are restored.
  • Resumed prompts include summaries of historical attachments without reattaching those files.
  • Session records store attachment counts rather than image data.
• Bug Fixes
  • Image counts now reflect only images sent to the model; images excluded for models without vision support are not counted.

[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: c0b74e7a-9c8c-46d9-bc51-6d51771e63b0

📥 Commits

Reviewing files that changed from the base of the PR and between 51bb11f and 49f77f4.

📒 Files selected for processing: 6

• internal/acp/agent.go
• internal/acp/image_history_test.go
• internal/cli/exec.go
• internal/cli/exec_streamjson_images_test.go
• internal/workspaceindex/gitignore.go
• internal/workspaceindex/gitignore_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 attachment counts for images sent to the model. Restored user-message context includes attachment markers without storing image data. Workspace index imports now use the Twigpine module path.

Changes

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, then renders attachment markers. FormatExecPrompt adds those markers to user-message context. Tests cover malformed payloads, bounded counts, legacy messages, and metadata.
ACP event recording and restoration internal/acp/agent.go, internal/acp/agent_test.go, internal/acp/image_history_test.go	ACP user events record counts for images remaining after vision filtering. In-memory and restored user history includes attachment markers. Tests cover prompt handling, persistence, load, and resume.
Exec event recording and resume internal/cli/exec.go, internal/cli/exec_streamjson_images_test.go	Exec user events record counts for images sent to the model. Tests cover stream-JSON images, stored events, and resume behavior.
Workspace index import path internal/workspaceindex/gitignore.go, internal/workspaceindex/gitignore_test.go	The implementation and test now import gitsafe from github.com/Twigpine/zero/internal/gitsafe.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant ACPAgent
  participant SessionStore
  participant ACPHistory
  ACPAgent->>ACPAgent: Filter images by model vision support
  ACPAgent->>SessionStore: Store user text and sent-image count
  SessionStore->>ACPHistory: Restore user message event
  ACPHistory->>ACPHistory: Add attachment marker to user text

Loading

Suggested reviewers: gnanam1990, vasanthdev2004, pierrunoyt

Merge Risk: ⚪ Minimal · up to 49f77

The change preserves short image and document markers in resumed session history without storing or resending image data. No merge-blocking risk was identified.

Security architecture risk: ⚪ Minimal · up to 49f77

The change preserves attachment indicators without saving image contents or resending historical images. The reviewed changes do not expand input permissions or image delivery to external services.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The added exposure is attachment-presence metadata in existing session events and derived history text. Image forwarding from stream-json and file inputs already passed through the same vision gate at the supplied base, so this PR does not introduce that external image-data flow.

Trust Boundaries and Controls

• observed — Persisted counts are treated as untrusted: wrong types, nonpositive values and fractional values are ignored. Valid oversized counts are capped before integer conversion, and rendering independently clamps counts. Count metadata produces fixed-format markers rather than image-loading instructions.

Resilience and Maintainability Implications

• observed — Existing durability limitations remain explicit: ACP can retain completed history in memory after a persistence failure, and exec stops further recording after its first recording error and warns on exit. Store appends use a session lock and sync the event log before updating metadata; this is not a cross-file transaction. Attachment metadata adds no separate recovery state.

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 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Issue #1135 is open and directly linked. ACP and exec now record count-only image metadata after the vision check. ACP in-memory history, session/load replay, and resumed context show markers such a…
Out of Scope Changes check		The production changes support issue #1135 through ACP history, exec recording, resume formatting, and shared attachment parsing. The added tests verify these behaviors. The two workspaceindex impor…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: preserving image attachment summaries when sessions resume.

Full details: Linked Issues check

Explanation

Issue #1135 is open and directly linked. ACP and exec now record count-only image metadata after the vision check. ACP in-memory history, session/load replay, and resumed context show markers such as [1 image] instead of empty user turns. Historical image data is not stored or resent. Tests cover vision and non-vision models, fresh load and resume, replay, legacy events, malformed counts, bounds, combined exec input, and failed turns.

Full details: Out of Scope Changes check

Explanation

The production changes support issue #1135 through ACP history, exec recording, resume formatting, and shared attachment parsing. The added tests verify these behaviors. The two workspaceindex import-path changes restore compilation after the rebase and do not change workspace-index behavior. No unrelated change is identified.

• 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

[Vasanthdev2004]

Thanks, 49f77f4 is exactly it. With the count moved back above the vision gate, the ignored-by-model cases fail in both TestACPImageAttachmentHistory and TestRunExecImageAttachmentHistory, and the ACP user event still goes in ahead of anything the run queues. The rebase only changed the import paths in the first commit.

d8a6f71 is the same two-line import fix as #1153, which main needs on its own, so whichever lands first, the other merges cleanly.

internal/acp, internal/cli, internal/sessions and internal/workspaceindex pass here, and CI is 10/10. Approving.