fix(acp): preserve image paths as fallback

[snandi1603]

Problem / Motivation

When KiroCrew converts a local image path into an ACP image block, it replaces the path in the text with [image: <name>]. If the image block is dropped downstream, the agent receives neither the image nor a usable path to the source file.

Why it matters

The agent cannot inspect the image or recover with its file-reading tools. Users must manually transcribe the screenshot or run OCR outside the normal workflow.

What changed (motivation → approach → change)

The image block remains the primary vision input, but the original local path now stays in the text block as a fail-open fallback. If image delivery works, the model receives the image as before. If it fails, a tool-capable agent can still read the source file.

The single-image, multiple-image, and repeated-path tests now verify that source paths remain available while each image is still encoded into an ACP image block.

Tests

• test/test_acp_prompt_blocks.py: 87 passed, 1 skipped
• Verified both modified Python files with py_compile
• The three updated regression tests failed before the implementation change and passed afterward

Manual verification

Confirmed the latest origin/main still replaces source paths with [image: <name>], so this fix was not already present.

Verified the final branch is based on the latest origin/main, contains one focused commit, and modifies only the prompt-block implementation and its tests.

Related Issues

N/A — no existing issue linked.

Pattern harvest

Rule candidate: review-prompt
Pattern: When converting a recoverable local reference into an opaque payload, retain the original reference if downstream delivery can fail silently.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (function contract updated)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

N/A — the repository template states that the exact CLA wording is still pending from OSPO.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of f7b4f52425e8b487183245ab28238b9bce847164 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Hedges an unevidenced failure mode by regressing the common case, and silently retires this module's own stated design goal without updating its spec.

Watch

• No downstream image-block dropper is named or linked ("If the image block is dropped downstream" — every failure the builder can see already falls back to path-in-text at prompt_blocks.py:187-219). Meanwhile, when delivery works, the model now sees a raw path with no signal it corresponds to the attached image, inviting a redundant tool read of a file whose pixels are already in the prompt — and kiro-cli replays full history every turn, so a second inlined copy compounds.
• The change contradicts the module header's load-bearing claim ("stops any channel from shipping a filesystem path to the model as text", prompt_blocks.py:13-15) and leaves it, the wire-shape example at line 22, and docs/system-specs/modules/acp-client.md step 6 ("Replaces the path in the text with [image: filename.png]") stale — the spec must move in the same commit, and retiring that design goal should be explicit, not incidental.

Suggestions

• A marker that carries the full path — e.g. [image attached: /tmp/shot.png] — dominates both designs: it keeps the "already attached" signal that suppresses redundant reads while preserving the tool-readable fallback this PR wants.

[DESIGN-REVIEWED] f7b4f52

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed f7b4f52425e8b487183245ab28238b9bce847164 via the fork AI-review pipeline; updated in place on each push.

Review details

FINDING -- src/kiro_crew/acp/prompt_blocks.py:141 -- "remaining in the text" contradicts the module and ACP client contracts requiring successful image paths to be replaced -> Fix: restore the replacement and prior contract text.
[GPT-REVIEWED] f7b4f52

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed f7b4f52425e8b487183245ab28238b9bce847164 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings that block; one advisory documentation-drift finding.

FINDING — src/kiro_crew/acp/prompt_blocks.py:13 — deleting the text.replace(raw, f"[image: {path.name}]") step leaves stale prose: the module docstring still claims a single implementation "stops any channel from shipping a filesystem path to the model as text" while the code now deliberately keeps the raw path in text, and the wire example (line 22) "look at this [image: shot.png]" and the acp-client.md spec step 6 ("Replaces the path in the text with [image: filename.png]", line 566) both describe output the code no longer produces → Fix: update the module docstring rationale, the line-22 wire example, and acp-client.md step 6 to state the source path is intentionally retained in the text as a tool-readable fallback (no [image: …] substitution).

[OPUS-REVIEWED] f7b4f52

[github-actions]

First Principles Review (Fable 5, fork) — 🔴 BLOCK

Premise-level review of f7b4f52425e8b487183245ab28238b9bce847164 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. supports_image_prompt is capability-derived and fails closed; once a block is built it goes straight onto the wire — nothing in this repo drops an image block after construction. The path-preserving fallback already exists on every failure branch the builder can observe (capability absent, oversize, read refused, undecodable). Final review:

First-Principles-Verdict: BLOCK

The guarded failure — "if the image block is dropped downstream" — has zero named instances, and the path-fallback already exists on every observable failure branch.

What this change ships

Intent: keep the original file path in the prompt text so an agent can read the file if the inline image never arrives — framed as a FIX.

1. Model now sees the raw local path alongside the inline image on every image turn — speculative, zero named instances
2. [image: <name>] placeholder removed from prompts and persisted history — undeclared cost: history keeps temp paths that die at end of turn
3. Function docstring rewritten to call the redundancy intentional — rides along; module docstring's wire example (prompt_blocks.py:23) still shows the old shape

Blockers

A fix for a defect nobody has hit, deleting documented deliberate behavior. The description concedes it: "N/A — no existing issue linked", and its "Manual verification" only confirms main behaves as designed. In the base, the only case that replaces the path is the success case — the block was built and sent (session_handle.py:735); supports_image_prompt fails closed, and I counted 4 branches in prompt_blocks.py (lines 145–149, 185–194, 200–204, 208–219) that already "leave the path in the text" whenever delivery can actually fail from this process's viewpoint. The rationale is stated as a generic pattern ("Pattern harvest: … retain the original reference if downstream delivery can fail silently") — analogy, not a constraint. Meanwhile the change pays a real per-turn cost the description never weighs: the module's own comment (prompt_blocks.py:106) records that channel temp attachments are "deleted at end of turn, leaving a dead reference", so persisted history now carries dead paths, and a tool-capable agent is invited to redundantly re-read a file it already received as pixels. Subtraction: drop the change; if a specific backend ever drops advertised-capability image blocks, fix that backend's promptCapabilities advertisement — the existing allow_image=False path then does exactly what this PR wants.

[FIRST-PRINCIPLES-REVIEWED] f7b4f52

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6305 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6305: CONTINUE_DEVELOPMENT. Complementary hunks in one function; the primary's assertions about the path surviving in the text hold under either ordering. Files: src/kiro_crew/acp/prompt_blocks.py.
• This PR is OVERLAPPING with PR #974. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8238: CONTINUE_DEVELOPMENT. 8238 reverses a documented deliberate decision from 974 rather than fixing a defect in it, so the design owner should confirm the retirement explicitly. Files: src/kiro_crew/acp/prompt_blocks.py.
• This PR is OVERLAPPING with PR #4079. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8238: CONTINUE_DEVELOPMENT. Different user goal, disjoint hunks, and a clean merge. The only coupling is the acp-client.md Image Support step that 4079 preserves and 8238 silently invalidates; whichever lands second must reconcile that line. Files: src/kiro_crew/acp/prompt_blocks.py, docs/system-specs/modules/acp-client.md.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.