fix: close post-merge archive safety gaps

[steipete]

Summary

• allow the documented third Discord CDN redirect while rejecting the fourth
• keep trusted injected HTTP transports compatible when successful responses omit request metadata, without weakening validation when a final response URL is available
• validate message tombstones during routine incremental snapshot imports before archive mutation
• add the next Unreleased changelog section

Root cause

The post-merge review of #134 found that the redirect guard counted the initial request as a redirect and that final-response validation treated optional transport metadata as mandatory. The post-merge review of #135 found that routine incremental imports used a separate filter that skipped the shared tombstone validator.

Proof

• GOWORK=off go test ./... -count=1
• GOWORK=off go test -race ./internal/media ./internal/share -count=1
• isolated compiled behavior contract: three redirects pass, the fourth is denied, a response without request metadata fetches successfully, and malformed incremental tombstones fail without changing archive or merge state
• real built CLI: version, attachment help, and update help smoke pass
• AutoReview: clean; no accepted or actionable findings

Risk

Low. Changes are confined to attachment redirect/transport compatibility and the existing snapshot-row validator at the incremental import boundary.

[clawsweeper]

Codex review: found issues before merge. Reviewed July 18, 2026, 3:17 PM ET / 19:17 UTC.

Summary
The branch permits three Discord CDN redirects, tolerates missing response request metadata for injected HTTP clients, validates incremental-import tombstones before archive mutation, and adds regression coverage.

Reproducibility: yes. from source and the added regression cases: the affected paths are the attachment redirect/final-response boundary and incremental snapshot import before archive mutation. A live local reproduction was not independently run because the read-only terminal sandbox failed before repository inspection commands could execute.

Review metrics: 2 noteworthy metrics.

• Patch surface: 5 files affected; 124 added, 3 removed. The implementation is concentrated in two runtime paths, two regression-test files, and one release-note file.
• Regression coverage: 108 test lines added across 2 files. The branch adds focused coverage for redirect limits, injected-client behavior, and pre-mutation tombstone rejection.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Remove the CHANGELOG.md addition before merge.

Next step before merge

• A collaborator-authored PR with a release-ownership cleanup should remain in normal maintainer review rather than enter an autonomous repair lane.

Security
Cleared: The diff changes existing validation and tests only; it adds no dependencies, executable workflow changes, secrets handling, or supply-chain inputs.

Review findings

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:3-9

Review details

Best possible solution:

Keep the focused redirect and incremental-import fixes with their regression tests, remove the release-owned CHANGELOG.md edit, and let the normal maintainer review complete against the exact PR head.

Do we have a high-confidence way to reproduce the issue?

Yes, from source and the added regression cases: the affected paths are the attachment redirect/final-response boundary and incremental snapshot import before archive mutation. A live local reproduction was not independently run because the read-only terminal sandbox failed before repository inspection commands could execute.

Is this the best way to solve the issue?

Yes, mostly. Correcting the redirect threshold and routing incremental rows through the existing validator are narrow fixes; the release-owned changelog addition is not part of the implementation solution and should be removed.

Full review comments:

• [P3] Remove the release-owned changelog entry — CHANGELOG.md:3-9
CHANGELOG.md is release-owned under the review policy. Keep the user-visible release context in this PR body or commit message and let the release flow update the changelog.
  Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.72

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 15af97146636.

Label changes

Label changes:

• add P2: This is a bounded correctness and archive-safety follow-up with limited blast radius, not an active availability emergency.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The author is a repository collaborator, so the external-contributor real-behavior-proof gate does not apply; the PR body nevertheless records focused tests, race checks, and a built-CLI smoke run.

Label justifications:

• P2: This is a bounded correctness and archive-safety follow-up with limited blast radius, not an active availability emergency.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The author is a repository collaborator, so the external-contributor real-behavior-proof gate does not apply; the PR body nevertheless records focused tests, race checks, and a built-CLI smoke run.

Evidence reviewed

What I checked:

• Attachment redirect boundary: The proposed change updates the redirect condition from rejecting three redirects to rejecting a fourth, and adds a regression test covering both three allowed redirects and a rejected fourth redirect. (internal/media/cache.go:141, 1e7f686ee1bf)
• Incremental import validation: The incremental snapshot filter now invokes the shared row validator before import, with a test asserting malformed tombstones do not alter archive data or the prior merged manifest. (internal/share/share.go:661, 1e7f686ee1bf)
• Related merged provenance: The PR explicitly follows the merged media hardening and tombstone-validation work from fix(media): constrain attachment redirects #134 and fix(share): validate imported tombstones #135; both are identified in the supplied context as merged on July 17, 2026. (05f4d7866367)
• Release-note ownership: The branch adds an Unreleased section to the release-owned changelog, while the review policy states normal PRs should carry release context in their PR body rather than edit CHANGELOG.md. (CHANGELOG.md:3, 1e7f686ee1bf)

Likely related people:

• steipete: The supplied history identifies this contributor as the author of the merged attachment redirect hardening and tombstone-import validation work that this follow-up refines. (role: recent area contributor; confidence: high; commits: 05f4d7866367, 04bfb70d8729, 1e7f686ee1bf; files: internal/media/cache.go, internal/share/share.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.