feat: opt in to chat reuse by idempotency-key

[mafredri]

Adds opt-in idempotency by chat label. When idempotency-label-key is set, the action lists chats filtered by the sanitized label scoped to the GitHub target and reuses the most recent non-archived match (sending a follow-up via createChatMessage) instead of creating a duplicate. Default behavior (key unset) is unchanged: always create, no listChats call.

What changed

• src/coder-client.ts: listChats takes optional { label, archived } and emits one repeated ?label=k:v query parameter per entry plus ?q=archived:false when requested. CreateChatRequest gains an optional labels map.
• src/action.ts: when idempotency-label-key is set and existing-chat-id is not, look up chats by <sanitized-key>:true ANDed with gh-target:<owner>/<repo>#<n>, sort by updated_at desc, send a follow-up to the most recent match (re-fetching afterward so outputs reflect post-message state), and core.warning listing duplicates on multi-match. On lookup miss (or all-archived), create a new chat carrying coder-agent-chat-action: "true", gh-target: <owner>/<repo>#<n>, and <sanitized-key>: "true".
• src/sanitize-label-key.ts (new): sanitizeLabelKey enforces the chats API's chat-label regex ([a-z0-9._/-], 64-byte cap) on the user input. RESERVED_LABEL_KEYS are checked upstream of the lookup so an idempotency-label-key: "gh-target" is rejected before any API call.
• src/comment.ts: commentOnIssue accepts a verb parameter ("created" default, "reused" for the auto-reuse path); deriveCommentKey runs the user-provided key through sanitizeLabelKey so it cannot break the <!-- coder-agent-chat-action:KEY --> HTML-comment boundary.
• src/test-helpers.ts: MockCoderClient.listChats accepts the typed ListChatsOptions argument.
• src/action.test.ts, src/coder-client.test.ts, src/comment.test.ts, src/sanitize-label-key.test.ts: cover unset, no-match, single match, race, archived-only, existing-chat-id wins, lookup-throws, reserved-key collision, the multi-label ?label= URL contract, the marker-injection regression, and the four documented sanitizeLabelKey edge cases.
• README.md: new "Idempotency by label" section covering the label set, sanitization rule (with an example), lookup query, race limitation, and the future Idempotency-Key header path.
• dist/index.js rebuilt.

---

Closes CODAGT-289

🤖 Authored by Coder Agents.

[mafredri]

/coder-agents-review

[mafredri]

/coder-agents-review

[coder-agents-review]

First-pass review (Netero). 2 P2, 2 P3, 1 Nit. Findings below are mechanical checks from the first-pass reviewer. The full review panel has not yet reviewed this PR; it will run after these are addressed.

The idempotency feature is well-structured: clean separation between lookup, label construction, and follow-up; solid test coverage for the integration paths; and a clear README section documenting the contract and known limitations.

"If two workflow runs trigger at the same time with the same idempotency-label-key, both can pass the lookup before either creates its chat, and both will create." Netero liked that the race is documented and the warning message names the duplicates.

🤖 This review was automatically generated with Coder Agents.

[coder-agents-review]

Round 2 is blocked. All 5 findings from the first-pass review (Round 1) are silent: no code changes, no author responses.

Unaddressed findings:

• DEREM-1 (P2) src/action.ts:397: sanitizeLabelKey has no direct unit tests for its four documented edge cases.
• DEREM-2 (P2) src/action.ts:379: buildIdempotencyLabels allows the sanitized key to collide with reserved label keys (gh-target, coder-agent-chat-action).
• DEREM-3 (P3) src/action.ts:362: Warning message includes the reused chat in the "ignoring" list.
• DEREM-4 (P3) src/action.ts:268: Idempotency-match follow-up path duplicates existing-chat-id path.
• DEREM-5 (Nit) src/test-helpers.ts:114: Mock uses inline type instead of exported ListChatsOptions.

Further review is blocked until the author pushes fixes or responds to the findings. The full review panel will run after these are addressed.

🤖 This review was automatically generated with Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

Panel review (18 reviewers). All 5 R1 findings verified fixed. 3 P2, 10 P3, 1 Note, 3 Nit new.

The DEREM-4 fix (sendFollowUpMessage extraction) is clean, and the sanitizeLabelKey unit tests are thorough. The collision guard, the warning message regression test, and the ListChatsOptions mock fix are all solid.

Three P2 findings stand out. The highest-convergence finding (11/18 reviewers) is the stale outputs on the idempotency-match path: sendFollowUpMessage unified the message logic but left the getChat re-fetch only in the existing-chat-id caller. The cross-issue contamination finding (3 reviewers) traces a scenario where a static idempotency-label-key routes Issue #2's follow-up into Issue #1's chat. The collision check bypass (3 reviewers) notes that DEREM-2's fix only covers the create path; the match path never reaches buildIdempotencyLabels.

