Feedback v2: processing lifecycle, server-side dedupe, canonical companion skill

Server (migration 002: processed_at, payload_hash):
- frictions: content-hash duplicate absorption within 24h -> 200 + existing
  row; the auto-collected context object is excluded from identity
- reviews: replay with different content -> 409 replay_mismatch instead of
  silently discarding the new payload
- processed_at + POST /api/v1/submissions/processed (batch mark/unmark with
  per-id outcomes), processed list filter, friction category/summary/project/
  harness surfaced on list rows
- generous length caps (protect indexed columns, never police content)
- /ready pings Postgres; deploy script verifies /ready instead of /health
- drop spoofable RealIP middleware; untrack committed build binary

Companion skill, canonical at skills/agent-feedback/ (distribution handled
outside this repo; SKILL.md documents installation):
- submit-friction: --stdin JSON mode, --dry-run, machine-readable outcome
  lines, auto-collected context (occurred_at, cwd, git repo/branch/commit/
  dirty, os/arch, session id, agent, effort, profile, client version)
- harness-agnostic attribution: markers verified from each harness's installed
  code (claude-code/opencode/pi/omp/codex), inner-harness-wins nesting, no
  prefix sniffing (profile API-key leaks), AGENT_FEEDBACK_HARNESS/MODEL/
  SESSION_ID overrides
- submit-review: 409 handling, honest exit codes; query: URL-encoded and
  read-only by default; new process.sh (list/done/undo)
- simplified spool: both types retry-safe, .inflight pruned, identity-checked
  replays

Docs and tests:
- docs/agent-usage.md rewritten to match reality (base URL, run_id recipe,
  new contract); CLAUDE.md rules updated (four-artifact API changes,
  write-once with the processed_at exception)
- e2e: rerun-safe unique IDs, mktemp, 33 checks incl. 413/409/dedupe/processed
- hermetic skill suite (tests/skill/, Python mock server, 41 checks) +
  shellcheck wired into CI; image build now depends on both test jobs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: foae

.github/workflows/ci.yml

@@ -46,8 +46,19 @@ jobs:
           TEST_POSTGRES_URL: postgres://postgres:test@127.0.0.1:5432/feedback_test?sslmode=disable
         run: go test -race -count=1 ./...
 
