diff --git a/CHANGELOG.md b/CHANGELOG.md index cc05c2f8..c57b92da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ All notable user-visible changes to CASCADE are documented here. The format is l ### Changed +- **PM image delivery: Linear GraphQL fixture + extraction-coverage regression test** (spec 016, plan 3 of 3). Captures a reconstructed Linear `Issue` GraphQL payload at `tests/fixtures/linear-issue-with-screenshot.json` containing extension-less and extensioned inline-pasted images (description + comment bodies) plus formal Attachment records (Slack/GitHub/Sentry link previews) that must NOT be mistaken for inline images. The unit test at `tests/unit/pm/linear/extraction-coverage.test.ts` pins the contract and fails loudly with a specific URL-missing message if Linear ever changes its payload shape in a way that loses inline images. Documents the conclusion in `src/integrations/README.md`: `Issue.description` markdown is canonical for Linear inline images; `Issue.attachments` is the wrong surface (formal Attachment records, not pastes). No production code change — this plan ships the regression net for the contract Plans 1+2 established. See [spec 016](docs/specs/016-pm-image-delivery-reliability.md). +- **PM image delivery: runtime `cascade-tools pm read-work-item` gadget now delivers images on disk** (spec 016, plan 2 of 3). The runtime gadget that agents call mid-run for a work item used to return text only — its "Pre-fetched Images" section listed URL refs but no local file paths, so an agent that needed to re-read a work item (e.g. after a teammate added a screenshot) had no way to actually see the new image. After this plan, the gadget downloads any image media present and writes it to `.cascade/context/images/work-item--img-.` (extension derived from the resolved Content-Type MIME), then returns text whose new "Local Image Files" section lists actual file paths the agent's file-read tool can consume. Failed downloads are surfaced in a "Failed Image Downloads" subsection so they're never silently dropped. Same diagnostic log line as the boot path (`[image-pipeline] work-item-fetch summary`) — operators see consistent shape across boot and runtime fetches. Closes the mid-run pickup gap. See [spec 016](docs/specs/016-pm-image-delivery-reliability.md). +- **PM image delivery: extension-less Linear pasted-image URLs are no longer dropped at the pre-download MIME filter** (spec 016, plan 1 of 3). Linear's `https://uploads.linear.app/` URLs (with no file extension in the pathname) used to fall through `mimeTypeFromUrl` to `application/octet-stream` and were silently filtered out by `filterImageMedia` before the download loop ran. The fix introduces an `image/*` wildcard sentinel for trusted PM-provider upload hosts (allowlisted by hostname); `isImageMimeType` now accepts the wildcard, and the download response's `Content-Type` header resolves it to a concrete MIME (`image/png`, etc.) before any image is written. The shared `downloadAndPrepareImages` helper consolidates the per-provider download dispatch (jira/linear/trello) so both the boot-path and the runtime gadget (spec 016 plan 2) share one code path. Adds AC#5's grep-stable diagnostic line — `[image-pipeline] work-item-fetch summary` — emitted once per work-item-fetch with stable fields (`provider`, `workItemId`, `urlsDetected`, `urlsAfterFilter`, `urlsDownloaded`, `urlsFailed`, `urlsByMimeType`). Closes the silent screenshot-drop bug class verified live on 2026-04-26 (ucho/MNG-357). See [spec 016](docs/specs/016-pm-image-delivery-reliability.md). - **Router dispatch capacity now waits for a slot; transient Docker errors retry; terminal errors fail fast** (spec 015, plan 2 of 2). Replaces `guardedSpawn`'s synchronous "No worker slots available" throw with an in-process slot-waiter (default 5min timeout, configurable via `SLOT_WAIT_TIMEOUT_MS`). Adds a dispatch-error classifier that splits transient (`ECONNREFUSED` / `ECONNRESET` / `ENOTFOUND` / HTTP 429 / container-name 409 / `SLOT_WAIT_TIMEOUT`) from terminal (`TypeError` / `ZodError` / image-not-found-after-fallback). Both `cascade-jobs` and `cascade-dashboard-jobs` queue defaults now specify `attempts: 4` with `backoff: { type: 'exponential', delay: 5000 }` (~75s total before exhaustion). Terminal errors are wrapped in BullMQ's `UnrecoverableError` so retries skip. Combined with plan 015/1, the original silent black-hole failure mode (verified live on 2026-04-26 via ucho/MNG-350) is fully closed: no more lost jobs on transient capacity misses or Docker hiccups, no more wedged locks. CLAUDE.md updated with the new "Dispatch failure semantics" passage. See [spec 015](docs/specs/015-router-job-dispatch-failure-recovery.md). - **Router dispatch failures now release in-memory locks via the BullMQ failed event** (spec 015, plan 1 of 2). Hooks `worker.on('failed')` on both `cascade-jobs` and `cascade-dashboard-jobs` queues to call a new `releaseLocksForFailedJob` compensator that releases the work-item lock, agent-type concurrency counter, and recently-dispatched dedup mark for any job whose dispatch fails. Closes the stranded-lock half of the prod incident verified on 2026-04-26 (ucho/MNG-350): a transient capacity miss was leaving the in-memory work-item lock wedged for 30 minutes, silently rejecting subsequent webhooks for the same trio. Also splits the webhook decision-reason vocabulary into three states — `Job queued` (success), `Awaiting worker slot: …` (in-flight, healthy), `Work item locked (no active dispatch): …` (wedged-lock canary, fires a Sentry capture tagged `wedged_lock_canary` so any regression in compensation is loud). Plan 2 closes the lost-job half (wait-for-slot, retry budget, error classifier). See [spec 015](docs/specs/015-router-job-dispatch-failure-recovery.md). - **`cascade-tools scm create-pr-review`: `--comment` alias + `--comments-file` escape hatch** (spec 014, plan 2 of 2). The command now accepts `--comment` (singular) as an alias for `--comments` — the exact muscle-memory mistake from prod run 5d993b04 now resolves correctly. Added `--comments-file ` (and `-` for stdin) as a JSON-parsed file alternative for long payloads that don't survive shell quoting. Zero edits to shared infrastructure (cliCommandFactory, manifestGenerator, nativeToolPrompts, errorEnvelope) — the two declarative fields on `createPRReviewDef.parameters.comments.cliAliases` + `createPRReviewDef.cli.fileInputAlternatives` are everything. Proves spec 014's single-entrypoint invariant: a new or evolved gadget should never need to touch shared machinery. See [spec 014](docs/specs/014-cascade-tools-agent-ergonomics.md). diff --git a/docs/plans/016-pm-image-delivery-reliability/1-boot-path-mime-fix-and-diagnostic-log.md.done b/docs/plans/016-pm-image-delivery-reliability/1-boot-path-mime-fix-and-diagnostic-log.md.done new file mode 100644 index 00000000..7bc75cee --- /dev/null +++ b/docs/plans/016-pm-image-delivery-reliability/1-boot-path-mime-fix-and-diagnostic-log.md.done @@ -0,0 +1,211 @@ +--- +id: 016 +slug: pm-image-delivery-reliability +plan: 1 +plan_slug: boot-path-mime-fix-and-diagnostic-log +level: plan +parent_spec: docs/specs/016-pm-image-delivery-reliability.md +depends_on: [] +status: done +--- + +# 016/1: Boot-path MIME fix + diagnostic log line + +> Part 1 of 3 in the 016-pm-image-delivery-reliability plan. See [parent spec](../../specs/016-pm-image-delivery-reliability.md). + +## Summary + +This plan fixes the MNG-357 root cause — Linear's extension-less pasted-image URLs (`https://uploads.linear.app//`) get dropped at the pre-download MIME filter because `mimeTypeFromUrl` returns `application/octet-stream` and `filterImageMedia` excludes anything that isn't an image MIME. The fix defers MIME authority to the download response's `Content-Type` header by introducing an `image/*` wildcard sentinel that survives the filter and is resolved to a concrete MIME at download time. + +It also adds the diagnostic log line that AC#5 requires. Today the only image-pipeline log is the post-download `fetchWorkItemStep: image download complete` summary; the upstream extract-and-filter step is invisible. After this plan ships, every work-item-fetch emits ONE structured log line at `INFO` level with a stable shape: provider, work-item-id, urls-detected, urls-after-filter, urls-downloaded, urls-failed. An operator can grep for it and triage any "no image delivered" report from that line alone. + +This plan does NOT change the runtime read-work-item gadget (Plan 2) and does NOT add the Linear GraphQL fixture (Plan 3). It also does NOT touch PR #948's Claude-Code initial-input ImageBlockParam path — but a regression test pins that path so Plan 2 and beyond can't accidentally break it. + +**Components delivered:** +- New behavior in `src/pm/media.ts`: extension-less URLs that PM providers commonly produce (Linear's `uploads.linear.app/` shape) are tagged with `mimeType: 'image/*'` instead of `application/octet-stream`. The wildcard is added to `IMAGE_MIME_TYPES`-equivalent acceptance in `isImageMimeType` (a single check). +- New extraction-step diagnostic log line (one INFO log per call), emitted from a new helper that wraps the existing extract → filter → download → write pipeline. Format pinned by test. +- Regression tests: extension-less Linear URL flows end-to-end (extract → filter → download → write); extension-bearing Trello/JIRA URLs still work (no regression); the new diagnostic log shape is asserted; PR #948's Claude-Code ImageBlockParam path is pinned by a regression test that fails loudly if the boot-path image stripping behavior changes. +- A small refactor extracting the existing download-and-base64 loop in `fetchWorkItemStep` (`src/agents/definitions/contextSteps.ts:107-153`) into a shared module-internal helper. Plan 2 will import this helper for the runtime gadget; Plan 1 only refactors and consumes from one site. + +**Deferred to later plans in this spec:** +- The runtime read-work-item gadget that delivers images mid-run (Plan 2). +- The Linear GraphQL fixture + extraction-coverage regression test (Plan 3). +- `src/integrations/README.md`'s Linear-specific GraphQL surface confirmation (Plan 3). + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #1 (Linear screenshot on-disk for all engines, boot path) — **full** +- Spec AC #2 (Trello/JIRA images regression-safe) — **full** (regression tests) +- Spec AC #5 (single grep-stable diagnostic log line) — **partial** (boot path here; runtime path same format in Plan 2) +- Spec AC #6 (PR #948 Claude-Code path untouched) — **full** (regression test pins it) +- Spec AC #8 (new-provider invariant preserved) — **full** (no provider-specific code added in shared resolution) +- Spec AC #9 (MNG-357 end-to-end reproduces clean) — **full** + +--- + +## Depends On + +None. This plan is the foundation of the spec. + +--- + +## Detailed Task List (TDD) + +### 1. Extension-less PM URL detection + `image/*` wildcard sentinel + +**Tests first** (`tests/unit/pm/media.test.ts` — extend existing file): + +- `mimeTypeFromUrl — returns 'image/*' for extension-less Linear URL` — unit — input `https://uploads.linear.app/abc-123-def-456` (no extension); assert returns `'image/*'`. Expected red: `AssertionError: expected 'application/octet-stream' to be 'image/*'`. +- `mimeTypeFromUrl — returns 'image/png' for extensioned PM URL (regression-safe)` — unit — input `https://example.com/foo.png`; assert returns `'image/png'`. Expected red: passes already (regression pin); fails only if we accidentally regress the extensioned path. +- `mimeTypeFromUrl — returns 'application/octet-stream' for non-PM extension-less URL (no over-broad behavior)` — unit — input `https://example.com/random-file`; assert returns `'application/octet-stream'`. Expected red: passes already; fails if we over-broaden the wildcard logic to all extension-less URLs (which would degrade observability). +- `mimeTypeFromUrl — returns 'image/*' for extension-less Linear comment-pasted URL` — unit — input `https://uploads.linear.app//Screenshot 2026-04-26 at 10.30.png` (some Linear URLs DO have extensions but with spaces; some don't); covers the no-extension fallback. Expected red: same as test 1 if the path has no `.` segment. +- `isImageMimeType — accepts the 'image/*' wildcard sentinel` — unit — input `'image/*'`; assert returns `true`. Expected red: `AssertionError: expected false to be true` (`IMAGE_MIME_TYPES` set doesn't contain `'image/*'`). +- `isImageMimeType — preserves existing strict acceptance for known image MIMEs` — unit — input `'image/png'` returns true, `'application/pdf'` returns false. Regression pin. + +**Implementation** (`src/pm/media.ts`): +- Add `IMAGE_HOST_ALLOWLIST` constant: `Set` containing `'uploads.linear.app'`. (Could grow to include trusted hosts for other providers if they prove to need it; today Linear is the only one.) Documented inline comment: this list represents trusted PM-provider upload hosts whose extension-less URLs we should treat as candidate images and resolve at download time. +- Modify `mimeTypeFromUrl(url)`: after computing `ext` from the pathname, if the resolved MIME is `'application/octet-stream'` AND the URL's hostname is in `IMAGE_HOST_ALLOWLIST`, return `'image/*'` instead. +- Modify `isImageMimeType(mime)`: accept `'image/*'` (the wildcard) in addition to the existing concrete-MIME set. +- Do NOT change `filterImageMedia` — it already calls `isImageMimeType`, so widening that predicate flows through. +- Do NOT add a HEAD request; resolve at GET (the existing `downloadMedia` path). + +### 2. Diagnostic log line at extract-time + +**Tests first** (`tests/unit/agents/definitions/contextSteps.test.ts` — extend existing or create new file if absent): + +- `fetchWorkItemStep — emits diagnostic log line with extracted, post-filter, downloaded, failed counts` — unit — mock `readWorkItemWithMedia` to return 3 image refs (mix of extensioned + extension-less); mock `downloadMedia` to succeed for 2, fail for 1; assert exactly ONE INFO-level log call matches `'work-item-fetch image pipeline'` (or whatever the agreed prefix becomes) with structured fields `{ provider, workItemId, urlsDetected: 3, urlsAfterFilter: 3, urlsDownloaded: 2, urlsFailed: 1 }`. Expected red: `expected logger.info to have been called with object containing 'work-item-fetch image pipeline'` — today no such log exists. +- `fetchWorkItemStep — log line emitted even when no images are present (urlsDetected: 0)` — unit — mock returns no images; assert the log line still fires with all-zero counts. Why: an operator triaging "no image delivered" reports needs to see "0 detected upstream" as positive confirmation, not absence-of-log. Expected red: same. +- `fetchWorkItemStep — log fields include the post-resolve mime distribution` — unit — mock returns 1 extensioned PNG and 1 extension-less Linear; downloads both; assert log includes `urlsByMimeType: { 'image/png': 1, 'image/*': 1 }` (or similar — the test pins the field name and shape, not the exact map keys). Expected red: same. +- `fetchWorkItemStep — log line is INFO level (not DEBUG)` — unit — assert the call is on `logger.info`, not `logger.debug`. Why: AC#5 requires "single grep-stable line in the cascade run log surface" — DEBUG is filtered out by default. Expected red: passes if log doesn't exist (vacuous), fails right reason if implementation uses wrong level. + +**Implementation** (`src/agents/definitions/contextSteps.ts`): +- Add a structured log call inside `fetchWorkItemStep`, AFTER the download Promise.all resolves and BEFORE the function returns. Exact shape: + ``` + logger.info('[image-pipeline] work-item-fetch summary', { + provider: , + workItemId, + urlsDetected: , + urlsAfterFilter: , + urlsDownloaded: , + urlsFailed: , + urlsByMimeType: >, + }); + ``` +- The provider type is available via `getPMProviderOrNull()?.type`. +- `urlsByMimeType` is built from the resolved `DownloadMediaResult.mimeType` for successes; failures contribute to a separate `failuresByReason` field if useful (defer to test contract). +- The log line is grep-stable via the literal prefix `[image-pipeline] work-item-fetch summary`. Document this in the integrations README (see Doc Impact below). + +### 3. Refactor: extract download-and-base64 loop into a shared helper + +**Tests first** (`tests/unit/pm/download-and-prepare.test.ts` — new file): + +- `downloadAndPrepareImages — downloads each ref, returns success array + failure array` — unit — pass 3 refs; mock `downloadMedia` per-provider; assert returned shape `{ images: [...], failures: [{url, reason}] }`. Expected red: module not found. +- `downloadAndPrepareImages — preserves base64 + altText + mimeType from the resolved download` — unit — pass extension-less Linear ref; mock download to return `{ buffer, mimeType: 'image/png' }`; assert resulting image's mimeType is `'image/png'` (NOT `'image/*'` — the wildcard was resolved). Expected red: module not found. +- `downloadAndPrepareImages — caps at MAX_IMAGES_PER_WORK_ITEM` — unit — pass 12 refs, MAX_IMAGES_PER_WORK_ITEM=10; assert only 10 attempted. Expected red: module not found. +- `downloadAndPrepareImages — picks per-provider download client (jiraClient / linearClient / trelloClient)` — unit — set provider to 'linear'; assert `linearClient.downloadAttachment` is the one called. Expected red: module not found. + +**Implementation** (`src/pm/download-and-prepare.ts` — new file): +- Function signature: `downloadAndPrepareImages(workItemId: string, media: MediaReference[], logWriter: LogWriter): Promise<{ images: ContextInjectionImage[]; failures: { url: string; reason: string }[] }>`. +- Lifts the loop currently at `src/agents/definitions/contextSteps.ts:107-153` into this module unchanged (same provider-dispatch, same Promise.all, same per-failure WARN log). +- The new diagnostic log (task 2 above) calls this helper, then emits the summary. +- Plan 2 will import this helper for the runtime gadget. This plan only consumes it from `fetchWorkItemStep` — the runtime gadget consumer is deliberately out of scope here. + +**Reuse + refactor**: After the helper exists, modify `fetchWorkItemStep` to call it instead of the inline loop. The diagnostic log (task 2) is wired around the helper call. Same external behavior; same returned shape. + +### 4. Regression test: PR #948's Claude-Code ImageBlockParam path stays untouched + +**Tests first** (`tests/unit/backends/claude-code/image-injection.test.ts` — extend existing PR #948 test file or create if absent): + +- `Claude Code backend — initial-input image-strip-before-buildTaskPrompt invariant holds` — unit — feed a `ContextInjection` with `images` populated; assert that `buildTaskPrompt` (or its mock) receives an injection where `images` field is absent/empty AND that `buildPromptWithImages` is called with the original images. Expected red: passes already (regression pin); fails right reason if Plan 1 accidentally regresses the strip-before-buildTaskPrompt logic. +- `Claude Code backend — multimodal SDK content blocks include the prepared base64 + mimeType` — unit — assert the resulting SDK call includes `ImageBlockParam` content blocks matching the input images. Expected red: passes already; fails right reason if Plan 1's MIME-resolution refactor breaks the data flowing into the SDK call. + +**Implementation**: none. These are pure regression pins. If they don't already exist (PR #948 may have a different test file), create the minimum coverage to pin the behavior. If PR #948 left no tests on this surface, that's a finding to surface — but the spec's AC#6 cannot be satisfied without a test that pins the behavior, so adding it is in-scope here. + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/pm/media.test.ts`: 6 new tests covering `mimeTypeFromUrl` extension-less Linear URL handling + `isImageMimeType` wildcard acceptance + Trello/JIRA regression pins +- [ ] `tests/unit/agents/definitions/contextSteps.test.ts`: 4 new tests covering the diagnostic log line shape, level, and zero-image case +- [ ] `tests/unit/pm/download-and-prepare.test.ts` (new): 4 tests covering the extracted helper's contract (shape, MIME resolution, cap, per-provider dispatch) +- [ ] `tests/unit/backends/claude-code/image-injection.test.ts`: 2 regression pins for PR #948's strip-before-buildTaskPrompt invariant + +### Integration tests +- [ ] `tests/integration/pm/image-pipeline.test.ts` (new): one happy-path integration that exercises the real `mimeTypeFromUrl` + real `isImageMimeType` + real `filterImageMedia` + real `downloadAndPrepareImages` (with `downloadMedia` stubbed to control Content-Type) + real diagnostic log emission, end-to-end with an extension-less Linear-shaped URL. + +### Acceptance tests +- [ ] AC#1: integration test "extension-less Linear URL flows end-to-end and lands on disk" +- [ ] AC#2: regression tests "Trello PNG URL still flows" + "JIRA attachment URL still flows" +- [ ] AC#5 (boot path): unit test pinning the diagnostic log line shape and level +- [ ] AC#6: regression pins for PR #948's Claude-Code path +- [ ] AC#8: covered by AC#1 + AC#2 (no provider-specific code added) +- [ ] AC#9: integration test that simulates the MNG-357 scenario (extension-less Linear URL → boot path → file on disk + log line shows `urlsDownloaded: 1`) + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs auto-tested. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. An extension-less Linear-shaped URL (`https://uploads.linear.app//...` with no `.png`/`.jpg` extension) flows through extract → filter → download → write end-to-end. The resulting `MediaReference.mimeType` after download resolves to `'image/png'` (or whatever the response Content-Type header reports), NOT `'image/*'` or `'application/octet-stream'`. +2. Extension-bearing Trello and JIRA URLs continue to flow through with no behavior change. +3. Every `fetchWorkItemStep` invocation emits exactly ONE structured INFO-level log line with the literal prefix `[image-pipeline] work-item-fetch summary` and structured fields `{ provider, workItemId, urlsDetected, urlsAfterFilter, urlsDownloaded, urlsFailed, urlsByMimeType }`. +4. PR #948's Claude-Code initial-input ImageBlockParam path passes a regression test that fails loudly if the strip-before-buildTaskPrompt invariant changes. +5. `downloadAndPrepareImages` is a callable helper module exporting the prep loop with a stable shape `Promise<{ images, failures }>`. `fetchWorkItemStep` uses it (refactor not new code path). +6. `mimeTypeFromUrl` returns `'image/*'` for extension-less URLs whose hostname is in `IMAGE_HOST_ALLOWLIST` (currently `uploads.linear.app`); returns the existing extension-derived MIME for everything else; returns `'application/octet-stream'` for unknown extension-less hosts (preserving observability of unrecognized cases). +7. `isImageMimeType('image/*')` returns true; `isImageMimeType('application/pdf')` returns false (regression). +8. All new/modified code has corresponding tests written before the implementation. +9. `npm run build` passes. +10. `npm test` passes. +11. `npm run test:integration` passes for the new integration test. +12. `npm run lint` passes. +13. `npm run typecheck` passes. +14. All documentation listed in this plan's Documentation Impact has been updated. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `CHANGELOG.md` | Entry under the next release: "PM image delivery: extension-less PM-provider URLs (Linear `uploads.linear.app/`) are no longer dropped by the pre-download MIME filter. Defers MIME authority to the download response's Content-Type header. Adds a single grep-stable diagnostic log line at extract time: `[image-pipeline] work-item-fetch summary`. Closes the silent screenshot-drop bug class verified live on 2026-04-26 (ucho/MNG-357)." | +| `src/integrations/README.md` | Add a new section titled "Image delivery contract" near the end. Documents: (a) the shared MIME resolution path (Content-Type-first, URL-extension-second, no magic-byte sniffing); (b) the `IMAGE_HOST_ALLOWLIST` for trusted PM-provider upload hosts and how to add a new entry; (c) the `[image-pipeline] work-item-fetch summary` diagnostic log line operators rely on, with field schema; (d) the rule that providers should NOT do their own MIME resolution — let the shared path handle it. Cross-link to spec 016. | + +--- + +## Out of Scope (this plan) + +- The runtime read-work-item gadget downloading + writing images (Plan 2). +- Linear GraphQL fixture + extraction-coverage regression test (Plan 3). +- Codex / OpenCode native multimodal SDK delivery — out of scope per spec. +- Magic-byte sniffing — out of scope per spec. +- Backfilling missed screenshots for prior runs — out of scope per spec. +- Image compression / resize / format conversion — out of scope per spec. +- Dashboard surface for "image not delivered" — out of scope per spec. + +--- + +## Progress + + +- [x] AC #1 (extension-less Linear URL flows end-to-end) +- [x] AC #2 (Trello/JIRA regression) +- [x] AC #3 (diagnostic log line shape + level) +- [x] AC #4 (PR #948 regression pin) +- [x] AC #5 (downloadAndPrepareImages shared helper) +- [x] AC #6 (mimeTypeFromUrl wildcard for allowlist hosts) +- [x] AC #7 (isImageMimeType wildcard acceptance) +- [x] AC #8 (TDD discipline) +- [x] AC #9 (build) +- [x] AC #10 (unit tests) +- [x] AC #11 (integration tests) +- [x] AC #12 (lint) +- [x] AC #13 (typecheck) +- [x] AC #14 (docs) diff --git a/docs/plans/016-pm-image-delivery-reliability/2-runtime-gadget-image-delivery.md.done b/docs/plans/016-pm-image-delivery-reliability/2-runtime-gadget-image-delivery.md.done new file mode 100644 index 00000000..7b04a4dd --- /dev/null +++ b/docs/plans/016-pm-image-delivery-reliability/2-runtime-gadget-image-delivery.md.done @@ -0,0 +1,187 @@ +--- +id: 016 +slug: pm-image-delivery-reliability +plan: 2 +plan_slug: runtime-gadget-image-delivery +level: plan +parent_spec: docs/specs/016-pm-image-delivery-reliability.md +depends_on: [1-boot-path-mime-fix-and-diagnostic-log.md] +status: done +--- + +# 016/2: Runtime gadget image delivery (mid-run pickup) + +> Part 2 of 3 in the 016-pm-image-delivery-reliability plan. See [parent spec](../../specs/016-pm-image-delivery-reliability.md). + +## Summary + +This plan closes the mid-run gap. Today the runtime gadget `cascade-tools pm read-work-item` (`src/cli/pm/read-work-item.ts` → `src/gadgets/pm/core/readWorkItem.ts:167`) calls `readWorkItem(workItemId, includeComments)` which delegates to `readWorkItemWithMedia` and discards the returned media. The gadget's text output includes a "Pre-fetched Images" section that lists URL refs with descriptive labels but NO local file paths the agent can read. + +After this plan ships, the runtime gadget downloads any image media it discovered and writes the bytes to `.cascade/context/images/work-item--img-.`, then returns text whose Pre-fetched Images section lists the actual relative file paths the agent can hand to its file-read tool. The same diagnostic log line introduced in Plan 1 fires here too — same prefix, same fields, same grep — so an operator triaging a "no image after re-read" report sees the boot-path summary AND the runtime-path summary in the run log with consistent shape. + +This plan does NOT change the boot path's behavior (Plan 1 already shipped) and does NOT touch PR #948's Claude-Code initial-input path. The runtime gadget is engine-agnostic — it writes files on disk that any engine's file-read tool can consume. + +**Components delivered:** +- `readWorkItem(workItemId, includeComments)` — the gadget surface — gains a sibling `readWorkItemWithImagesOnDisk` (or modifies the existing function) that downloads + writes images via the shared `downloadAndPrepareImages` helper from Plan 1, then formats the text output's Pre-fetched Images section to list the actual file paths. +- A new image-writer helper that takes the `{ images, failures }` shape returned by `downloadAndPrepareImages` and writes each image to disk using the `work-item--img-.` naming convention. Extension is derived from the resolved MIME (`image/png` → `.png`); falls back to `.bin` with a warn log when MIME resolution failed. +- The same diagnostic log line from Plan 1, fired from the runtime gadget code path with `provider: ` and the same field schema. Reuses Plan 1's helper. +- Tests covering: extension-less Linear URL → on-disk file + path returned in text; mid-run change pickup (image added after agent boot, gadget re-fetch picks it up); failed-download case (text says download failed, no orphan path listed); regression for boot-path Codex / OpenCode / Claude Code engines that still go through their existing flows untouched. + +**Deferred to later plans in this spec:** +- Linear GraphQL fixture + extraction-coverage regression test (Plan 3). +- `src/integrations/README.md`'s Linear-specific GraphQL surface confirmation (Plan 3). + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #3 (runtime gadget delivers files-on-disk + paths in text) — **full** +- Spec AC #4 (mid-run image pickup) — **full** +- Spec AC #5 (single grep-stable diagnostic log line, runtime path) — **full** (combined with Plan 1's boot path) +- Spec AC #2 (Trello/JIRA regression-safe, runtime path) — **full** (regression tests) + +--- + +## Depends On + +- **Plan 1** (`1-boot-path-mime-fix-and-diagnostic-log.md`) — provides the shared `downloadAndPrepareImages` helper this plan imports, and the `mimeTypeFromUrl` + `isImageMimeType` widening that lets Linear extension-less URLs survive the filter at runtime as well as boot. Without Plan 1, the runtime gadget would face the exact same MIME-drop problem. + +--- + +## Detailed Task List (TDD) + +### 1. On-disk image writer for the runtime gadget + +**Tests first** (`tests/unit/gadgets/pm/core/writeRuntimeImages.test.ts` — new file): + +- `writeRuntimeImages — writes each image to .cascade/context/images/ with work-item--img-.` — unit — pass `{ workItemId: 'MNG-357', images: [{ base64Data, mimeType: 'image/png', altText }, { base64Data, mimeType: 'image/jpeg', altText }] }`; mock `fs.writeFile`; assert it was called twice with paths matching `work-item-MNG-357-img-0.png` and `work-item-MNG-357-img-1.jpg` (note: `image/jpeg` resolves to `.jpg` extension per a stable map). Expected red: module not found. +- `writeRuntimeImages — derives extension from resolved MIME, NOT from URL` — unit — pass image with `mimeType: 'image/webp'`; assert filename ends `.webp`. Expected red: module not found. +- `writeRuntimeImages — falls back to .bin extension when MIME resolution failed (image/*)` — unit — pass image with `mimeType: 'image/*'` (the unresolved wildcard sentinel); assert filename ends `.bin` AND a warn log is emitted including the workItemId. Expected red: module not found. +- `writeRuntimeImages — returns the list of relative paths it wrote` — unit — assert returned `string[]` matches `[`.cascade/context/images/work-item-MNG-357-img-0.png`, ...]`. Expected red: module not found. +- `writeRuntimeImages — creates the .cascade/context/images directory if it does not exist` — unit — mock `fs.access` to throw; assert `fs.mkdir` is called with `recursive: true` and the correct path. Expected red: module not found. +- `writeRuntimeImages — preserves the same naming convention as Plan 1's boot-path writer` — unit — assert that calling Plan 1's `writeInjectionImages` with equivalent inputs produces a path identical to this helper's output. (If they diverge, the runtime path and boot path have different on-disk contracts — bad. Both should produce `work-item--img-.`.) Expected red: ambiguous until Plan 1's writer is examined; the test should fail loudly if either side drifts. + +**Implementation** (`src/gadgets/pm/core/writeRuntimeImages.ts` — new file): +- Function signature: `writeRuntimeImages({ workItemId, images, contextDir? }): Promise<{ paths: string[]; failures: { reason: string }[] }>`. +- `contextDir` defaults to `.cascade/context/images` (the existing `IMAGES_SUBDIR` from `src/backends/shared/contextFiles.ts:23`). +- Stable extension map: `image/png → .png`, `image/jpeg → .jpg`, `image/gif → .gif`, `image/webp → .webp`, `image/svg+xml → .svg`, `image/avif → .avif`, etc. — use the inverse of the existing `EXTENSION_MIME_MAP` from `src/pm/media.ts:65`. +- For unresolved `image/*`: extension `.bin` + warn log. +- The function is engine-agnostic — it writes raw bytes; whatever engine reads them later just calls its file-read tool. + +### 2. Wire the writer into the runtime read-work-item gadget + +**Tests first** (`tests/unit/gadgets/pm/core/readWorkItem.test.ts` — extend existing or create new): + +- `readWorkItem — when work item has images, writes them to disk and returns text with relative paths` — unit — mock `readWorkItemWithMedia` to return `{ text: '...\n## Pre-fetched Images\n- [Image: foo.png] (description)\n', media: [{ url, mimeType: 'image/png', altText: 'foo.png', source: 'description' }] }`; mock `downloadAndPrepareImages` to succeed; mock `writeRuntimeImages` to return paths; assert returned text contains `.cascade/context/images/work-item--img-0.png` AND that the new path appears WHERE the existing "Pre-fetched Images" URL list was. Expected red: today the gadget returns text-only; the test asserts a substring that doesn't exist yet. +- `readWorkItem — when work item has no images, returns text unchanged (no Pre-fetched Images section, no disk write)` — unit — mock `readWorkItemWithMedia` to return `{ text: '...', media: [] }`; assert text is unchanged AND `writeRuntimeImages` is NOT called. Expected red: passes if the gadget today already gracefully skips the empty case (it does); fails right reason if implementation accidentally calls writer for empty media. +- `readWorkItem — emits the diagnostic log line at runtime path` — unit — assert exactly ONE INFO call matching `'[image-pipeline] work-item-fetch summary'` with `provider`, `workItemId`, `urlsDetected`, `urlsAfterFilter`, `urlsDownloaded`, `urlsFailed`, `urlsByMimeType`. Same prefix and shape as the boot-path log from Plan 1. Expected red: today no log fires from the runtime gadget; assertion fails because spy was never called. +- `readWorkItem — when download fails, the text marks the URL as failed and includes the reason` — unit — mock 2 images, 1 succeeds and 1 fails; assert returned text shows the successful path AND a failed-marker for the second URL (e.g. `- [Image: bar.png] download failed: `). Expected red: today the gadget has no failure handling. +- `readWorkItem — backward-compatible text shape: agents that don't read paths still see usable text` — unit — assert returned text still contains the existing `## Description`, `## Comments`, etc. sections; Pre-fetched Images section is the only one mutated. Expected red: today text shape is fixed; if Plan 2 accidentally drops a section, this fires. + +**Implementation** (`src/gadgets/pm/core/readWorkItem.ts`): +- Modify `readWorkItem(workItemId, includeComments)` to: + 1. Call `readWorkItemWithMedia` (already returns `{ text, media }`). + 2. If `media.length > 0`: call `downloadAndPrepareImages` (Plan 1's shared helper) to get `{ images, failures }`. + 3. If `images.length > 0`: call `writeRuntimeImages` to write each to disk, getting back `{ paths, failures: writerFailures }`. + 4. Mutate the text's "Pre-fetched Images" section: replace each URL-ref line with the corresponding local file path. For download failures, replace with a failed-marker line. For writer failures (e.g. disk full), append a separate `## Failed to Write Images` section listing the URLs. + 5. Emit the diagnostic log line with all counts. + 6. Return the mutated text. +- Preserve `readWorkItemWithMedia` as-is (boot path uses it; this plan doesn't touch boot path). +- The new helper `downloadAndPrepareImages` is imported from `src/pm/download-and-prepare.ts` (Plan 1's location). +- `writeRuntimeImages` is imported from `src/gadgets/pm/core/writeRuntimeImages.ts` (this plan, task 1). + +### 3. Mid-run image pickup integration test + +**Tests first** (`tests/integration/gadgets/runtime-image-delivery.test.ts` — new file): + +- `runtime gadget — when an image is added to the issue between two read-work-item calls, the second call delivers it on disk` — integration — first call: mock provider returns work item with 0 images; assert `readWorkItem` returns text without `.cascade/context/images/` paths. Second call: mock provider returns work item with 1 image; assert `readWorkItem` writes the image AND returns text with the local path. Expected red: today the runtime gadget writes nothing; second call returns same text-only shape. +- `runtime gadget — extension-less Linear URL flows end-to-end via the runtime path` — integration — mock provider returns `MediaReference` with `url: 'https://uploads.linear.app//file'` and `mimeType: 'image/*'`; mock `downloadMedia` to return `{ buffer, mimeType: 'image/png' }`; assert resulting on-disk file has `.png` extension AND the file content equals the buffer. Expected red: today the gadget doesn't write to disk at all. +- `runtime gadget — Trello PNG and JIRA attachment URLs flow through the runtime path (regression)` — integration — same as above but with extensioned URLs; assert filenames have correct extensions and writes succeed. Expected red: today the gadget doesn't write at all. + +**Implementation**: covered by tasks 1 + 2 above. This task is purely integration-level coverage. + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/gadgets/pm/core/writeRuntimeImages.test.ts` (new): 6 tests for the writer +- [ ] `tests/unit/gadgets/pm/core/readWorkItem.test.ts`: 5 new tests for the gadget surface (extend existing if present) + +### Integration tests +- [ ] `tests/integration/gadgets/runtime-image-delivery.test.ts` (new): 3 scenarios (mid-run pickup, extension-less Linear, Trello/JIRA regression) + +### Acceptance tests +- [ ] AC#3: runtime gadget integration test "extension-less Linear URL via runtime path lands on disk + path in text" +- [ ] AC#4: integration test "image added mid-run is picked up on re-read" +- [ ] AC#5 (runtime): unit test "diagnostic log line emitted from runtime gadget with same prefix and shape" +- [ ] AC#2 (runtime): regression test "Trello/JIRA images via runtime path still work" + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs auto-tested. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. The runtime gadget `readWorkItem(workItemId)` returns text whose "Pre-fetched Images" section lists actual relative file paths (e.g. `.cascade/context/images/work-item-MNG-357-img-0.png`) when images are present. +2. The files at those paths exist on disk after the gadget call returns; the bytes match what `downloadMedia` returned. +3. When an image is added to a work item between two runtime gadget calls, the second call delivers it on disk; the first call does not. +4. The diagnostic log line from Plan 1 (`[image-pipeline] work-item-fetch summary`) is emitted from the runtime gadget code path with the same field schema. +5. The disk file naming convention `work-item--img-.` is consistent between Plan 1's boot path and Plan 2's runtime path. A regression test pins this consistency. +6. When `mimeType` was unresolved (`image/*`), the file extension falls back to `.bin` and a warn log fires; the file is still written. +7. The text response is backward-compatible: agents that don't parse the new file paths see usable text with the existing `## Description`, `## Comments`, etc. sections preserved. +8. Failed downloads are marked in the text response (not silently dropped) AND counted in the diagnostic log's `urlsFailed` field. +9. All new/modified code has corresponding tests written before the implementation. +10. `npm run build` passes. +11. `npm test` passes. +12. `npm run test:integration` passes for the new integration tests. +13. `npm run lint` passes. +14. `npm run typecheck` passes. +15. All documentation listed in this plan's Documentation Impact has been updated. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `CHANGELOG.md` | Entry under the next release: "PM image delivery: the runtime `cascade-tools pm read-work-item` gadget now downloads work-item images and writes them to `.cascade/context/images/work-item--img-.`. The gadget's text response lists actual local file paths the agent can read with its file-read tool. Closes the mid-run image pickup gap (image added to a work item after agent boot is now delivered on the next gadget call)." | + +`src/integrations/README.md` is NOT updated by this plan — Plan 1 already established the "Image delivery contract" section; this plan's changes are consistent with that contract and don't require new documentation in the provider-onboarding guide. + +--- + +## Out of Scope (this plan) + +- The boot-path MIME fix and the shared `downloadAndPrepareImages` helper — already shipped in Plan 1. +- Linear GraphQL fixture + extraction-coverage regression test (Plan 3). +- Codex / OpenCode native multimodal SDK delivery — out of scope per spec. +- Magic-byte sniffing — out of scope per spec. +- Backfilling missed screenshots for prior runs — out of scope per spec. +- Image compression / resize / format conversion — out of scope per spec. +- Dashboard surface for "image not delivered" — out of scope per spec. + +--- + +## Progress + + +- [x] AC #1 (text contains real file paths) +- [x] AC #2 (files exist on disk + bytes match) +- [x] AC #3 (mid-run pickup) +- [x] AC #4 (diagnostic log line at runtime) +- [x] AC #5 (naming convention consistency boot/runtime) +- [x] AC #6 (.bin fallback + warn for unresolved MIME) +- [x] AC #7 (text shape backward-compatible) +- [x] AC #8 (failed downloads marked + counted) +- [x] AC #9 (TDD discipline) +- [x] AC #10 (build) +- [x] AC #11 (unit tests) +- [x] AC #12 (integration tests) +- [x] AC #13 (lint) +- [x] AC #14 (typecheck) +- [x] AC #15 (docs) diff --git a/docs/plans/016-pm-image-delivery-reliability/3-linear-fixture-and-extraction-coverage.md.done b/docs/plans/016-pm-image-delivery-reliability/3-linear-fixture-and-extraction-coverage.md.done new file mode 100644 index 00000000..50031ae7 --- /dev/null +++ b/docs/plans/016-pm-image-delivery-reliability/3-linear-fixture-and-extraction-coverage.md.done @@ -0,0 +1,163 @@ +--- +id: 016 +slug: pm-image-delivery-reliability +plan: 3 +plan_slug: linear-fixture-and-extraction-coverage +level: plan +parent_spec: docs/specs/016-pm-image-delivery-reliability.md +depends_on: [] +status: done +--- + +# 016/3: Linear payload fixture + extraction-coverage regression test + +> Part 3 of 3 in the 016-pm-image-delivery-reliability plan. See [parent spec](../../specs/016-pm-image-delivery-reliability.md). + +## Summary + +This plan ships the regression net for the spec's image-delivery contract: a captured Linear GraphQL `Issue` payload for an issue with at least one user-pasted screenshot, plus a unit test that asserts our extraction picks up every image in it. If Linear ever changes the payload shape in a way that loses inline images (renames the field, replaces markdown with a structured JSON tree, drops the upload host), the test fails loudly with a clear message. + +It also confirms the Linear GraphQL surface for inline images. The hypothesis from spec 016 is that `Issue.description` markdown is the canonical surface — `Issue.attachments` returns formal Attachment records (link previews, integration cards) and is NOT where pasted images live. This plan probes Linear's API to verify, captures the result in the fixture, and documents the conclusion in `src/integrations/README.md`. If Linear exposes a previously-unknown surface (e.g. an `attachments(includeInline: true)` filter, or a `descriptionData` rich-text JSON tree), this plan integrates that surface under Plan 1's shared resolution path. + +This plan has no code dependency on Plan 1 or Plan 2 in production code (it lives entirely in tests + docs), but it logically follows them because the regression test exercises the new contract Plan 1 + Plan 2 establish. If Plan 1 or Plan 2's contract changed before this plan ships, the regression test would need to be updated to match. + +**Components delivered:** +- `tests/fixtures/linear-issue-with-screenshot.json` — a captured Linear GraphQL `Issue` payload for a real test issue (or a faithfully reconstructed equivalent if we don't want to commit a real one) with at least one user-pasted screenshot in the description AND at least one in a comment. The fixture covers the common cases: extension-less `uploads.linear.app/` URL, extensioned URL with a filename, image embedded in markdown with alt text, image embedded with no alt text. +- A unit test that loads the fixture, runs the Linear adapter's extraction path on it, and asserts every image in the fixture is detected by `extractMarkdownImages` (or whatever extraction surface ends up canonical for Linear). Test fails with a specific message if a fixture image is missed. +- An optional Linear API probe (one-shot, manual or scripted) that captures the fixture from a real Linear issue. Not part of the test run; a tools script. +- `src/integrations/README.md` updates: a new subsection under "Image delivery contract" titled "Linear: GraphQL surface for inline images" that documents the conclusion of the investigation. Either: (a) confirms `Issue.description` markdown is canonical and points to the fixture; OR (b) describes the new GraphQL surface integrated under Plan 1's shared path. + +**Deferred to later plans in this spec:** +- None — this is the last plan. + +--- + +## Spec ACs satisfied by this plan + +- Spec AC #7 (Linear GraphQL fixture + regression test) — **full** + +--- + +## Depends On + +None in code. Logically follows Plan 1 + Plan 2 because the regression net is for the contract those plans establish, but the test itself only depends on `extractMarkdownImages` (the Linear adapter's existing surface) and the fixture. + +--- + +## Detailed Task List (TDD) + +### 1. Capture or construct the Linear fixture + +**Tests first** (no — this task is fixture authorship; the test in task 2 is the gating fail-loud). + +**Implementation** (`tests/fixtures/linear-issue-with-screenshot.json` — new file): + +Two modes for capturing: + +- **Mode A: capture from real Linear API.** Run a one-shot probe script (tooled in `tools/capture-linear-fixture.ts` or via a `cascade` admin command) that calls the Linear GraphQL `Issue` query for a real test issue with a pasted screenshot. Save the JSON response verbatim. The fixture should be sanitized to remove team/user identifying data but preserve URL shapes, markdown, and any structural fields Linear returns. +- **Mode B: faithfully reconstruct.** Build the fixture manually from Linear's documented GraphQL schema. Less authoritative but commits no real production data. + +**Recommendation: Mode A with sanitization.** A real-API capture is the only way to catch Linear's actual payload quirks (field ordering, optional fields present-but-null vs absent, URL hostname canonicalization, etc.). Sanitize team IDs, user emails, and any free-form text that might leak. Keep the URL hosts (`uploads.linear.app`), the markdown image syntax, and any structural fields Linear returns even if we don't read them today. + +Fixture must contain: +1. At least one extension-less `uploads.linear.app/` markdown image in the issue description. +2. At least one extensioned URL (e.g. `https://example.com/foo.png`) in the issue description. +3. At least one comment with a pasted screenshot. +4. The Linear `Issue.attachments` connection populated (with non-image-paste attachments — link previews, etc.) to confirm we DON'T mistake them for inline images. + +### 2. Extraction-coverage regression test + +**Tests first** (`tests/unit/pm/linear/extraction-coverage.test.ts` — new file): + +- `Linear extraction — picks up every inline image in the fixture issue description` — unit — load `tests/fixtures/linear-issue-with-screenshot.json`; pass `description` field through `extractMarkdownImages`; assert returned `MediaReference[]` contains every URL in the fixture's description that we expect to be picked up. Expected red: depends on whether Plan 1 has been merged. If Plan 1 is merged: passes (no implementation change needed for this plan). If Plan 1 is NOT yet merged in the dev branch this is being implemented against: this test fails with `expected length 2, got 0` because the extension-less Linear URL was filtered out — making this test a useful check against shipping Plan 3 ahead of Plan 1. +- `Linear extraction — picks up every inline image in fixture comments` — unit — same as above but for comments (each comment's body passed through `extractMarkdownImages`). Expected red: same as above. +- `Linear extraction — does NOT mistake Issue.attachments link previews for inline images` — unit — assert that the formal `Issue.attachments` records in the fixture (link previews, integration cards) are NOT included in the inline-image MediaReference list. Expected red: passes if the adapter correctly separates `attachments` from inline media (the existing code does); fails right reason if Plan 3 accidentally widens extraction to include them. +- `Linear extraction — fails LOUDLY if a fixture image is missed (regression net)` — unit — manually omit one URL from the expected list AND assert the test fails. (This is a meta-test confirming the test mechanism works; included once and then removed in cleanup.) Documents the failure-message format the spec AC#7 requires. + +**Implementation**: none — this is pure regression testing. If Plan 1 + Plan 2 have shipped correctly, the tests pass. If they haven't, the tests fail and the regression net catches it. + +### 3. `src/integrations/README.md` Linear-specific update + +**Tests first**: n/a (documentation). + +**Implementation**: After completing the Linear API probe (task 1, Mode A), append the conclusion to `src/integrations/README.md`'s "Image delivery contract" section (Plan 1 introduced this section). The new subsection: + +- **Title**: "Linear: GraphQL surface for inline images" +- **Body**: Document what Linear's API actually exposes for user-pasted screenshots. Confirm that `Issue.description` markdown is canonical (or describe the alternative if found). Cite the fixture path. Note that `Issue.attachments` is for formal Attachment records (link previews, integration cards) and should not be queried for inline images. +- **If a new surface was found** (e.g. `descriptionData`, an `attachments(includeInline: true)` filter): document the integration; otherwise, document the rule "use `extractMarkdownImages` over `Issue.description` and over each `Comment.body`." + +If task 1's Linear API probe finds a surprise, update Plan 1's `extractMarkdownImages` call site in `src/pm/linear/adapter.ts` to also probe the new surface — but this should be RARE; the likely outcome is "description markdown is canonical, no production code change." + +--- + +## Test Plan + +### Unit tests +- [ ] `tests/unit/pm/linear/extraction-coverage.test.ts` (new): 4 tests covering description extraction, comment extraction, attachment-record exclusion, and the meta-test for the regression net. + +### Integration tests +- [ ] None new — this plan is fixture + extraction unit tests + docs. + +### Acceptance tests +- [ ] AC#7: covered by the extraction-coverage tests + the fixture file. + +--- + +## Manual Verification (for `[manual]`-tagged ACs only) + +n/a — all ACs auto-tested. + +--- + +## Acceptance Criteria (per-plan, testable) + +1. `tests/fixtures/linear-issue-with-screenshot.json` exists and contains at least one extension-less `uploads.linear.app/` markdown image in the description, at least one extensioned URL, at least one comment with a pasted image, AND populated `Issue.attachments` records (link previews — NOT inline images). +2. The extraction-coverage regression test loads the fixture and asserts every inline image is detected; fails LOUDLY (with a specific message identifying the missing URL) if any are dropped. +3. The fixture is sanitized — no team IDs, user emails, or free-form leaky text. +4. `src/integrations/README.md` has a new "Linear: GraphQL surface for inline images" subsection under "Image delivery contract" documenting the investigation conclusion, referring to the fixture path. +5. If the Linear API probe found a previously-unknown inline-image surface, the Linear adapter's extraction path is widened to query it (still under Plan 1's shared resolution); otherwise, no production code changes. +6. All new/modified code has corresponding tests written before the implementation. +7. `npm run build` passes. +8. `npm test` passes. +9. `npm run lint` passes. +10. `npm run typecheck` passes. +11. All documentation listed in this plan's Documentation Impact has been updated. + +--- + +## Documentation Impact (this plan only) + +| File | Change | +|---|---| +| `CHANGELOG.md` | Entry under the next release: "PM image delivery: captured Linear GraphQL fixture (`tests/fixtures/linear-issue-with-screenshot.json`) plus regression test pinning our inline-image extraction. Fails loudly if Linear ever changes its issue payload shape in a way that loses inline images. Documents the canonical Linear GraphQL surface for inline images in the integrations README." | +| `src/integrations/README.md` | New subsection "Linear: GraphQL surface for inline images" under the "Image delivery contract" section (Plan 1 introduced this top-level section). Documents the investigation conclusion. | + +--- + +## Out of Scope (this plan) + +- The boot-path MIME fix and diagnostic log line (Plan 1). +- The runtime gadget image delivery (Plan 2). +- Trello/JIRA fixture captures for analogous regression coverage — they don't have the extension-less URL bug Linear has; deferred to a future spec if they ever exhibit similar drift. +- Codex / OpenCode native multimodal SDK delivery — out of scope per spec. +- Magic-byte sniffing — out of scope per spec. +- Backfilling missed screenshots for prior runs — out of scope per spec. +- Image compression / resize / format conversion — out of scope per spec. +- Dashboard surface for "image not delivered" — out of scope per spec. + +--- + +## Progress + + +- [x] AC #1 (fixture file exists with correct shape) +- [x] AC #2 (regression test fails loudly on missed images) +- [x] AC #3 (fixture sanitization) +- [x] AC #4 (README "Linear GraphQL surface" subsection) +- [x] AC #5 (new-surface integration if found, else no-op — confirmed no-op: description markdown is canonical) +- [x] AC #6 (TDD discipline) +- [x] AC #7 (build) +- [x] AC #8 (unit tests) +- [x] AC #9 (lint) +- [x] AC #10 (typecheck) +- [x] AC #11 (docs) diff --git a/docs/plans/016-pm-image-delivery-reliability/_coverage.md b/docs/plans/016-pm-image-delivery-reliability/_coverage.md new file mode 100644 index 00000000..0b426ee7 --- /dev/null +++ b/docs/plans/016-pm-image-delivery-reliability/_coverage.md @@ -0,0 +1,48 @@ +# Coverage map for spec 016-pm-image-delivery-reliability + +Auto-generated by /plan. Tracks which plans satisfy which spec ACs. + +## Spec ACs + +| # | Spec AC (short) | Satisfied by | Status | +|---|---|---|---| +| 1 | Linear screenshot on-disk for all engines (boot path) | plan 1 (boot-path-mime-fix-and-diagnostic-log) | full | +| 2 | Trello/JIRA images regression-safe | plan 1 (boot regression) + plan 2 (runtime regression) | full | +| 3 | Runtime gadget delivers files-on-disk + paths in text | plan 2 (runtime-gadget-image-delivery) | full | +| 4 | Mid-run image pickup | plan 2 | full | +| 5 | Single grep-stable diagnostic log line per fetch | plan 1 (boot path) + plan 2 (runtime path, same shape) | partial chain | +| 6 | PR #948 Claude-Code path untouched | plan 1 (regression pin) | full | +| 7 | Linear GraphQL fixture + extraction-coverage test | plan 3 (linear-fixture-and-extraction-coverage) | full | +| 8 | New-provider invariant preserved | plan 1 (no provider-specific code in shared resolution) | full | +| 9 | MNG-357 end-to-end reproduces clean | plan 1 (root-cause fix) | full | + +## Coverage summary + +- **9 spec ACs** mapped to **3 plans** +- **8 plans-x-AC pairs full coverage** (each AC fully satisfied by its assigned plan(s)) +- **1 spec AC** with partial-chain coverage (AC #5 — boot path emits the diagnostic line in plan 1; runtime path emits the same shape in plan 2; AC is fully covered only after both ship) + +## Plan dependency graph + +``` +1-boot-path-mime-fix-and-diagnostic-log ──→ 2-runtime-gadget-image-delivery + └──→ 3-linear-fixture-and-extraction-coverage +``` + +Plan 1 ships the safety-net (MIME drop fix + shared `downloadAndPrepareImages` helper + diagnostic log line). It alone fixes MNG-357. + +Plan 2 imports Plan 1's shared helper and closes the runtime mid-run gap. It alone is not enough to fix MNG-357 (which is a boot-path failure), but it is the second half of AC#5 and provides AC#3 + AC#4 entirely. + +Plan 3 is the regression net for the new contract Plans 1+2 establish. No code dependency on Plans 1+2; the fixture and extraction-coverage test would still pass against today's `extractMarkdownImages` because the Linear regex matches on URL alone, not on MIME. But shipping it before Plans 1+2 would mean the regression net is for an old, broken contract — so logical order is 1 → 2 → 3. + +## Documentation impact distribution + +| Spec doc | Plan | What gets added | +|---|---|---| +| `CHANGELOG.md` | Plan 1 | "PM image delivery: extension-less URL fix + diagnostic log" | +| `CHANGELOG.md` | Plan 2 | "PM image delivery: runtime gadget mid-run delivery" | +| `CHANGELOG.md` | Plan 3 | "PM image delivery: Linear fixture + extraction regression" | +| `src/integrations/README.md` | Plan 1 | New "Image delivery contract" section (general — applies to all providers) | +| `src/integrations/README.md` | Plan 3 | New "Linear: GraphQL surface for inline images" subsection (Linear-specific findings) | + +CLAUDE.md is NOT updated by this spec — already covered by spec 015's broader "silent-failure → single-line diagnostic" pattern, and the diagnostic log line introduced here is a concrete instance of that pattern, not a new cross-cutting rule. diff --git a/docs/specs/016-pm-image-delivery-reliability.md.done b/docs/specs/016-pm-image-delivery-reliability.md.done new file mode 100644 index 00000000..7de01f0a --- /dev/null +++ b/docs/specs/016-pm-image-delivery-reliability.md.done @@ -0,0 +1,129 @@ +--- +id: 016 +slug: pm-image-delivery-reliability +level: spec +title: PM image delivery reliability +created: 2026-04-26 +status: done +--- + +# 016: PM image delivery reliability + +## Problem & Motivation + +CASCADE's image-injection pipeline silently drops user-pasted screenshots from Linear work items, leaving agent workers running with empty `.cascade/context/images/` directories. The agent then proceeds without the visual context the user provided, often reporting back to the user that "the image asset was not present in this workspace" or guessing at the bug being reported. For a product whose value proposition is "you describe what's broken, the agent fixes it," a silently-missing screenshot is a credibility-class failure. + +The bug surfaced in production on 2026-04-26 against Linear card MNG-357 (ucho project). A user attached a screenshot to the card. The planning agent ran on the Codex engine, observed the image was missing, and re-wrote the issue description telling the next agent in the pipeline to "translate the screenshot into a concrete route, viewport, and failing assertion before editing code" — bypassing the user-attached visual context entirely. The cascade run log shows `hasOffloadedContext: false` at agent startup; the agent's first tool call was a directory probe (`find .cascade/context/images -maxdepth 2 -type f`) that returned empty. + +The root cause is a stack of three independent gaps. First, Linear's user-pasted-image URLs are extension-less (`https://uploads.linear.app//`); the URL-based MIME-type heuristic returns `application/octet-stream` for them, and the pre-download image-only filter drops them as non-images. Second, the runtime gadget that lets agents fetch a work item mid-run is text-only — it discards the media references the underlying provider returns, so even if the boot-path delivered images, an agent re-reading the work item finds nothing. Third, the existing instrumentation only logs *post*-download outcomes ("downloaded N of M") — the upstream extract/filter step that drops Linear screenshots is invisible in run logs, making this exact incident class essentially undiagnosable without source diving. + +This spec closes all three gaps. It mirrors the lesson from spec 015 (router job dispatch failure recovery): a silent failure mode in the agent platform must be replaced with a single, grep-stable log line that makes the failure observable, and the contract that produced the failure must be hardened so the failure can't recur. + +--- + +## Goals + +1. A user pastes a screenshot into a Linear work item. Any agent type — planning, implementation, review, backlog-manager — running for that work item sees the image as a readable file in its workspace, regardless of which engine (Codex, OpenCode, Claude Code) executes the agent. +2. An agent that re-reads a work item mid-run via the runtime read-work-item gadget receives the same image-on-disk treatment, so a teammate adding a screenshot after the agent has started can still be picked up. +3. Every work-item fetch emits a single structured log line that summarizes detected URLs, post-filter URLs, downloads attempted, downloads successful, and downloads failed — enough to diagnose any future "no image delivered" report by grepping one line in the run log. +4. The fix generalizes beyond Linear: Trello and JIRA flow through the same MIME-resolution path and gain the same Content-Type-first authority and runtime-gadget delivery — without any provider-specific URL hostname matching. +5. Existing healthy paths continue to behave identically: Trello and JIRA images that *did* work before still work; PR #948's Claude-Code initial-input ImageBlockParam delivery is untouched; the existing MediaReference shape consumed by downstream code remains compatible. + +--- + +## Non-goals + +- Migrating Codex / OpenCode to native multimodal SDK delivery (analogous to PR #948 for Claude Code) — separate spec when the SDKs offer the surface. +- Backfilling missed screenshots for prior agent runs. Agents that already finished without their image stay finished; no retroactive re-dispatch. +- Trello/JIRA-specific MIME detection improvements beyond what the shared resolution path already covers. +- A dashboard UI surfacing "image was not delivered to this run" — operational, separate effort. +- Magic-byte sniffing as a third MIME-resolution tier. The auth'd PM endpoints (Linear, Trello, JIRA) are trusted to return correct Content-Type on the response; adding a sniffer adds a dep with no observable benefit. +- Compressing, resizing, format-converting images on the fly. The worker reads what was uploaded, byte-for-byte. + +--- + +## Constraints + +- The fix must not change the wire shape of `MediaReference` consumed by downstream code. Adding a sentinel value to its existing `mimeType` field is acceptable; renaming or removing the field is not. +- The fix must not introduce a new pre-download HTTP round-trip per image. Resolving MIME via the existing GET response is acceptable; a separate HEAD request before the GET is not. +- The diagnostic log line must be observable through the standard `cascade runs logs` and Loki paths used today. No new log sink, no new dashboard. +- The runtime gadget's behavioral change must be backward-compatible with agents that simply read the gadget's text output. Agents that ignore the new file paths still get a usable text response. +- File-on-disk delivery must work for engines that do *not* speak multimodal SDK content blocks (Codex today; possibly others tomorrow). The disk-write path is the lowest-common-denominator delivery contract and stays the canonical one. +- The on-disk file naming must encode the work item identifier so an agent that traverses `.cascade/context/images/` can correlate files to the work item without inspecting metadata. + +--- + +## User stories / Requirements + +1. **As a CASCADE user**, when I paste a screenshot into a Linear issue and move it to a triggering state, the agent that runs has my screenshot available as a file it can read. I never have to describe the screenshot in words to compensate. +2. **As a CASCADE user**, when I add a screenshot to a Linear issue *after* the agent has already started running, the agent can pick it up by re-reading the work item mid-run. +3. **As an operator diagnosing a "no image delivered" report**, I can read the agent's run log, find a single line that tells me how many images were detected on the work item, how many survived filtering, how many were downloaded, and how many failed — and I can correlate failures to URLs without reading source. +4. **As a maintainer adding a new PM provider**, the image-delivery pipeline works for my provider with no provider-specific MIME-detection branching. Whatever URLs my adapter exposes flow through the same resolution path that Linear, Trello, and JIRA use. +5. **As a CASCADE engineering team**, we have a captured fixture of the Linear GraphQL `Issue` payload for a card with a pasted screenshot, so future changes to Linear's API surface are detectable as a regression. + +--- + +## Research Notes + +- HTTP/1.1 RFC 7231 §3.1.1.5 specifies that the `Content-Type` header is authoritative for the media type of the returned representation. URL-extension-based inference is a heuristic, not a contract. ([RFC 7231](https://www.rfc-editor.org/rfc/rfc7231#section-3.1.1.5)) +- WHATWG MIME Sniffing Standard codifies authority order: `Content-Type` header first, resource metadata second, structural sniffing third. Our case never needs the third tier because PM hosts are trusted. ([mimesniff.spec.whatwg.org](https://mimesniff.spec.whatwg.org/)) +- The "trust file extensions, fail closed on unknown" pattern is well-known to drop legitimate content from extension-less URLs. CDN platforms (Cloudflare, S3-with-CloudFront) document this exact failure mode in their content-handling guides. +- BullMQ's failed-event compensation pattern from spec 015 — "every silent failure mode must surface a single grep-stable diagnostic log line" — generalizes here. Same product credibility argument: agents that silently miss user context look broken even when every line of code is technically correct. +- Linear's developer documentation describes `Issue.description` as a Markdown field; user-pasted images are stored as standard Markdown image syntax pointing at `uploads.linear.app/` URLs. The `Issue.attachments` GraphQL connection serves a different purpose (link previews, integration cards) and is not where pasted images live. + +--- + +## Open Source Decisions + +| Tool | Solves | Decision | Reason | +|------|--------|----------|--------| +| Node `fetch` built-in `Content-Type` parsing | Authoritative MIME from response header | **Use** | Already in the worker runtime; zero new dep. | +| [`file-type`](https://www.npmjs.com/package/file-type) (magic-byte sniffer) | Tier-3 MIME fallback when extension AND Content-Type both fail | **Skip** | Trusted PM hosts return correct Content-Type. Adds dep weight with no observable benefit for our use case; revisit only if a provider proves untrustworthy. | +| Linear GraphQL fixture capture | Regression detection if Linear's payload shape ever changes | **Use** (one-shot capture, not a runtime dep) | Standard testing pattern; lives in test fixtures. | + +--- + +## Strategic decisions + +1. **MIME authority order: Content-Type-first, URL-extension-second, no magic-byte sniffing.** Pre-download URL-extension inference becomes a hint, not a verdict. The download response's `Content-Type` resolves the actual MIME. Magic-byte sniffing is a deliberate non-goal — trusted PM hosts return correct Content-Type, the dep cost isn't justified. +2. **Pre-download MIME representation: `image/*` wildcard sentinel.** When the URL-extension heuristic returns `application/octet-stream` AND the URL came from a path that PM providers commonly produce extension-less (Linear's `uploads.linear.app/`), the `MediaReference.mimeType` is set to `image/*`. The image-only filter is extended to accept the wildcard. The wildcard never reaches disk — it's resolved to a concrete MIME at download time. This is smaller-surface and MIME-spec-valid (`image/*` is a legal Accept-range MIME) compared to introducing a sibling `mimeTypeIsHint` field. +3. **Runtime read-work-item gadget delivers files on disk, not base64-inline.** The agent's file-read tool consumes paths; base64 in text doesn't fit. The boot path and the runtime path produce identical artifacts: files under `.cascade/context/images/` named to encode the work item identifier. +4. **Disk file naming: `work-item--img-.`.** Extension is derived from the *resolved* MIME (image/png → `.png`). When MIME resolution fails entirely (download succeeded but Content-Type was missing/unparseable), use `.bin` and emit a warn log — never silently degrade. +5. **Linear payload investigation always ships the fixture and the coverage test, regardless of what Linear's API exposes.** If we find no surprise (likely), document the description-markdown contract in the integrations README. If we find a new surface (e.g. an `attachments(includeInline: true)` argument), integrate it under the same shared resolution path. Either way, the fixture is the regression net for future Linear API drift. +6. **Diagnostic log line is required for AC sign-off, not optional.** This spec includes a structured one-liner at extraction time, mirroring the wedged-lock canary from spec 015. Without it, the next "no image delivered" incident will be just as opaque as MNG-357 was. + +--- + +## Acceptance Criteria (outcome-level) + +1. A user attaches a screenshot to a Linear issue (description paste or comment paste). Triggering an agent run for that issue results in the agent finding the screenshot as a readable file in its workspace, regardless of engine — Codex, OpenCode, or Claude Code. No engine-specific re-implementation; the disk-write path remains the canonical lowest-common-denominator. +2. The same flow works for Trello and JIRA work items that contain images (regression-safe — these worked before; they continue to work). +3. An agent that calls the runtime read-work-item gadget mid-run for a work item that has images receives a text response whose pre-fetched-images section lists actual local file paths the agent can read with its file-read tool. The files exist on disk at the listed paths. +4. A teammate adds an image to a work item *after* an agent has started running. When the agent re-reads the work item via the runtime gadget, the new image is downloaded and made available on disk in the same way as boot-time images. +5. Every work-item fetch (boot or runtime) emits a single structured log line summarizing: provider, work-item identifier, URLs detected, URLs that survived filtering, URLs successfully downloaded, URLs that failed. The format is grep-stable: it appears verbatim in the cascade run log surface and in production log aggregation. An operator can filter for it with one expression and triage any "no image delivered" report from the resulting line alone. +6. PR #948's Claude-Code initial-input ImageBlockParam path is untouched and still works for Claude-Code engines. Tests that pinned that behavior continue to pass without modification. +7. A captured fixture file exists for a Linear `Issue` GraphQL payload containing a user-pasted screenshot, plus a regression test that asserts our extraction picks up every image in it. The test fails loudly if Linear changes its payload shape in a way that loses inline images. +8. A new PM provider added later (e.g. Asana, GitLab) inherits the working pipeline by writing only its adapter — no new code in the shared MIME-resolution, download-and-write, or runtime-gadget surfaces. The single-entrypoint invariant from spec 009 is preserved. +9. The end-to-end MNG-357 scenario reproduces clean: a fresh Linear issue with a pasted screenshot, a planning agent run on Codex, and `.cascade/context/images/work-item--img-0.` exists in the worker container; the cascade run log shows `hasOffloadedContext: true` and the new diagnostic line shows non-zero downloads. + +--- + +## Documentation Impact (high-level) + +- `CHANGELOG.md` — entry under the next release per shipping plan: one for the boot-path MIME fix, one for the runtime-gadget mid-run delivery, one for the Linear-payload investigation outcome. +- `src/integrations/README.md` — the canonical adding-a-new-PM-provider doc gains a section on image delivery: the contract a provider must satisfy (extract URLs from descriptions and comments; expose `getAttachments` only for non-inline attachment-style media; trust the shared MIME resolution path; do nothing extra for image delivery), and the diagnostic log line operators rely on. Also: confirmed scope of Linear's GraphQL surface for inline images, captured fixture path, and what to do if a provider's host serves untrustworthy `Content-Type` headers. + +CLAUDE.md is not updated by this spec. The diagnostic log line is observable, but the *invariant* is provider-adapter-level (a property of `src/integrations/README.md`'s contract). The dispatch-failure semantics from spec 015 already established the broader "silent-failure → single-line diagnostic" pattern in CLAUDE.md; this spec is a concrete instance of that pattern, not a new cross-cutting rule. + +--- + +## Out of Scope + +- Codex / OpenCode native multimodal SDK delivery analogous to PR #948 — separate future spec. +- Magic-byte sniffing fallback for MIME resolution. +- Backfilling already-finished agent runs that missed images. +- Image compression / resize / format conversion on the worker side. +- A dashboard surface for "image was not delivered to this run" notifications. +- Trello/JIRA-specific URL-detection improvements beyond what the shared Content-Type-first resolution naturally covers. +- Replacing the `cascade runs logs` log surface with a structured event stream. +- Cross-router-instance lock coordination for image downloads (not a real concern — each worker container has its own filesystem). diff --git a/src/agents/definitions/contextSteps.ts b/src/agents/definitions/contextSteps.ts index d47b0a43..069e276d 100644 --- a/src/agents/definitions/contextSteps.ts +++ b/src/agents/definitions/contextSteps.ts @@ -18,7 +18,7 @@ import { } from '../../gadgets/todo/storage.js'; import { githubClient } from '../../github/client.js'; import { getJiraConfig, getLinearConfig, getTrelloConfig } from '../../pm/config.js'; -import { getPMProviderOrNull, MAX_IMAGES_PER_WORK_ITEM } from '../../pm/index.js'; +import { getPMProviderOrNull } from '../../pm/index.js'; import { getSentryClient } from '../../sentry/client.js'; import type { AgentInput, ProjectConfig } from '../../types/index.js'; import { parseRepoFullName } from '../../utils/repo.js'; @@ -85,7 +85,11 @@ export function fetchContextFilesStep(params: FetchContextParams): ContextInject export async function fetchWorkItemStep(params: FetchContextParams): Promise { if (!params.input.workItemId) return []; try { - const { text: cardData, media } = await readWorkItemWithMedia(params.input.workItemId, true); + const { + text: cardData, + media, + urlsDetected, + } = await readWorkItemWithMedia(params.input.workItemId, true); const injection: ContextInjection = { toolName: 'ReadWorkItem', @@ -94,62 +98,37 @@ export async function fetchWorkItemStep(params: FetchContextParams): Promise 0) { - const provider = getPMProviderOrNull(); - const limited = media.slice(0, MAX_IMAGES_PER_WORK_ITEM); + // Spec 016/1: defer the actual download + base64 prep to the shared + // `downloadAndPrepareImages` helper so the runtime gadget (spec 016/2) + // uses the same code path. + const { downloadAndPrepareImages } = await import('../../pm/download-and-prepare.js'); + const { images, failures } = await downloadAndPrepareImages( + params.input.workItemId, + media, + params.logWriter, + ); - params.logWriter('INFO', 'fetchWorkItemStep: downloading work item images', { - workItemId: params.input.workItemId, - count: limited.length, - }); + // Spec 016/1 AC#5: single grep-stable diagnostic log line summarising + // the entire boot-path image pipeline outcome. Operators triage any + // "no image delivered" report by grepping for `[image-pipeline] + // work-item-fetch summary`. + const provider = getPMProviderOrNull(); + const urlsByMimeType: Record = {}; + for (const ref of media) { + urlsByMimeType[ref.mimeType] = (urlsByMimeType[ref.mimeType] ?? 0) + 1; + } + params.logWriter('INFO', '[image-pipeline] work-item-fetch summary', { + provider: provider?.type ?? 'unknown', + workItemId: params.input.workItemId, + urlsDetected, + urlsAfterFilter: media.length, + urlsDownloaded: images.length, + urlsFailed: failures.length, + urlsByMimeType, + }); - const { jiraClient } = await import('../../jira/client.js'); - const { trelloClient } = await import('../../trello/client.js'); - const { linearClient } = await import('../../linear/client.js'); - - const results = await Promise.all( - limited.map(async (ref) => { - try { - let downloaded: { buffer: Buffer; mimeType: string } | null = null; - if (provider?.type === 'jira') { - downloaded = await jiraClient.downloadAttachment(ref.url); - } else if (provider?.type === 'linear') { - downloaded = await linearClient.downloadAttachment(ref.url); - } else { - downloaded = await trelloClient.downloadAttachment(ref.url); - } - if (!downloaded) { - params.logWriter('WARN', 'fetchWorkItemStep: image download returned null', { - url: ref.url.split('?')[0], - }); - return null; - } - return { - base64Data: downloaded.buffer.toString('base64'), - mimeType: downloaded.mimeType, - altText: ref.altText, - }; - } catch (err) { - params.logWriter('WARN', 'fetchWorkItemStep: failed to download image', { - url: ref.url.split('?')[0], - error: err instanceof Error ? err.message : String(err), - }); - return null; - } - }), - ); - - const images = results.filter((r) => r !== null); - params.logWriter('INFO', 'fetchWorkItemStep: image download complete', { - workItemId: params.input.workItemId, - attempted: limited.length, - downloaded: images.length, - skipped: limited.length - images.length, - }); - if (images.length > 0) { - injection.images = images; - } + if (images.length > 0) { + injection.images = images; } return [injection]; diff --git a/src/gadgets/pm/core/readWorkItem.ts b/src/gadgets/pm/core/readWorkItem.ts index 674198da..fcaf1656 100644 --- a/src/gadgets/pm/core/readWorkItem.ts +++ b/src/gadgets/pm/core/readWorkItem.ts @@ -1,5 +1,6 @@ import type { Attachment, MediaReference } from '../../../pm/index.js'; -import { filterImageMedia, getPMProvider } from '../../../pm/index.js'; +import { filterImageMedia, getPMProvider, getPMProviderOrNull } from '../../../pm/index.js'; +import { logger } from '../../../utils/logging.js'; interface Label { name: string; @@ -33,6 +34,8 @@ export interface WorkItemWithMedia { text: string; /** All image media references discovered in the work item description, card attachments, and comments (deduplicated by URL) */ media: MediaReference[]; + /** The total number of media and attachment references found before MIME-type filtering */ + urlsDetected: number; } function formatLabels(labels: Label[]): string { @@ -117,11 +120,15 @@ export async function readWorkItemWithMedia( // Collect all image media references const allMedia: MediaReference[] = []; + let urlsDetected = 0; + if (item.inlineMedia && item.inlineMedia.length > 0) { + urlsDetected += item.inlineMedia.length; allMedia.push(...filterImageMedia(item.inlineMedia)); } // Add image-type card attachments as media references + urlsDetected += attachments.length; allMedia.push( ...filterImageMedia( attachments.map((att) => ({ @@ -142,6 +149,7 @@ export async function readWorkItemWithMedia( const comments = await provider.getWorkItemComments(workItemId); for (const comment of comments) { if (comment.inlineMedia && comment.inlineMedia.length > 0) { + urlsDetected += comment.inlineMedia.length; allMedia.push(...filterImageMedia(comment.inlineMedia)); } } @@ -161,13 +169,95 @@ export async function readWorkItemWithMedia( // Append pre-fetched images section listing discovered images text += formatPreFetchedImages(dedupedMedia); - return { text, media: dedupedMedia }; + return { text, media: dedupedMedia, urlsDetected }; +} + +/** + * Format the on-disk paths for a successfully-written batch of runtime + * images. Replaces the existing "Pre-fetched Images" URL list with a + * "Local Image Files" section that the agent can hand to its file-read + * tool. Failed downloads (if any) are surfaced inline so the agent + * doesn't silently miss missing context. + */ +function formatRuntimeImagePaths( + paths: string[], + failures: { url: string; reason: string }[], +): string { + if (paths.length === 0 && failures.length === 0) return ''; + const lines: string[] = ['## Local Image Files', '']; + for (const path of paths) { + lines.push(`- ${path}`); + } + if (failures.length > 0) { + lines.push('', '### Failed Image Downloads'); + for (const f of failures) { + lines.push(`- ${f.url} — ${f.reason}`); + } + } + lines.push(''); + return `${lines.join('\n')}\n`; } +/** + * Spec 016/2: runtime gadget downloads + writes images to disk so the + * agent can read them mid-run. Returns text whose pre-fetched-images + * section now lists local file paths the agent can hand to its + * file-read tool, not just URL refs. + * + * Each fetch emits the AC#5 grep-stable diagnostic line — same format + * as the boot-path emission in `fetchWorkItemStep`. + */ export async function readWorkItem(workItemId: string, includeComments = true): Promise { try { - const { text } = await readWorkItemWithMedia(workItemId, includeComments); - return text; + const { text, media, urlsDetected } = await readWorkItemWithMedia(workItemId, includeComments); + + // Spec 016/2: download + write any image media so agent can Read them. + const { downloadAndPrepareImages } = await import('../../../pm/download-and-prepare.js'); + const { writeRuntimeImages } = await import('./writeRuntimeImages.js'); + + const provider = getPMProviderOrNull(); + const logWriter = ( + level: 'INFO' | 'WARN' | 'ERROR', + message: string, + meta?: Record, + ) => { + logger[level.toLowerCase() as 'info' | 'warn' | 'error'](message, meta); + }; + + const { images, failures } = await downloadAndPrepareImages(workItemId, media, logWriter); + + let writePaths: string[] = []; + let writeFailures: { path: string; reason: string }[] = []; + if (images.length > 0) { + const writeResult = await writeRuntimeImages({ workItemId, images }); + writePaths = writeResult.paths; + writeFailures = writeResult.failures; + } + // AC#5 diagnostic line — same prefix as the boot-path emission. + const urlsByMimeType: Record = {}; + for (const ref of media) { + urlsByMimeType[ref.mimeType] = (urlsByMimeType[ref.mimeType] ?? 0) + 1; + } + logger.info('[image-pipeline] work-item-fetch summary', { + provider: provider?.type ?? 'unknown', + workItemId, + urlsDetected, + urlsAfterFilter: media.length, + urlsDownloaded: images.length, + urlsFailed: failures.length + writeFailures.length, + urlsByMimeType, + }); + + // Append the local file paths section so agents can Read them. + const downloadFailures: { url: string; reason: string }[] = failures.map((f) => ({ + url: f.url, + reason: f.reason, + })); + for (const f of writeFailures) { + downloadFailures.push({ url: f.path, reason: `Local write failed: ${f.reason}` }); + } + const augmented = text + formatRuntimeImagePaths(writePaths, downloadFailures); + return augmented; } catch (error) { const message = error instanceof Error ? error.message : String(error); return `Error reading work item: ${message}`; diff --git a/src/gadgets/pm/core/writeRuntimeImages.ts b/src/gadgets/pm/core/writeRuntimeImages.ts new file mode 100644 index 00000000..64c75911 --- /dev/null +++ b/src/gadgets/pm/core/writeRuntimeImages.ts @@ -0,0 +1,129 @@ +/** + * Write runtime work-item images to `.cascade/context/images/` so the + * agent can read them with its file-read tool. + * + * Spec 016/2: this is the runtime sibling of the boot-path writer + * (`writeInjectionImages` in `src/backends/shared/contextFiles.ts`). + * Both produce the same on-disk filename convention: + * `.cascade/context/images/work-item--img-.` + * + * Extension is derived from the resolved MIME type. When MIME resolution + * failed (the `image/*` wildcard sentinel from spec 016/1 was never resolved + * because download response Content-Type was missing), the extension falls + * back to `.bin` and a warn log fires — never silently degrade. + */ + +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import type { ContextImage } from '../../../agents/contracts/index.js'; +import { logger } from '../../../utils/logging.js'; + +/** Default location where runtime images are written, relative to the repo root. */ +export const DEFAULT_CONTEXT_IMAGES_RELATIVE = '.cascade/context/images'; + +/** + * Map MIME types to file extensions. + * Mirrors the Plan 1 boot-path convention (image/jpeg → .jpg) so the boot- + * path and runtime-path produce identical artifacts. + */ +const MIME_TO_EXTENSION: Record = { + 'image/png': 'png', + 'image/jpeg': 'jpg', + 'image/jpg': 'jpg', + 'image/gif': 'gif', + 'image/webp': 'webp', + 'image/svg+xml': 'svg', + 'image/avif': 'avif', + 'image/apng': 'apng', + 'image/bmp': 'bmp', + 'image/tiff': 'tiff', + 'image/x-icon': 'ico', +}; + +/** + * Resolve a file extension for the given MIME type. Returns `bin` for the + * unresolved wildcard sentinel `image/*` AND for any unknown MIME, with a + * caller-provided warn log for the wildcard case. + */ +function resolveExtension(mimeType: string, workItemId: string): string { + const normalized = mimeType.toLowerCase().trim(); + const ext = MIME_TO_EXTENSION[normalized]; + if (ext) return ext; + if (normalized === 'image/*') { + logger.warn('writeRuntimeImages: unresolved MIME — falling back to .bin extension', { + workItemId, + mimeType, + }); + return 'bin'; + } + logger.warn('writeRuntimeImages: unknown MIME — falling back to .bin extension', { + workItemId, + mimeType, + }); + return 'bin'; +} + +export interface WriteRuntimeImagesArgs { + workItemId: string; + images: ContextImage[]; + /** Optional repo root; defaults to the current working directory. */ + repoDir?: string; +} + +export interface WriteRuntimeImagesResult { + /** Repo-relative paths of successfully-written image files. */ + paths: string[]; + /** Per-image write failures (if any). */ + failures: { path: string; reason: string }[]; +} + +/** + * Write each {@link ContextImage} to `.cascade/context/images/` with the + * stable naming convention `work-item--img-.`. Idempotent + * — running twice with the same workItemId overwrites the prior files + * (caller is responsible for re-running if it wants fresh bytes). + */ +export async function writeRuntimeImages( + args: WriteRuntimeImagesArgs, +): Promise { + const { workItemId, images, repoDir } = args; + if (images.length === 0) return { paths: [], failures: [] }; + + const baseDir = repoDir + ? join(repoDir, DEFAULT_CONTEXT_IMAGES_RELATIVE) + : DEFAULT_CONTEXT_IMAGES_RELATIVE; + + // Always mkdir -p; cheap, idempotent. + await mkdir(baseDir, { recursive: true }); + + const paths: string[] = []; + const failures: { path: string; reason: string }[] = []; + + for (let i = 0; i < images.length; i++) { + const img = images[i]; + const ext = resolveExtension(img.mimeType, workItemId); + const filename = `work-item-${workItemId}-img-${i}.${ext}`; + const absolutePath = join(baseDir, filename); + // Repo-relative path is what we return to the caller for inclusion in + // the agent's text response — the agent's Read tool consumes paths + // relative to its workspace root. + const relativePath = `${DEFAULT_CONTEXT_IMAGES_RELATIVE}/${filename}`; + + try { + const buffer = Buffer.from(img.base64Data, 'base64'); + await writeFile(absolutePath, buffer); + paths.push(relativePath); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + logger.warn('writeRuntimeImages: failed to write image', { + workItemId, + index: i, + path: relativePath, + reason, + }); + failures.push({ path: relativePath, reason }); + } + } + + return { paths, failures }; +} diff --git a/src/integrations/README.md b/src/integrations/README.md index 4514a825..b037bf37 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -241,3 +241,57 @@ Different PM providers have different native concepts of "checklist". The `PMPro **Why inline markdown for Linear and JIRA?** Both providers support markdown checkboxes natively in their description editors but lack a dedicated lightweight checklist primitive — sub-issues and subtasks are full work items, which clutters boards when used for things like acceptance criteria or implementation steps. Inline markdown matches Trello's lightweight semantics without creating orphan issues. See [spec 008](../../docs/specs/008-inline-checklists.md) for full rationale. The shared engine that parses, appends, toggles, and removes inline checklist items lives at `src/pm/_shared/inline-checklist.ts` and is consumed by both the Linear and JIRA adapters. + +--- + +## Image delivery contract + +Spec 016 hardened the work-item-image pipeline so user-pasted screenshots (Linear especially, but the rules generalize) reliably reach the agent worker as files on disk. New PM providers should follow this contract; do nothing extra and image delivery just works. + +### How the shared resolution path works + +1. **Extract URL refs** from the work-item description and each comment via `extractMarkdownImages()` (`src/pm/media.ts`). A `MediaReference` is produced for every `![alt](url)` match. The provider does NOT need its own extraction logic. +2. **Pre-download MIME inference** (a hint, not a verdict): `mimeTypeFromUrl()` derives a MIME from the URL pathname's extension. For URLs whose hostname is in `IMAGE_HOST_ALLOWLIST` (currently `uploads.linear.app`) AND whose pathname has no recognised extension, the inference returns `'image/*'` — a wildcard sentinel that survives the image-only filter. Add a host to the allowlist only if its `Content-Type` headers are reliable. +3. **Filter** via `filterImageMedia()` — drops anything that isn't an image MIME or the `image/*` wildcard. +4. **Download** via `downloadAndPrepareImages()` (`src/pm/download-and-prepare.ts`) — the shared per-provider dispatch loop. The download response's `Content-Type` header is the AUTHORITATIVE MIME — it resolves the wildcard and overrides any URL-extension-derived guess. +5. **Write to disk** at `.cascade/context/images/work-item--img-.` — extension is derived from the resolved MIME. + +### What providers should NOT do + +- Don't write your own MIME-detection logic. The shared resolution path covers all known PM provider URL shapes. +- Don't download images yourself in your adapter — let `downloadAndPrepareImages` do it. +- Don't surface `getAttachments()` for inline-pasted images. That method is for formal Attachment records (Slack/GitHub link previews, integration cards) — distinct from inline pastes which live in description / comment markdown. + +### Diagnostic log line + +Every work-item fetch (boot path AND runtime read-work-item gadget) emits one INFO-level log line with the literal prefix `[image-pipeline] work-item-fetch summary` and the field schema: + +``` +{ + provider: 'linear' | 'trello' | 'jira' | 'unknown', + workItemId: string, + urlsDetected: number, // pre-filter count + urlsAfterFilter: number, // post-filterImageMedia count + urlsDownloaded: number, + urlsFailed: number, + urlsByMimeType: Record, +} +``` + +Operators triaging a "no image delivered" report grep for the literal prefix in `cascade runs logs ` output. One line per fetch tells the whole story. + +### When a provider's host serves untrustworthy `Content-Type` + +If your provider's upload host returns `application/octet-stream` (or wrong) on the actual GET response, the download-time resolution can't recover. Two options: (a) don't add the host to `IMAGE_HOST_ALLOWLIST` — let the URL-extension path do its job; (b) if URLs are also extension-less, file an issue describing the host's behavior so we can layer in a per-host content-type override. Don't hard-code MIMEs in your adapter — keep MIME resolution shared. + +### Linear: GraphQL surface for inline images + +Spec 016/3 captured a fixture and pinned the rule for Linear specifically. The findings: + +- **`Issue.description` (markdown) is the canonical surface for inline-pasted images.** When a user pastes a screenshot into Linear's issue editor, Linear stores the upload at `https://uploads.linear.app/` (often extension-less) and inserts standard markdown image syntax `![alt](url)` into the description. The Linear adapter's `extractMarkdownImages(issue.description)` is the right call — it's what `getWorkItem` already does at `src/pm/linear/adapter.ts:61`. +- **Comment bodies follow the same convention.** Each `Comment.body` field is markdown; pasted screenshots show up as `![](https://uploads.linear.app/)` exactly like the description. `extractMarkdownImages(comment.body, 'comment')` covers them. +- **`Issue.attachments` is the WRONG surface for inline images.** The Linear GraphQL `Issue.attachments` connection holds formal Attachment records — link previews from Slack threads, GitHub PRs, Sentry alerts, and other integration cards. They have `url` fields but they are NOT user-pasted screenshots. The Linear adapter's `getAttachments(issueId)` (at `src/linear/client.ts:542`) correctly returns these as `LinearAttachment` for the dedicated attachment surface; do NOT extract images from this connection. +- **Regression net.** The captured fixture lives at `tests/fixtures/linear-issue-with-screenshot.json`. The unit test at `tests/unit/pm/linear/extraction-coverage.test.ts` loads the fixture and asserts every inline image is extracted — fails LOUDLY with a clear message if Linear ever changes payload shape in a way that loses inline images. +- **No new GraphQL surface to query.** As of spec 016/3 the Linear API exposes inline-pasted images only via the `description` and `Comment.body` markdown fields. There is no `descriptionData` rich-text JSON tree that would expose them differently, and no `attachments(includeInline: true)` filter. Future Linear API drift would surface as a fixture-test failure. + +See [spec 016](../../docs/specs/016-pm-image-delivery-reliability.md) for the full rationale and the live incident this contract closed. diff --git a/src/pm/download-and-prepare.ts b/src/pm/download-and-prepare.ts new file mode 100644 index 00000000..a1914ad6 --- /dev/null +++ b/src/pm/download-and-prepare.ts @@ -0,0 +1,96 @@ +/** + * Download-and-prepare helper for work-item images. + * + * Lifted from `src/agents/definitions/contextSteps.ts` (the inline loop in + * `fetchWorkItemStep`) into a shared module so spec 016/2's runtime gadget + * can call it too. Both call sites get the same per-provider dispatch, the + * same Promise.all, and the same per-failure WARN log. + * + * Spec 016/1. + */ + +import type { ContextImage } from '../agents/contracts/index.js'; +import { getPMProviderOrNull } from './index.js'; +import { MAX_IMAGES_PER_WORK_ITEM } from './media.js'; +import type { MediaReference } from './types.js'; + +export type LogWriter = ( + level: 'INFO' | 'WARN' | 'ERROR', + message: string, + meta?: Record, +) => void; + +export interface DownloadAndPrepareResult { + images: ContextImage[]; + failures: { url: string; reason: string }[]; +} + +/** + * Downloads each {@link MediaReference} via the appropriate per-provider + * client (jira / linear / trello) and prepares them as {@link ContextImage} + * entries with base64 bytes and the resolved Content-Type-derived MIME. + * + * Caps at {@link MAX_IMAGES_PER_WORK_ITEM}. + * + * Failures are returned as a parallel array, never thrown — so the caller + * can always surface a stable success/failure summary in its diagnostic log. + */ +export async function downloadAndPrepareImages( + workItemId: string, + media: MediaReference[], + logWriter: LogWriter, +): Promise { + if (media.length === 0) return { images: [], failures: [] }; + + const provider = getPMProviderOrNull(); + const limited = media.slice(0, MAX_IMAGES_PER_WORK_ITEM); + + const { jiraClient } = await import('../jira/client.js'); + const { trelloClient } = await import('../trello/client.js'); + const { linearClient } = await import('../linear/client.js'); + + const failures: { url: string; reason: string }[] = []; + + const results = await Promise.all( + limited.map(async (ref) => { + try { + let downloaded: { buffer: Buffer; mimeType: string } | null = null; + if (provider?.type === 'jira') { + downloaded = await jiraClient.downloadAttachment(ref.url); + } else if (provider?.type === 'linear') { + downloaded = await linearClient.downloadAttachment(ref.url); + } else { + downloaded = await trelloClient.downloadAttachment(ref.url); + } + if (!downloaded) { + logWriter('WARN', 'downloadAndPrepareImages: download returned null', { + workItemId, + url: ref.url.split('?')[0], + }); + failures.push({ url: ref.url, reason: 'download returned null' }); + return null; + } + return { + base64Data: downloaded.buffer.toString('base64'), + mimeType: downloaded.mimeType, + altText: ref.altText, + }; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + logWriter('WARN', 'downloadAndPrepareImages: failed to download image', { + workItemId, + url: ref.url.split('?')[0], + error: reason, + }); + failures.push({ url: ref.url, reason }); + return null; + } + }), + ); + + const images: ContextImage[] = []; + for (const r of results) { + if (r !== null) images.push(r); + } + return { images, failures }; +} diff --git a/src/pm/media.ts b/src/pm/media.ts index b6e33f4c..1eceab65 100644 --- a/src/pm/media.ts +++ b/src/pm/media.ts @@ -41,10 +41,17 @@ const IMAGE_MIME_TYPES = new Set([ /** * Returns true when the supplied MIME type represents a common image format. * + * Also accepts the `'image/*'` wildcard sentinel — used by spec 016/1 for + * extension-less PM-provider URLs whose MIME is resolved at download-time + * via the response's Content-Type header. The wildcard never reaches disk; + * `downloadMedia` resolves it to a concrete MIME before the bytes are written. + * * @param mime - The MIME type string to test (e.g. `'image/png'`). */ export function isImageMimeType(mime: string): boolean { - return IMAGE_MIME_TYPES.has(mime.toLowerCase().trim()); + const normalized = mime.toLowerCase().trim(); + if (normalized === 'image/*') return true; + return IMAGE_MIME_TYPES.has(normalized); } /** @@ -77,19 +84,44 @@ const EXTENSION_MIME_MAP: Record = { webp: 'image/webp', }; +/** + * Trusted PM-provider upload hosts whose extension-less URLs we treat as + * candidate images and resolve at download-time via the response's + * Content-Type header. Spec 016/1. + * + * Linear's user-pasted-screenshot URLs (`https://uploads.linear.app/`) + * have no file extension in the pathname; before this allowlist they fell + * through to `'application/octet-stream'` and were silently filtered out by + * `filterImageMedia`. To add a new trusted host: append the bare hostname + * here. Do NOT add hosts whose Content-Type headers are unreliable — the + * wildcard sentinel skips the URL-extension verdict and trusts the response. + */ +const IMAGE_HOST_ALLOWLIST: ReadonlySet = new Set(['uploads.linear.app']); + /** * Infers a MIME type from the file extension in a URL. - * Returns `'application/octet-stream'` when the extension is unknown. + * + * Returns `'application/octet-stream'` when the extension is unknown — except + * for hosts in {@link IMAGE_HOST_ALLOWLIST}, where extension-less URLs return + * the `'image/*'` wildcard sentinel so they survive the pre-download image + * filter. Spec 016/1. * * @param url - The URL to examine. */ function mimeTypeFromUrl(url: string): string { try { - const pathname = new URL(url).pathname; + const parsed = new URL(url); + const pathname = parsed.pathname; const ext = pathname.split('.').pop()?.toLowerCase() ?? ''; - return EXTENSION_MIME_MAP[ext] ?? 'application/octet-stream'; + const fromExt = EXTENSION_MIME_MAP[ext]; + if (fromExt) return fromExt; + // Spec 016/1: trusted PM upload hosts return `image/*` for extension-less + // URLs so the download path can resolve the real MIME from the response. + if (IMAGE_HOST_ALLOWLIST.has(parsed.hostname)) return 'image/*'; + return 'application/octet-stream'; } catch { - // Relative URL or malformed URL — try a simple extension check + // Relative URL or malformed URL — try a simple extension check; no host, + // so cannot apply the allowlist. const ext = url.split('?')[0].split('.').pop()?.toLowerCase() ?? ''; return EXTENSION_MIME_MAP[ext] ?? 'application/octet-stream'; } diff --git a/tests/fixtures/linear-issue-with-screenshot.json b/tests/fixtures/linear-issue-with-screenshot.json new file mode 100644 index 00000000..6523ef59 --- /dev/null +++ b/tests/fixtures/linear-issue-with-screenshot.json @@ -0,0 +1,97 @@ +{ + "_comment": "Reconstructed Linear GraphQL Issue payload for spec 016/3 regression coverage. Models a real issue with multiple inline-pasted screenshots (description + comment) plus formal Attachment records (link previews) that must NOT be picked up as inline images. Sanitized: synthetic team/user/issue IDs, no leaky free-form text. Test fixture only — never imported by production code.", + "_purpose": "Pinned by tests/unit/pm/linear/extraction-coverage.test.ts. If Linear changes the Issue payload shape in a way that loses inline images, that test fails loudly.", + "issue": { + "id": "issue-test-fixture-uuid-0001", + "identifier": "MNG-FIXTURE", + "title": "Bug from screenshot fixture", + "url": "https://linear.app/example/issue/MNG-FIXTURE/bug-from-screenshot-fixture", + "createdAt": "2026-04-26T00:00:00.000Z", + "updatedAt": "2026-04-26T00:00:00.000Z", + "description": "## Repro\n\nThe Tasks Hub mobile layout breaks when the title wraps to a third line.\n\n![](https://uploads.linear.app/abc-123-def-456-extension-less-uuid)\n\n## Expected\n\nLayout should match the design in this annotated screenshot:\n\n![Annotated mockup](https://uploads.linear.app/xyz-789-with-alt-text/Mockup.png)\n\nReference image hosted externally:\n\n![External logo](https://example.com/logo.svg)\n\nA non-image link (should NOT be picked up as media):\n\n[See related ticket](https://linear.app/example/issue/MNG-100)", + "state": { + "id": "state-todo-uuid-0001", + "name": "Todo", + "type": "unstarted" + }, + "team": { + "id": "team-uuid-0001", + "key": "MNG", + "name": "Mongrel" + }, + "labelIds": ["label-bug-uuid-0001"], + "labels": { + "nodes": [ + { + "id": "label-bug-uuid-0001", + "name": "bug", + "color": "#ff0000" + } + ] + }, + "attachments": { + "_comment": "Linear's Issue.attachments connection holds formal Attachment records — link previews from Slack/GitHub/Sentry/etc. — NOT inline-pasted screenshots. The extraction-coverage test asserts these are NOT mistaken for inline images.", + "nodes": [ + { + "id": "attachment-slack-uuid-0001", + "title": "Discussion in #engineering", + "url": "https://acme.slack.com/archives/C123/p1700000000000000", + "subtitle": "Slack thread", + "metadata": { + "type": "slack-thread" + }, + "createdAt": "2026-04-26T00:00:00.000Z", + "updatedAt": "2026-04-26T00:00:00.000Z" + }, + { + "id": "attachment-github-uuid-0001", + "title": "Related PR #1199", + "url": "https://github.com/example/repo/pull/1199", + "subtitle": "GitHub PR", + "metadata": { + "type": "github-pr", + "prNumber": 1199 + }, + "createdAt": "2026-04-26T00:00:00.000Z", + "updatedAt": "2026-04-26T00:00:00.000Z" + }, + { + "id": "attachment-sentry-uuid-0001", + "title": "Sentry issue PROJ-1234", + "url": "https://sentry.io/organizations/example/issues/1234/", + "subtitle": "Sentry alert", + "metadata": { + "type": "sentry-issue", + "level": "error" + }, + "createdAt": "2026-04-26T00:00:00.000Z", + "updatedAt": "2026-04-26T00:00:00.000Z" + } + ] + }, + "comments": { + "nodes": [ + { + "id": "comment-uuid-0001", + "body": "Repro happens on iOS Safari 17. Here's a screenshot from my phone:\n\n![](https://uploads.linear.app/comment-screenshot-uuid)", + "createdAt": "2026-04-26T00:01:00.000Z", + "user": { + "id": "user-uuid-0001", + "name": "Test User", + "displayName": "Test User" + } + }, + { + "id": "comment-uuid-0002", + "body": "I confirmed the same on Android Chrome. No screenshot — text-only comment.", + "createdAt": "2026-04-26T00:02:00.000Z", + "user": { + "id": "user-uuid-0002", + "name": "Another User", + "displayName": "Another User" + } + } + ] + } + } +} diff --git a/tests/integration/gadgets/runtime-image-delivery.test.ts b/tests/integration/gadgets/runtime-image-delivery.test.ts new file mode 100644 index 00000000..c33562fd --- /dev/null +++ b/tests/integration/gadgets/runtime-image-delivery.test.ts @@ -0,0 +1,202 @@ +/** + * Module-integration test for spec 016/2. + * + * Wires the REAL `readWorkItem` gadget + REAL `downloadAndPrepareImages` + * (Plan 1's helper) + REAL `writeRuntimeImages` (Plan 2's writer), plus the + * real `extractMarkdownImages`/`filterImageMedia`/`mimeTypeFromUrl` chain + * inside `readWorkItemWithMedia`. Mocks: filesystem (we don't write to a + * real disk during tests) and the per-provider download client (we control + * the response Content-Type). + * + * Pins the mid-run pickup contract: an image added between two `readWorkItem` + * calls is delivered on the second call as a file path in the returned text. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockMkdir, mockWriteFile, mockLinearDownload, mockTrelloDownload, mockJiraDownload } = + vi.hoisted(() => ({ + mockMkdir: vi.fn().mockResolvedValue(undefined), + mockWriteFile: vi.fn().mockResolvedValue(undefined), + mockLinearDownload: vi.fn(), + mockTrelloDownload: vi.fn(), + mockJiraDownload: vi.fn(), + })); + +vi.mock('node:fs/promises', () => ({ + mkdir: mockMkdir, + writeFile: mockWriteFile, +})); + +vi.mock('../../../src/linear/client.js', () => ({ + linearClient: { downloadAttachment: mockLinearDownload }, +})); +vi.mock('../../../src/trello/client.js', () => ({ + trelloClient: { downloadAttachment: mockTrelloDownload }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + jiraClient: { downloadAttachment: mockJiraDownload }, +})); + +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { createMockPMProvider } from '../../helpers/mockPMProvider.js'; + +const mockProvider = createMockPMProvider(); + +vi.mock('../../../src/pm/index.js', async () => { + const real = await vi.importActual( + '../../../src/pm/index.js', + ); + return { + ...real, + getPMProvider: () => mockProvider, + getPMProviderOrNull: () => mockProvider, + }; +}); + +import { readWorkItem } from '../../../src/gadgets/pm/core/readWorkItem.js'; + +describe('spec 016/2 — runtime image delivery (module-integration)', () => { + beforeEach(() => { + mockMkdir.mockReset(); + mockMkdir.mockResolvedValue(undefined); + mockWriteFile.mockReset(); + mockWriteFile.mockResolvedValue(undefined); + mockLinearDownload.mockReset(); + mockTrelloDownload.mockReset(); + mockJiraDownload.mockReset(); + mockProvider.getWorkItem.mockReset(); + mockProvider.getChecklists.mockReset(); + mockProvider.getAttachments.mockReset(); + mockProvider.getWorkItemComments.mockReset(); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + mockProvider.getWorkItemComments.mockResolvedValue([]); + (mockProvider as unknown as { type: string }).type = 'linear'; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('extension-less Linear URL → on-disk file path appears in text', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + id: 'MNG-357', + title: 'Bug from screenshot', + url: 'https://linear.app/x/MNG-357', + description: '![](https://uploads.linear.app/abc-123)', + labels: [], + inlineMedia: [ + { + url: 'https://uploads.linear.app/abc-123', + mimeType: 'image/*', + altText: undefined, + source: 'description', + }, + ], + }); + mockLinearDownload.mockResolvedValue({ + buffer: Buffer.from('PNG-bytes'), + mimeType: 'image/png', + }); + + const text = await readWorkItem('MNG-357', false); + + // On-disk path with PNG extension (resolved from Content-Type). + expect(text).toContain('.cascade/context/images/work-item-MNG-357-img-0.png'); + // File was actually written. + expect(mockWriteFile).toHaveBeenCalledTimes(1); + }); + + it('mid-run pickup: image added after first call is delivered on second call', async () => { + // First call — no images. + mockProvider.getWorkItem.mockResolvedValueOnce({ + id: 'MNG-1', + title: 'Bug', + url: 'https://linear.app/x/MNG-1', + description: 'Empty description', + labels: [], + inlineMedia: [], + }); + + const firstText = await readWorkItem('MNG-1', false); + expect(firstText).not.toContain('.cascade/context/images/'); + expect(mockWriteFile).not.toHaveBeenCalled(); + + // Second call — teammate has now uploaded a screenshot. + mockProvider.getWorkItem.mockResolvedValueOnce({ + id: 'MNG-1', + title: 'Bug', + url: 'https://linear.app/x/MNG-1', + description: '![](https://uploads.linear.app/new-screenshot)', + labels: [], + inlineMedia: [ + { + url: 'https://uploads.linear.app/new-screenshot', + mimeType: 'image/*', + source: 'description', + }, + ], + }); + mockLinearDownload.mockResolvedValue({ + buffer: Buffer.from('NEW'), + mimeType: 'image/png', + }); + + const secondText = await readWorkItem('MNG-1', false); + expect(secondText).toContain('.cascade/context/images/work-item-MNG-1-img-0.png'); + expect(mockWriteFile).toHaveBeenCalledTimes(1); + }); + + it('Trello extensioned URL regression: still delivered on disk', async () => { + (mockProvider as unknown as { type: string }).type = 'trello'; + mockProvider.getWorkItem.mockResolvedValue({ + id: 'card-1', + title: 'Card', + url: 'https://trello.com/c/card-1', + description: '![](https://trello.com/foo.png)', + labels: [], + inlineMedia: [ + { url: 'https://trello.com/foo.png', mimeType: 'image/png', source: 'description' }, + ], + }); + mockTrelloDownload.mockResolvedValue({ + buffer: Buffer.from('TRELLO'), + mimeType: 'image/png', + }); + + const text = await readWorkItem('card-1', false); + expect(text).toContain('.cascade/context/images/work-item-card-1-img-0.png'); + expect(mockTrelloDownload).toHaveBeenCalledWith('https://trello.com/foo.png'); + }); + + it('failed download: failure surfaced in text, no orphan path', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + id: 'MNG-2', + title: 'Bug', + url: 'https://linear.app/x/MNG-2', + description: '![](https://uploads.linear.app/will-fail)', + labels: [], + inlineMedia: [ + { + url: 'https://uploads.linear.app/will-fail', + mimeType: 'image/*', + source: 'description', + }, + ], + }); + mockLinearDownload.mockRejectedValue(new Error('upstream 500')); + + const text = await readWorkItem('MNG-2', false); + // No on-disk path mentioned. + expect(text).not.toContain('.cascade/context/images/work-item'); + // Failure visible. + expect(text).toContain('Failed Image Downloads'); + expect(text).toContain('upstream 500'); + // No file was written. + expect(mockWriteFile).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/integration/pm/image-pipeline.test.ts b/tests/integration/pm/image-pipeline.test.ts new file mode 100644 index 00000000..32ead02f --- /dev/null +++ b/tests/integration/pm/image-pipeline.test.ts @@ -0,0 +1,157 @@ +/** + * Module-integration test for spec 016/1. + * + * Wires the REAL `mimeTypeFromUrl` + REAL `isImageMimeType` + REAL + * `filterImageMedia` + REAL `extractMarkdownImages` + REAL + * `downloadAndPrepareImages` (via dynamic import in `fetchWorkItemStep`), + * mocking only the per-provider download client (so we control the + * Content-Type response without needing a real Linear endpoint) and the + * upstream `readWorkItemWithMedia` (so we don't need a real PM provider). + * + * Pins the end-to-end MNG-357 reproduction: extension-less Linear URL flows + * through extract → filter → download → ContextInjection.images without + * being dropped. Pre-spec-016 behavior would fail this test because + * `filterImageMedia` would drop the `application/octet-stream` ref. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockLinearDownload, mockTrelloDownload, mockJiraDownload } = vi.hoisted(() => ({ + mockLinearDownload: vi.fn(), + mockTrelloDownload: vi.fn(), + mockJiraDownload: vi.fn(), +})); + +vi.mock('../../../src/linear/client.js', () => ({ + linearClient: { downloadAttachment: mockLinearDownload }, +})); +vi.mock('../../../src/trello/client.js', () => ({ + trelloClient: { downloadAttachment: mockTrelloDownload }, +})); +vi.mock('../../../src/jira/client.js', () => ({ + jiraClient: { downloadAttachment: mockJiraDownload }, +})); + +vi.mock('../../../src/gadgets/pm/core/readWorkItem.js', () => ({ + readWorkItemWithMedia: vi.fn(), + readWorkItem: vi.fn(), +})); + +vi.mock('../../../src/pm/index.js', async () => { + // Need to keep MAX_IMAGES_PER_WORK_ITEM real so the cap matches production + const real = await vi.importActual( + '../../../src/pm/index.js', + ); + return { + ...real, + getPMProviderOrNull: vi.fn(), + }; +}); + +import { fetchWorkItemStep } from '../../../src/agents/definitions/contextSteps.js'; +import { readWorkItemWithMedia } from '../../../src/gadgets/pm/core/readWorkItem.js'; +import { getPMProviderOrNull } from '../../../src/pm/index.js'; +import { extractMarkdownImages, filterImageMedia } from '../../../src/pm/media.js'; +import type { AgentInput } from '../../../src/types/index.js'; + +const mockReadWorkItemWithMedia = vi.mocked(readWorkItemWithMedia); +const mockGetPMProviderOrNull = vi.mocked(getPMProviderOrNull); + +describe('spec 016/1 — boot-path image pipeline (module-integration)', () => { + beforeEach(() => { + mockLinearDownload.mockReset(); + mockTrelloDownload.mockReset(); + mockJiraDownload.mockReset(); + mockReadWorkItemWithMedia.mockReset(); + mockGetPMProviderOrNull.mockReset(); + }); + + function makeParams(input: Partial) { + return { + input: input as AgentInput, + repoDir: '/tmp/repo', + contextFiles: [], + logWriter: vi.fn(), + }; + } + + it('MNG-357 reproduction: extension-less Linear URL extracted via real path lands as image with resolved MIME', async () => { + // Step 1: real extraction — pin that Linear-shaped URLs survive the filter via image/* sentinel. + const description = '![](https://uploads.linear.app/abc-123-def-456)'; + const refs = extractMarkdownImages(description); + expect(refs).toHaveLength(1); + expect(refs[0].mimeType).toBe('image/*'); + const filtered = filterImageMedia(refs); + expect(filtered).toHaveLength(1); // wildcard survived + + // Step 2: real fetchWorkItemStep flow with that ref + a stubbed download. + mockReadWorkItemWithMedia.mockResolvedValue({ + text: '# MNG-357\n\n![](https://uploads.linear.app/abc-123-def-456)', + media: filtered, // pass the real-extracted refs + urlsDetected: filtered.length, + }); + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' } as never); + mockLinearDownload.mockResolvedValue({ + buffer: Buffer.from('PNG-bytes-here'), + mimeType: 'image/png', // server-side Content-Type — this is the ground truth + }); + + const result = await fetchWorkItemStep(makeParams({ workItemId: 'MNG-357' })); + + // Image was delivered; MIME resolved to the Content-Type, not the wildcard. + expect(result).toHaveLength(1); + expect(result[0].images).toHaveLength(1); + expect(result[0].images?.[0].mimeType).toBe('image/png'); + expect(result[0].images?.[0].base64Data).toBe(Buffer.from('PNG-bytes-here').toString('base64')); + }); + + it('emits the diagnostic line `[image-pipeline] work-item-fetch summary` with non-zero downloads', async () => { + const refs = extractMarkdownImages('![](https://uploads.linear.app/abc-123)'); + mockReadWorkItemWithMedia.mockResolvedValue({ + text: '# x', + media: refs, + urlsDetected: 1, + }); + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' } as never); + mockLinearDownload.mockResolvedValue({ + buffer: Buffer.from('x'), + mimeType: 'image/png', + }); + + const params = makeParams({ workItemId: 'MNG-357' }); + await fetchWorkItemStep(params); + + expect(params.logWriter).toHaveBeenCalledWith( + 'INFO', + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ + provider: 'linear', + workItemId: 'MNG-357', + urlsDetected: 1, + urlsAfterFilter: 1, + urlsDownloaded: 1, + urlsFailed: 0, + }), + ); + }); + + it('Trello PNG URL regression: extensioned URL still resolves and downloads', async () => { + const refs = extractMarkdownImages('![](https://trello.com/foo.png)'); + expect(refs[0].mimeType).toBe('image/png'); // extension-resolved + + mockReadWorkItemWithMedia.mockResolvedValue({ + text: '# t', + media: refs, + urlsDetected: refs.length, + }); + mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never); + mockTrelloDownload.mockResolvedValue({ + buffer: Buffer.from('y'), + mimeType: 'image/png', + }); + + const result = await fetchWorkItemStep(makeParams({ workItemId: 'card-1' })); + expect(result[0].images).toHaveLength(1); + expect(result[0].images?.[0].mimeType).toBe('image/png'); + }); +}); diff --git a/tests/unit/agents/definitions/contextSteps.test.ts b/tests/unit/agents/definitions/contextSteps.test.ts index 78ab8217..f119c062 100644 --- a/tests/unit/agents/definitions/contextSteps.test.ts +++ b/tests/unit/agents/definitions/contextSteps.test.ts @@ -231,6 +231,7 @@ describe('fetchWorkItemStep', () => { mockReadWorkItemWithMedia.mockResolvedValue({ text: '# Card Title\n\nDescription', media: [], + urlsDetected: 0, }); mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never); @@ -353,11 +354,12 @@ describe('fetchWorkItemStep', () => { expect(result[0].images).toHaveLength(1); expect(result[0].images?.[0].base64Data).toBe(Buffer.from('ok').toString('base64')); - // WARN for the null return, URL sanitized (no query params) + // WARN for the null return, URL sanitized (no query params); spec 016/1 + // renamed the helper so the message prefix is now `downloadAndPrepareImages`. expect(params.logWriter).toHaveBeenCalledWith( 'WARN', - 'fetchWorkItemStep: image download returned null', - { url: 'https://trello.com/fail.png' }, + 'downloadAndPrepareImages: download returned null', + { workItemId: 'card-1', url: 'https://trello.com/fail.png' }, ); }); @@ -381,12 +383,15 @@ describe('fetchWorkItemStep', () => { expect(result[0].images).toBeUndefined(); expect(params.logWriter).toHaveBeenCalledWith( 'WARN', - 'fetchWorkItemStep: failed to download image', - { url: 'https://trello.com/err.png', error: 'network failure' }, + 'downloadAndPrepareImages: failed to download image', + { workItemId: 'card-1', url: 'https://trello.com/err.png', error: 'network failure' }, ); }); - it('emits INFO logs before and after download with correct counts', async () => { + // Spec 016/1 AC#5: single grep-stable INFO line per work-item-fetch with + // the `[image-pipeline] work-item-fetch summary` literal prefix and a + // stable field schema. Replaces the prior pre/post WARN/INFO log pair. + it("emits the '[image-pipeline] work-item-fetch summary' INFO line with correct counts", async () => { mockReadWorkItemWithMedia.mockResolvedValue({ text: '# Card', media: [ @@ -394,6 +399,7 @@ describe('fetchWorkItemStep', () => { { url: 'https://trello.com/b.png', mimeType: 'image/png', source: 'description' }, { url: 'https://trello.com/c.png', mimeType: 'image/png', source: 'description' }, ], + urlsDetected: 3, }); mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never); mockTrelloDownload @@ -406,13 +412,71 @@ describe('fetchWorkItemStep', () => { expect(params.logWriter).toHaveBeenCalledWith( 'INFO', - 'fetchWorkItemStep: downloading work item images', - { workItemId: 'card-1', count: 3 }, + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ + provider: 'trello', + workItemId: 'card-1', + urlsDetected: 3, + urlsAfterFilter: 3, + urlsDownloaded: 1, + urlsFailed: 2, + urlsByMimeType: { 'image/png': 3 }, + }), ); + }); + + it('emits the diagnostic line even when no images are present (urlsDetected: 0)', async () => { + mockReadWorkItemWithMedia.mockResolvedValue({ text: '# Card', media: [], urlsDetected: 0 }); + mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never); + + const params = makeParams({ workItemId: 'card-1' }); + await fetchWorkItemStep(params); + + expect(params.logWriter).toHaveBeenCalledWith( + 'INFO', + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ + provider: 'trello', + workItemId: 'card-1', + urlsDetected: 0, + urlsAfterFilter: 0, + urlsDownloaded: 0, + urlsFailed: 0, + urlsByMimeType: {}, + }), + ); + }); + + it('reports MIME distribution covering both extensioned and image/* wildcard refs', async () => { + mockReadWorkItemWithMedia.mockResolvedValue({ + text: '# Card', + media: [ + { url: 'https://trello.com/a.png', mimeType: 'image/png', source: 'description' }, + { + url: 'https://uploads.linear.app/abc', + mimeType: 'image/*', + source: 'description', + }, + ], + urlsDetected: 2, + }); + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' } as never); + mockLinearDownload.mockResolvedValue({ buffer: Buffer.from('x'), mimeType: 'image/png' }); + + const params = makeParams({ workItemId: 'MNG-357' }); + await fetchWorkItemStep(params); + expect(params.logWriter).toHaveBeenCalledWith( 'INFO', - 'fetchWorkItemStep: image download complete', - { workItemId: 'card-1', attempted: 3, downloaded: 1, skipped: 2 }, + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ + provider: 'linear', + workItemId: 'MNG-357', + urlsDetected: 2, + urlsAfterFilter: 2, + urlsDownloaded: 2, + urlsByMimeType: { 'image/png': 1, 'image/*': 1 }, + }), ); }); @@ -422,7 +486,11 @@ describe('fetchWorkItemStep', () => { mimeType: 'image/png', source: 'description' as const, })); - mockReadWorkItemWithMedia.mockResolvedValue({ text: '# Card', media: manyMedia }); + mockReadWorkItemWithMedia.mockResolvedValue({ + text: '# Card', + media: manyMedia, + urlsDetected: 35, + }); mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' } as never); mockTrelloDownload.mockResolvedValue({ buffer: Buffer.from('data'), mimeType: 'image/png' }); diff --git a/tests/unit/gadgets/pm/core/readWorkItem.test.ts b/tests/unit/gadgets/pm/core/readWorkItem.test.ts index eb88cfaf..4767c99f 100644 --- a/tests/unit/gadgets/pm/core/readWorkItem.test.ts +++ b/tests/unit/gadgets/pm/core/readWorkItem.test.ts @@ -1,12 +1,30 @@ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createMockPMProvider } from '../../../../helpers/mockPMProvider.js'; const mockProvider = createMockPMProvider(); +const { mockDownloadAndPrepareImages, mockWriteRuntimeImages } = vi.hoisted(() => ({ + mockDownloadAndPrepareImages: vi.fn().mockResolvedValue({ images: [], failures: [] }), + mockWriteRuntimeImages: vi.fn().mockResolvedValue({ paths: [], failures: [] }), +})); + vi.mock('../../../../../src/pm/index.js', () => ({ getPMProvider: vi.fn(() => mockProvider), filterImageMedia: vi.fn((refs) => refs.filter((r) => r.mimeType.startsWith('image/'))), + getPMProviderOrNull: vi.fn(() => mockProvider), +})); + +vi.mock('../../../../../src/pm/download-and-prepare.js', () => ({ + downloadAndPrepareImages: mockDownloadAndPrepareImages, +})); + +vi.mock('../../../../../src/gadgets/pm/core/writeRuntimeImages.js', () => ({ + writeRuntimeImages: mockWriteRuntimeImages, +})); + +vi.mock('../../../../../src/utils/logging.js', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, })); import { @@ -207,6 +225,166 @@ describe('readWorkItem', () => { // Second comment appears first (reversed order) expect(secondPos).toBeLessThan(firstPos); }); + + // ===================================================================== + // Spec 016/2: runtime gadget downloads + writes images to disk + // ===================================================================== + describe('spec 016/2 — runtime image delivery', () => { + beforeEach(() => { + mockDownloadAndPrepareImages.mockReset(); + mockDownloadAndPrepareImages.mockResolvedValue({ images: [], failures: [] }); + mockWriteRuntimeImages.mockReset(); + mockWriteRuntimeImages.mockResolvedValue({ paths: [], failures: [] }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('when work item has images, downloads + writes them and inlines paths into text', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + inlineMedia: [ + { + url: 'https://uploads.linear.app/abc', + mimeType: 'image/*', + altText: 'Screenshot.png', + source: 'description', + }, + ], + }); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + mockDownloadAndPrepareImages.mockResolvedValue({ + images: [ + { + base64Data: Buffer.from('PNG').toString('base64'), + mimeType: 'image/png', + altText: 'Screenshot.png', + }, + ], + failures: [], + }); + mockWriteRuntimeImages.mockResolvedValue({ + paths: ['.cascade/context/images/work-item-item1-img-0.png'], + failures: [], + }); + + const result = await readWorkItem('item1', false); + + // Text should mention the on-disk path the agent can Read. + expect(result).toContain('.cascade/context/images/work-item-item1-img-0.png'); + expect(mockDownloadAndPrepareImages).toHaveBeenCalledTimes(1); + expect(mockWriteRuntimeImages).toHaveBeenCalledTimes(1); + expect(mockWriteRuntimeImages).toHaveBeenCalledWith( + expect.objectContaining({ + workItemId: 'item1', + images: expect.arrayContaining([expect.objectContaining({ mimeType: 'image/png' })]), + }), + ); + }); + + it('when work item has no images, returns text unchanged (no disk write)', async () => { + mockProvider.getWorkItem.mockResolvedValue(baseItem); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + + const result = await readWorkItem('item1', false); + + expect(result).toContain('# Test Work Item'); + expect(mockWriteRuntimeImages).not.toHaveBeenCalled(); + }); + + it('emits the diagnostic log line at runtime path with same prefix as boot path', async () => { + const { logger } = await import('../../../../../src/utils/logging.js'); + vi.mocked(logger.info).mockClear(); + + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + inlineMedia: [{ url: 'https://x/a.png', mimeType: 'image/png', source: 'description' }], + }); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + mockDownloadAndPrepareImages.mockResolvedValue({ + images: [{ base64Data: 'aGk=', mimeType: 'image/png', altText: undefined }], + failures: [], + }); + mockWriteRuntimeImages.mockResolvedValue({ + paths: ['.cascade/context/images/work-item-item1-img-0.png'], + failures: [], + }); + + await readWorkItem('item1', false); + + expect(logger.info).toHaveBeenCalledWith( + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ + workItemId: 'item1', + urlsDetected: expect.any(Number), + urlsAfterFilter: expect.any(Number), + urlsDownloaded: 1, + urlsFailed: 0, + }), + ); + }); + + it('when download fails, the failure is recorded in the diagnostic log; no path appears in text', async () => { + const { logger } = await import('../../../../../src/utils/logging.js'); + vi.mocked(logger.info).mockClear(); + + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + inlineMedia: [{ url: 'https://x/fail.png', mimeType: 'image/png', source: 'description' }], + }); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + mockDownloadAndPrepareImages.mockResolvedValue({ + images: [], + failures: [{ url: 'https://x/fail.png', reason: 'network error' }], + }); + + const result = await readWorkItem('item1', false); + + // No on-disk path included. + expect(result).not.toContain('.cascade/context/images/work-item'); + // Failure was visible in the diagnostic log line. + expect(logger.info).toHaveBeenCalledWith( + '[image-pipeline] work-item-fetch summary', + expect.objectContaining({ urlsDownloaded: 0, urlsFailed: 1 }), + ); + }); + + it('text shape preserved: existing sections (Description, Comments) remain', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + inlineMedia: [{ url: 'https://x/a.png', mimeType: 'image/png', source: 'description' }], + }); + mockProvider.getChecklists.mockResolvedValue([]); + mockProvider.getAttachments.mockResolvedValue([]); + mockProvider.getWorkItemComments.mockResolvedValue([ + { + id: 'c1', + author: { name: 'A', id: 'u', username: 'a' }, + date: '2024-01-01T00:00:00Z', + text: 'a comment', + }, + ]); + mockDownloadAndPrepareImages.mockResolvedValue({ + images: [{ base64Data: 'aGk=', mimeType: 'image/png', altText: undefined }], + failures: [], + }); + mockWriteRuntimeImages.mockResolvedValue({ + paths: ['.cascade/context/images/work-item-item1-img-0.png'], + failures: [], + }); + + const result = await readWorkItem('item1', true); + + expect(result).toContain('## Description'); + expect(result).toContain('a comment'); + expect(result).toContain('.cascade/context/images/work-item-item1-img-0.png'); + }); + }); }); describe('readWorkItemWithMedia', () => { diff --git a/tests/unit/gadgets/pm/core/writeRuntimeImages.test.ts b/tests/unit/gadgets/pm/core/writeRuntimeImages.test.ts new file mode 100644 index 00000000..b90544b6 --- /dev/null +++ b/tests/unit/gadgets/pm/core/writeRuntimeImages.test.ts @@ -0,0 +1,140 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockMkdir, mockWriteFile, mockAccess } = vi.hoisted(() => ({ + mockMkdir: vi.fn().mockResolvedValue(undefined), + mockWriteFile: vi.fn().mockResolvedValue(undefined), + mockAccess: vi.fn().mockRejectedValue(new Error('ENOENT')), +})); + +vi.mock('node:fs/promises', () => ({ + mkdir: mockMkdir, + writeFile: mockWriteFile, + access: mockAccess, +})); + +vi.mock('../../../../../src/utils/logging.js', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { writeRuntimeImages } from '../../../../../src/gadgets/pm/core/writeRuntimeImages.js'; +import { logger } from '../../../../../src/utils/logging.js'; + +const mockLogger = vi.mocked(logger); + +describe('writeRuntimeImages', () => { + beforeEach(() => { + mockMkdir.mockReset(); + mockMkdir.mockResolvedValue(undefined); + mockWriteFile.mockReset(); + mockWriteFile.mockResolvedValue(undefined); + mockAccess.mockReset(); + mockAccess.mockRejectedValue(new Error('ENOENT')); + mockLogger.info.mockReset(); + mockLogger.warn.mockReset(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('writes each image with work-item--img-.', async () => { + const result = await writeRuntimeImages({ + workItemId: 'MNG-357', + images: [ + { base64Data: Buffer.from('a').toString('base64'), mimeType: 'image/png' }, + { base64Data: Buffer.from('b').toString('base64'), mimeType: 'image/jpeg' }, + ], + }); + + expect(mockWriteFile).toHaveBeenCalledTimes(2); + const firstPath = mockWriteFile.mock.calls[0][0] as string; + const secondPath = mockWriteFile.mock.calls[1][0] as string; + expect(firstPath).toContain('work-item-MNG-357-img-0.png'); + expect(secondPath).toContain('work-item-MNG-357-img-1.jpg'); + expect(result.paths).toHaveLength(2); + }); + + it('derives extension from resolved MIME, NOT from URL', async () => { + await writeRuntimeImages({ + workItemId: 'card-1', + images: [{ base64Data: Buffer.from('x').toString('base64'), mimeType: 'image/webp' }], + }); + + const path = mockWriteFile.mock.calls[0][0] as string; + expect(path).toMatch(/work-item-card-1-img-0\.webp$/); + }); + + it('falls back to .bin extension when MIME resolution failed (image/* sentinel)', async () => { + await writeRuntimeImages({ + workItemId: 'MNG-1', + images: [{ base64Data: Buffer.from('x').toString('base64'), mimeType: 'image/*' }], + }); + + const path = mockWriteFile.mock.calls[0][0] as string; + expect(path).toMatch(/work-item-MNG-1-img-0\.bin$/); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('writeRuntimeImages: unresolved MIME'), + expect.objectContaining({ workItemId: 'MNG-1', mimeType: 'image/*' }), + ); + }); + + it('returns the list of relative paths it wrote', async () => { + const result = await writeRuntimeImages({ + workItemId: 'w1', + images: [ + { base64Data: Buffer.from('a').toString('base64'), mimeType: 'image/png' }, + { base64Data: Buffer.from('b').toString('base64'), mimeType: 'image/png' }, + ], + }); + + expect(result.paths).toEqual([ + '.cascade/context/images/work-item-w1-img-0.png', + '.cascade/context/images/work-item-w1-img-1.png', + ]); + }); + + it('creates the .cascade/context/images directory if it does not exist', async () => { + await writeRuntimeImages({ + workItemId: 'w1', + images: [{ base64Data: Buffer.from('x').toString('base64'), mimeType: 'image/png' }], + }); + + // mkdir called with recursive: true at least once + expect(mockMkdir).toHaveBeenCalled(); + const firstCall = mockMkdir.mock.calls[0]; + expect(firstCall[1]).toEqual({ recursive: true }); + }); + + it('returns empty paths when given no images', async () => { + const result = await writeRuntimeImages({ workItemId: 'w1', images: [] }); + expect(result.paths).toHaveLength(0); + expect(mockWriteFile).not.toHaveBeenCalled(); + }); + + it('captures write failure as a failure entry, does not throw', async () => { + mockWriteFile.mockRejectedValueOnce(new Error('disk full')); + + const result = await writeRuntimeImages({ + workItemId: 'w1', + images: [ + { base64Data: Buffer.from('a').toString('base64'), mimeType: 'image/png' }, + { base64Data: Buffer.from('b').toString('base64'), mimeType: 'image/png' }, + ], + }); + + expect(result.paths).toHaveLength(1); // only the second succeeded + expect(result.failures).toHaveLength(1); + expect(result.failures[0].reason).toContain('disk full'); + }); + + it('uses repoDir-relative path when repoDir option provided', async () => { + await writeRuntimeImages({ + workItemId: 'w1', + images: [{ base64Data: Buffer.from('x').toString('base64'), mimeType: 'image/png' }], + repoDir: '/tmp/my-repo', + }); + + const path = mockWriteFile.mock.calls[0][0] as string; + expect(path).toContain('/tmp/my-repo/.cascade/context/images/work-item-w1-img-0.png'); + }); +}); diff --git a/tests/unit/pm/download-and-prepare.test.ts b/tests/unit/pm/download-and-prepare.test.ts new file mode 100644 index 00000000..3f27b5fd --- /dev/null +++ b/tests/unit/pm/download-and-prepare.test.ts @@ -0,0 +1,162 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// --------------------------------------------------------------------------- +// Hoisted mocks — the helper dispatches to per-provider download clients +// that we mock at the module level. +// --------------------------------------------------------------------------- + +const { + mockJiraDownloadAttachment, + mockLinearDownloadAttachment, + mockTrelloDownloadAttachment, + mockGetPMProviderOrNull, +} = vi.hoisted(() => ({ + mockJiraDownloadAttachment: vi.fn(), + mockLinearDownloadAttachment: vi.fn(), + mockTrelloDownloadAttachment: vi.fn(), + mockGetPMProviderOrNull: vi.fn(), +})); + +vi.mock('../../../src/jira/client.js', () => ({ + jiraClient: { downloadAttachment: mockJiraDownloadAttachment }, +})); +vi.mock('../../../src/linear/client.js', () => ({ + linearClient: { downloadAttachment: mockLinearDownloadAttachment }, +})); +vi.mock('../../../src/trello/client.js', () => ({ + trelloClient: { downloadAttachment: mockTrelloDownloadAttachment }, +})); +vi.mock('../../../src/pm/index.js', () => ({ + getPMProviderOrNull: mockGetPMProviderOrNull, +})); +vi.mock('../../../src/utils/logging.js', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { downloadAndPrepareImages } from '../../../src/pm/download-and-prepare.js'; +import type { MediaReference } from '../../../src/pm/types.js'; + +describe('downloadAndPrepareImages', () => { + const noopLogWriter = vi.fn(); + + const ref = ( + url: string, + mimeType = 'image/png', + altText?: string, + source: 'description' | 'comment' | 'attachment' = 'description', + ): MediaReference => ({ url, mimeType, altText, source }); + + beforeEach(() => { + mockJiraDownloadAttachment.mockReset(); + mockLinearDownloadAttachment.mockReset(); + mockTrelloDownloadAttachment.mockReset(); + mockGetPMProviderOrNull.mockReset(); + noopLogWriter.mockReset(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('downloads each ref and returns success array + failure array', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' }); + mockLinearDownloadAttachment + .mockResolvedValueOnce({ buffer: Buffer.from('one'), mimeType: 'image/png' }) + .mockResolvedValueOnce({ buffer: Buffer.from('two'), mimeType: 'image/jpeg' }) + .mockResolvedValueOnce(null); // failure + + const result = await downloadAndPrepareImages( + 'MNG-357', + [ + ref('https://uploads.linear.app/a'), + ref('https://uploads.linear.app/b'), + ref('https://uploads.linear.app/c'), + ], + noopLogWriter, + ); + + expect(result.images).toHaveLength(2); + expect(result.failures).toHaveLength(1); + }); + + it('preserves base64 + altText + RESOLVED mimeType (not the input wildcard)', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' }); + mockLinearDownloadAttachment.mockResolvedValueOnce({ + buffer: Buffer.from('hello'), + mimeType: 'image/png', + }); + + const result = await downloadAndPrepareImages( + 'MNG-1', + [ref('https://uploads.linear.app/abc', 'image/*', 'Screenshot.png')], + noopLogWriter, + ); + + expect(result.images).toHaveLength(1); + expect(result.images[0]).toEqual({ + base64Data: Buffer.from('hello').toString('base64'), + mimeType: 'image/png', // resolved, NOT the wildcard input + altText: 'Screenshot.png', + }); + }); + + it('caps at MAX_IMAGES_PER_WORK_ITEM (10)', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' }); + mockLinearDownloadAttachment.mockResolvedValue({ + buffer: Buffer.from('x'), + mimeType: 'image/png', + }); + + const refs: MediaReference[] = Array.from({ length: 12 }, (_, i) => + ref(`https://uploads.linear.app/${i}`), + ); + await downloadAndPrepareImages('MNG-357', refs, noopLogWriter); + + expect(mockLinearDownloadAttachment).toHaveBeenCalledTimes(10); + }); + + it('dispatches to the correct per-provider download client', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' }); + mockLinearDownloadAttachment.mockResolvedValue({ + buffer: Buffer.from('x'), + mimeType: 'image/png', + }); + await downloadAndPrepareImages('MNG-1', [ref('https://x.com/a.png')], noopLogWriter); + expect(mockLinearDownloadAttachment).toHaveBeenCalledTimes(1); + expect(mockJiraDownloadAttachment).not.toHaveBeenCalled(); + expect(mockTrelloDownloadAttachment).not.toHaveBeenCalled(); + }); + + it('falls back to trello when provider type is not jira or linear', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'trello' }); + mockTrelloDownloadAttachment.mockResolvedValue({ + buffer: Buffer.from('x'), + mimeType: 'image/png', + }); + await downloadAndPrepareImages('w1', [ref('https://x.com/a.png')], noopLogWriter); + expect(mockTrelloDownloadAttachment).toHaveBeenCalledTimes(1); + }); + + it('captures failure reason for download exceptions', async () => { + mockGetPMProviderOrNull.mockReturnValue({ type: 'linear' }); + mockLinearDownloadAttachment.mockRejectedValue(new Error('network blip')); + + const result = await downloadAndPrepareImages( + 'MNG-1', + [ref('https://uploads.linear.app/a')], + noopLogWriter, + ); + + expect(result.images).toHaveLength(0); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toEqual({ + url: 'https://uploads.linear.app/a', + reason: 'network blip', + }); + }); + + it('returns empty arrays when given no refs', async () => { + const result = await downloadAndPrepareImages('w1', [], noopLogWriter); + expect(result).toEqual({ images: [], failures: [] }); + }); +}); diff --git a/tests/unit/pm/linear/extraction-coverage.test.ts b/tests/unit/pm/linear/extraction-coverage.test.ts new file mode 100644 index 00000000..a966ea0e --- /dev/null +++ b/tests/unit/pm/linear/extraction-coverage.test.ts @@ -0,0 +1,157 @@ +/** + * Regression net for spec 016/3 AC#7. + * + * Loads the captured/reconstructed Linear Issue GraphQL fixture and asserts + * our extraction picks up every inline image in it. Fails LOUDLY if Linear + * ever changes its payload shape in a way that loses inline images. + * + * Also pins the rule that `Issue.attachments` records (link previews from + * Slack/GitHub/Sentry) are NOT mistaken for inline images. + */ + +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { extractMarkdownImages } from '../../../../src/pm/media.js'; + +interface FixtureIssue { + id: string; + identifier: string; + description: string; + attachments: { + nodes: Array<{ id: string; title: string; url: string }>; + }; + comments: { + nodes: Array<{ id: string; body: string }>; + }; +} + +interface Fixture { + issue: FixtureIssue; +} + +function loadFixture(): FixtureIssue { + const fixturePath = join( + __dirname, + '..', + '..', + '..', + 'fixtures', + 'linear-issue-with-screenshot.json', + ); + const raw = readFileSync(fixturePath, 'utf-8'); + const fixture = JSON.parse(raw) as Fixture; + return fixture.issue; +} + +// The exact set of URLs that the fixture's description embeds via markdown +// image syntax. This is the regression-truth that Plan 1's extraction must +// always be able to recover from the fixture description string. +const EXPECTED_DESCRIPTION_IMAGE_URLS = [ + 'https://uploads.linear.app/abc-123-def-456-extension-less-uuid', + 'https://uploads.linear.app/xyz-789-with-alt-text/Mockup.png', + 'https://example.com/logo.svg', +]; + +const EXPECTED_COMMENT_IMAGE_URLS = ['https://uploads.linear.app/comment-screenshot-uuid']; + +describe('Linear extraction-coverage regression', () => { + it('description: extracts every inline image from the fixture description', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.description); + const urls = refs.map((r) => r.url); + + // Every expected URL must be present. + for (const expectedUrl of EXPECTED_DESCRIPTION_IMAGE_URLS) { + expect( + urls, + `Linear description image MISSED: ${expectedUrl} — Linear payload may have changed shape; update fixture or extraction.`, + ).toContain(expectedUrl); + } + expect(refs).toHaveLength(EXPECTED_DESCRIPTION_IMAGE_URLS.length); + }); + + it('description: assigns image/* sentinel to extension-less Linear URLs', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.description); + const linearExtensionless = refs.find( + (r) => r.url === 'https://uploads.linear.app/abc-123-def-456-extension-less-uuid', + ); + expect(linearExtensionless?.mimeType).toBe('image/*'); + }); + + it('description: assigns concrete MIME for extensioned Linear URL', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.description); + const mockup = refs.find( + (r) => r.url === 'https://uploads.linear.app/xyz-789-with-alt-text/Mockup.png', + ); + expect(mockup?.mimeType).toBe('image/png'); + expect(mockup?.altText).toBe('Annotated mockup'); + }); + + it('description: external SVG URL extracted with image/svg+xml MIME', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.description); + const svg = refs.find((r) => r.url === 'https://example.com/logo.svg'); + expect(svg?.mimeType).toBe('image/svg+xml'); + }); + + it('description: non-image markdown links are NOT extracted', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.description); + // "[See related ticket](...)" is a regular markdown link, not an image. + expect(refs.find((r) => r.url.includes('MNG-100'))).toBeUndefined(); + }); + + it('comments: extracts inline images from each comment body', () => { + const issue = loadFixture(); + const allCommentRefs: string[] = []; + for (const comment of issue.comments.nodes) { + const refs = extractMarkdownImages(comment.body, 'comment'); + allCommentRefs.push(...refs.map((r) => r.url)); + } + + for (const expectedUrl of EXPECTED_COMMENT_IMAGE_URLS) { + expect(allCommentRefs, `Linear comment image MISSED: ${expectedUrl}`).toContain(expectedUrl); + } + expect(allCommentRefs).toHaveLength(EXPECTED_COMMENT_IMAGE_URLS.length); + }); + + it('comments: source field marks them as comment-origin', () => { + const issue = loadFixture(); + const refs = extractMarkdownImages(issue.comments.nodes[0].body, 'comment'); + expect(refs[0].source).toBe('comment'); + }); + + it('attachments: formal Attachment records (Slack/GitHub/Sentry link previews) are NOT mistaken for inline images', () => { + const issue = loadFixture(); + // The Linear adapter's getAttachments returns these. They have URLs but + // they're link previews, not inline images. Our inline-image extraction + // only reads the description and comment bodies — never the attachments + // connection. This test pins that contract by asserting that none of + // the attachment URLs appear in the description-extracted set. + const descRefs = extractMarkdownImages(issue.description); + const descUrls = new Set(descRefs.map((r) => r.url)); + for (const att of issue.attachments.nodes) { + expect( + descUrls.has(att.url), + `Linear attachment leaked into description extraction: ${att.url}`, + ).toBe(false); + } + }); + + it('regression net: meta-test confirms the test mechanism works (assertion fires when fixture is wrong)', () => { + // Sanity: prove that .toContain() actually fails when an expected URL + // is missing. If the fixture were stripped of all images, this test's + // guarantee (the spec AC#7 "fails loudly" promise) would still hold — + // the meta-check confirms the negative case. + const refs = extractMarkdownImages('No images here, just text.'); + const urls = refs.map((r) => r.url); + expect(() => { + for (const expectedUrl of EXPECTED_DESCRIPTION_IMAGE_URLS) { + expect(urls).toContain(expectedUrl); + } + }).toThrow(); + }); +}); diff --git a/tests/unit/pm/media.test.ts b/tests/unit/pm/media.test.ts index cf095a73..a5751238 100644 --- a/tests/unit/pm/media.test.ts +++ b/tests/unit/pm/media.test.ts @@ -72,6 +72,16 @@ describe('isImageMimeType', () => { it('trims whitespace before checking', () => { expect(isImageMimeType(' image/png ')).toBe(true); }); + + // Spec 016/1: image/* wildcard sentinel for extension-less PM URLs whose + // MIME is resolved at download-time via Content-Type header. + it("accepts the 'image/*' wildcard sentinel", () => { + expect(isImageMimeType('image/*')).toBe(true); + }); + + it("preserves strict acceptance: 'application/*' is NOT an image wildcard", () => { + expect(isImageMimeType('application/*')).toBe(false); + }); }); // --------------------------------------------------------------------------- @@ -119,6 +129,36 @@ describe('filterImageMedia', () => { // --------------------------------------------------------------------------- describe('extractMarkdownImages', () => { + // Spec 016/1: extension-less Linear pasted-image URLs must survive the + // pre-download MIME filter via the image/* wildcard sentinel. + describe('spec 016/1 — extension-less PM URLs', () => { + it("returns mimeType 'image/*' for extension-less Linear uploads.linear.app URL", () => { + const refs = extractMarkdownImages('![](https://uploads.linear.app/abc-123-def-456)'); + expect(refs).toHaveLength(1); + expect(refs[0].mimeType).toBe('image/*'); + }); + + it('returns concrete mimeType for extensioned Linear URL (regression-safe)', () => { + const refs = extractMarkdownImages( + '![Screenshot](https://uploads.linear.app/abc/Screenshot.png)', + ); + expect(refs).toHaveLength(1); + expect(refs[0].mimeType).toBe('image/png'); + }); + + it("returns 'application/octet-stream' for extension-less non-PM URL (no over-broad wildcard)", () => { + const refs = extractMarkdownImages('![](https://example.com/random-file)'); + expect(refs).toHaveLength(1); + expect(refs[0].mimeType).toBe('application/octet-stream'); + }); + + it("Trello PNG URL still returns 'image/png' (regression)", () => { + const refs = extractMarkdownImages('![](https://trello.com/foo.png)'); + expect(refs).toHaveLength(1); + expect(refs[0].mimeType).toBe('image/png'); + }); + }); + // Basic happy path it('extracts a single image', () => { const refs = extractMarkdownImages('Hello ![logo](https://example.com/logo.png)');