"Texture Surprise: sendFollowUpMessage looks like the complete follow-up abstraction, but it only handles half the contract." (Hisoka)

---

src/comment.ts:53

P3 [DEREM-18] deriveCommentKey returns inputs.idempotencyLabelKey verbatim (line 53). buildCommentMarker wraps it in <!-- coder-agent-chat-action:KEY -->. If the key contains -->, the HTML comment boundary breaks and everything after renders as visible Markdown. Attack chain: a workflow sets idempotency-label-key: ${{ github.event.label.name }}, an attacker creates a label with --> in it, the action fails, and the failure comment renders attacker-controlled Markdown (phishing links). GitHub sanitizes <script> but renders Markdown links as clickable.

The code is pre-existing, but this PR makes idempotencyLabelKey a documented first-class input. Fix: use sanitizeLabelKey(inputs.idempotencyLabelKey) instead of the raw value, since the sanitized output contains only [a-z0-9._/-]. (Kurapika)

🤖

src/action.ts:365

P3 [DEREM-19] listChats errors in findIdempotentMatch surface as generic "Coder API error: Bad Request" with no operation context. The error propagates through classifyError which discards the response body. The workflow log gives no indication it was the idempotency label lookup, what label was used, or what the API said. The listChats call with a label filter is the most likely new source of 400s in this PR (new query parameter, experimental endpoint).

Wrap the call so the error names the operation and the label:

try {
  chats = await this.coder.listChats({ label });
} catch (err) {
  throw new Error(
    `Failed to look up chats by idempotency label "${label}": ${err instanceof Error ? err.message : err}`,
    { cause: err },
  );
}

(Leorio)

🤖

🤖 This review was automatically generated with Coder Agents.

[mafredri]

In reply to DEREM-18 (P3, src/comment.ts:53): #24 (review)

deriveCommentKey now runs inputs.idempotencyLabelKey through sanitizeLabelKey before returning. The sanitized output is restricted to [a-z0-9._/-], which cannot break the <!-- coder-agent-chat-action:KEY --> HTML-comment boundary. This closes the markdown-injection vector when a workflow uses an attacker-controllable label name (e.g. idempotency-label-key: ${{ github.event.label.name }}).

This touches comment.ts, which is technically S5 land. Justified because S7 is the slice that turns idempotency-label-key into a documented first-class input; the helper accepts that input directly. Behavior change for S5: a marker like <!-- coder-agent-chat-action:My Key --> becomes <!-- coder-agent-chat-action:my-key -->. Both writers and readers go through deriveCommentKey, so S5's find-by-marker behavior remains stable as long as a workflow does not switch from sanitized to raw between runs (it cannot, since the helper is the only entry point).

Added a regression test in comment.test.ts that passes "x-->[click](http://evil)<!--" and asserts the result matches the platform key regex and contains no <, >, or -->. Fix in commit 819b8bb.

---

In reply to DEREM-19 (P3, src/action.ts:365): #24 (review)

findIdempotentMatch wraps listChats with try/catch and re-throws an Error whose message names the operation and the exact label filters: Failed to look up chats by idempotency labels [<key>:true, gh-target:<value>]: <inner> with cause: err preserved for debuggers. Combined with DEREM-14's new test, this pins both the contextful error message and the no-silent-fall-through behavior. Fix in commit 819b8bb.

🤖 Posted using /amend-review skill via Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

Verification round (6 reviewers). All 16 R3 findings verified fixed. 4 P3 new, no P2.

The R3 P2 fixes are solid: the getChat re-fetch on the idempotency path mirrors the existing-chat-id pattern, the multi-label AND scoping eliminates cross-issue contamination, and the collision check hoist covers both paths. The HTML injection fix in deriveCommentKey is correct and tested.

The fix commit (819b8bb) includes structural refactoring beyond the 20 findings (outputs.ts extraction, ClassifiedActionError, FailureDetail discriminated union, classifyError restructuring). Each change is individually clean, but reviewers scoping to the findings would miss it. Worth noting for human review.

"Texture Surprise: sendFollowUpMessage looks like the complete follow-up abstraction, but it only handles half the contract." (Hisoka, R3) The stale-outputs half is now fixed; the verb half has one remaining caller (DEREM-27).

🤖 This review was automatically generated with Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

All 31 prior findings resolved (26 author-fixed, 5 dropped). 1 minor P3 test gap remaining (DEREM-32). 144/144 tests pass. CI green.