+  skill:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: shellcheck client scripts
+        run: |
+          sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck
+          cd skills/agent-feedback/scripts && shellcheck -x ./*.sh
+      - name: hermetic client tests (mock server)
+        run: bash tests/skill/run-tests.sh
+
   image:
-    needs: test
+    needs: [test, skill]
     if: github.event_name == 'push' && github.ref == 'refs/heads/main'
     runs-on: ubuntu-latest
     steps:

.gitignore

@@ -32,3 +32,6 @@ backend/vendor
 # Editor/IDE
 # .idea/
 # .vscode/
+
+# Local build output (just build)
+backend/bin/

CLAUDE.md

@@ -14,7 +14,9 @@ your task needs it. If you are instead **submitting data to the running service*
 |---|---|
 | Wire a skill/producer to submit feedback | [docs/agent-usage.md](docs/agent-usage.md) — self-contained API contract, curl recipes |
 | Change service code | This file (rules below), then [docs/architecture.md](docs/architecture.md) for the layer model |
-| Change the API surface | Rules below — three artifacts must move together |
+| Change the API surface | Rules below — four artifacts must move together |
+| Change the companion skill (client scripts) | `skills/agent-feedback/` — this repo is the canonical source; run `tests/skill/run-tests.sh` |
+| Install/adopt the companion skill | `skills/agent-feedback/SKILL.md` — Installation section |
 | Understand naming/error/test conventions | [docs/conventions.md](docs/conventions.md) |
 | See how the service was scaffolded | [docs/adding-a-service.md](docs/adding-a-service.md), [docs/patterns.md](docs/patterns.md) |
 | Run / verify / deploy | Commands + Verification below; deploy runbook in [README.md](README.md) ("Deploy to production") |
@@ -26,11 +28,15 @@ backend/                      Go module root (single go.mod)
   pkg/                        shared infra (postgres pool, envutil, httputil, observability)
   services/feedback/
     cmd/feedback/main.go      wiring: config, DI, chi router, graceful shutdown
-    core/                     business logic: validation, idempotent create, list/get
+    core/                     business logic: validation, content hashing/dedupe, create, list/get, processed
     handler/                  HTTP: auth middleware, DTOs, sentinel-error → status mapping
     storage/postgres/         q.sql + migrations/ (inputs) → sqlc/ (generated, never edit)
 infra/agent-feedback/         docker-compose stack (postgres + service)
 scripts/e2e.sh                live-contract test suite (all endpoints)
+skills/agent-feedback/        CANONICAL companion skill (SKILL.md + client scripts ONLY —
+                              distributed as-is; sync/distribution handled outside this repo)
+tests/skill/                  hermetic tests for the skill scripts (repo-only; must NEVER
+                              travel with the skill directory)
 docs/agent-usage.md           THE API contract (producers integrate against this)
 ```
 
@@ -44,6 +50,7 @@ just test            # unit tests (integration tests skip without TEST_POSTGRES_
 just run-local       # needs .env (cp .env.example .env)
 just sqlc-generate   # regenerate storage/postgres/sqlc/ after editing q.sql or migrations
 bash scripts/e2e.sh <API_KEY> [BASE_URL]   # from repo root, against a running stack
+bash tests/skill/run-tests.sh   # hermetic client-script tests (mock server)
 ```
 
 Integration tests need a real Postgres:
@@ -64,14 +71,20 @@ docker rm -f pg-test
   `just sqlc-generate`. Never hand-edit generated files.
 - **Migrations are append-only** now that the service is deployed. New numbered
   file per change; never rewrite an existing one.
-- **An API change is a three-artifact change**: handler/core code,
-  [docs/agent-usage.md](docs/agent-usage.md), and `scripts/e2e.sh` move in the
-  same commit. The doc is a load-bearing contract — producer agents build their
-  recipes from it without reading the code.
-- **Write-once semantics are deliberate.** No update endpoints. Reviews dedupe on
-  `(submission_type, run_id)` — replay returns the existing row with 200. The
-  skill name `friction` is reserved. Don't add PATCH; a correction is a new
-  submission.
+- **An API change is a four-artifact change**: handler/core code,
+  [docs/agent-usage.md](docs/agent-usage.md), `scripts/e2e.sh`, and the client
+  scripts in `skills/agent-feedback/` move in the same commit. The doc is a
+  load-bearing contract — producer agents build their recipes from it without
+  reading the code.
+- **Write-once payloads, one mutable flag.** Submission content is never
+  updated; `processed_at` (set/cleared via `POST /api/v1/submissions/processed`)
+  is the only mutable state. Reviews dedupe on `(submission_type, run_id)`:
+  identical replay → 200 existing row; different content under the same key →
+  409 `replay_mismatch` (hash comparison via `payload_hash`) — a correction is a
+  new submission under a new run_id. Frictions dedupe on content hash within a
+  24 h window (`core.frictionDedupeWindow`) → duplicate returns 200 + existing
+  row; the auto-collected `context` object is EXCLUDED from that hash (it varies
+  between attempts of the same friction). The skill name `friction` is reserved.
 - **Identity PK is a deliberate exception** to the template's UUID convention
   (see note in [docs/conventions.md](docs/conventions.md)).
 - Create endpoints are strict: unknown JSON fields → 400, bodies over 10 MiB → 413.
@@ -98,3 +111,6 @@ docker rm -f pg-test
 3. If you touched the API surface: `docker compose up -d` in `infra/agent-feedback/`
    (needs `.env`, see `.env.example`), then `bash scripts/e2e.sh <key>` — all
    checks green — then `docker compose down -v` to remove test data.
+4. If you touched `skills/agent-feedback/`: `bash tests/skill/run-tests.sh`
+   (hermetic — needs python3, no running stack). Keep the skill directory free
+   of tests/tooling — it is distributed verbatim.

README.md

@@ -26,8 +26,10 @@ retry semantics, and copy-pasteable recipes are all in
 ## API
 
 Endpoints: `POST /api/v1/reviews`, `POST /api/v1/frictions`,
-`GET /api/v1/submissions` (filtered list), `GET /api/v1/submissions/{id}`.
-Unauthenticated: `/health`, `/ready`, `/metrics` (Prometheus).
+`GET /api/v1/submissions` (filtered list), `GET /api/v1/submissions/{id}`,
+`POST /api/v1/submissions/processed` (batch mark/unmark by the feedback processor).
+Unauthenticated: `/health` (liveness), `/ready` (readiness incl. Postgres ping),
+`/metrics` (Prometheus).
 
 The contract — request/response schemas per field, error bodies, status codes,
 idempotency, curl examples — lives in **[docs/agent-usage.md](docs/agent-usage.md)**
@@ -51,6 +53,8 @@ agent-feedback/
 │       └── .env.example
 ├── infra/agent-feedback/              # docker-compose stack (postgres + feedback)
 ├── scripts/e2e.sh                     # End-to-end contract suite (all endpoints)
+├── skills/agent-feedback/             # Canonical companion skill (SKILL.md + client scripts; see its Installation section)
+├── tests/skill/                       # Hermetic tests for the skill scripts (repo-only, never distributed)
 └── docs/
     ├── agent-usage.md                 # API contract for producer agents (start here for integration)
     ├── architecture.md                # Layer model, module strategy (template docs)
@@ -117,7 +121,9 @@ auth; olivier holds no registry credentials), streams it over SSH into
 `docker load`, syncs
 `infra/agent-feedback/docker-compose.deploy.yml` to `~/agent-feedback/` on
 olivier, generates credentials into `~/agent-feedback/.env` on first deploy
-(preserved on every later deploy), runs `docker compose up -d`, and health-checks.
+(preserved on every later deploy), runs `docker compose up -d`, and verifies
+`/ready` (which includes a Postgres ping — a deploy with a dead database fails
+the check instead of reporting healthy).
 Local prerequisites: `gh` (authed), `docker`, `crane` (`brew install crane`) —
 crane is used instead of `docker save` because docker's containerd image store
 can emit truncated save tars (see the script header).
@@ -131,6 +137,14 @@ bash scripts/e2e.sh <API_KEY> [BASE_URL]   # BASE_URL defaults to http://127.0.0
 ```
 
 Exercises every endpoint against a live deployment: auth failures, create,
-idempotent replay, validation errors (bad values, unknown fields, oversized
-bodies), list filters, get-by-id. It creates test submissions — clean up with
-`docker compose down -v` (or delete the rows) afterwards.
+identical replay, replay-mismatch 409, friction duplicate absorption, validation
+errors (bad values, unknown fields, length caps, oversized bodies), list filters
+incl. `processed`, processed mark/unmark, get-by-id. Safe to rerun against a
+persistent database (unique ids per run); it creates test submissions — clean up
+with `docker compose down -v` (or delete the rows) afterwards.
+
+The companion skill has its own hermetic suite (no stack needed):
+
+```bash
+bash tests/skill/run-tests.sh
+```

backend/bin/feedback

@@ -137,10 +137,11 @@ func main() {
 	// Router
 	r := chi.NewRouter()
 
+	// middleware.RealIP is deliberately absent: it rewrites RemoteAddr from
+	// spoofable headers (X-Forwarded-For etc.) and nothing proxies this service.
 	r.Use(
 		middleware.Recoverer,
 		middleware.RequestID,
-		middleware.RealIP,
 		buildLoggerMiddleware(logger, cfg.EnvMode),
 		httputil.ReqMonitor(cfg.ServiceName),
 	)
@@ -157,6 +158,17 @@ func main() {
 			_, _ = w.Write([]byte("SHUTTING_DOWN"))
 			return
 		}
+		// Readiness includes the database: a wedged Postgres must fail the
+		// probe (and the deploy script's post-deploy check), not report READY
+		// while every write spools client-side.
+		pingCtx, cancel := context.WithTimeout(r.Context(), 2*time.Second)
+		defer cancel()
+		if err := pg.DB().Ping(pingCtx); err != nil {
+			slog.Warn("readiness probe failed: postgres unreachable", "error", err)
+			w.WriteHeader(http.StatusServiceUnavailable)
+			_, _ = w.Write([]byte("DB_UNAVAILABLE"))
+			return
+		}
 		w.WriteHeader(http.StatusOK)
 		_, _ = w.Write([]byte("READY"))
 	})
@@ -169,6 +181,7 @@ func main() {
 		api.Post("/reviews", h.HandleCreateReview())
 		api.Post("/frictions", h.HandleCreateFriction())
 		api.Get("/submissions", h.HandleListSubmissions())
+		api.Post("/submissions/processed", h.HandleSetProcessed())
 		api.Get("/submissions/{id}", h.HandleGetSubmission())
 	})
 

backend/services/feedback/cmd/feedback/main.go

@@ -0,0 +1,33 @@
+package core
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+)
+
+// reviewCanonical / frictionCanonical are the exact shapes hashed into
+// submissions.payload_hash. Struct field order fixes the JSON encoding, so
+// identical content always produces the same digest. created_at is deliberately
+// excluded — the hash identifies content, not the moment of receipt.
+type reviewCanonical struct {
+	MachineName      string        `json:"machine_name"`
+	CoordinatorModel string        `json:"coordinator_model"`
+	Payload          reviewPayload `json:"payload"`
+}
+
+type frictionCanonical struct {
+	MachineName      string          `json:"machine_name"`
+	CoordinatorModel string          `json:"coordinator_model"`
+	Payload          frictionPayload `json:"payload"`
+}
+
+// canonicalHash returns the sha256 hex digest of v's JSON encoding.
+func canonicalHash(v any) (string, error) {
+	b, err := json.Marshal(v)
+	if err != nil {
+		return "", err
+	}
+	sum := sha256.Sum256(b)
+	return hex.EncodeToString(sum[:]), nil
+}