feat: reuse verified same-site banners in article imports #282

Merged

erik merged 1 commit from feat/task-79102318-reusable-banner-keys into main

2026-09-18 15:55:38 -05:00

erik commented

2026-09-18 15:34:30 -05:00

Owner

Summary

Task: task-79102318 — Support reusable media keys in article imports

Treat article banner: banners/technology.png as a same-site /media/<key> reference and banner: ./banner.png as a post-specific local upload.
Verify shared images with an unauthenticated, time-bounded HEAD request before mutations; require HTTP 200 and an image content type, reject redirects and unsafe paths, and report missing images separately from other verification failures.
Reuse the existing post banner URL API without adding media lookup endpoints. Preserve published article identity and omitted publication dates.
Remove failed article banner images in the browser without introducing a default, including banners deleted after import.
Document upload-once reuse, the ./ migration for existing local references, and shared-image replacement/deletion behavior.

Verification

./scripts/pre-pr.sh — passed (generated CSS, formatting, lint, all workspace typechecks and tests, including integration tests).
npm run build — passed.
npm run test:visual — 29 passed, including absent/deleted banner behavior.
CLI regression coverage: two drafts sharing a key, published updates, local uploads, invalid/missing references, non-image responses, HTTP failures and rejected redirects.
API regressions confirm sharing URLs without media records and preserving published identity/timestamp; public route tests confirm HEAD support and missing-image status.

Compatibility and limitations

Existing local references such as banner: images/hero.png must become banner: ./images/hero.png.
Shared references use the API origin's /media/ route; arbitrary external URLs are not supported by article import.
Verification is point-in-time. Display-time removal of unavailable banners requires JavaScript.
No release, merge, or task closure is included; awaiting CI and independent review before human merge approval.

## Summary Task: task-79102318 — Support reusable media keys in article imports - Treat article `banner: banners/technology.png` as a same-site `/media/<key>` reference and `banner: ./banner.png` as a post-specific local upload. - Verify shared images with an unauthenticated, time-bounded HEAD request before mutations; require HTTP 200 and an image content type, reject redirects and unsafe paths, and report missing images separately from other verification failures. - Reuse the existing post banner URL API without adding media lookup endpoints. Preserve published article identity and omitted publication dates. - Remove failed article banner images in the browser without introducing a default, including banners deleted after import. - Document upload-once reuse, the `./` migration for existing local references, and shared-image replacement/deletion behavior. ## Verification - `./scripts/pre-pr.sh` — passed (generated CSS, formatting, lint, all workspace typechecks and tests, including integration tests). - `npm run build` — passed. - `npm run test:visual` — 29 passed, including absent/deleted banner behavior. - CLI regression coverage: two drafts sharing a key, published updates, local uploads, invalid/missing references, non-image responses, HTTP failures and rejected redirects. - API regressions confirm sharing URLs without media records and preserving published identity/timestamp; public route tests confirm HEAD support and missing-image status. ## Compatibility and limitations - Existing local references such as `banner: images/hero.png` must become `banner: ./images/hero.png`. - Shared references use the API origin's `/media/` route; arbitrary external URLs are not supported by article import. - Verification is point-in-time. Display-time removal of unavailable banners requires JavaScript. - No release, merge, or task closure is included; awaiting CI and independent review before human merge approval.

erik added 1 commit

2026-09-18 15:34:30 -05:00

feat: reuse verified same-site banners in article imports

CI / build-lint-test (pull_request) Successful in 2m57s

Details

3b6f191305

Task: #task-79102318

erik commented

2026-09-18 15:39:21 -05:00

Author

Owner

Review update

PR: #282 (open)
Task: task-79102318
Reviewed commit: 3b6f1913058493810581ac2f6c225d6cb32e8a89
Result: approved
Summary: Independently reviewed the diff against the current task acceptance criteria, including relevant import, media-delivery, rendering, and regression-test code. Shared banners use bounded, unauthenticated same-origin HEAD verification before mutations; explicit local uploads retain their storage convention. No new lookup API, default banner, or source changes were introduced by this review.

Acceptance criteria

met — Two draft imports reuse the same /media/banners/technology.png URL, issue HEAD requests without downloading image bodies to the CLI, and perform no uploads (cli/src/__tests__/commands.test.ts).
met — Verification requests the actual delivery URL directly; no database lookup or new endpoint is added (cli/src/banner-import.ts; API regression verifies reuse without media records).
met — ./ paths resolve relative to the Markdown file and upload under posts/<slug>/banner.<extension>; the existing create/update upload test exercises this behavior.
met — Bare keys never fall back to local files; the failed-verification regression deliberately creates a matching local file and still observes only HEAD.
met — Missing images/files and invalid references fail before mutations. Parser and command tests cover URLs, absolute paths, traversal, malformed values, and missing references.
met — Non-image metadata, network failures, HTTP failures, unsupported HEAD, and redirects produce contextual verification errors distinct from missing images. Manual redirect handling prevents cross-origin follow-up; requests contain no API authorization.
met — Published updates omit unspecified publication dates and update by slug. CLI and API regressions verify banner replacement while preserving ID, slug, and publication timestamp.
met — Absent banners render no image; a head-installed capturing error listener removes failed article banners without a default or removal of content images. Unit and browser tests cover this behavior. View-time removal requires JavaScript, as explicitly documented.
met — Added/updated coverage spans URL construction, verification, uploads, invalid/missing inputs, failures/redirects, draft creation, published updates, and absent/deleted banner display.
met — cli/README.md includes upload-once reuse, both frontmatter forms, API-origin derivation, ./ migration, replacement/deletion effects, and import-time versus view-time behavior.

