feat: auto-resolve chat identity from github.context by mafredri · Pull Request #20 · coder/create-agent-chat-action

mafredri · 2026-04-29T16:44:00Z

Auto-resolves the chat identity from github.context so workflows no longer need to wire github-user-id: ${{ github.event.sender.id }} on every job. Explicit coder-username and github-user-id still take precedence.

What changed

src/action.ts: extract resolveCoderUsername() with a four-source precedence (coder-username > github-user-id > payload.sender.id > actor). The constructor takes a required ActionContext. The sender guard mirrors the Zod constraint on github-user-id (integer, strictly positive). schedule events are refused before any auto-resolve source because their actor is the workflow file's last editor, not a triggering identity. Intermediate API failures are wrapped to name the auto-resolved source, preserve the upstream error, and recommend coder-username as the bypass.
src/index.ts: pass github.context into the action.
src/test-helpers.ts: add createMockContext helper.
src/action.test.ts: cover the four resolution paths, the precedence tie-breaker, partial-sender fallthrough, sender id 0 and non-integer fallthrough, schedule refusal (with and without sender.id), and three intermediate-failure wraps.
README.md: document the resolution order, the schedule exclusion, and the wrapped-failure contract; drop the now-redundant github-user-id from the labeled-issue example.
dist/index.js rebuilt.

Identity auto-resolve trust gate

Follow-up to address hostile-trigger threat model concerns raised in review.

src/action.ts: add a trust gate between the schedule refusal and source 3. The gate refuses auto-resolve when the trigger is a fork pull request (a null head.repo, pull_request.head.repo.fork === true, or head.repo.full_name !== base.repo.full_name), or when comment.author_association / review.author_association (in that priority) is not one of OWNER, MEMBER, or COLLABORATOR. The gate deliberately does not read issue.author_association or pull_request.author_association: those fields describe the resource opener, not the event sender, so reading them would refuse trusted operations like a MEMBER labeling a NONE user's issue. Payloads with no fork signal and no comment / review association data (e.g. issues, workflow_dispatch, push, repository_dispatch) return no-signal and defer to GitHub's underlying event-permission model. Explicit coder-username / github-user-id inputs run before the gate and bypass it by design.
src/action.test.ts: trust-gate tests covering fork-PR refusal (with fork flag, via full_name mismatch, and via null head.repo), low-trust association refusals on comment and review, trusted-association acceptance via comment.author_association, comment-over-review precedence, the realistic issues: [labeled] MEMBER-labels-NONE-opener auto-resolve, the coder-username and github-user-id bypasses on a fork PR, and workflow_dispatch no-signal pass-through.
README.md: new ### Auto-resolve trust gate section under ## Identity resolution documenting the gate, why it ignores issue / pull_request associations, the no-signal deferral, the threat model, and the relationship to pull_request_target.
dist/index.js rebuilt.

Threat model summary

Without the gate, a workflow that runs on pull_request or issue_comment and lets the action auto-resolve identity would bind whichever Coder user matches sender.id to the chat run. An attacker who happens to have a Coder identity could then open a fork PR or drop a comment on an issue to execute attacker-controlled prompts under the workflow's Coder session token. The gate refuses these triggers by default; workflow authors who want to triage drive-by issues opt in via coder-username or github-user-id.