5 rounds, 3 panel reviews (18 + 6 + 3 reviewers). The three R3 P2 findings (stale outputs, cross-issue contamination, unreachable collision check) are correctly fixed and verified. The R4 P3 findings (verb propagation, predicate tightening, fallback test, verb test) are addressed.

Notable quality in the fix cycle: multi-label AND scoping for cross-issue isolation, defense-in-depth on the collision guard, getChat refresh fallback that degrades to stale data rather than failing the action, and the tightened comment predicate regex.

One item for human review: the fix commit (819b8bb) includes structural refactoring beyond the findings (outputs.ts extraction, ClassifiedActionError, FailureDetail discriminated union, classifyError restructuring). Each change is clean; the scope was undisclosed.

---

src/action.test.ts:477

P3 [DEREM-32] The "sends message to existing chat" test mocks createComment (line 486) but never asserts the body. This PR changed the existing-chat-id path to pass "reused" to sendFollowUpMessage (DEREM-27 fix). If someone removes that argument, the test passes silently and the comment reverts to "Agent chat created:". The idempotency-match test at line 1311 asserts /^Agent chat reused: / for its path, but the existing-chat-id path has no body assertion.

Fix: add expect(commentCall?.body).toMatch(/^Agent chat reused: /) after the existing assertions. (Bisky)

🤖

🤖 This review was automatically generated with Coder Agents.

[mafredri]

In reply to DEREM-32 (P3, src/action.test.ts:477): #24 (review)

Added expect(commentCall?.body).toMatch(/^Agent chat reused: /) to the "sends message to existing chat" test. A regression that drops the "reused" argument from the existing-chat-id call site now fails this test instead of silently reverting to Agent chat:. Fix in commit 2e401ea.

🤖 Posted using /amend-review skill via Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

All 32 findings resolved. Zero open items. CI green.

DEREM-32 (the last open P3, existing-chat-id verb assertion) fixed in commit 943d7fb. The test now asserts /^Agent chat reused: / on the comment body for the existing-chat-id path, matching the idempotency path's assertion.

6 rounds, 32 findings tracked, 27 author-fixed, 5 dropped. Clean.

🤖 This review was automatically generated with Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

All 32 findings resolved. Zero open items. 145/145 tests pass. CI green.

Delta since R6: one comment-only cleanup commit (9613bb30, "trim verbose code comments"). No behavior change. Netero confirmed all 8 sections clean.

7 rounds, 32 findings tracked, 27 author-fixed, 5 dropped. Ship it.

🤖 This review was automatically generated with Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

Verification of per-user scoping delta (4 reviewers). 3 P3 new, no P2. Prior 32 findings remain resolved.

The new commits add per-user idempotency scoping (coder-agent-chat-action-user:<userId> label), rename idempotency-label-key to idempotency-key, and add getCoderUserByUsername. The core logic is sound: the user ID is resolved early, threaded into both the lookup and the label builder, and tested via a cross-user hijack scenario. 154/154 tests pass.

Three test coverage gaps and one operational signal issue noted below.

---

src/action.test.ts:169

P3 [DEREM-34] No test pins the predicate match for an existing "Agent chat reused:" comment. The "updates existing Agent chat comment" test (line 169) only matches "Agent chat: old-url". If the optional group ( reused)? were removed from the regex during a refactor, a second reuse run would miss the first reuse comment and create a duplicate. No test catches it.

Sketch: set up an existing comment with body "Agent chat reused: old-url", call commentOnIssue with verb="reused", assert updateComment is called on that comment. (Bisky)

🤖

🤖 This review was automatically generated with Coder Agents.

[mafredri]

In reply to DEREM-34 (P3, src/action.test.ts:169): #24 (review)

Added a commentOnIssue test that seeds an existing "Agent chat reused: old-url" comment, invokes with verb="reused", and asserts updateComment is called on that comment (and createComment is not). Dropping the optional ( reused)? group from the predicate during a refactor now fails this test instead of silently creating a duplicate. Fix in commit 4f9bfd1.

🤖 Posted using /amend-review skill via Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

All 35 findings resolved (30 author-fixed, 5 dropped). Zero open items. CI green.

DEREM-33 through DEREM-35 (R8 P3s) verified fixed: per-user scoping tested via both identity resolution paths, "Agent chat reused:" predicate match pinned, and getCoderUserByUsername 404 now maps to user_not_found.

9 rounds, 35 findings tracked. Ship it.

🤖 This review was automatically generated with Coder Agents.

[mafredri]

/coder-agents-review

[coder-agents-review]

All 35 findings resolved. Zero open items. CI green. Rebase only since R9 (one action.yaml description tweak, no source changes).

10 rounds, 35 findings tracked, 30 author-fixed, 5 dropped.

🤖 This review was automatically generated with Coder Agents.