Verification and pipeline

CI gate: green/resolved — supplied Forgejo Actions run #467, commit 3b6f191305, job build-lint-test: success. Used this known result rather than the broken fj pr status relative-URL parser.
Existing local evidence inspected: /tmp/slug-banner-pre-pr.log reports all checks passed; /tmp/slug-banner-build.log records successful workspace builds; /tmp/slug-banner-visual.log reports 29 passed.
No broad checks rerun. No source code or task status changed. No merge performed.

Findings and verdict

Blocking issues: 0
Warnings: 0
Criteria: 10 total / 10 met / 0 missing
Final verdict: approved
Next: Await explicit human merge approval; this review does not authorize or perform a merge.

### Review update - PR: https://forge.caradoc.com/erik/slugkit/pulls/282 (open) - Task: task-79102318 - Reviewed commit: `3b6f1913058493810581ac2f6c225d6cb32e8a89` - Result: **approved** - Summary: Independently reviewed the diff against the current task acceptance criteria, including relevant import, media-delivery, rendering, and regression-test code. Shared banners use bounded, unauthenticated same-origin HEAD verification before mutations; explicit local uploads retain their storage convention. No new lookup API, default banner, or source changes were introduced by this review. ### Acceptance criteria 1. **met** — Two draft imports reuse the same `/media/banners/technology.png` URL, issue HEAD requests without downloading image bodies to the CLI, and perform no uploads (`cli/src/__tests__/commands.test.ts`). 2. **met** — Verification requests the actual delivery URL directly; no database lookup or new endpoint is added (`cli/src/banner-import.ts`; API regression verifies reuse without media records). 3. **met** — `./` paths resolve relative to the Markdown file and upload under `posts/<slug>/banner.<extension>`; the existing create/update upload test exercises this behavior. 4. **met** — Bare keys never fall back to local files; the failed-verification regression deliberately creates a matching local file and still observes only HEAD. 5. **met** — Missing images/files and invalid references fail before mutations. Parser and command tests cover URLs, absolute paths, traversal, malformed values, and missing references. 6. **met** — Non-image metadata, network failures, HTTP failures, unsupported HEAD, and redirects produce contextual verification errors distinct from missing images. Manual redirect handling prevents cross-origin follow-up; requests contain no API authorization. 7. **met** — Published updates omit unspecified publication dates and update by slug. CLI and API regressions verify banner replacement while preserving ID, slug, and publication timestamp. 8. **met** — Absent banners render no image; a head-installed capturing error listener removes failed article banners without a default or removal of content images. Unit and browser tests cover this behavior. View-time removal requires JavaScript, as explicitly documented. 9. **met** — Added/updated coverage spans URL construction, verification, uploads, invalid/missing inputs, failures/redirects, draft creation, published updates, and absent/deleted banner display. 10. **met** — `cli/README.md` includes upload-once reuse, both frontmatter forms, API-origin derivation, `./` migration, replacement/deletion effects, and import-time versus view-time behavior. ### Verification and pipeline - CI gate: **green/resolved** — supplied Forgejo Actions run **#467**, commit `3b6f191305`, job `build-lint-test`: success. Used this known result rather than the broken `fj pr status` relative-URL parser. - Existing local evidence inspected: `/tmp/slug-banner-pre-pr.log` reports all checks passed; `/tmp/slug-banner-build.log` records successful workspace builds; `/tmp/slug-banner-visual.log` reports **29 passed**. - No broad checks rerun. No source code or task status changed. No merge performed. ### Findings and verdict - Blocking issues: **0** - Warnings: **0** - Criteria: **10 total / 10 met / 0 missing** - Final verdict: **approved** - Next: Await explicit human merge approval; this review does not authorize or perform a merge.

erik merged commit 79a2e389b1 into main

2026-09-18 15:55:38 -05:00

erik deleted branch feat/task-79102318-reusable-banner-keys

2026-09-18 15:55:38 -05:00

erik referenced this pull request from a commit

2026-09-18 15:55:40 -05:00

Merge pull request 'feat: reuse verified same-site banners in article imports' (#282) from feat/task-79102318-reusable-banner-keys into main