The shared-idempotency chat-hijack concern (user Y sending messages into user X's chat via a gh-target: <owner>/<repo>#<n> label) is deferred to the idempotency slice, which owns the listChats filter shape. The recommendation there is to scope the lookup to the resolved Coder user id in addition to gh-target.

Closes CODAGT-284

🤖 Authored by Coder Agents.

mafredri · 2026-04-29T16:44:18Z

/coder-agents-review

mafredri · 2026-04-29T17:24:43Z

/coder-agents-review

coder-agents-review

Clean extraction of resolveCoderUsername() from run() with a well-designed data-presence-driven fallback chain. The four-source precedence is clearly documented, the structural ActionContext interface keeps the dependency surface small, and the TDD commit sequence (red tests, implementation, dist rebuild) is solid. The terminal error message at line 203 is a model: names the operation, names the context, prescribes two fixes.

Three P2s, seven P3s, two nits.

The P2s share a theme: the resolution chain's happy paths are well-tested and well-documented, but the intermediate failure and edge-case paths have gaps. When source 3 or 4 finds a value but the subsequent API call fails, the error that propagates is the raw upstream error with no mention of coder-username as a workaround. The validation applied to github-user-id (Zod: positive integer) is not applied to sender.id (bare typeof === "number"), opening a path to a bare-string throw. And for schedule events, context.actor silently resolves to the workflow file's last editor rather than failing visibly.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

mafredri · 2026-04-29T19:34:19Z

/coder-agents-review

coder-agents-review

All 12 round-1 findings verified as fixed. The three P2s (error wrapping, sender.id validation, schedule exclusion) are addressed at root cause with pinning tests. The error-wrapping pattern is thorough: each intermediate failure names the auto-resolved source, preserves the upstream error, and recommends coder-username as bypass. The schedule exclusion throws a clear error instead of silently misattributing. The sender.id guard mirrors the Zod constraint.

Two new P3s, one nit. Both P3s are documentation/structural, not behavioral.

DERM-14 is a fix-chain artifact from DEREM-1: the code logic is correct but the comment and README still frame source 4 as the workflow_dispatch path, contradicted by the test suite's own inline comment. DEREM-15 is a defensive-coding concern about the schedule guard's placement relative to the sender.id check.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

mafredri · 2026-04-30T11:38:35Z

/coder-agents-review

coder-agents-review

All 16 findings across three rounds are resolved. No new findings from Netero or the panel.

The resolver's four-source precedence chain is correctly implemented with semantic guards, thorough error wrapping, and 16 dedicated identity-resolution tests. The schedule exclusion is now semantic (fires before any auto-resolve source), the sender.id guard mirrors the Zod schema exactly (Number.isInteger + > 0), and the documentation accurately describes when each source activates.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

mafredri · 2026-04-30T13:40:35Z

/coder-agents-review

coder-agents-review

Post-rebase verification. The PR was rebased onto S1 round 3 (comment prefix change, buildOutputs hasPR gate, coder-organization warning, schema cleanup). All 16 prior findings remain fixed. The rebase accommodation commits (passing createMockContext to new S1 test sites, removing planning references) are correct. No new issues introduced.

Four rounds, 16 findings raised, all resolved. The identity resolution implementation is solid.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

mafredri · 2026-04-30T14:42:44Z

/coder-agents-review

coder-agents-review

Post-rebase verification (S1 R4). Identity resolution code unchanged. All 16 prior findings remain fixed. One new P3 and one nit from the test suite.

DEREM-17 is a repeat of the DEREM-10 pattern: the sender id=0 and id=1.5 fallthrough tests were added after the DEREM-10 fix and repeat the same defect (discarding the return value). DEREM-18 is a test hygiene issue where identity tests produce swallowed TypeErrors from the unmocked commenting API.

mafredri · 2026-04-30T15:31:35Z

/coder-agents-review

coder-agents-review

DEREM-17 and DEREM-18 verified fixed. All 18 findings across six rounds are resolved. No new findings from Netero or the panel.

Six rounds, 18 findings (3 P2, 11 P3, 3 Nit, 1 Note dropped), all resolved. The identity resolution implementation is complete and well-tested.

mafredri · 2026-05-11T09:14:18Z

/coder-agents-review

mafredri · 2026-05-11T09:14:33Z

Pushed two commits at ef5d184 (HEAD).

What's new

1e8b7f9 feat(create-agent-chat-action): trust-gate identity auto-resolve adds the trust gate, ActionContext extensions, and ten new tests.
ef5d184 chore(dist): rebuild dist after identity auto-resolve trust gate regenerates the bundle.

Threat model

Hostile-trigger (fork PR or drive-by comment under pull_request / issue_comment): without a gate, the action would bind whichever Coder user matched sender.id to the chat run, which means an attacker who happens to have a Coder identity could execute attacker-controlled prompts under the workflow's Coder session token. The gate now refuses these triggers by default. The gate sits between the schedule refusal and source 3 (sender.id), so it never runs when the workflow author explicitly set coder-username or github-user-id.

Trust signal precedence (most specific first):

Fork PR check: pull_request.head.repo.fork === true OR head.repo.full_name !== base.repo.full_name → refuse.
comment.author_association → review.author_association → pull_request.author_association → issue.author_association; first present decides; only OWNER, MEMBER, COLLABORATOR pass.
No association data and not a fork PR → no-signal, defer to GitHub's event-permission model (secret access, branch protection, workflow_dispatch write permission, etc.).

This mirrors the explicit gh api .../collaborators/<user>/permission gate the Coder workflows use for chat triage, expressed declaratively against payload fields the action already has.

Chat-hijack via shared idempotency label

The gh-target: <owner>/<repo>#<n> lookup attack (user Y sending messages into user X's chat) is owned by the idempotency wiring slice, not this one. The recommendation for that slice is to scope listChats to the resolved Coder user id in addition to the gh-target label, so the dedup key is (user, target), not just target. I'll carry that forward into the idempotency-slice plan rather than try to retrofit it here.

Verification

bun test: 111 pass, 0 fail (10 new tests in the trust-gate block).
bun run typecheck, bun run lint, bun run format clean.
bun run build reproducible: ./check_unstaged.sh clean after rebuild.
CI re-triggered: https://github.com/coder/create-agent-chat-action/actions/runs/25661233477.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

coder-agents-review

The trust gate architecture is well-designed: fork detection with dual signals, most-specific-first association priority, no-signal deferral for permission-gated events, and clean separation of classifyAutoResolveTrust as a pure function. The discriminated union (trusted/untrusted/no-signal) is the right abstraction.

One P2, three P3s. The P2 is a semantic mismatch between what the gate reads and what it needs: issue.author_association describes the issue opener, not the event sender. For issues.labeled, a MEMBER labeling a NONE user's issue gets refused. This breaks the README's primary example, which removed github-user-id as "now-redundant."

Four reviewers converged independently on the same P2, each tracing the same scenario through the code. Kite's fix option A (drop issue/pull_request from the associations array, keep only comment/review which reliably describe the sender) is the cleanest path.

mafredri · 2026-05-11T09:44:15Z

/coder-agents-review

coder-agents-review

Further review is blocked until the author responds to or pushes fixes for the four open findings from round 7.

Unaddressed findings:

DEREM-19 (P2): Trust gate checks issue/PR author's author_association instead of event sender's. Breaks the README's issues.labeled example when a MEMBER labels an issue opened by a NONE user. Four reviewers converged on this independently.
DEREM-20 (P3): Fork detection blind to null head.repo (deleted fork).
DEREM-21 (P3): No test exercises review.author_association as a trust gate signal.
DEREM-22 (P3): Happy-path tests use unrealistic payloads that mask DEREM-19.

The head SHA changed (rebase onto S1 R4), but no code or comment addresses these findings.

mafredri · 2026-05-11T11:12:52Z

/coder-agents-review

coder-agents-review

All four R7 trust-gate findings verified fixed. The gate now correctly reads only comment.author_association and review.author_association (sender-reliable fields), ignoring issue/pull_request associations that describe the resource opener. Fork detection handles null head.repo. The realistic issues.labeled test (MEMBER labels NONE opener's issue) passes under the narrowed gate and would have failed under the old one.

One Nit from Netero: the ActionContext doc comment at line 51 still lists issue as read by the trust gate. Stale after DEREM-19 fix.

Nine rounds, 23 findings (4 P2, 14 P3, 4 Nit, 1 Note dropped), 22 resolved, 1 Nit open.

mafredri · 2026-05-11T11:39:13Z

/coder-agents-review

coder-agents-review

DEREM-23 verified fixed. All 23 findings across ten rounds are resolved. No new findings.

Ten rounds, 23 findings (4 P2, 14 P3, 4 Nit, 1 Note dropped), all resolved. The identity resolution and trust gate implementation is complete.

mafredri · 2026-05-11T16:21:41Z

/coder-agents-review

coder-agents-review

Docs-only commit adding service-account mode documentation to the README. All 23 findings remain resolved. No new findings from Netero or the panel.

Round 1 review on PR #20 surfaced twelve findings (three P2, seven P3, two nits) clustered around: stale documentation about which events deliver `sender`, opaque intermediate failure messages, sender input not validated like the explicit `github-user-id`, schedule events silently misattributing the chat, and several test gaps. What changed: - Resolver: tighten the sender guard to `> 0` to match the `github-user-id` Zod constraint (rejects 0 and NaN). Refuse `schedule` events explicitly because `context.actor` for cron-triggered runs is the workflow file's last editor, not a triggering identity. Wrap each intermediate API call so failures name the auto-resolved source (sender id or actor login), preserve the upstream error, and recommend `coder-username` as the bypass. - Constructor: `context` is now required. The seventeen pre-existing test sites pass `createMockContext()` as the fourth argument, so a future refactor that drops the production-side context wiring fails at compile time. - Comments and README: replace the inaccurate `workflow_dispatch` references in the actor-fallback comment, the README, and a test name; document the schedule exclusion and the wrapped-failure contract. - Tests: add the precedence tie-breaker (#1 vs #2), partial-sender fallthrough, sender id 0 fallthrough, schedule refusal, and three intermediate-failure path tests. Replace the redundant `pull_request` test (same code path as the issues case). Add return-value assertions to the four happy-path tests. Out of scope: - The bare-string `throw` at `coder-client.ts:69` (DEREM-3 secondary). The sender guard prevents the auto-resolve path from reaching it, but the underlying defect remains and is owned by S6's slice scope. Refs CODAGT-284.

…e (round 2) Round 2 review on PR #20 came back APPROVED with three follow-up findings, all documentation/structural artifacts of the round-1 fixes: - DEREM-14 (P3): the actor-branch comment and README source 4 still framed source 4 as the `workflow_dispatch` path, contradicted by the test fixture's own inline note that `workflow_dispatch` payloads include `sender.id` (so source 3 handles them). - DEREM-15 (P3): the schedule guard sat in the actor branch, after the sender.id check. Today's `schedule` payloads omit `sender`, so the two paths converge by accident, but if a future `schedule` payload shape ever delivers `sender.id` (GHES extensions, custom dispatch chains), source 3 would silently resolve and reintroduce DEREM-4's misattribution. - DEREM-16 (Nit): the sender guard claimed to mirror Zod's `z.number().int().positive()` but only checked `typeof === "number" && > 0`, admitting non-integer numbers. What changed: - Move the `schedule` guard above the sender.id check so the exclusion is semantic, not positional. New test `refuses to auto-resolve schedule events even when sender.id is present` pins it. - Tighten the sender guard to `typeof === "number" && Number.isInteger(senderId) && > 0`, matching the Zod schema exactly. New test `treats non-integer sender id as missing and falls through to actor` covers the `1.5` case. - Reword the actor-branch comment, the `resolveCoderUsername` doc comment, the README "Identity resolution" section, and the Overview bullet to describe what source 4 actually covers (events whose payload lacks a usable `sender.id`) instead of naming `workflow_dispatch`. All 85 tests pass; the round 1 contract (wrapped failures, schedule exclusion, return-value assertions) holds. Refs CODAGT-284.

…text Auto-resolve the Coder user from `github.context` when neither `coder-username` nor `github-user-id` is provided. Resolution order: 1. `coder-username` input (direct). 2. `github-user-id` input (Coder lookup by GitHub id). 3. `context.payload.sender.id` (sender-driven webhook events). 4. `context.actor` (events whose payload omits a usable sender.id). A trust gate refuses auto-resolve before sources 3 and 4 when the trigger is a fork pull request (`pull_request.head.repo` is null, `head.repo.fork === true`, or `head.repo.full_name !== base.repo.full_name`) or when `comment.author_association` / `review.author_association` (in that priority) is not `OWNER`, `MEMBER`, or `COLLABORATOR`. The gate does not read `issue.author_association` or `pull_request.author_association`: those describe the resource opener, not the event sender. Setting `coder-username` or `github-user-id` bypasses the gate by design. `schedule` events are refused before any auto-resolve source: their `actor` is the workflow file's last editor, not a triggering user. Constructor signature: adds a required `context: ActionContext` parameter between `inputs` and `clock`. The `Clock` parameter remains optional with a default. Co-changes: README documents the resolution order, the trust gate, the threat model, the no-signal deferral to GitHub's permission model, and a copy-pasteable service-account mode example for workflows like doc-check that want one identity for every event.

mafredri force-pushed the s2-identity-sEwMTa branch 2 times, most recently from 250b41c to 2212937 Compare April 29, 2026 16:59

coder-agents-review Bot reviewed Apr 29, 2026

View reviewed changes

mafredri force-pushed the s2-identity-sEwMTa branch from 5868d6b to 4e94f0e Compare April 29, 2026 18:24

coder-agents-review Bot approved these changes Apr 29, 2026

View reviewed changes

Comment thread src/action.ts Outdated

Comment thread src/action.ts Outdated

Comment thread src/action.ts Outdated

mafredri force-pushed the s2-identity-sEwMTa branch from 011205a to fe4d632 Compare April 29, 2026 21:09

coder-agents-review Bot approved these changes Apr 30, 2026

View reviewed changes

mafredri force-pushed the s2-identity-sEwMTa branch from 6f313fe to cc6b864 Compare April 30, 2026 13:11

coder-agents-review Bot approved these changes Apr 30, 2026

View reviewed changes

mafredri force-pushed the s2-identity-sEwMTa branch from cc6b864 to 7641630 Compare April 30, 2026 14:23

coder-agents-review Bot approved these changes Apr 30, 2026

View reviewed changes

Comment thread src/action.test.ts Outdated

Comment thread src/action.test.ts

coder-agents-review Bot approved these changes Apr 30, 2026

View reviewed changes

mafredri force-pushed the s2-identity-sEwMTa branch from ef5d184 to 1751b10 Compare May 11, 2026 09:17

coder-agents-review Bot reviewed May 11, 2026

View reviewed changes

Comment thread src/action.ts Outdated

Comment thread src/action.ts

Comment thread src/action.test.ts Outdated

Comment thread src/action.test.ts

coder-agents-review Bot reviewed May 11, 2026

View reviewed changes

coder-agents-review Bot approved these changes May 11, 2026

View reviewed changes

Comment thread src/action.ts Outdated

coder-agents-review Bot approved these changes May 11, 2026

View reviewed changes

johnstcn approved these changes May 11, 2026

View reviewed changes

mafredri changed the base branch from s1-schema-VOtF3n to main May 11, 2026 16:36

mafredri force-pushed the s2-identity-sEwMTa branch from 8d3fd48 to ee0f5a3 Compare May 11, 2026 16:39

mafredri marked this pull request as ready for review May 11, 2026 16:42

mafredri changed the title ~~feat(create-agent-chat-action): S2 identity auto-resolve from github.context~~ feat: auto-resolve chat identity from github.context May 11, 2026

mafredri added 2 commits May 11, 2026 16:50

chore(dist): rebuild dist after rebase onto main with S4 and S6

1283f5a

mafredri force-pushed the s2-identity-sEwMTa branch from ee0f5a3 to 1283f5a Compare May 11, 2026 16:50

mafredri merged commit a290f9a into main May 11, 2026
2 checks passed

Conversation

mafredri commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Identity auto-resolve trust gate

Threat model summary

Uh oh!

mafredri commented Apr 29, 2026

Uh oh!

mafredri commented Apr 29, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mafredri commented Apr 29, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mafredri commented Apr 30, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mafredri commented Apr 30, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mafredri commented Apr 30, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mafredri commented Apr 30, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mafredri commented May 11, 2026

Uh oh!

mafredri commented May 11, 2026

What's new

Threat model

Chat-hijack via shared idempotency label

Verification

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mafredri commented May 11, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mafredri commented May 11, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mafredri commented Apr 29, 2026 •

edited

Loading