Skip to content

Artifact dedup by SHA-256 is spec'd but not implemented (spec/impl drift in §d) #321

Description

@bg-playground

Problem

docs/specs/external_results_v1.md §d ("Idempotency rules → Artifacts — deduplication by SHA-256") documents the following behavior:

If the body of a new artifact upload has the same SHA-256 as an artifact already attached to the same case_result_id, BGSTM returns the existing artifact row (200 OK) rather than creating a duplicate.

The error-code registry (§f) also lists artifact.duplicate as a valid response code.

As of the artifact endpoint shipped in #319, none of this is implemented. The handler:

  • Does not compute a SHA-256 of the upload body.
  • Does not query existing external_case_artifacts rows for a hash match.
  • Always creates a fresh row + file on every successful upload.

There is also no sha256 (or equivalent) column on the external_case_artifacts table, so dedup couldn't work even if the handler tried.

Why this matters

  1. Spec/impl drift erodes trust in the spec doc as a source of truth.
  2. The smoke workflow in PR Add BGSTM external-results smoke workflow pinned to reporter SHA ab5d7c1 with main-branch, audit-log, and artifact-path compatibility hardening #314 may eventually want to assert dedup behavior; it currently can't.
  3. Reporters that retry uploads (e.g. on transient network errors) will today create duplicate rows + duplicate files on disk/S3 — exactly what the documented dedup was meant to prevent.

Decision needed

Pick one path:

(A) Implement dedup as documented. Adds a sha256 column to external_case_artifacts (with an index on (case_result_id, sha256)), hashes the body during the existing chunked write loop, and short-circuits to 200 OK + existing row on match. Worth pairing with #320 (true streaming size enforcement) since both touch the upload loop.

(B) Delete the dedup section from the spec. If we don't actually want this behavior in v1, remove §d's artifact-dedup paragraph and the artifact.duplicate code from §f's error registry. Document the rationale in the commit message.

Either is fine; the current state (spec says one thing, code does another) is the worst option.

Acceptance criteria

  • Decision made between (A) implement vs. (B) delete from spec.
  • If (A): sha256 column added via Alembic migration; hash computed during upload; dedup hit returns 200 OK + existing row (409 artifact.duplicate retired or repurposed); tests cover hit and miss; spec wording confirmed accurate.
  • If (B): §d artifact-dedup paragraph removed; §f error registry pruned; PR feat(#298): artifact upload endpoint + pluggable storage backend #319's response-code table cross-checked.
  • No remaining references to SHA-256 dedup in docs unless they reflect actual behavior.

References

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions