fix: verify issue-update writes instead of inferring success (#12051)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents report task state to the control plane with `PATCH
/api/issues/{id}` at the end of each heartbeat
> - On remote sandbox targets those writes cross a relay that can fail
at the connection level
> - An agent that pipes its status curl through `head` cannot see that
failure; the write is lost but the run reports success
> - The issue then stays `in_progress` with no disposition, and the
missing-disposition recovery must repair it
> - This pull request makes the issue-update helper verify every write,
and it teaches the shared skill to require verified writes
> - The benefit is that a lost status write becomes a visible, retried
failure instead of a silent success

## Linked Issues or Issue Description

No public issue exists for this defect. The description below follows
the bug report template.

**What happened?**

A sandboxed heartbeat run answered its issue in a comment. It then sent
`PATCH /api/issues/{id}` with `status: done` through `curl -sf ... |
head -c 400`. The relay dropped the connection. The `-f` flag suppressed
the error output, and the pipe replaced curl's exit code with the exit
code of `head`. The agent saw empty output and exit 0. It reported the
write as an "empty 2xx" success and exited. The issue stayed
`in_progress`, and the successful-run recovery had to close it in a
corrective run.

**Expected behavior**

A status write that does not reach the server must surface as a failure.
The helper script must retry transient failures. It must exit non-zero
when the write is unconfirmed. Skill guidance must forbid write patterns
that hide failures.

**Steps to reproduce**

1. Point `PAPERCLIP_API_URL` at an endpoint that drops connections
intermittently.
2. Finalize an issue with `curl -sf -X PATCH
"$PAPERCLIP_API_URL/api/issues/$ID" -d '{"status":"done"}' | head -c
400`.
3. Observe exit code 0 with empty output while the server never received
the PATCH.

## What Changed

- `scripts/paperclip-issue-update.sh` now captures `%{http_code}`,
retries a retryable failure (connection-level, 429, 5xx) once — two
attempts total, which matches the shared bounded-write-retry rule —
rejects an empty 2xx body, and confirms the response echoes the
requested status before it exits 0.
- Failure output states plainly that the write was NOT saved, so the
calling agent reports it accurately.
- `skills/paperclip/SKILL.md` Step 8 adds a required "Verify writes —
never infer them" rule: a successful PATCH always returns the updated
issue JSON, disposition writes must never run through `head`/`tail`
pipelines, and an unconfirmed write must be reported as FAILED.
- `server/src/__tests__/paperclip-skill-utils.test.ts` pins the new
skill rule; a new `paperclip-issue-update-helper.test.ts` exercises the
helper's behavior end-to-end.

## Verification

- `bash -n scripts/paperclip-issue-update.sh`
- `server/src/__tests__/paperclip-issue-update-helper.test.ts` runs the
helper end-to-end against a local HTTP server: confirmed-echo success
(exit 0), empty 2xx (exit 1), wrong echoed status (exit 1), 422 reject
(exit 1, exactly one request), 503 then success (two requests),
connection refused (two attempts, then exit 1 with a "NOT saved"
report).
- `npx vitest run
server/src/__tests__/paperclip-issue-update-helper.test.ts
server/src/__tests__/paperclip-skill-utils.test.ts
server/src/__tests__/cli-invocation-safety.test.ts` — 50 passed.

## Risks

- Low risk. The success-path output is unchanged (the updated issue
JSON).
- The helper now exits non-zero on unconfirmed writes. Callers that
previously missed silent failures now see explicit errors. That is the
intended behavior change.
- The single retry re-sends the PATCH after a retryable failure. If the
first request committed and only its response was lost, an attached
comment can post twice. The duplicate is visible and benign; the prior
behavior lost the write silently.

## Model Used

- Claude Fable 5 (Anthropic), model id `claude-fable-5`, extended
thinking enabled, agentic tool use via Claude Code (CLI harness), 200k
context window.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

scripts/paperclip-issue-update.sh

@@ -102,9 +102,65 @@ if [[ -z "${PAPERCLIP_API_URL:-}" || -z "${PAPERCLIP_API_KEY:-}" || -z "${PAPERC
   exit 1
 fi
 
-curl -sS -X PATCH \
-  "$PAPERCLIP_API_URL/api/issues/$issue_id" \
-  -H "Authorization: Bearer $PAPERCLIP_API_KEY" \
-  -H "X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID" \
-  -H 'Content-Type: application/json' \
-  --data-binary "$payload"
+# A successful PATCH always returns the updated issue JSON. An empty body or a
+# connection-level failure means the write did NOT land, even when a pipeline
+# exit code says otherwise, so verify the response instead of inferring success.
+# Two attempts total: the shared heartbeat policy stops a control-plane write
+# after two consecutive failures, so the helper must not send a third.
+max_attempts=2
+attempt=1
+while :; do
+  http_code=""
+  body=""
+  set +e
+  response="$(
+    curl -sS -m 30 -X PATCH \
+      "$PAPERCLIP_API_URL/api/issues/$issue_id" \
+      -H "Authorization: Bearer $PAPERCLIP_API_KEY" \
+      -H "X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID" \
+      -H 'Content-Type: application/json' \
+      --data-binary "$payload" \
+      -w '\n%{http_code}'
+  )"
+  curl_exit=$?
+  set -e
+
+  if [[ "$curl_exit" -eq 0 ]]; then
+    http_code="${response##*$'\n'}"
+    body="${response%$'\n'*}"
+  fi
+
+  if [[ "$curl_exit" -eq 0 && "$http_code" == 2* ]]; then
+    if [[ -z "$body" ]]; then
+      printf 'Issue update FAILED: HTTP %s with an empty response body. A real update echoes the issue JSON; treat this write as not saved.\n' "$http_code" >&2
+      exit 1
+    fi
+    if [[ -n "$status" ]]; then
+      returned_status="$(jq -r '.status // empty' <<<"$body" 2>/dev/null || true)"
+      if [[ "$returned_status" != "$status" ]]; then
+        printf 'Issue update FAILED: server echoed status %s instead of requested %s.\n' "${returned_status:-<none>}" "$status" >&2
+        printf '%s\n' "$body" >&2
+        exit 1
+      fi
+    fi
+    printf '%s\n' "$body"
+    exit 0
+  fi
+
+  # 4xx (other than 429) is a definitive rejection; retrying cannot change it.
+  if [[ "$curl_exit" -eq 0 && "$http_code" == 4* && "$http_code" != "429" ]]; then
+    printf 'Issue update rejected (HTTP %s).\n' "$http_code" >&2
+    [[ -n "$body" ]] && printf '%s\n' "$body" >&2
+    exit 1
+  fi
+
+  if (( attempt >= max_attempts )); then
+    printf 'Issue update FAILED after %d attempts (curl exit %s, HTTP %s). The status/comment was NOT saved — report this write as failed, do not assume it landed.\n' "$max_attempts" "$curl_exit" "${http_code:-000}" >&2
+    [[ -n "$body" ]] && printf '%s\n' "$body" >&2
+    exit 1
+  fi
+
+  printf 'Issue update attempt %d/%d failed (curl exit %s, HTTP %s); retrying...\n' "$attempt" "$max_attempts" "$curl_exit" "${http_code:-000}" >&2
+  sleep $((attempt * 2))
+  attempt=$((attempt + 1))
+done

server/src/__tests__/paperclip-issue-update-helper.test.ts

@@ -0,0 +1,186 @@
+import { spawn } from "node:child_process";
+import http from "node:http";
+import type { AddressInfo } from "node:net";
+import path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+
+// End-to-end coverage for scripts/paperclip-issue-update.sh: the helper must
+// only exit 0 when the server confirms the write by echoing the update, must
+// classify failures (retry connection-level faults and 5xx, never retry a
+// definitive 4xx), and must stop at two attempts total to honor the shared
+// bounded-write-retry rule.
+const HELPER_PATH = path.resolve("scripts/paperclip-issue-update.sh");
+
+interface HelperResult {
+  code: number | null;
+  stdout: string;
+  stderr: string;
+}
+
+interface RecordedRequest {
+  method: string;
+  url: string;
+  body: string;
+}
+
+describe("paperclip issue update helper", () => {
+  const cleanupFns: Array<() => Promise<void>> = [];
+
+  afterEach(async () => {
+    while (cleanupFns.length > 0) {
+      const cleanup = cleanupFns.pop();
+      if (!cleanup) continue;
+      await cleanup().catch(() => undefined);
+    }
+  });
+
+  async function startServer(
+    respond: (request: RecordedRequest, attempt: number, res: http.ServerResponse) => void,
+  ): Promise<{ baseUrl: string; requests: RecordedRequest[] }> {
+    const requests: RecordedRequest[] = [];
+    const server = http.createServer((req, res) => {
+      let body = "";
+      req.on("data", (chunk) => {
+        body += chunk;
+      });
+      req.on("end", () => {
+        const recorded: RecordedRequest = {
+          method: req.method ?? "",
+          url: req.url ?? "",
+          body,
+        };
+        requests.push(recorded);
+        respond(recorded, requests.length, res);
+      });
+    });
+    await new Promise<void>((resolve) => {
+      server.listen(0, "127.0.0.1", resolve);
+    });
+    cleanupFns.push(
+      () =>
+        new Promise<void>((resolve) => {
+          server.close(() => resolve());
+        }),
+    );
+    const { port } = server.address() as AddressInfo;
+    return { baseUrl: `http://127.0.0.1:${port}`, requests };
+  }
+
+  function runHelper(apiUrl: string, args: string[]): Promise<HelperResult> {
+    return new Promise((resolve, reject) => {
+      const child = spawn("bash", [HELPER_PATH, ...args], {
+        env: {
+          ...process.env,
+          PAPERCLIP_API_URL: apiUrl,
+          PAPERCLIP_API_KEY: "test-key",
+          PAPERCLIP_RUN_ID: "test-run",
+        },
+        stdio: ["ignore", "pipe", "pipe"],
+      });
+      let stdout = "";
+      let stderr = "";
+      child.stdout.on("data", (chunk) => {
+        stdout += chunk;
+      });
+      child.stderr.on("data", (chunk) => {
+        stderr += chunk;
+      });
+      child.on("error", reject);
+      child.on("close", (code) => resolve({ code, stdout, stderr }));
+    });
+  }
+
+  const doneArgs = ["--issue-id", "issue-1", "--status", "done", "--comment", "closing note"];
+
+  it("exits 0 and prints the issue JSON when the server echoes the requested status", async () => {
+    const { baseUrl, requests } = await startServer((request, _attempt, res) => {
+      const payload = JSON.parse(request.body) as { status?: string; comment?: string };
+      res.writeHead(200, { "content-type": "application/json" });
+      res.end(JSON.stringify({ id: "issue-1", status: payload.status }));
+    });
+
+    const result = await runHelper(baseUrl, doneArgs);
+
+    expect(result.code).toBe(0);
+    expect(JSON.parse(result.stdout)).toMatchObject({ id: "issue-1", status: "done" });
+    expect(requests).toHaveLength(1);
+    expect(requests[0]?.method).toBe("PATCH");
+    expect(requests[0]?.url).toBe("/api/issues/issue-1");
+    expect(JSON.parse(requests[0]?.body ?? "{}")).toEqual({ status: "done", comment: "closing note" });
+  });
+
+  it("fails an empty 2xx body instead of treating it as success", async () => {
+    const { baseUrl } = await startServer((_request, _attempt, res) => {
+      res.writeHead(200, { "content-length": "0" });
+      res.end();
+    });
+
+    const result = await runHelper(baseUrl, doneArgs);
+
+    expect(result.code).toBe(1);
+    expect(result.stdout).toBe("");
+    expect(result.stderr).toContain("empty response body");
+  });
+
+  it("fails when the server echoes a different status than requested", async () => {
+    const { baseUrl } = await startServer((_request, _attempt, res) => {
+      res.writeHead(200, { "content-type": "application/json" });
+      res.end(JSON.stringify({ id: "issue-1", status: "in_progress" }));
+    });
+
+    const result = await runHelper(baseUrl, doneArgs);
+
+    expect(result.code).toBe(1);
+    expect(result.stderr).toContain("echoed status in_progress");
+  });
+
+  it("does not retry a definitive 4xx rejection", async () => {
+    const { baseUrl, requests } = await startServer((_request, _attempt, res) => {
+      res.writeHead(422, { "content-type": "application/json" });
+      res.end(JSON.stringify({ error: "validation" }));
+    });
+
+    const result = await runHelper(baseUrl, doneArgs);
+
+    expect(result.code).toBe(1);
+    expect(result.stderr).toContain("rejected (HTTP 422)");
+    expect(requests).toHaveLength(1);
+  });
+
+  it("retries a 5xx once and succeeds when the retry lands", async () => {
+    const { baseUrl, requests } = await startServer((request, attempt, res) => {
+      if (attempt === 1) {
+        res.writeHead(503, { "content-type": "application/json" });
+        res.end(JSON.stringify({ error: "unavailable" }));
+        return;
+      }
+      const payload = JSON.parse(request.body) as { status?: string };
+      res.writeHead(200, { "content-type": "application/json" });
+      res.end(JSON.stringify({ id: "issue-1", status: payload.status }));
+    });
+
+    const result = await runHelper(baseUrl, doneArgs);
+
+    expect(result.code).toBe(0);
+    expect(requests).toHaveLength(2);
+    expect(result.stderr).toContain("retrying");
+  });
+
+  it("stops after two attempts on connection-level failure and reports the write as not saved", async () => {
+    // Bind and close a listener so the port is real but refuses connections.
+    const probe = http.createServer();
+    await new Promise<void>((resolve) => {
+      probe.listen(0, "127.0.0.1", resolve);
+    });
+    const { port } = probe.address() as AddressInfo;
+    await new Promise<void>((resolve) => {
+      probe.close(() => resolve());
+    });
+
+    const result = await runHelper(`http://127.0.0.1:${port}`, doneArgs);
+
+    expect(result.code).toBe(1);
+    expect(result.stderr).toContain("FAILED after 2 attempts");
+    expect(result.stderr).toContain("NOT saved");
+  }, 15_000);
+});

server/src/__tests__/paperclip-skill-utils.test.ts

@@ -90,6 +90,17 @@ describe("paperclip skill utils", () => {
     expect(skillBody).toContain("`assigneeUserId` is null");
   });
 
+  it("requires issue-update writes to be verified, not inferred", async () => {
+    const skillBody = await fs.readFile(path.resolve("skills/paperclip/SKILL.md"), "utf8");
+
+    expect(skillBody).toContain("Verify writes — never infer them");
+    expect(skillBody).toContain("An empty response body means the write FAILED");
+    expect(skillBody).toContain("Never pipe a disposition write through `head`/`tail`");
+    // The helper's verification behavior (HTTP status parsing, retry
+    // classification, attempt bound, exit codes) is exercised end-to-end in
+    // paperclip-issue-update-helper.test.ts against a live local server.
+  });
+
   it("keeps the create-issue-interaction-ui guide as a maintainer-only skill", async () => {
     const skillPath = path.resolve(".agents/skills/create-issue-interaction-ui/SKILL.md");
     const skillBody = await fs.readFile(skillPath, "utf8");

skills/paperclip/SKILL.md

@@ -112,6 +112,8 @@ For technical upload instructions, read `references/artifacts.md`.
 
 **Bounded write retry.** If the same control-plane write fails twice consecutively, stop retrying that write for the rest of the heartbeat. Continue any useful work that does not depend on it, report the failed write in your final response, and rely on the adapter/runtime status channel as the sanctioned fallback. Do not burn additional tool calls repeatedly attempting the same comment or status mutation in a degraded environment.
 
+**Verify writes — never infer them.** A successful `PATCH /api/issues/{id}` always returns the updated issue JSON. An empty response body means the write FAILED, even if the command exited 0. Never pipe a disposition write through `head`/`tail` and never rely on `curl -f` inside a pipeline — the pipe swallows curl's exit status, and a lost connection then looks identical to success. Use `scripts/paperclip-issue-update.sh` (it checks the HTTP status, retries connection-level failures, and confirms the echoed `status`); if you must hand-roll curl, capture `-w '%{http_code}'` and check the response echoes your update. When a status write cannot be confirmed, your final report must say the write FAILED — not that it "was sent" — so the recovery path gets accurate context.
+
 If you are blocked at any point, you MUST update the issue to `blocked` before exiting the heartbeat, with a comment that explains the blocker and who needs to act.
 
 Before ending any heartbeat, apply this final-disposition checklist: