diff --git a/README.md b/README.md index ac0a6ea..dcfac39 100644 --- a/README.md +++ b/README.md @@ -2,11 +2,12 @@ GitHub Action that starts a [Coder Agents](https://coder.com/docs/ai-coder/agents) chat against a GitHub issue or pull request, optionally waits for it to finish, and posts the result as a comment. Re-running the workflow on the same target continues the existing chat instead of duplicating. +The chat owner is always the user the `coder-token` belongs to. Read [Security model](#security-model) before adopting on a public repo. + ## Requirements - Coder deployment with Agents enabled (experimental). -- Coder session token with permission to read users in the target organization and to create chats. -- For GitHub-id identity resolution: deployment configured with [GitHub OAuth](https://coder.com/docs/admin/external-auth#configure-a-github-oauth-app) and the Coder user has linked their GitHub account. +- Coder session token belonging to the user the chats should run as. Treat this token as a high-value secret: anyone holding it acts as that Coder user via the agent's tool plane (see [Security model](#security-model)). ## Quickstart @@ -38,27 +39,25 @@ jobs: github-token: ${{ github.token }} ``` -The chat runs under the Coder user linked to the GitHub user who applied the label. Set `coder-username` for service-account workflows. +The chat runs as whoever the `coder-token` belongs to; that identity is the only one the chats API supports. Workflows that gate triggers loosely (`issue_comment`, `pull_request_target`, etc.) own the trust decision via an `if:` filter; see [Security model](#security-model). ## Inputs | Name | Required | Default | Description | | ---------------------- | -------- | ------- | ----------- | | `coder-url` | yes | | Coder deployment URL. | -| `coder-token` | yes | | Coder session token. | +| `coder-token` | yes | | Coder session token. The user this token belongs to is the chat owner; the chats API has no owner override. | | `chat-prompt` | yes | | Prompt to send to the agent. | -| `github-url` | yes | | Issue or pull request URL. | +| `github-url` | yes | | Issue or pull request URL. Host is validated; only `https://github.com///issues/` and `https://github.com///pull/` are accepted. | | `github-token` | yes | | Used to post and update comments. | -| `coder-username` | no | | Run the chat as this Coder user. Mutually exclusive with `github-user-id`. Bypasses the [trust gate](#security-model). | -| `github-user-id` | no | | Resolve to a Coder user by linked GitHub id. Mutually exclusive with `coder-username`. | -| `coder-organization` | no | | Coder organization name. Recommended for multi-org users. | +| `coder-organization` | no | | Coder organization name. Recommended for multi-org token owners. | | `workspace-id` | no | | Pin the chat to an existing workspace. | | `model-config-id` | no | | Model configuration to use. | | `existing-chat-id` | no | | Send a follow-up to a known chat. Skips chat-reuse lookup. Mutually exclusive with `force-new-chat`. | | `comment-on-issue` | no | `true` | Post the result on `github-url`. | | `wait` | no | `none` | `complete` polls every 5s until terminal status or `wait-timeout-seconds`. | | `wait-timeout-seconds` | no | `600` | Max wait when `wait: complete`. | -| `idempotency-key` | no | | Optional sharding key. See [Chat reuse](#chat-reuse). | +| `idempotency-key` | no | | Optional sharding key on the reuse scope. See [Chat reuse](#chat-reuse). | | `force-new-chat` | no | `false` | Skip chat-reuse lookup and always create. Mutually exclusive with `existing-chat-id`. | ## Outputs @@ -70,7 +69,7 @@ The chat runs under the Coder user linked to the GitHub user who applied the lab | `chat-created` | `true` if newly created, `false` if a message was sent to an existing chat. | | `chat-status` | `waiting`, `pending`, `running`, `paused`, `completed`, `error`. | | `chat-title` | Chat title. | -| `coder-username` | Coder username the chat ran as. | +| `coder-username` | Coder username the `coder-token` belongs to (always the chat owner). | | `workspace-id` | Workspace UUID. | | `pull-request-url` | PR or branch URL when the chat tracks changes. | | `pull-request-state` | `open`, `closed`, `merged`. | @@ -90,39 +89,33 @@ PR/diff outputs come from the chat's `diff_status` and are only reliable when th ### Identity resolution -The action picks the Coder user to run the chat as. First source wins: - -1. `coder-username` input. Used directly. -2. `github-user-id` input. Looked up by linked GitHub id; deleted Coder users are filtered. -3. `github.context.payload.sender.id`. Available on most webhook events. -4. `github.context.actor`. Resolved to a GitHub id via Octokit. Excluded for `schedule` events (the actor is the workflow file editor, not a triggering user). - -If nothing resolves, the action fails and names the inputs to set. +There is one Coder identity in play. `POST /api/experimental/chats` binds the chat owner to the user the session token belongs to; the API has no owner override. The action calls `GET /api/v2/users/me` once to read the token owner's username and organization memberships, then creates the chat. The `coder-username` output is the token owner. ### Organization resolution 1. `coder-organization` input. Looked up by name. -2. First org membership of the resolved user. Non-deterministic for multi-org users; the action warns and recommends pinning `coder-organization`. +2. First org membership of the token owner. Non-deterministic for multi-org users; the action warns and recommends pinning `coder-organization`. Either path fails with `chat-error-kind=org_not_found` when the org doesn't exist or the user has no memberships. ### Chat reuse -By default the action reuses the most recent non-archived chat scoped to the same `github-url`, the same Coder user, and (when `GITHUB_WORKFLOW` is set) the same workflow name. Two workflows targeting the same PR keep separate chats. Re-running the same workflow continues one chat. +By default the action reuses the most recent non-archived chat scoped to the same `github-url` and (when `GITHUB_WORKFLOW` is set) the same workflow name. Two workflows targeting the same PR keep separate chats. Re-running the same workflow continues one chat. -Opt out per call with `force-new-chat: true`. Shard the scope further with `idempotency-key` to maintain multiple parallel chats on one target/user/workflow (for example, one per matrix dimension). `existing-chat-id` takes priority over both and skips the lookup. +All chats this action creates are owned by the `coder-token` holder, so the reuse scope deliberately omits a per-actor label. Workflows that want per-actor separation pass it through `idempotency-key` themselves, for example `idempotency-key: ${{ github.actor }}`. + +Opt out per call with `force-new-chat: true`. Shard the scope further with `idempotency-key` to maintain multiple parallel chats on one target/workflow (for example, one per matrix dimension). `existing-chat-id` takes priority over both and skips the lookup. The action writes these labels on every chat it creates: -| Label | Value | -| ------------------------------------- | --------------------------- | -| `coder-agents-chat-action` | `"true"` | -| `gh-target` | `/#` | -| `coder-agents-chat-action-user` | `` | -| `coder-agents-chat-action-workflow` | `` (when set) | -| `` | `"true"` (when set) | +| Label | Value | +| -------------------------------------- | ------------------------------ | +| `coder-agents-chat-action` | `"true"` | +| `gh-target` | `/#` | +| `coder-agents-chat-action-workflow` | `` (when set) | +| `coder-agents-chat-action-idempotency` | `` (when set) | -The `idempotency-key` input is sanitized to fit the platform's label-key regex (`^[a-zA-Z0-9][a-zA-Z0-9._/-]*$`, 64 bytes): lowercased, characters outside `[a-z0-9._/-]` replaced with `-`, leading non-alphanumerics trimmed, truncated. A value that sanitizes to a reserved label key is rejected at startup. +The `idempotency-key` input is sanitized to fit the platform's label-value regex (`^[a-zA-Z0-9][a-zA-Z0-9._/-]*$`, 64 bytes): lowercased, characters outside `[a-z0-9._/-]` replaced with `-`, leading non-alphanumerics trimmed, truncated. The sanitizer is lossy: `MyKey!` and `MyKey?` both collapse to `mykey-`. Pass values you control (commit SHAs, label slugs, `github.actor`) rather than free-form titles. ### Wait mode @@ -140,7 +133,7 @@ The action maintains one comment per `github-url` per workflow using a hidden HT ## Recipes -### Doc-check on every PR, under a service account +### Doc-check on every PR (gated against fork PRs) ```yaml name: Doc check @@ -155,6 +148,12 @@ permissions: jobs: doc-check: + # Internal PRs only. `pull_request_target` exposes `secrets.*` to fork + # PRs, so the workflow filters trust before invoking the action. Swap + # to a label-allowlist `if:` (for example, + # `contains(github.event.pull_request.labels.*.name, 'safe-to-review')`) + # to gate on a maintainer-applied label instead. + if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest steps: - uses: coder/agents-chat-action@v0 @@ -162,7 +161,6 @@ jobs: coder-url: ${{ secrets.CODER_URL }} coder-token: ${{ secrets.CODER_TOKEN }} coder-organization: ${{ secrets.CODER_ORG }} # required if the bot belongs to more than one org - coder-username: doc-check-bot chat-prompt: | Use the doc-check skill to review PR ${{ github.event.pull_request.html_url }}. @@ -171,7 +169,7 @@ jobs: wait: complete ``` -`pull_request_target` runs against the base repo and has access to secrets even for fork PRs. The service-account identity bypasses the trust gate so fork PRs are reviewed under a known bot. +`pull_request_target` runs against the base repo and exposes `secrets.*` to fork PRs. The workflow-level `if:` is the canonical place to gate trust: it short-circuits before the runner starts the step. The chat is owned by the `coder-token` holder; the prompt is benign, but the agent will read PR content with its tools (see [Security model](#security-model)). ### Send a follow-up @@ -224,10 +222,8 @@ The action sets `chat-error-kind` and `chat-error-message` on failure, posts a c | `chat-error-kind` | What happened | What to do | | ----------------- | ------------- | ---------- | | `spend_exceeded` | Chat spend limit reached. Spent and limit are in the comment. | Wait for reset or raise the deployment's per-user limit. | -| `user_not_found` | No Coder user matched the GitHub identity. | Pass `coder-username`, or have the user link their GitHub account in Coder. | -| `user_ambiguous` | Multiple live Coder users share the GitHub id. | Set `coder-username` to disambiguate. | -| `org_not_found` | Org missing or the user has no memberships. The comment names which. | Fix or set `coder-organization`. | -| `api_error` | Any other Coder API error. The comment includes the underlying message; wrapped errors carry the original `CoderAPIError` via `Error.cause` and the workflow log renders the full cause chain. | Common causes: bad token, bad `workspace-id`, deployment unreachable. | +| `org_not_found` | Org missing or the token owner has no memberships. The comment names which. | Fix or set `coder-organization`. | +| `api_error` | Any other Coder API error. The comment includes the underlying message in a code block; wrapped errors carry the original `CoderAPIError` via `Error.cause`, and the workflow log renders the full cause chain. | Common causes: bad token, bad `workspace-id`, deployment unreachable, non-github.com `github-url`. | | `timeout` | `wait: complete` didn't reach terminal in time. | Raise `wait-timeout-seconds`, or split the work. | Branch on the kind without parsing the message: @@ -239,16 +235,45 @@ Branch on the kind without parsing the message: ## Security model -Identity auto-resolve binds the Coder user matching the GitHub event sender to the chat. The trust gate refuses to auto-resolve when the trigger is untrusted: +### Chat ownership + +`POST /api/experimental/chats` binds the chat owner to whoever the session token authenticates as. There is no owner override. Anyone who can read `secrets.CODER_TOKEN` acts as that Coder user end-to-end, including the agent's tool plane (shell, `gh`, `git push`, `coder external-auth`, MCP servers). Treat the token as a high-value secret. If your platform exposes per-user spend caps, template allowlists, tool allowlists, or scoped external_auth grants, use them on the token owner; this action cannot constrain what the agent can do once a chat exists. + +### Trigger gating + +The action does not gate triggers. The workflow author defines trigger policy with `if:`. GitHub already gates the load-bearing case: `secrets.*` is not exposed to `pull_request` runs from forks, so a fork-PR run that depends on `coder-token` cannot reach the action. Workflows that opt into broader trigger surfaces (`pull_request_target`, `issue_comment`, `pull_request_review`, `pull_request_review_comment`) opt out of that default and must restate the policy themselves. + +Patterns: + +```yaml +# Internal PRs only (on pull_request_target, which exposes secrets to fork PRs). +if: github.event.pull_request.head.repo.full_name == github.repository +``` + +```yaml +# Trusted commenters only (on issue_comment / pull_request_review_comment). +if: | + github.event.comment.author_association == 'OWNER' || + github.event.comment.author_association == 'MEMBER' || + github.event.comment.author_association == 'COLLABORATOR' +``` + +```yaml +# Maintainer-applied label allowlist. +if: contains(github.event.pull_request.labels.*.name, 'safe-to-run') +``` + +See GitHub's [Events that trigger workflows](https://docs.github.com/en/actions/reference/events-that-trigger-workflows) for the full event matrix and the `pull_request_target` and secrets-on-forks rules. -- Fork PRs (`head.repo` null, `head.repo.fork === true`, or `head.repo.full_name !== base.repo.full_name`). -- Comment or review events whose `comment.author_association` or `review.author_association` is not `OWNER`, `MEMBER`, or `COLLABORATOR`. +### Indirect prompt injection -The gate doesn't read `issue.author_association` or `pull_request.author_association` because those describe the resource opener, not the event sender (a MEMBER labeling a NONE user's issue is fine). +The agent reads attacker-authored content during its run: PR titles, PR bodies, issue comments, diffs, and anything else the prompt tells it to fetch (`gh pr view`, `gh issue view --comments`, `gh pr diff`). The agent is a language model; it will follow embedded instructions in that content if they look plausible. Treat any public-repo trigger as adversarial; nothing in this action constrains what the chat reads once it runs. -For other events the action defers to GitHub's own event-permission model. Setting `coder-username` or `github-user-id` explicitly bypasses the gate; the workflow author has chosen the identity. +The action ships no defense against this class. Mitigations live deployment-side: -Independent of the gate: fork PRs that need secrets must run under `pull_request_target`, not `pull_request`. +- Pin a hardened workspace template via `workspace-id` (minimal tools, no shell, scoped network egress). +- Use Coder's platform controls to allowlist templates, restrict tool registrations, and scope the token owner's `external_auth` grants. See [Coder Agents platform controls](https://coder.com/docs/ai-coder/agents/platform-controls). +- Keep `coder-token` on a dedicated, minimally-privileged Coder user. The chat's blast radius is whatever that user can reach inside Coder (workspaces, external auth grants, mounted secrets). ## Limitations diff --git a/action.yaml b/action.yaml index 358bd46..8642a29 100644 --- a/action.yaml +++ b/action.yaml @@ -27,16 +27,8 @@ inputs: description: "GitHub token used to post and update issue comments." required: true - github-user-id: - description: "GitHub user ID to resolve to a Coder user. Mutually exclusive with coder-username." - required: false - - coder-username: - description: "Coder username to use directly. Mutually exclusive with github-user-id; useful for service-account workflows." - required: false - coder-organization: - description: "Coder organization name. Looked up by name to resolve the organization UUID for chat creation. Recommended when the resolved Coder user belongs to more than one organization, since the fallback choice is non-deterministic." + description: "Coder organization name. Looked up by name to resolve the organization UUID for chat creation. Recommended when the Coder user belongs to more than one organization, since the fallback choice is non-deterministic." required: false workspace-id: @@ -67,7 +59,7 @@ inputs: default: "600" idempotency-key: - description: "Optional sharding key to narrow the default per-workflow scope. By default the action reuses the most recent non-archived chat scoped to `gh-target`, the resolved Coder user, and the workflow name. Set this to maintain multiple parallel chats on the same target/user/workflow (for example, one per matrix dimension)." + description: "Optional sharding key to narrow the default reuse scope. By default the action reuses the most recent non-archived chat scoped to `gh-target` and the workflow name (when `GITHUB_WORKFLOW` is set). All chats are owned by the `coder-token` holder, so the scope does not include a per-actor component. Set this to maintain multiple parallel chats on the same target/workflow (for example, one per matrix dimension). Pass `${{ github.actor }}` here if you want per-actor separation." required: false force-new-chat: @@ -77,7 +69,7 @@ inputs: outputs: coder-username: - description: "The Coder username resolved from the GitHub user." + description: "The Coder username the `coder-token` belongs to (always the chat owner; the chats API has no owner override)." chat-id: description: "The chat ID." @@ -125,7 +117,7 @@ outputs: description: "Base branch name when available." chat-error-kind: - description: "Machine-readable error kind when the chat fails (one of `spend_exceeded`, `user_not_found`, `user_ambiguous`, `org_not_found`, `api_error`, `timeout`)." + description: "Machine-readable error kind when the chat fails (one of `spend_exceeded`, `org_not_found`, `api_error`, `timeout`)." chat-error-message: description: "Human-readable error message when the chat fails." diff --git a/dist/index.js b/dist/index.js index 79d2000..dbe0b43 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2,7 +2,6 @@ var __create = Object.create; var __getProtoOf = Object.getPrototypeOf; var __defProp = Object.defineProperty; var __getOwnPropNames = Object.getOwnPropertyNames; -var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __hasOwnProp = Object.prototype.hasOwnProperty; function __accessProp(key) { return this[key]; @@ -29,23 +28,6 @@ var __toESM = (mod, isNodeMode, target) => { cache.set(mod, to); return to; }; -var __toCommonJS = (from) => { - var entry = (__moduleCache ??= new WeakMap).get(from), desc; - if (entry) - return entry; - entry = __defProp({}, "__esModule", { value: true }); - if (from && typeof from === "object" || typeof from === "function") { - for (var key of __getOwnPropNames(from)) - if (!__hasOwnProp.call(entry, key)) - __defProp(entry, key, { - get: __accessProp.bind(from, key), - enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable - }); - } - __moduleCache.set(from, entry); - return entry; -}; -var __moduleCache; var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports); var __returnValue = (v) => v; function __exportSetter(name, newValue) { @@ -19128,7 +19110,7 @@ var require_before_after_hook = __commonJS((exports2, module2) => { // node_modules/@octokit/endpoint/dist-node/index.js var require_dist_node2 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -19139,16 +19121,16 @@ var require_dist_node2 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { endpoint: () => endpoint }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var import_universal_user_agent = require_dist_node(); var VERSION = "9.0.6"; var userAgent = `octokit-endpoint.js/${VERSION} ${(0, import_universal_user_agent.getUserAgent)()}`; @@ -19542,7 +19524,7 @@ var require_once = __commonJS((exports2, module2) => { var require_dist_node4 = __commonJS((exports2, module2) => { var __create2 = Object.create; var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __getProtoOf2 = Object.getPrototypeOf; var __hasOwnProp2 = Object.prototype.hasOwnProperty; @@ -19554,17 +19536,17 @@ var require_dist_node4 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; var __toESM2 = (mod, isNodeMode, target) => (target = mod != null ? __create2(__getProtoOf2(mod)) : {}, __copyProps(isNodeMode || !mod || !mod.__esModule ? __defProp2(target, "default", { value: mod, enumerable: true }) : target, mod)); - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { RequestError: () => RequestError }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var import_deprecation = require_dist_node3(); var import_once = __toESM2(require_once()); var logOnceCode = (0, import_once.default)((deprecation) => console.warn(deprecation)); @@ -19612,7 +19594,7 @@ var require_dist_node4 = __commonJS((exports2, module2) => { // node_modules/@octokit/request/dist-node/index.js var require_dist_node5 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -19623,16 +19605,16 @@ var require_dist_node5 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { request: () => request }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var import_endpoint = require_dist_node2(); var import_universal_user_agent = require_dist_node(); var VERSION = "8.4.1"; @@ -19811,7 +19793,7 @@ var require_dist_node5 = __commonJS((exports2, module2) => { // node_modules/@octokit/graphql/dist-node/index.js var require_dist_node6 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -19822,18 +19804,18 @@ var require_dist_node6 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var index_exports = {}; __export2(index_exports, { GraphqlResponseError: () => GraphqlResponseError, graphql: () => graphql2, withCustomRequest: () => withCustomRequest }); - module2.exports = __toCommonJS2(index_exports); + module2.exports = __toCommonJS(index_exports); var import_request3 = require_dist_node5(); var import_universal_user_agent = require_dist_node(); var VERSION = "7.1.1"; @@ -19935,7 +19917,7 @@ var require_dist_node6 = __commonJS((exports2, module2) => { // node_modules/@octokit/auth-token/dist-node/index.js var require_dist_node7 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -19946,16 +19928,16 @@ var require_dist_node7 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { createTokenAuth: () => createTokenAuth }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var REGEX_IS_INSTALLATION_LEGACY = /^v1\./; var REGEX_IS_INSTALLATION = /^ghs_/; var REGEX_IS_USER_TO_SERVER = /^ghu_/; @@ -19998,7 +19980,7 @@ var require_dist_node7 = __commonJS((exports2, module2) => { // node_modules/@octokit/core/dist-node/index.js var require_dist_node8 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -20009,16 +19991,16 @@ var require_dist_node8 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var index_exports = {}; __export2(index_exports, { Octokit: () => Octokit }); - module2.exports = __toCommonJS2(index_exports); + module2.exports = __toCommonJS(index_exports); var import_universal_user_agent = require_dist_node(); var import_before_after_hook = require_before_after_hook(); var import_request = require_dist_node5(); @@ -20134,7 +20116,7 @@ var require_dist_node8 = __commonJS((exports2, module2) => { // node_modules/@octokit/plugin-rest-endpoint-methods/dist-node/index.js var require_dist_node9 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -20145,17 +20127,17 @@ var require_dist_node9 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { legacyRestEndpointMethods: () => legacyRestEndpointMethods, restEndpointMethods: () => restEndpointMethods }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var VERSION = "10.4.1"; var Endpoints = { actions: { @@ -22271,7 +22253,7 @@ var require_dist_node9 = __commonJS((exports2, module2) => { // node_modules/@octokit/plugin-paginate-rest/dist-node/index.js var require_dist_node10 = __commonJS((exports2, module2) => { var __defProp2 = Object.defineProperty; - var __getOwnPropDesc2 = Object.getOwnPropertyDescriptor; + var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __getOwnPropNames2 = Object.getOwnPropertyNames; var __hasOwnProp2 = Object.prototype.hasOwnProperty; var __export2 = (target, all) => { @@ -22282,11 +22264,11 @@ var require_dist_node10 = __commonJS((exports2, module2) => { if (from && typeof from === "object" || typeof from === "function") { for (let key of __getOwnPropNames2(from)) if (!__hasOwnProp2.call(to, key) && key !== except) - __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc2(from, key)) || desc.enumerable }); + __defProp2(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable }); } return to; }; - var __toCommonJS2 = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); + var __toCommonJS = (mod) => __copyProps(__defProp2({}, "__esModule", { value: true }), mod); var dist_src_exports = {}; __export2(dist_src_exports, { composePaginateRest: () => composePaginateRest, @@ -22294,7 +22276,7 @@ var require_dist_node10 = __commonJS((exports2, module2) => { paginateRest: () => paginateRest, paginatingEndpoints: () => paginatingEndpoints }); - module2.exports = __toCommonJS2(dist_src_exports); + module2.exports = __toCommonJS(dist_src_exports); var VERSION = "9.2.2"; function normalizePaginatedListResponse(response) { if (!response.data) { @@ -22744,11 +22726,6 @@ var require_github = __commonJS((exports2) => { }); // src/index.ts -var exports_src = {}; -__export(exports_src, { - parseGithubUserID: () => parseGithubUserID -}); -module.exports = __toCommonJS(exports_src); var core4 = __toESM(require_core(), 1); var github = __toESM(require_github(), 1); @@ -26770,40 +26747,9 @@ class RealCoderClient { } return response.json(); } - async getCoderUserByGitHubId(githubUserId) { - if (githubUserId === undefined) { - throw new CoderAPIError("GitHub user ID cannot be undefined", 400); - } - if (githubUserId === 0) { - throw new CoderAPIError("GitHub user ID cannot be 0", 400); - } - const filter = `github_com_user_id:${githubUserId}`; - const endpoint = `/api/v2/users?q=${encodeURIComponent(filter)}`; - const response = await this.request(endpoint); - const userList = CoderSDKGetUsersResponseSchema.parse(response); - const liveUsers = userList.users.filter((u) => !u.deleted); - if (liveUsers.length === 0) { - throw new CoderAPIError(`No Coder user found with GitHub user ID ${githubUserId}`, 404, undefined, "user_not_found"); - } - if (liveUsers.length > 1) { - throw new CoderAPIError(`Multiple Coder users found with GitHub user ID ${githubUserId}`, 409, undefined, "user_ambiguous"); - } - return CoderSDKUserSchema.parse(liveUsers[0]); - } - async getCoderUserByUsername(username) { - if (!username) { - throw new CoderAPIError("Coder username cannot be empty", 400); - } - const endpoint = `/api/v2/users/${encodeURIComponent(username)}`; - try { - const response = await this.request(endpoint); - return CoderSDKUserSchema.parse(response); - } catch (err) { - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new CoderAPIError(`No Coder user found with username "${username}"`, 404, err.response, "user_not_found"); - } - throw err; - } + async getAuthenticatedUser() { + const response = await this.request("/api/v2/users/me"); + return CoderSDKUserSchema.parse(response); } async getOrganizationByName(name) { if (!name) { @@ -26861,9 +26807,6 @@ var CoderSDKUserSchema = exports_external.object({ github_com_user_id: exports_external.number().optional(), deleted: exports_external.boolean().optional() }); -var CoderSDKGetUsersResponseSchema = exports_external.object({ - users: exports_external.array(CoderSDKUserSchema) -}); var CoderOrganizationSchema = exports_external.object({ id: exports_external.string().uuid(), name: exports_external.string(), @@ -26932,37 +26875,26 @@ var CreateChatMessageRequestSchema = exports_external.object({ var CreateChatMessageResponseSchema = exports_external.object({ queued: exports_external.boolean() }); -var ChatErrorKindSchema = exports_external.enum([ - "user_not_found", - "user_ambiguous", - "org_not_found", - "spend_exceeded", - "api_error", - "timeout" -]); class CoderAPIError extends Error { statusCode; response; - kind; - constructor(message, statusCode, response, kind) { + constructor(message, statusCode, response) { super(message); this.statusCode = statusCode; this.response = response; - this.kind = kind; this.name = "CoderAPIError"; } } -// src/sanitize-label-key.ts +// src/sanitize-label-token.ts var ACTION_LABEL_KEYS = { marker: "coder-agents-chat-action", target: "gh-target", - user: "coder-agents-chat-action-user", - workflow: "coder-agents-chat-action-workflow" + workflow: "coder-agents-chat-action-workflow", + idempotency: "coder-agents-chat-action-idempotency" }; -var RESERVED_LABEL_KEYS = new Set(Object.values(ACTION_LABEL_KEYS)); -function sanitizeLabelKey(input) { +function sanitizeLabelToken(input) { const lowered = input.toLowerCase(); const replaced = lowered.replace(/[^a-z0-9._/-]/g, "-"); const trimmed = replaced.replace(/^[^a-z0-9]+/, ""); @@ -26972,7 +26904,21 @@ function sanitizeLabelKey(input) { // src/comment.ts var core = __toESM(require_core(), 1); -var GITHUB_URL_REGEX = /([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)\/?(?:[?#].*)?$/; +var GITHUB_URL_REGEX = /^https:\/\/github\.com\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)\/?(?:[?#].*)?$/; +function parseGithubItemURL(input) { + if (!input) { + return; + } + const match = input.match(GITHUB_URL_REGEX); + if (!match) { + return; + } + return { + owner: match[1], + repo: match[2], + number: Number.parseInt(match[3], 10) + }; +} var COMMENT_MARKER_PREFIX = ""; function buildCommentMarker(key) { @@ -26980,14 +26926,14 @@ function buildCommentMarker(key) { } function deriveCommentKey(inputs) { if (inputs.idempotencyKey) { - return sanitizeLabelKey(inputs.idempotencyKey); + return sanitizeLabelToken(inputs.idempotencyKey); } - const match = inputs.githubURL.match(GITHUB_URL_REGEX); + const parsed = parseGithubItemURL(inputs.githubURL); let base; - if (!match) { + if (!parsed) { base = inputs.githubURL; } else { - base = `${match[1]}/${match[2]}#${match[3]}`; + base = `${parsed.owner}/${parsed.repo}#${parsed.number}`; } if (inputs.workflow) { return `${base}:${inputs.workflow}`; @@ -26996,10 +26942,6 @@ function deriveCommentKey(inputs) { } function classifyError(err) { if (err instanceof CoderAPIError) { - const code = mapErrorCodeToKind(err.kind); - if (code) { - return { kind: code, message: err.message }; - } const spend = parseSpendExceededBody(err.response); if (err.statusCode === 409 && spend) { return { @@ -27020,15 +26962,6 @@ function classifyError(err) { } return { kind: "api_error", message: String(err) }; } -function mapErrorCodeToKind(code) { - switch (code) { - case "user_not_found": - case "user_ambiguous": - return code; - default: - return; - } -} function parseSpendExceededBody(response) { const obj = parseJSONObject(response); if (!obj) { @@ -27072,11 +27005,21 @@ function formatMicrosAsDollars(micros) { const dollars = micros / 1e6; return `$${dollars.toFixed(2)}`; } +var DETAIL_BLOCK_MAX_CHARS = 4000; +function renderDetailBlock(message) { + const stripped = message.replace(/[\x00-\x08\x0B-\x1F\x7F]/g, ""); + const capped = stripped.length > DETAIL_BLOCK_MAX_CHARS ? stripped.slice(0, DETAIL_BLOCK_MAX_CHARS) : stripped; + const safe = capped.replace(/`{4,}/g, "```"); + return `- Detail: +\`\`\`\` +${safe} +\`\`\`\``; +} function buildFailureCommentBody(detail, ctx) { const runPhase = isRunPhaseFailure(detail.kind, ctx); const heading = runPhase ? "**Coder Agents Chat: failed**" : "**Coder Agents Chat: failed to start**"; const lines = [heading, ""]; - const linkLine = ctx.chatUrl ? `View the chat in the Coder deployment: ${ctx.chatUrl}` : `View chats in the Coder deployment: ${ctx.chatsUrl}`; + const linkLine = ctx.chatUrl ? `View the chat in the Coder deployment: ${ctx.chatUrl}` : `View agents in the Coder deployment: ${ctx.agentsUrl}`; switch (detail.kind) { case "spend_exceeded": lines.push("The Coder deployment's chat spend limit was reached, so this " + "chat could not be created.", "", `- chat-error-kind=${detail.kind}`, `- Spent: ${formatMicrosAsDollars(detail.spentMicros)}`, `- Limit: ${formatMicrosAsDollars(detail.limitMicros)}`); @@ -27085,25 +27028,19 @@ function buildFailureCommentBody(detail, ctx) { } lines.push("", linkLine); break; - case "user_not_found": - lines.push("No Coder user could be resolved for this run. Adjust either " + "the `github-user-id` input (the GitHub identity is not linked " + "to a Coder user) or pass `coder-username` directly.", "", `- chat-error-kind=${detail.kind}`, `- Detail: ${detail.message}`, "", linkLine); - break; - case "user_ambiguous": - lines.push("Multiple Coder users matched the GitHub identity. Set the " + "`coder-username` input to the specific account this workflow " + "should run as.", "", `- chat-error-kind=${detail.kind}`, `- Detail: ${detail.message}`, "", linkLine); - break; case "org_not_found": - lines.push("The resolved Coder user has no matching organization. Set the " + "`coder-organization` input or grant the user a membership.", "", `- chat-error-kind=${detail.kind}`, `- Detail: ${detail.message}`, "", linkLine); + lines.push("The Coder user has no matching organization. Set the " + "`coder-organization` input or grant the user a membership.", "", `- chat-error-kind=${detail.kind}`, renderDetailBlock(detail.message), "", linkLine); break; case "api_error": lines.push(apiErrorPhrase(runPhase, ctx), ""); - lines.push(`- chat-error-kind=${detail.kind}`, `- Detail: ${detail.message}`); + lines.push(`- chat-error-kind=${detail.kind}`, renderDetailBlock(detail.message)); if (ctx.chatStatus === "error") { lines.push("- Hint: the agent itself failed mid-run; inspect " + "`last_error` on the chat (e.g. provider rate limits) " + "rather than action connectivity."); } lines.push("", linkLine); break; case "timeout": - lines.push("`wait: complete` polling did not reach a terminal status within " + "`wait-timeout-seconds`.", "", `- chat-error-kind=${detail.kind}`, `- Detail: ${detail.message}`, "", linkLine); + lines.push("`wait: complete` polling did not reach a terminal status within " + "`wait-timeout-seconds`.", "", `- chat-error-kind=${detail.kind}`, renderDetailBlock(detail.message), "", linkLine); break; default: { const _exhaustive = detail; @@ -27226,8 +27163,8 @@ async function upsertCommentByMarker(args) { logLabel: "failure comment" }); } -function buildDeploymentChatsUrl(coderURL) { - return `${normalizeBaseUrl(coderURL)}/chats`; +function buildDeploymentAgentsUrl(coderURL) { + return `${normalizeBaseUrl(coderURL)}/agents`; } // src/action.ts @@ -27257,90 +27194,34 @@ class ActionFailureError extends Error { coderUsername; chatUrl; } -function describeError(err) { - if (err instanceof Error) { - return err.message; - } - if (typeof err === "string") { - return err; - } - try { - return JSON.stringify(err); - } catch { - return String(err); - } -} -var TRUSTED_AUTHOR_ASSOCIATIONS = new Set([ - "OWNER", - "MEMBER", - "COLLABORATOR" -]); -function classifyAutoResolveTrust(context) { - const pr = context.payload.pull_request; - if (pr) { - const headRepo = pr.head?.repo; - const baseRepo = pr.base?.repo; - const headFullName = headRepo?.full_name; - const baseFullName = baseRepo?.full_name; - const isFork = headRepo === null || headRepo?.fork === true || typeof headFullName === "string" && typeof baseFullName === "string" && headFullName !== baseFullName; - if (isFork) { - return { - kind: "untrusted", - reason: "the pull request is from a fork; auto-resolve refuses to bind " + "the workflow's Coder identity to a fork-PR author" - }; - } - } - const associations = [ - { source: "comment", value: context.payload.comment?.author_association }, - { source: "review", value: context.payload.review?.author_association } - ]; - for (const { source, value } of associations) { - if (typeof value !== "string" || value.length === 0) { - continue; - } - if (TRUSTED_AUTHOR_ASSOCIATIONS.has(value)) { - return { - kind: "trusted", - reason: `${source}.author_association is ${value}` - }; - } - return { - kind: "untrusted", - reason: `${source}.author_association is ${value}, which lacks ` + "repository write access" - }; - } - return { kind: "no-signal" }; -} class CoderAgentChatAction { coder; octokit; inputs; - context; clock; - constructor(coder, octokit, inputs, context, clock = defaultClock) { + constructor(coder, octokit, inputs, clock = defaultClock) { this.coder = coder; this.octokit = octokit; this.inputs = inputs; - this.context = context; this.clock = clock; } parseGithubURL() { if (!this.inputs.githubURL) { throw new Error("Missing GitHub URL"); } - const match = this.inputs.githubURL.match(GITHUB_URL_REGEX); - if (!match) { - throw new Error(`Invalid GitHub URL: ${this.inputs.githubURL}`); + const parsed = parseGithubItemURL(this.inputs.githubURL); + if (!parsed) { + throw new Error(`Invalid \`github-url\` input "${this.inputs.githubURL}". ` + "Expected `https://github.com///issues/` or " + "`https://github.com///pull/`. The action " + "rejects non-github.com hosts so a workflow that templates " + "user-controlled content into this input cannot redirect the " + "action to an attacker-chosen repository."); } return { - githubOrg: match[1], - githubRepo: match[2], - githubIssueNumber: parseInt(match[3], 10) + githubOrg: parsed.owner, + githubRepo: parsed.repo, + githubIssueNumber: parsed.number }; } generateChatUrl(chatId) { - return `${normalizeBaseUrl(this.inputs.coderURL)}/chats/${chatId}`; + return `${normalizeBaseUrl(this.inputs.coderURL)}/agents/${chatId}`; } async commentOnIssue(args) { const workflow = process.env.GITHUB_WORKFLOW || undefined; @@ -27367,7 +27248,6 @@ class CoderAgentChatAction { marker }); } - warnUnwiredInputs() {} buildOutputs(coderUsername, chat, chatCreated) { const diff = chat.diff_status; const hasPR = diff?.pr_number != null; @@ -27463,67 +27343,7 @@ class CoderAgentChatAction { throw err; } } - async resolveCoderUsername() { - if (this.inputs.coderUsername) { - core2.info(`Using provided Coder username: ${this.inputs.coderUsername}`); - let coderUser; - try { - coderUser = await this.coder.getCoderUserByUsername(this.inputs.coderUsername); - } catch (err) { - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new ActionFailureError("user_not_found", `Coder user '${this.inputs.coderUsername}' not found. ` + "Check the `coder-username` input value.", undefined, { cause: err }); - } - throw err; - } - return { username: coderUser.username, user: coderUser }; - } - if (this.inputs.githubUserID !== undefined) { - core2.info(`Looking up Coder user by GitHub user ID: ${this.inputs.githubUserID}`); - const coderUser = await this.coder.getCoderUserByGitHubId(this.inputs.githubUserID); - return { username: coderUser.username, user: coderUser }; - } - if (this.context.eventName === "schedule") { - throw new Error("Cannot auto-resolve a GitHub identity for `schedule` events: " + "`github.context.actor` for cron-triggered runs is the workflow " + "file's last editor, not the triggering user. " + "Set the `coder-username` input to a Coder username, or set " + "`github-user-id` to the GitHub numeric user id of the user the " + "chat should run as."); - } - const trust = classifyAutoResolveTrust(this.context); - if (trust.kind === "untrusted") { - throw new Error("Refusing to auto-resolve a GitHub identity: " + `${trust.reason}. ` + "Set the `coder-username` input to a Coder username, or set " + "`github-user-id` to the GitHub numeric user id of the user " + "the chat should run as."); - } - if (trust.kind === "trusted") { - core2.info(`Auto-resolve trust check passed: ${trust.reason}`); - } - const senderId = this.context.payload?.sender?.id; - if (typeof senderId === "number" && Number.isInteger(senderId) && senderId > 0) { - core2.info(`Auto-resolving Coder user from github.context.payload.sender.id: ${senderId}`); - try { - const coderUser = await this.coder.getCoderUserByGitHubId(senderId); - return { username: coderUser.username, user: coderUser }; - } catch (err) { - throw new Error(`Failed to resolve Coder user from github.context.payload.sender.id (${senderId}): ${describeError(err)}. ` + "Set the `coder-username` input to bypass auto-resolution."); - } - } - const actor = this.context.actor; - if (actor) { - core2.info(`Auto-resolving Coder user from github.context.actor: ${actor}`); - let actorId; - try { - const { data } = await this.octokit.rest.users.getByUsername({ - username: actor - }); - actorId = data.id; - } catch (err) { - throw new Error(`Failed to resolve GitHub user id for github.context.actor (${actor}): ${describeError(err)}. ` + "Set the `coder-username` input to bypass auto-resolution."); - } - try { - const coderUser = await this.coder.getCoderUserByGitHubId(actorId); - return { username: coderUser.username, user: coderUser }; - } catch (err) { - throw new Error(`Failed to resolve Coder user for github.context.actor (${actor}, GitHub user id ${actorId}): ${describeError(err)}. ` + "Set the `coder-username` input to bypass auto-resolution."); - } - } - throw new Error("Could not auto-resolve a GitHub identity from the workflow context. " + "Set the `coder-username` input to a Coder username, or set " + "`github-user-id` to the GitHub numeric user id of the user the " + "chat should run as."); - } - async resolveOrganizationID(coderUsername, resolvedUser) { + async resolveOrganizationID(user) { if (this.inputs.coderOrganization) { core2.info(`Resolving Coder organization by name: ${this.inputs.coderOrganization}`); try { @@ -27536,19 +27356,6 @@ class CoderAgentChatAction { throw err; } } - let user; - if (resolvedUser) { - user = resolvedUser; - } else { - try { - user = await this.coder.getCoderUserByUsername(coderUsername); - } catch (err) { - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new ActionFailureError("user_not_found", `Coder user '${coderUsername}' not found. ` + "Check the `coder-username` input value.", undefined, { cause: err }); - } - throw err; - } - } const orgID = user.organization_ids[0]; if (!orgID) { throw new ActionFailureError("org_not_found", `Coder user '${user.username}' has no organization memberships. ` + "Set the `coder-organization` input to the organization the chat " + "should run in."); @@ -27574,7 +27381,7 @@ class CoderAgentChatAction { } } async handleFailure(error3) { - const detail = classifyError(error3); + const detail = error3 instanceof ActionFailureError && error3.kind !== "spend_exceeded" ? { kind: error3.kind, message: error3.message } : classifyError(error3); const failure = error3 instanceof ActionFailureError ? error3 : new ActionFailureError(detail.kind, detail.message, undefined, { cause: error3 }); @@ -27591,7 +27398,7 @@ class CoderAgentChatAction { const workflow = process.env.GITHUB_WORKFLOW || undefined; const marker = buildCommentMarker(deriveCommentKey({ ...this.inputs, workflow })); const body = buildFailureCommentBody(detail, { - chatsUrl: buildDeploymentChatsUrl(this.inputs.coderURL), + agentsUrl: buildDeploymentAgentsUrl(this.inputs.coderURL), marker, chatUrl: failure.chatUrl, chatStatus: failure.chat?.status @@ -27607,13 +27414,13 @@ class CoderAgentChatAction { return failure; } async runInner() { - this.warnUnwiredInputs(); - const { username: coderUsername, user: resolvedUser } = await this.resolveCoderUsername(); const { githubOrg, githubRepo, githubIssueNumber } = this.parseGithubURL(); core2.info(`GitHub owner: ${githubOrg}`); core2.info(`GitHub repo: ${githubRepo}`); core2.info(`GitHub item number: ${githubIssueNumber}`); - core2.info(`Coder username: ${coderUsername}`); + const tokenOwner = await this.coder.getAuthenticatedUser(); + const coderUsername = tokenOwner.username; + core2.info(`Resolved Coder user from \`coder-token\` (users/me): ${coderUsername}`); if (this.inputs.existingChatId) { core2.info(`Sending message to existing chat: ${this.inputs.existingChatId}`); const chatId = ChatIdSchema.parse(this.inputs.existingChatId); @@ -27626,16 +27433,13 @@ class CoderAgentChatAction { githubIssueNumber }); } - const sanitizedKey = this.inputs.idempotencyKey ? sanitizeLabelKey(this.inputs.idempotencyKey) : undefined; - if (sanitizedKey && RESERVED_LABEL_KEYS.has(sanitizedKey)) { - throw new Error(`idempotency-key sanitizes to a reserved chat-label key ("${sanitizedKey}"). ` + `Reserved keys: ${[...RESERVED_LABEL_KEYS].join(", ")}. ` + "Choose a different idempotency-key value."); - } + const sanitizedIdempotency = this.inputs.idempotencyKey ? sanitizeLabelToken(this.inputs.idempotencyKey) : undefined; const ghTarget = `${githubOrg}/${githubRepo}#${githubIssueNumber}`; const workflow = process.env.GITHUB_WORKFLOW || undefined; if (this.inputs.forceNewChat) { core2.info("force-new-chat=true: skipping chat-reuse lookup"); } else { - const follow = await this.findReuseMatch(ghTarget, resolvedUser.id, workflow, sanitizedKey); + const follow = await this.findReuseMatch(ghTarget, workflow, sanitizedIdempotency); if (follow) { core2.info(`Reusing existing chat: ${follow.id}`); return this.runFollowUp({ @@ -27649,13 +27453,13 @@ class CoderAgentChatAction { } } core2.info("Creating new agents chat..."); - const organizationID = await this.resolveOrganizationID(coderUsername, resolvedUser); + const organizationID = await this.resolveOrganizationID(tokenOwner); const req = { organization_id: organizationID, content: [{ type: "text", text: this.inputs.chatPrompt }], workspace_id: this.inputs.workspaceId, model_config_id: this.inputs.modelConfigId, - labels: this.buildChatLabels(ghTarget, resolvedUser.id, workflow, sanitizedKey) + labels: this.buildChatLabels(ghTarget, workflow, sanitizedIdempotency) }; const createdChat = await this.coder.createChat(req); core2.info(`Agents chat created successfully (id: ${createdChat.id}, status: ${createdChat.status})`); @@ -27736,17 +27540,16 @@ class CoderAgentChatAction { chatCreated: false }; } - async findReuseMatch(ghTarget, coderUserId, workflow, sanitizedKey) { + async findReuseMatch(ghTarget, workflow, sanitizedIdempotency) { const labels = [ `${ACTION_LABEL_KEYS.marker}:true`, - `${ACTION_LABEL_KEYS.target}:${ghTarget}`, - `${ACTION_LABEL_KEYS.user}:${coderUserId}` + `${ACTION_LABEL_KEYS.target}:${ghTarget}` ]; if (workflow) { labels.push(`${ACTION_LABEL_KEYS.workflow}:${workflow}`); } - if (sanitizedKey) { - labels.push(`${sanitizedKey}:true`); + if (sanitizedIdempotency) { + labels.push(`${ACTION_LABEL_KEYS.idempotency}:${sanitizedIdempotency}`); } let chats; try { @@ -27775,20 +27578,16 @@ class CoderAgentChatAction { } return live[0]; } - buildChatLabels(ghTarget, coderUserId, workflow, sanitizedKey) { - if (sanitizedKey && RESERVED_LABEL_KEYS.has(sanitizedKey)) { - throw new Error(`idempotency-key sanitizes to a reserved chat-label key ("${sanitizedKey}"). ` + `Reserved keys: ${[...RESERVED_LABEL_KEYS].join(", ")}. ` + "Choose a different idempotency-key value."); - } + buildChatLabels(ghTarget, workflow, sanitizedIdempotency) { const labels = { [ACTION_LABEL_KEYS.marker]: "true", - [ACTION_LABEL_KEYS.target]: ghTarget, - [ACTION_LABEL_KEYS.user]: coderUserId + [ACTION_LABEL_KEYS.target]: ghTarget }; if (workflow) { labels[ACTION_LABEL_KEYS.workflow] = workflow; } - if (sanitizedKey) { - labels[sanitizedKey] = "true"; + if (sanitizedIdempotency) { + labels[ACTION_LABEL_KEYS.idempotency] = sanitizedIdempotency; } return labels; } @@ -27852,8 +27651,6 @@ var ActionInputsObjectSchema = exports_external.object({ coderOrganization: exports_external.string().min(1).optional(), githubURL: exports_external.string().url(), githubToken: exports_external.string().min(1), - githubUserID: exports_external.number().int().positive().optional(), - coderUsername: exports_external.string().min(1).optional(), workspaceId: exports_external.string().uuid().optional(), modelConfigId: exports_external.string().uuid().optional(), existingChatId: exports_external.string().uuid().optional(), @@ -27863,17 +27660,12 @@ var ActionInputsObjectSchema = exports_external.object({ idempotencyKey: exports_external.string().min(1).optional(), forceNewChat: exports_external.boolean().default(false) }); -var ActionInputsSchema = ActionInputsObjectSchema.refine((data) => !(data.githubUserID !== undefined && data.coderUsername !== undefined), { - message: "Cannot set both github-user-id and coder-username; choose one.", - path: ["coderUsername"] -}).refine((data) => !(data.existingChatId !== undefined && data.forceNewChat === true), { +var ActionInputsSchema = ActionInputsObjectSchema.refine((data) => !(data.existingChatId !== undefined && data.forceNewChat === true), { message: "Cannot set both existing-chat-id and force-new-chat; choose one.", path: ["forceNewChat"] }); -var ChatErrorKindSchema2 = exports_external.enum([ +var ChatErrorKindSchema = exports_external.enum([ "spend_exceeded", - "user_not_found", - "user_ambiguous", "org_not_found", "api_error", "timeout" @@ -27895,21 +27687,13 @@ var ActionOutputsSchema = exports_external.object({ changedFiles: exports_external.number().optional(), headBranch: exports_external.string().optional(), baseBranch: exports_external.string().optional(), - chatErrorKind: ChatErrorKindSchema2.optional(), + chatErrorKind: ChatErrorKindSchema.optional(), chatErrorMessage: exports_external.string().optional() }); // src/index.ts -function parseGithubUserID(raw) { - if (!raw) - return; - if (!/^\d+$/.test(raw)) - return Number.NaN; - return Number(raw); -} async function main() { try { - const githubUserID = parseGithubUserID(core4.getInput("github-user-id")); const inputs = ActionInputsSchema.parse({ coderURL: core4.getInput("coder-url", { required: true }), coderToken: core4.getInput("coder-token", { required: true }), @@ -27917,8 +27701,6 @@ async function main() { coderOrganization: core4.getInput("coder-organization") || undefined, githubURL: core4.getInput("github-url", { required: true }), githubToken: core4.getInput("github-token", { required: true }), - githubUserID, - coderUsername: core4.getInput("coder-username") || undefined, workspaceId: core4.getInput("workspace-id") || undefined, modelConfigId: core4.getInput("model-config-id") || undefined, existingChatId: core4.getInput("existing-chat-id") || undefined, @@ -27934,7 +27716,7 @@ async function main() { const coder = new RealCoderClient(inputs.coderURL, inputs.coderToken); const octokit = github.getOctokit(inputs.githubToken); core4.debug("Clients initialized"); - const action = new CoderAgentChatAction(coder, octokit, inputs, github.context); + const action = new CoderAgentChatAction(coder, octokit, inputs); const outputs = await action.run(); setActionOutputs(outputs); core4.debug("Action completed successfully"); diff --git a/src/action.test.ts b/src/action.test.ts index feb1730..ba9526f 100644 --- a/src/action.test.ts +++ b/src/action.test.ts @@ -8,18 +8,13 @@ import { } from "./action"; import type { Octokit } from "./action"; import { CoderAPIError } from "./coder-client"; -import { - ChatIdSchema, - type CoderChat, - type CoderSDKUser, -} from "./coder-client"; +import { ChatIdSchema, type CoderChat } from "./coder-client"; import { ActionOutputsSchema } from "./schemas"; import { MockCoderClient, createFakeClock, createMockOctokit, createMockInputs, - createMockContext, mockUser, mockUserNoOrgs, mockChat, @@ -50,7 +45,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.parseGithubURL(); @@ -70,7 +64,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.parseGithubURL(); @@ -88,7 +81,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); expect(() => action.parseGithubURL()).toThrowError("Missing GitHub URL"); @@ -100,11 +92,10 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); expect(() => action.parseGithubURL()).toThrowError( - "Invalid GitHub URL: not-a-url", + /Invalid `github-url` input/, ); }); @@ -116,10 +107,11 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); - expect(() => action.parseGithubURL()).toThrowError("Invalid GitHub URL"); + expect(() => action.parseGithubURL()).toThrowError( + /Invalid `github-url` input/, + ); }); test("accepts a trailing slash after the issue number", () => { @@ -130,7 +122,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.parseGithubURL(); @@ -150,7 +141,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.parseGithubURL(); @@ -170,7 +160,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.parseGithubURL(); @@ -182,7 +171,13 @@ describe("CoderAgentChatAction", () => { }); }); - test("handles non-github.com URL", () => { + test("rejects non-github.com hostnames", () => { + // The regex used to accept any host because it was end-anchored + // only. Coercing the action to comment on + // `https://attacker.example/coder/coder/issues/1` would have + // called `octokit.rest.issues.createComment` with owner=coder, + // repo=coder, number=1 under the workflow's `github-token`. The + // action now refuses. const inputs = createMockInputs({ githubURL: "https://code.acme.com/owner/repo/issues/123", }); @@ -190,16 +185,26 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); - const result = action.parseGithubURL(); + expect(() => action.parseGithubURL()).toThrowError( + /non-github.com hosts/, + ); + }); - expect(result).toEqual({ - githubOrg: "owner", - githubRepo: "repo", - githubIssueNumber: 123, + test("rejects an attacker-redirect via a non-github host that mimics the issue path", () => { + const inputs = createMockInputs({ + githubURL: "https://attacker.example/coder/coder/issues/1", }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + expect(() => action.parseGithubURL()).toThrowError( + /Invalid `github-url` input/, + ); }); }); @@ -210,12 +215,11 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.generateChatUrl(mockChat.id); - expect(result).toBe(`https://coder.test/chats/${mockChat.id}`); + expect(result).toBe(`https://coder.test/agents/${mockChat.id}`); }); test("handles URL with trailing junk", () => { @@ -226,12 +230,11 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = action.generateChatUrl(mockChat.id); - expect(result).toBe(`https://coder.test/chats/${mockChat.id}`); + expect(result).toBe(`https://coder.test/agents/${mockChat.id}`); }); }); @@ -249,7 +252,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.commentOnIssue({ @@ -294,7 +296,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.commentOnIssue({ @@ -327,7 +328,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect( @@ -349,23 +349,21 @@ describe("CoderAgentChatAction", () => { }); test("creates new chat successfully", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, commentOnIssue: false, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = await action.run(); - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith(12345); + expect(coderClient.mockGetAuthenticatedUser).toHaveBeenCalled(); expect(coderClient.mockCreateChat).toHaveBeenCalledWith( expect.objectContaining({ content: [{ type: "text", text: "Test prompt" }], @@ -381,7 +379,7 @@ describe("CoderAgentChatAction", () => { expect(parsedResult.chatTitle).toBe("Test chat"); expect(parsedResult.workspaceId).toBe(mockChat.workspace_id ?? undefined); expect(parsedResult.chatUrl).toMatch( - /^https:\/\/coder\.test\/chats\/[a-f0-9-]+$/, + /^https:\/\/coder\.test\/agents\/[a-f0-9-]+$/, ); }); @@ -392,7 +390,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const out = action.buildOutputs(mockUser.username, mockChat, true); @@ -420,7 +417,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const out = action.buildOutputs( @@ -453,7 +449,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const diff = mockChatWithDiff.diff_status; if (!diff) { @@ -479,7 +474,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const diff = mockChatWithDiff.diff_status; if (!diff) { @@ -510,7 +504,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const diff = mockChatWithDiff.diff_status; if (!diff) { @@ -540,7 +533,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const diff = mockChatWithDiff.diff_status; if (!diff) { @@ -574,7 +566,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const chatWithError: typeof mockChat = { ...mockChat, @@ -594,7 +585,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const out = action.buildOutputs(mockUser.username, mockChat, true); @@ -603,33 +593,8 @@ describe("CoderAgentChatAction", () => { }); }); - test("creates chat using direct coder-username", async () => { - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: mockUser.username, - commentOnIssue: false, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(coderClient.mockCreateChat).toHaveBeenCalled(); - - const parsedResult = ActionOutputsSchema.parse(result); - expect(parsedResult.coderUsername).toBe(mockUser.username); - expect(parsedResult.chatCreated).toBe(true); - }); - test("sends message to existing chat", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -643,14 +608,12 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = await action.run(); @@ -675,21 +638,19 @@ describe("CoderAgentChatAction", () => { }); test("rejects a malformed existing-chat-id at runtime (defense in depth past the schema)", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); // `createMockInputs` casts to `ActionInputs` without running // `ActionInputsSchema`. That lets this test prove the // `ChatIdSchema.parse` in the existing-chat branch refuses non-UUID // input even if a future caller skips the upstream schema parse. const inputs = createMockInputs({ - githubUserID: 12345, existingChatId: "not-a-uuid", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -697,1061 +658,115 @@ describe("CoderAgentChatAction", () => { }); test("falls back to minimal outputs when getChat fails after follow-up", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); coderClient.mockGetChat.mockRejectedValue(new Error("transient API error")); - const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; - const inputs = createMockInputs({ - githubUserID: 12345, - existingChatId, - commentOnIssue: false, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - // The follow-up message succeeded, so the action should not fail red - // just because the chat fetch did. The outputs degrade gracefully. - const result = await action.run(); - - expect(coderClient.mockCreateChatMessage).toHaveBeenCalled(); - expect(coderClient.mockGetChat).toHaveBeenCalledWith(existingChatId); - - const parsedResult = ActionOutputsSchema.parse(result); - expect(parsedResult.chatCreated).toBe(false); - expect(parsedResult.chatId).toBe(existingChatId); - expect(parsedResult.chatStatus).toBeUndefined(); - expect(parsedResult.chatTitle).toBeUndefined(); - expect(parsedResult.workspaceId).toBeUndefined(); - }); - - test("creates chat with workspace-id", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const workspaceId = "550e8400-e29b-41d4-a716-446655440000"; - const inputs = createMockInputs({ - githubUserID: 12345, - workspaceId, - commentOnIssue: false, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await action.run(); - - expect(coderClient.mockCreateChat).toHaveBeenCalledWith( - expect.objectContaining({ - workspace_id: workspaceId, - }), - ); - }); - - describe("commentOnIssue toggle", () => { - test("does not comment when commentOnIssue is false", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: 12345, - commentOnIssue: false, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await action.run(); - - expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); - expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); - }); - - test("comments when commentOnIssue is true", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - octokit.rest.issues.listComments.mockResolvedValue({ - data: [], - } as ReturnType); - octokit.rest.issues.createComment.mockResolvedValue( - {} as ReturnType, - ); - - const inputs = createMockInputs({ - githubUserID: 12345, - githubURL: "https://github.com/owner/repo/issues/123", - commentOnIssue: true, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await action.run(); - - expect(octokit.rest.issues.listComments).toHaveBeenCalled(); - expect(octokit.rest.issues.createComment).toHaveBeenCalled(); - }); - }); - - describe("warnUnwiredInputs", () => { - test("does not warn for wait=complete", () => { - const warning = spyOn(core, "warning").mockImplementation(() => {}); - try { - const inputs = createMockInputs({ wait: "complete" }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - action.warnUnwiredInputs(); - - expect(warning).not.toHaveBeenCalledWith( - expect.stringContaining("`wait: complete`"), - ); - } finally { - warning.mockRestore(); - } - }); - - test("does not warn at defaults", () => { - const warning = spyOn(core, "warning").mockImplementation(() => {}); - try { - const inputs = createMockInputs(); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - action.warnUnwiredInputs(); - - expect(warning).not.toHaveBeenCalled(); - } finally { - warning.mockRestore(); - } - }); - }); - - describe("Identity resolution", () => { - test("uses coder-username directly without GitHub-id lookup", async () => { - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: mockUser.username, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - payload: { sender: { id: 99999 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("prefers coder-username over github-user-id when both bypass the schema", async () => { - // The Zod schema rejects setting both inputs simultaneously, but the - // resolver is a unit and the precedence #1 vs #2 must hold even if a - // future caller bypasses the schema. Constructing the action directly - // pins the precedence in the unit's contract. - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = { - ...createMockInputs(), - coderUsername: mockUser.username, - githubUserID: 12345, - commentOnIssue: false, - }; - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext({ eventName: "issues" }), - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("looks up by github-user-id when set", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: 12345, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - payload: { sender: { id: 99999 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith( - 12345, - ); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("falls back to context.payload.sender.id when both inputs are unset", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - actor: "some-actor", - payload: { sender: { id: 424242 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith( - 424242, - ); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("falls through to actor when sender exists without a numeric id", async () => { - // Bot-triggered events sometimes deliver a partial sender object - // (e.g. `{ login: "bot" }` with no `id`). The resolver guards - // `sender.id` with `typeof === "number" && > 0` and falls through. - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 333 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - actor: "octocat", - payload: { sender: {} }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(octokit.rest.users.getByUsername).toHaveBeenCalledWith({ - username: "octocat", - }); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("treats sender id of 0 as missing and falls through to actor", async () => { - // Mirrors the Zod schema's positive constraint on `github-user-id`. - // Without the guard, `0` reaches a bare-string throw inside the - // Coder client and surfaces as "Unknown error occurred". - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 444 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - actor: "octocat", - payload: { sender: { id: 0 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalledWith( - 0, - ); - expect(octokit.rest.users.getByUsername).toHaveBeenCalledWith({ - username: "octocat", - }); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("treats non-integer sender id as missing and falls through to actor", async () => { - // Mirrors the Zod schema's `.int()` constraint on `github-user-id`. - // GitHub user IDs are integers in practice, but the runtime guard - // should match the schema's shape rather than admitting `1.5`. - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 444 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - actor: "octocat", - payload: { sender: { id: 1.5 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalledWith( - 1.5, - ); - expect(octokit.rest.users.getByUsername).toHaveBeenCalledWith({ - username: "octocat", - }); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("falls back to actor lookup for manual triggers", async () => { - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 555 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - // `workflow_dispatch` payloads do include `sender`, so use a payload - // shape (sender absent) that genuinely forces the actor branch. - const context = createMockContext({ - eventName: "workflow_dispatch", - actor: "octocat", - payload: {}, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(octokit.rest.users.getByUsername).toHaveBeenCalledWith({ - username: "octocat", - }); - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith(555); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("refuses to auto-resolve schedule events even when actor is present", async () => { - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "schedule", - actor: "workflow-editor", - payload: {}, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("schedule"); - expect(message).toContain("coder-username"); - expect(message).toContain("github-user-id"); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("refuses to auto-resolve schedule events even when sender.id is present", async () => { - // The schedule guard must be semantic, not positional. Today's - // `schedule` payloads omit `sender`, but if a future GHES extension - // or custom dispatch chain delivers `sender.id`, we still refuse - // rather than silently misattribute. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "schedule", - actor: "workflow-editor", - payload: { sender: { id: 12345 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("schedule"); - expect(message).toContain("coder-username"); - expect(message).toContain("github-user-id"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - }); - - test("fails with a clear error when no source resolves", async () => { - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "repository_dispatch", - actor: "", - payload: {}, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("coder-username"); - expect(message).toContain("github-user-id"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - }); - - test("wraps sender lookup failure with source and bypass instructions", async () => { - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new Error("No Coder user found with GitHub user ID 424242"), - ); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "issues", - payload: { sender: { id: 424242 } }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("github.context.payload.sender.id"); - expect(message).toContain("424242"); - expect(message).toContain( - "No Coder user found with GitHub user ID 424242", - ); - expect(message).toContain("coder-username"); - }); - - test("wraps actor getByUsername failure with source and bypass instructions", async () => { - octokit.rest.users.getByUsername.mockRejectedValue( - new Error("Not Found"), - ); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "workflow_dispatch", - actor: "missing-user", - payload: {}, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("github.context.actor"); - expect(message).toContain("missing-user"); - expect(message).toContain("Not Found"); - expect(message).toContain("coder-username"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("wraps actor Coder lookup failure with source and bypass instructions", async () => { - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 555 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new Error("No Coder user found with GitHub user ID 555"), - ); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "workflow_dispatch", - actor: "octocat", - payload: {}, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("github.context.actor"); - expect(message).toContain("octocat"); - expect(message).toContain("555"); - expect(message).toContain("No Coder user found with GitHub user ID 555"); - expect(message).toContain("coder-username"); - }); - - test("refuses auto-resolve on a fork pull request even with a sender.id", async () => { - // Hostile-trigger threat model: an attacker who happens to have a - // Coder identity could open a PR from a fork to bind their identity - // to the workflow's chat run. The trust gate refuses before any - // Coder API call. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "pull_request", - actor: "attacker", - payload: { - sender: { id: 99999 }, - pull_request: { - head: { - repo: { fork: true, full_name: "attacker/fork" }, - }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("fork"); - expect(message).toContain("coder-username"); - expect(message).toContain("github-user-id"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - expect(octokit.rest.users.getByUsername).not.toHaveBeenCalled(); - }); - - test("detects fork by head/base repo full_name mismatch when fork flag is absent", async () => { - // Some webhook deliveries omit `fork`. Fall back to comparing - // `full_name` so the gate still refuses cross-repo PRs. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "pull_request", - actor: "attacker", - payload: { - sender: { id: 99999 }, - pull_request: { - head: { repo: { full_name: "attacker/fork" } }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - expect((caught as Error).message).toContain("fork"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("refuses auto-resolve when head.repo is null (deleted fork)", async () => { - // When a fork's source repository is deleted, GitHub delivers - // `pull_request.head.repo` as `null`. The fork flag and the - // full_name comparison both yield falsy under optional chaining, - // so the gate must treat `null` head repo as a fork explicitly. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "pull_request", - actor: "attacker", - payload: { - sender: { id: 99999 }, - pull_request: { - head: { repo: null }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - expect((caught as Error).message).toContain("fork"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("refuses auto-resolve when comment.author_association is CONTRIBUTOR", async () => { - // Drive-by issue comment from a non-write user. The sender id - // would resolve under the old behavior; the trust gate must - // refuse before any Coder lookup. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "issue_comment", - actor: "drive-by", - payload: { - sender: { id: 99999 }, - comment: { author_association: "CONTRIBUTOR" }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("CONTRIBUTOR"); - expect(message).toContain("author_association"); - expect(message).toContain("coder-username"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("auto-resolves when MEMBER labels NONE opener's issue (sender is the labeler)", async () => { - // Realistic `issues: [labeled]` payload. The sender is a trusted - // MEMBER labeling an issue opened by a NONE user. The gate must - // NOT read `issue.author_association` (the opener) and refuse; - // it must auto-resolve the labeler's sender.id. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issues", - actor: "member-labeler", - payload: { - sender: { id: 424242 }, - issue: { author_association: "NONE" }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith( - 424242, - ); - expect(result.coderUsername).toBe(mockUser.username); - }); - - test("allows auto-resolve when comment.author_association is MEMBER", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issue_comment", - actor: "member-user", - payload: { - sender: { id: 424242 }, - comment: { author_association: "MEMBER" }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); - - const result = await action.run(); - - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith( - 424242, - ); - expect(result.coderUsername).toBe(mockUser.username); + const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; + const inputs = createMockInputs({ + existingChatId, + commentOnIssue: false, }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); - test("allows auto-resolve via comment.author_association for OWNER and COLLABORATOR", async () => { - for (const association of ["OWNER", "COLLABORATOR"] as const) { - const freshClient = new MockCoderClient(); - freshClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - freshClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "issue_comment", - actor: "trusted", - payload: { - sender: { id: 7 }, - comment: { author_association: association }, - }, - }); - const action = new CoderAgentChatAction( - freshClient, - octokit as unknown as Octokit, - inputs, - context, - ); + // The follow-up message succeeded, so the action should not fail red + // just because the chat fetch did. The outputs degrade gracefully. + const result = await action.run(); - const result = await action.run(); - expect(result.coderUsername).toBe(mockUser.username); - expect(freshClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith(7); - } - }); + expect(coderClient.mockCreateChatMessage).toHaveBeenCalled(); + expect(coderClient.mockGetChat).toHaveBeenCalledWith(existingChatId); - test("refuses auto-resolve when review.author_association is NONE", async () => { - // On `pull_request_review`, the reviewer is the sender. A NONE - // reviewer should not be able to drive auto-resolve even when the - // PR is same-repo. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "pull_request_review", - actor: "drive-by-reviewer", - payload: { - sender: { id: 99999 }, - review: { author_association: "NONE" }, - pull_request: { - head: { repo: { fork: false, full_name: "owner/repo" } }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); + const parsedResult = ActionOutputsSchema.parse(result); + expect(parsedResult.chatCreated).toBe(false); + expect(parsedResult.chatId).toBe(existingChatId); + expect(parsedResult.chatStatus).toBeUndefined(); + expect(parsedResult.chatTitle).toBeUndefined(); + expect(parsedResult.workspaceId).toBeUndefined(); + }); - let caught: unknown; - try { - await action.run(); - } catch (e) { - caught = e; - } - expect(caught).toBeInstanceOf(Error); - const message = (caught as Error).message; - expect(message).toContain("review.author_association"); - expect(message).toContain("NONE"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); - }); - - test("prefers comment.author_association over review.author_association", async () => { - // On `pull_request_review_comment`, both `comment` and `review` - // can appear in the payload. The comment is the more specific - // signal (it identifies the line-level commenter, not the - // containing review thread author), so comment wins. - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - }); - const context = createMockContext({ - eventName: "pull_request_review_comment", - actor: "drive-by", - payload: { - sender: { id: 99999 }, - comment: { author_association: "NONE" }, - review: { author_association: "MEMBER" }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, - ); + test("creates chat with workspace-id", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockCreateChat.mockResolvedValue(mockChat); - await expect(action.run()).rejects.toThrow(/NONE/); + const workspaceId = "550e8400-e29b-41d4-a716-446655440000"; + const inputs = createMockInputs({ + workspaceId, + commentOnIssue: false, }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); - test("coder-username bypasses the trust gate on a fork PR", async () => { - // Workflow author explicitly opted into running as a known - // service-account identity. The trust gate must not refuse: the - // fork PR's prompt is still attacker-controlled, but the workflow - // author has accepted the responsibility of that opt-in. - coderClient.mockGetCoderUserByUsername.mockResolvedValue({ - ...mockUser, - username: "bot-user", - }); + await action.run(); + + expect(coderClient.mockCreateChat).toHaveBeenCalledWith( + expect.objectContaining({ + workspace_id: workspaceId, + }), + ); + }); + + describe("commentOnIssue toggle", () => { + test("does not comment when commentOnIssue is false", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: "bot-user", commentOnIssue: false, }); - const context = createMockContext({ - eventName: "pull_request", - actor: "attacker", - payload: { - sender: { id: 99999 }, - pull_request: { - head: { repo: { fork: true, full_name: "attacker/fork" } }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - context, ); - const result = await action.run(); - expect(result.coderUsername).toBe("bot-user"); - expect(coderClient.mockGetCoderUserByGithubID).not.toHaveBeenCalled(); + await action.run(); + + expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); }); - test("github-user-id bypasses the trust gate on a fork PR", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("comments when commentOnIssue is true", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ - githubUserID: 7, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "pull_request", - actor: "attacker", - payload: { - sender: { id: 99999 }, - pull_request: { - head: { repo: { fork: true, full_name: "attacker/fork" } }, - base: { repo: { full_name: "owner/repo" } }, - }, - }, - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - context, + octokit.rest.issues.listComments.mockResolvedValue({ + data: [], + } as ReturnType); + octokit.rest.issues.createComment.mockResolvedValue( + {} as ReturnType, ); - const result = await action.run(); - expect(result.coderUsername).toBe(mockUser.username); - expect(coderClient.mockGetCoderUserByGithubID).toHaveBeenCalledWith(7); - }); - - test("workflow_dispatch carries no trust signal and auto-resolves", async () => { - // `workflow_dispatch` payloads carry neither pull_request nor - // author_association data. The gate returns `no-signal` and - // auto-resolve proceeds; GitHub already gates who can trigger - // `workflow_dispatch` (write access to the repo). - octokit.rest.users.getByUsername.mockResolvedValue({ - data: { id: 555 }, - } as unknown as ReturnType); - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: undefined, - commentOnIssue: false, - }); - const context = createMockContext({ - eventName: "workflow_dispatch", - actor: "trusted-user", - payload: {}, + githubURL: "https://github.com/owner/repo/issues/123", + commentOnIssue: true, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - context, ); - const result = await action.run(); - expect(result.coderUsername).toBe(mockUser.username); + await action.run(); + + expect(octokit.rest.issues.listComments).toHaveBeenCalled(); + expect(octokit.rest.issues.createComment).toHaveBeenCalled(); }); }); describe("wait=complete polling", () => { test("wait=none honors the wait gate: no getChat, no clock sleep", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, wait: "none", commentOnIssue: false, }); @@ -1760,7 +775,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1775,7 +789,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete polls getChat every 5 seconds until terminal", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -1786,7 +800,6 @@ describe("CoderAgentChatAction", () => { .mockResolvedValueOnce({ ...mockChat, status: "completed" }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -1796,7 +809,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1814,7 +826,7 @@ describe("CoderAgentChatAction", () => { // Polling must complete before the comment goes out, otherwise a // failure mid-poll would leave a stale "Agents Chat:" comment on // the issue while the workflow step itself fails. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -1830,7 +842,6 @@ describe("CoderAgentChatAction", () => { ); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: true, @@ -1840,7 +851,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1860,7 +870,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete fails with chat-error-kind=timeout when timeout reached", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -1871,7 +881,6 @@ describe("CoderAgentChatAction", () => { }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 10, commentOnIssue: false, @@ -1881,7 +890,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1901,7 +909,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete fails when chat enters error during polling", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -1915,7 +923,6 @@ describe("CoderAgentChatAction", () => { }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -1925,7 +932,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1936,8 +942,8 @@ describe("CoderAgentChatAction", () => { caught = err; } - // CODAGT-290 will refine last_error mapping; until then, - // every error terminal surfaces as api_error. + // Until last_error mapping is refined, every error terminal + // surfaces as api_error. expect(caught).toBeInstanceOf(ActionFailureError); const err = caught as ActionFailureError; expect(err.kind).toBe("api_error"); @@ -1947,7 +953,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete reaches terminal status, outputs reflect final chat state", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); const initialChat = { ...mockChat, status: "running" as const, @@ -1964,7 +970,6 @@ describe("CoderAgentChatAction", () => { .mockResolvedValueOnce(finalChat); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -1974,7 +979,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -1986,7 +990,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete also polls when existing-chat-id is set", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2001,7 +1005,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 600, @@ -2012,7 +1015,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2032,7 +1034,7 @@ describe("CoderAgentChatAction", () => { }); test("wait=complete fails with chat-error-kind=api_error when getChat throws", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -2042,7 +1044,6 @@ describe("CoderAgentChatAction", () => { ); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -2052,7 +1053,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2082,7 +1082,7 @@ describe("CoderAgentChatAction", () => { expect(err.chat).toBeDefined(); expect(err.chat?.status).toBe("running"); expect(err.chatId).toBeDefined(); - expect(err.chatUrl).toContain("/chats/"); + expect(err.chatUrl).toContain("/agents/"); expect(err.coderUsername).toBe(mockUser.username); }); @@ -2090,7 +1090,7 @@ describe("CoderAgentChatAction", () => { // `waiting` is terminal but ambiguous (agent done vs agent // waiting for input); pin the success path explicitly so a // regression that drops it from TERMINAL_STATUSES fails here. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -2101,7 +1101,6 @@ describe("CoderAgentChatAction", () => { }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -2111,7 +1110,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2123,7 +1121,7 @@ describe("CoderAgentChatAction", () => { test("wait=complete fails with default message when chat error has no last_error", async () => { // Covers the `last_error || fallback` branch. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -2135,7 +1133,6 @@ describe("CoderAgentChatAction", () => { }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -2145,7 +1142,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2167,7 +1163,7 @@ describe("CoderAgentChatAction", () => { // not return immediately on the first poll. // requireNonTerminalFirst forces the loop to observe the // agent transitioning before accepting any terminal. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2182,7 +1178,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 600, @@ -2193,7 +1188,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2211,7 +1205,7 @@ describe("CoderAgentChatAction", () => { // New-chat branch leaves requireNonTerminalFirst false: // createChat returns a fresh chat, so a terminal on the // first poll is real. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -2222,7 +1216,6 @@ describe("CoderAgentChatAction", () => { }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -2232,7 +1225,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2246,7 +1238,7 @@ describe("CoderAgentChatAction", () => { // First two getChat calls reject (transient outage); the third // returns a terminal status. The loop must stay alive across // the failures rather than failing fast on the first one. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue({ ...mockChat, status: "running", @@ -2257,7 +1249,6 @@ describe("CoderAgentChatAction", () => { .mockResolvedValueOnce({ ...mockChat, status: "completed" }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -2267,7 +1258,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2284,7 +1274,7 @@ describe("CoderAgentChatAction", () => { // atCreation, so the rewrap path is skipped: err.chat stays // undefined but err.chatId, chatUrl, and coderUsername are // still decorated for the failure outputs. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2294,7 +1284,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 600, @@ -2305,7 +1294,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2321,7 +1309,7 @@ describe("CoderAgentChatAction", () => { expect(err.kind).toBe("api_error"); expect(err.chat).toBeUndefined(); expect(String(err.chatId)).toBe(existingChatId); - expect(err.chatUrl).toContain("/chats/"); + expect(err.chatUrl).toContain("/agents/"); expect(err.coderUsername).toBe(mockUser.username); }); @@ -2330,7 +1318,7 @@ describe("CoderAgentChatAction", () => { // already in. The loop hits the timeout without ever // observing a non-terminal observation; the failure message // distinguishes this from a normal "ran out of time" timeout. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2341,7 +1329,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 10, @@ -2352,7 +1339,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2378,7 +1364,7 @@ describe("CoderAgentChatAction", () => { // only `sawNonTerminal` would treat both polls as stale; the // loop must accept the second terminal because it differs from // the first. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2392,7 +1378,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 600, @@ -2403,7 +1388,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2420,7 +1404,7 @@ describe("CoderAgentChatAction", () => { // Chat is in `waiting`, follow-up sent, agent fails within one // poll interval. Second poll sees `error` (different terminal): // the loop must reach throwOnChatError, not time out. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2434,7 +1418,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 600, @@ -2445,7 +1428,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2467,7 +1449,7 @@ describe("CoderAgentChatAction", () => { // MAX_CONSECUTIVE_POLL_FAILURES is reached. latest stays // undefined, so error.chat is undefined too, but error.chatId // must be populated from the options so chat-id output is set. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -2475,7 +1457,6 @@ describe("CoderAgentChatAction", () => { const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ - githubUserID: 12345, existingChatId, wait: "complete", waitTimeoutSeconds: 4, @@ -2486,7 +1467,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -2506,47 +1486,8 @@ describe("CoderAgentChatAction", () => { }); describe("Error Scenarios", () => { - test("throws error when Coder user not found", async () => { - // The real RealCoderClient.getCoderUserByGitHubId throws - // CoderAPIError with status 404; the mock must match so - // classifyError sees user_not_found rather than the api_error - // fallback. - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new CoderAPIError( - "No Coder user found with GitHub user ID 12345", - 404, - undefined, - "user_not_found", - ), - ); - octokit.rest.issues.listComments.mockResolvedValue({ - data: [], - } as ReturnType); - octokit.rest.issues.createComment.mockResolvedValue( - {} as ReturnType, - ); - - const inputs = createMockInputs({ githubUserID: 12345 }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await expect(action.run()).rejects.toThrow( - "No Coder user found with GitHub user ID 12345", - ); - // Assert the failure went through user_not_found classification - // (the comment body kind line proves classifyError matched). - const call = octokit.rest.issues.createComment.mock.calls[0]?.[0] as - | { body: string } - | undefined; - expect(call?.body).toContain("chat-error-kind=user_not_found"); - }); - test("throws error when chat creation fails", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new Error("Failed to create chat"), ); @@ -2557,12 +1498,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow("Failed to create chat"); @@ -2572,7 +1512,7 @@ describe("CoderAgentChatAction", () => { "posts a failure comment with chat-error-kind=spend_exceeded " + "and spent/limit amounts on 409 spend-exceeded shape", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError( "Coder API error: Conflict", @@ -2592,12 +1532,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2609,49 +1548,7 @@ describe("CoderAgentChatAction", () => { expect(call?.body).toContain("chat-error-kind=spend_exceeded"); expect(call?.body).toContain("$7.50"); expect(call?.body).toContain("$10.00"); - expect(call?.body).toContain("https://coder.test/chats"); - expect(call?.body).toContain( - "", - ); - }, - ); - - test( - "posts a failure comment with chat-error-kind=user_not_found and " + - "names the input that needs adjusting", - async () => { - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new CoderAPIError( - "No Coder user found with GitHub user ID 12345", - 404, - undefined, - "user_not_found", - ), - ); - octokit.rest.issues.listComments.mockResolvedValue({ - data: [], - } as ReturnType); - octokit.rest.issues.createComment.mockResolvedValue( - {} as ReturnType, - ); - - const inputs = createMockInputs({ githubUserID: 12345 }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await expect(action.run()).rejects.toThrow(); - - expect(octokit.rest.issues.createComment).toHaveBeenCalledTimes(1); - const call = octokit.rest.issues.createComment.mock.calls[0]?.[0] as - | { body: string } - | undefined; - expect(call?.body).toContain("chat-error-kind=user_not_found"); - expect(call?.body).toContain("github-user-id"); - expect(call?.body).toContain("coder-username"); + expect(call?.body).toContain("https://coder.test/agents"); expect(call?.body).toContain( "", ); @@ -2659,17 +1556,14 @@ describe("CoderAgentChatAction", () => { ); test( - "posts a failure comment with chat-error-kind=user_ambiguous and " + - "suggests coder-username", + "posts a failure comment with the org_not_found template body when " + + "resolveOrganizationID throws ActionFailureError(org_not_found)", async () => { - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new CoderAPIError( - "Multiple Coder users found with GitHub user ID 12345", - 409, - undefined, - "user_ambiguous", - ), - ); + // Without the ActionFailureError pre-check in handleFailure, this + // path classifies the thrown ActionFailureError as `api_error` + // and renders the api_error template, so the comment body + // disagrees with the `chat-error-kind` output. + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUserNoOrgs); octokit.rest.issues.listComments.mockResolvedValue({ data: [], } as ReturnType); @@ -2677,12 +1571,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({ coderOrganization: undefined }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2691,16 +1584,14 @@ describe("CoderAgentChatAction", () => { const call = octokit.rest.issues.createComment.mock.calls[0]?.[0] as | { body: string } | undefined; - expect(call?.body).toContain("chat-error-kind=user_ambiguous"); - expect(call?.body).toContain("coder-username"); - expect(call?.body).toContain( - "", - ); + expect(call?.body).toContain("chat-error-kind=org_not_found"); + expect(call?.body).toContain("no matching organization"); + expect(call?.body).not.toContain("An unexpected error occurred"); }, ); test("falls back to chat-error-kind=api_error for unknown 4xx shapes", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); @@ -2711,12 +1602,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2732,20 +1622,18 @@ describe("CoderAgentChatAction", () => { }); test("posts no failure comment when commentOnIssue=false", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); const inputs = createMockInputs({ - githubUserID: 12345, commentOnIssue: false, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2759,7 +1647,7 @@ describe("CoderAgentChatAction", () => { "updates existing failure comment in place when re-run with the " + "same marker key", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); @@ -2778,12 +1666,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2803,7 +1690,7 @@ describe("CoderAgentChatAction", () => { async () => { process.env.GITHUB_WORKFLOW = "doc-check"; try { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); @@ -2814,12 +1701,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2841,7 +1727,7 @@ describe("CoderAgentChatAction", () => { "posts a failure comment when github-url is a pull request URL " + "(end-to-end PR support)", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); @@ -2853,14 +1739,12 @@ describe("CoderAgentChatAction", () => { ); const inputs = createMockInputs({ - githubUserID: 12345, githubURL: "https://github.com/test-org/test-repo/pull/77", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow(); @@ -2884,7 +1768,7 @@ describe("CoderAgentChatAction", () => { "throws ActionFailureError carrying chat-error-* outputs on " + "the failure path", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); @@ -2895,12 +1779,11 @@ describe("CoderAgentChatAction", () => { {} as ReturnType, ); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -2925,19 +1808,18 @@ describe("CoderAgentChatAction", () => { "propagates the classified error when GitHub comment posting " + "itself fails", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( new CoderAPIError("Coder API error: Bad Request", 400, ""), ); // paginate (which findCommentByPredicate uses) rejects. octokit.paginate.mockRejectedValue(new Error("boom")); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -2951,72 +1833,55 @@ describe("CoderAgentChatAction", () => { }, ); - // `handleFailure`'s defensive catch around `parseGithubURL` keeps a - // malformed github-url from masking the original API error. The - // schema only validates URL syntax, so a URL like - // `https://github.com/foo` passes the schema but the regex does not - // match. - test( - "degrades gracefully when github-url passes schema but fails " + - "the issue/PR regex", - async () => { - // Failing the user lookup means parseGithubURL never runs in - // runInner; only handleFailure's defensive call hits the bad URL. - coderClient.mockGetCoderUserByGithubID.mockRejectedValue( - new CoderAPIError( - "No Coder user found with GitHub user ID 12345", - 404, - undefined, - "user_not_found", - ), - ); + // `parseGithubURL` runs first in `runInner` so a malformed + // `github-url` fails fast with a URL-parser error instead of + // masking some later API error. The schema only validates URL + // syntax, so a URL like `https://github.com/foo` passes the schema + // but the regex does not match. + test("fails fast when github-url passes schema but fails the issue/PR regex", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); - const inputs = createMockInputs({ - githubUserID: 12345, - // Passes schema (.url()) but does not match the issue/PR regex. - githubURL: "https://github.com/owner-only", - }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); + const inputs = createMockInputs({ + // Passes schema (.url()) but does not match the issue/PR regex. + githubURL: "https://github.com/owner-only", + }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); - let caught: unknown; - try { - await action.run(); - } catch (error) { - caught = error; - } - // The classified error survived the parseGithubURL throw inside - // handleFailure: chat-error-kind is user_not_found, not the - // parser's "Invalid GitHub URL" string. - expect(caught).toBeInstanceOf(ActionFailureError); - expect((caught as ActionFailureError).kind).toBe("user_not_found"); - expect((caught as ActionFailureError).message).toContain( - "No Coder user found", - ); - // No comment posted because the parser rejected the URL. - expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); - }, - ); + let caught: unknown; + try { + await action.run(); + } catch (error) { + caught = error; + } + // runInner parses github-url before reaching users/me; + // `getAuthenticatedUser` is never called. + expect(coderClient.mockGetAuthenticatedUser).not.toHaveBeenCalled(); + expect(caught).toBeInstanceOf(ActionFailureError); + expect((caught as ActionFailureError).kind).toBe("api_error"); + expect((caught as ActionFailureError).message).toContain( + "Invalid `github-url`", + ); + // No comment posted because the parser rejected the URL. + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + }); }); describe("Organization resolution", () => { test("resolves org by name to a UUID when coder-organization is set", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: "coder", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3031,19 +1896,17 @@ describe("CoderAgentChatAction", () => { ); }); - test("defaults to the resolved user's first org membership when coder-organization is unset", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("defaults to the Coder user's first org membership when coder-organization is unset", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3056,27 +1919,22 @@ describe("CoderAgentChatAction", () => { ); }); - test("defaults via getCoderUserByUsername when only coder-username is set", async () => { - coderClient.mockGetCoderUserByUsername.mockResolvedValue(mockUser); + test("resolves the token owner via getAuthenticatedUser when coder-organization is unset", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: mockUser.username, coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); - expect(coderClient.mockGetCoderUserByUsername).toHaveBeenCalledWith( - mockUser.username, - ); + expect(coderClient.mockGetAuthenticatedUser).toHaveBeenCalled(); expect(coderClient.mockGetOrganizationByName).not.toHaveBeenCalled(); expect(coderClient.mockCreateChat).toHaveBeenCalledWith( expect.objectContaining({ @@ -3085,18 +1943,16 @@ describe("CoderAgentChatAction", () => { ); }); - test("fails with chat-error-kind=org_not_found when the resolved user has no org memberships", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUserNoOrgs); + test("fails with chat-error-kind=org_not_found when the Coder user has no org memberships", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUserNoOrgs); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3112,20 +1968,18 @@ describe("CoderAgentChatAction", () => { }); test("wraps getOrganizationByName 404 in ActionFailureError(org_not_found)", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockGetOrganizationByName.mockRejectedValue( new CoderAPIError("Coder API error: Not Found", 404), ); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: "does-not-exist", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3144,20 +1998,18 @@ describe("CoderAgentChatAction", () => { }); test("non-404 CoderAPIError from getOrganizationByName is not classified as org_not_found", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockGetOrganizationByName.mockRejectedValue( new CoderAPIError("Coder API error: Unauthorized", 401), ); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: "coder", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3175,7 +2027,7 @@ describe("CoderAgentChatAction", () => { }); test("existing-chat-id flow does not resolve the organization", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUserNoOrgs); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUserNoOrgs); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); @@ -3185,7 +2037,6 @@ describe("CoderAgentChatAction", () => { // not trigger that resolution because createChatMessage inherits // the chat's organization. const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: undefined, existingChatId: "990e8400-e29b-41d4-a716-446655440000", }); @@ -3193,33 +2044,32 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const result = await action.run(); expect(coderClient.mockGetOrganizationByName).not.toHaveBeenCalled(); - expect(coderClient.mockGetCoderUserByUsername).not.toHaveBeenCalled(); + // users/me is still called once (the action emits the username + // output regardless of the existing-chat-id path), but the + // organization-resolution branch is correctly skipped. + expect(coderClient.mockGetAuthenticatedUser).toHaveBeenCalledTimes(1); expect(coderClient.mockCreateChatMessage).toHaveBeenCalled(); expect(coderClient.mockCreateChat).not.toHaveBeenCalled(); expect(result.chatCreated).toBe(false); }); - test("wraps getCoderUserByUsername 404 in ActionFailureError(user_not_found)", async () => { - coderClient.mockGetCoderUserByUsername.mockRejectedValue( + test("surfaces users/me 404 as api_error", async () => { + coderClient.mockGetAuthenticatedUser.mockRejectedValue( new CoderAPIError("Coder API error: Not Found", 404), ); const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: "missing-user", coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3229,31 +2079,29 @@ describe("CoderAgentChatAction", () => { caught = e; } + // users/me failures classify as api_error: a 404 here means the + // `coder-token` does not authenticate, not that some named user + // is missing. The original CoderAPIError is preserved on cause. expect(caught).toBeInstanceOf(ActionFailureError); - expect((caught as ActionFailureError).kind).toBe("user_not_found"); - expect((caught as ActionFailureError).message).toContain("missing-user"); - // Cause chain preserves the original CoderAPIError for debugging. + expect((caught as ActionFailureError).kind).toBe("api_error"); expect((caught as ActionFailureError).cause).toBeInstanceOf( CoderAPIError, ); expect(coderClient.mockCreateChat).not.toHaveBeenCalled(); }); - test("non-404 CoderAPIError from getCoderUserByUsername is not classified as user_not_found", async () => { - coderClient.mockGetCoderUserByUsername.mockRejectedValue( + test("users/me 401 (non-404) classifies as api_error", async () => { + coderClient.mockGetAuthenticatedUser.mockRejectedValue( new CoderAPIError("Coder API error: Unauthorized", 401), ); const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: "some-user", coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3264,7 +2112,7 @@ describe("CoderAgentChatAction", () => { } expect(caught).toBeInstanceOf(ActionFailureError); - expect((caught as ActionFailureError).kind).not.toBe("user_not_found"); + expect((caught as ActionFailureError).kind).toBe("api_error"); expect((caught as ActionFailureError).cause).toBeInstanceOf( CoderAPIError, ); @@ -3278,20 +2126,18 @@ describe("CoderAgentChatAction", () => { "770e8400-e29b-41d4-a716-446655440000", ], }; - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(multiOrgUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(multiOrgUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const warningSpy = spyOn(core, "warning").mockImplementation(() => {}); try { const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3312,20 +2158,18 @@ describe("CoderAgentChatAction", () => { }); test("single-org user does not emit a warning", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const warningSpy = spyOn(core, "warning").mockImplementation(() => {}); try { const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: undefined, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3337,7 +2181,7 @@ describe("CoderAgentChatAction", () => { }); test("ActionFailureError preserves the original error as `cause` when wrapping a 404", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); const originalError = new CoderAPIError( "Coder API error: Not Found", 404, @@ -3345,14 +2189,12 @@ describe("CoderAgentChatAction", () => { coderClient.mockGetOrganizationByName.mockRejectedValue(originalError); const inputs = createMockInputs({ - githubUserID: 12345, coderOrganization: "does-not-exist", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); let caught: unknown; @@ -3367,17 +2209,16 @@ describe("CoderAgentChatAction", () => { }); describe("Chat reuse", () => { - test("default: listChats is called with the gh-target and per-user scope before creating", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("default: listChats is called with the gh-target scope before creating", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([]); coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3386,10 +2227,12 @@ describe("CoderAgentChatAction", () => { const arg = coderClient.mockListChats.mock.calls[0]?.[0] as | { label?: string[]; archived?: boolean } | undefined; + // The per-user label is intentionally absent: all chats this + // action creates are owned by the `coder-token` holder, so a + // per-actor label would have added no isolation. expect(arg?.label).toEqual([ "coder-agents-chat-action:true", "gh-target:test-org/test-repo#123", - `coder-agents-chat-action-user:${mockUser.id}`, ]); expect(arg?.archived).toBe(false); expect(coderClient.mockCreateChat).toHaveBeenCalledTimes(1); @@ -3398,16 +2241,15 @@ describe("CoderAgentChatAction", () => { test("default: GITHUB_WORKFLOW is included in the lookup and on the created chat", async () => { process.env.GITHUB_WORKFLOW = "doc-check"; try { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([]); coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3429,17 +2271,16 @@ describe("CoderAgentChatAction", () => { } }); - test("default: writes the three core labels on the new chat", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("default: writes the two core labels on the new chat", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([]); coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3449,17 +2290,17 @@ describe("CoderAgentChatAction", () => { | undefined; expect(req?.labels?.["coder-agents-chat-action"]).toBe("true"); expect(req?.labels?.["gh-target"]).toBe("test-org/test-repo#123"); - expect(req?.labels?.["coder-agents-chat-action-user"]).toBe(mockUser.id); + // No per-user label: the chat owner is the token holder. + expect(req?.labels?.["coder-agents-chat-action-user"]).toBeUndefined(); // Workflow env unset; no workflow label and no sharding key. expect(Object.keys(req?.labels ?? {}).sort()).toEqual([ "coder-agents-chat-action", - "coder-agents-chat-action-user", "gh-target", ]); }); test("default + match: sends a follow-up via createChatMessage and does not create", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([ { ...mockChat, archived: false }, ]); @@ -3476,7 +2317,6 @@ describe("CoderAgentChatAction", () => { const modelConfigId = "d3a2b1c4-5678-49ab-bcde-1234567890ab"; const inputs = createMockInputs({ - githubUserID: 12345, chatPrompt: "continue the work", modelConfigId, }); @@ -3484,7 +2324,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const outputs = await action.run(); @@ -3515,12 +2354,11 @@ describe("CoderAgentChatAction", () => { }); test("default + match + wait=complete: polls until terminal status (no silent skip)", async () => { - // Regression test for DEREM-2: the reuse follow-up path must - // honor wait=complete the same way the existing-chat-id path - // does. A reuse-path follow-up to a chat already in a terminal - // status would otherwise return on the pre-message snapshot - // before the agent transitions. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + // The reuse follow-up path must honor wait=complete the same way + // the existing-chat-id path does. A reuse-path follow-up to a + // chat already in a terminal status would otherwise return on + // the pre-message snapshot before the agent transitions. + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([ { ...mockChat, archived: false, status: "waiting" }, ]); @@ -3536,7 +2374,6 @@ describe("CoderAgentChatAction", () => { .mockResolvedValueOnce({ ...mockChat, status: "completed" }); const inputs = createMockInputs({ - githubUserID: 12345, wait: "complete", waitTimeoutSeconds: 600, commentOnIssue: false, @@ -3546,7 +2383,6 @@ describe("CoderAgentChatAction", () => { coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), clock, ); @@ -3567,19 +2403,18 @@ describe("CoderAgentChatAction", () => { archived: false, status: "waiting" as const, }; - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([snapshot]); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); coderClient.mockGetChat.mockRejectedValue(new Error("network")); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); const outputs = await action.run(); @@ -3603,7 +2438,7 @@ describe("CoderAgentChatAction", () => { updated_at: "2026-02-01T00:00:00.000000Z", archived: false, }; - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([older, newer]); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, @@ -3611,12 +2446,11 @@ describe("CoderAgentChatAction", () => { coderClient.mockGetChat.mockResolvedValue(newer); const warnSpy = spyOn(core, "warning"); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3630,18 +2464,17 @@ describe("CoderAgentChatAction", () => { }); test("default + only archived match: creates a new chat (does not unarchive)", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([ { ...mockChat, archived: true }, ]); coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3651,21 +2484,19 @@ describe("CoderAgentChatAction", () => { }); test("existing-chat-id wins: lookup is skipped", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); coderClient.mockGetChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, existingChatId: mockChat.id, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3676,18 +2507,16 @@ describe("CoderAgentChatAction", () => { }); test("force-new-chat: skips lookup and creates a new chat with the action labels", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, forceNewChat: true, }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3703,19 +2532,17 @@ describe("CoderAgentChatAction", () => { | undefined; expect(req?.labels?.["coder-agents-chat-action"]).toBe("true"); expect(req?.labels?.["gh-target"]).toBe("test-org/test-repo#123"); - expect(req?.labels?.["coder-agents-chat-action-user"]).toBe(mockUser.id); }); test("listChats throws: error propagates with operation context", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockRejectedValue(new Error("boom")); - const inputs = createMockInputs({ githubUserID: 12345 }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await expect(action.run()).rejects.toThrow( @@ -3723,96 +2550,53 @@ describe("CoderAgentChatAction", () => { ); }); - test("distinct users on the same target each get their own chat (no cross-user hijack)", async () => { - // User B's lookup must carry their own user label so the chats API - // cannot AND-match a chat created with mockUser.id, and the new - // chat must be stamped with User B's UUID. - const userB: CoderSDKUser = { - ...mockUser, - id: "770e8400-e29b-41d4-a716-446655440777", - username: "userB", - }; - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(userB); - coderClient.mockListChats.mockResolvedValue([]); - coderClient.mockCreateChat.mockResolvedValue(mockChat); - - const inputs = createMockInputs({ githubUserID: 67890 }); - const action = new CoderAgentChatAction( - coderClient, - octokit as unknown as Octokit, - inputs, - createMockContext(), - ); - - await action.run(); - - const listArg = coderClient.mockListChats.mock.calls[0]?.[0] as - | { label?: string[] } - | undefined; - expect(listArg?.label).toContain( - `coder-agents-chat-action-user:${userB.id}`, - ); - expect(listArg?.label).not.toContain( - `coder-agents-chat-action-user:${mockUser.id}`, - ); - const createReq = coderClient.mockCreateChat.mock.calls[0]?.[0] as - | { labels?: Record } - | undefined; - expect(createReq?.labels?.["coder-agents-chat-action-user"]).toBe( - userB.id, - ); - }); - - test("coder-username path: per-user scope is applied via getCoderUserByUsername", async () => { - coderClient.mockGetCoderUserByUsername.mockResolvedValue(mockUser); + test("the reuse scope is intentionally not partitioned by acting user", async () => { + // All chats this action creates are owned by the `coder-token` + // holder, so the reuse scope does not include a per-actor label; + // workflows that want per-actor separation set + // `idempotency-key: ${{ github.actor }}` themselves. This test + // pins the absence of the per-user label so a regression that + // re-introduces it is caught. + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([]); coderClient.mockCreateChat.mockResolvedValue(mockChat); - const inputs = createMockInputs({ - githubUserID: undefined, - coderUsername: mockUser.username, - }); + const inputs = createMockInputs({}); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); - expect(coderClient.mockGetCoderUserByUsername).toHaveBeenCalledWith( - mockUser.username, - ); const listArg = coderClient.mockListChats.mock.calls[0]?.[0] as | { label?: string[] } | undefined; - expect(listArg?.label).toContain( - `coder-agents-chat-action-user:${mockUser.id}`, - ); + for (const label of listArg?.label ?? []) { + expect(label).not.toMatch(/^coder-agents-chat-action-user:/); + } const createReq = coderClient.mockCreateChat.mock.calls[0]?.[0] as | { labels?: Record } | undefined; - expect(createReq?.labels?.["coder-agents-chat-action-user"]).toBe( - mockUser.id, - ); + expect( + createReq?.labels?.["coder-agents-chat-action-user"], + ).toBeUndefined(); }); describe("idempotency-key sharding", () => { - test("adds the sanitized key to lookup and to the new chat's labels", async () => { - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("adds the sanitized key as the value of the fixed idempotency label", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); coderClient.mockListChats.mockResolvedValue([]); coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, idempotencyKey: "My Custom Key!", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); await action.run(); @@ -3820,43 +2604,47 @@ describe("CoderAgentChatAction", () => { const listArg = coderClient.mockListChats.mock.calls[0]?.[0] as | { label?: string[] } | undefined; - // The sanitized key is added as a `:true` filter. - expect( - listArg?.label?.some((l) => /^my-custom-key.*:true$/.test(l)), - ).toBe(true); + // The sanitized key is the value of a fixed key. User input + // cannot collide with an action-owned key under this scheme. + expect(listArg?.label).toContain( + "coder-agents-chat-action-idempotency:my-custom-key-", + ); const createReq = coderClient.mockCreateChat.mock.calls[0]?.[0] as | { labels?: Record } | undefined; - const extraKeys = Object.keys(createReq?.labels ?? {}).filter( - (k) => - k !== "coder-agents-chat-action" && - k !== "gh-target" && - k !== "coder-agents-chat-action-user", - ); - expect(extraKeys).toHaveLength(1); - expect(extraKeys[0]).toMatch(/^my-custom-key/); - expect(createReq?.labels?.[extraKeys[0]]).toBe("true"); + expect( + createReq?.labels?.["coder-agents-chat-action-idempotency"], + ).toBe("my-custom-key-"); }); - test("sharding key that sanitizes to a reserved label key is rejected fast", async () => { - // `idempotency-key: "gh-target"` would silently overwrite an - // action-owned scope label. Reject before any API call. - coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + test("idempotency-key cannot overwrite an action-owned label", async () => { + // The sanitized idempotency-key is the VALUE of the fixed + // `coder-agents-chat-action-idempotency` key, so even an + // `idempotency-key` of `gh-target` cannot displace the + // `gh-target` key; it just sets + // `coder-agents-chat-action-idempotency: gh-target`. + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockListChats.mockResolvedValue([]); + coderClient.mockCreateChat.mockResolvedValue(mockChat); const inputs = createMockInputs({ - githubUserID: 12345, idempotencyKey: "gh-target", }); const action = new CoderAgentChatAction( coderClient, octokit as unknown as Octokit, inputs, - createMockContext(), ); - await expect(action.run()).rejects.toThrow(/reserved chat-label key/); - expect(coderClient.mockListChats).not.toHaveBeenCalled(); - expect(coderClient.mockCreateChat).not.toHaveBeenCalled(); + await action.run(); + + const createReq = coderClient.mockCreateChat.mock.calls[0]?.[0] as + | { labels?: Record } + | undefined; + expect(createReq?.labels?.["gh-target"]).toBe("test-org/test-repo#123"); + expect( + createReq?.labels?.["coder-agents-chat-action-idempotency"], + ).toBe("gh-target"); }); }); }); diff --git a/src/action.ts b/src/action.ts index a4e093f..0f3f01a 100644 --- a/src/action.ts +++ b/src/action.ts @@ -9,21 +9,17 @@ import type { CoderSDKUser, CreateChatRequest, } from "./coder-client"; -import { - ACTION_LABEL_KEYS, - RESERVED_LABEL_KEYS, - sanitizeLabelKey, -} from "./sanitize-label-key"; +import { ACTION_LABEL_KEYS, sanitizeLabelToken } from "./sanitize-label-token"; import { buildCommentMarker, - buildDeploymentChatsUrl, + buildDeploymentAgentsUrl, buildFailureCommentBody, buildSuccessCommentBody, classifyError, deriveCommentKey, type FailureDetail, - GITHUB_URL_REGEX, normalizeBaseUrl, + parseGithubItemURL, upsertCommentByMarker, } from "./comment"; import type { ActionInputs, ActionOutputs, ChatErrorKind } from "./schemas"; @@ -95,202 +91,11 @@ export class ActionFailureError extends Error { chatUrl?: string; } -/** - * Stringify an unknown thrown value for a wrapping error message. Library - * code may throw `Error`s, bare strings, or arbitrary values. - */ -function describeError(err: unknown): string { - if (err instanceof Error) { - return err.message; - } - if (typeof err === "string") { - return err; - } - try { - return JSON.stringify(err); - } catch { - return String(err); - } -} - -/** - * GitHub `author_association` values that map to repository write access in - * the action's auto-resolve trust model. `OWNER` and `MEMBER` cover org - * and personal-repo owners; `COLLABORATOR` covers invited collaborators. - * Any other association (including `CONTRIBUTOR`, `FIRST_TIMER`, - * `FIRST_TIME_CONTRIBUTOR`, `MANNEQUIN`, `NONE`) is treated as untrusted. - * - * See: https://docs.github.com/en/graphql/reference/enums#commentauthorassociation - */ -const TRUSTED_AUTHOR_ASSOCIATIONS = new Set([ - "OWNER", - "MEMBER", - "COLLABORATOR", -]); - -/** - * Structural subset of `@actions/github`'s `Context` covering the fields the - * action reads. Production callers pass `github.context`; tests build - * fixtures via `createMockContext`. - * - * The auto-resolve trust gate (`classifyAutoResolveTrust`) reads - * `pull_request.head.repo` / `pull_request.base.repo` for fork detection, - * and `comment.author_association` / `review.author_association` as the - * sender-reliable trust signals. `issue.author_association` and - * `pull_request.author_association` are typed on the payload for - * completeness but the gate deliberately does not read them (they - * describe the resource opener, not the event sender). Fields are - * typed loosely because the full webhook schemas are large and - * event-specific. - */ -export interface ActionContext { - eventName: string; - actor: string; - payload: { - sender?: { - id?: number; - [key: string]: unknown; - }; - pull_request?: { - author_association?: string; - head?: { - repo?: { - fork?: boolean; - full_name?: string; - [key: string]: unknown; - } | null; - [key: string]: unknown; - }; - base?: { - repo?: { - full_name?: string; - [key: string]: unknown; - } | null; - [key: string]: unknown; - }; - [key: string]: unknown; - }; - issue?: { - author_association?: string; - [key: string]: unknown; - }; - comment?: { - author_association?: string; - [key: string]: unknown; - }; - review?: { - author_association?: string; - [key: string]: unknown; - }; - [key: string]: unknown; - }; -} - -/** - * Outcome of the auto-resolve trust gate. `trusted` means the gate found a - * repository-write-level signal and auto-resolve may proceed. `untrusted` - * means the gate found a signal that fails the bar (fork PR, low-trust - * association) and auto-resolve must refuse. `no-signal` means the - * payload carried nothing the gate can act on, so the gate defers to - * GitHub's underlying event-permission model (secret access, branch - * protection, etc.). - */ -type TrustClassification = - | { kind: "trusted"; reason: string } - | { kind: "untrusted"; reason: string } - | { kind: "no-signal" }; - -/** - * Classify whether the triggering identity from `context` is trusted for - * auto-resolve. - * - * Two layers of signal, applied in order: - * - * 1. Fork pull requests always refuse. An attacker who opens a PR from a - * fork must not be able to bind the workflow's Coder token to their - * own Coder identity (if they happen to have one) and execute - * attacker-controlled prompts. A `null` `head.repo` (deleted fork) is - * also treated as a fork: the only way `head.repo` becomes null is - * when the fork's source repository was deleted, which collapses the - * same-repo check below into a false negative. - * - * 2. `author_association` on `comment` or `review`, in that order. These - * are the only fields where the association describes the event - * *sender* rather than the resource *author*. On `issue_comment`, - * `comment.user` is the sender; on `pull_request_review`, - * `review.user` is the sender. By contrast, `issue.author_association` - * and `pull_request.author_association` describe the resource opener, - * not the labeler / assigner / reviewer who actually triggered the - * event. Reading them would refuse a trusted MEMBER labeling an - * issue opened by a NONE user. - * - * Returning `no-signal` is deliberate: events like `issues`, - * `pull_request` (same-repo), `workflow_dispatch`, `push`, and - * `repository_dispatch` carry no sender-association data the gate can - * trust, and the underlying GitHub permission model already gates who - * can trigger them. The trust gate is layered on top of, not in place - * of, those controls. - */ -function classifyAutoResolveTrust(context: ActionContext): TrustClassification { - const pr = context.payload.pull_request; - if (pr) { - const headRepo = pr.head?.repo; - const baseRepo = pr.base?.repo; - const headFullName = headRepo?.full_name; - const baseFullName = baseRepo?.full_name; - const isFork = - headRepo === null || - headRepo?.fork === true || - (typeof headFullName === "string" && - typeof baseFullName === "string" && - headFullName !== baseFullName); - if (isFork) { - return { - kind: "untrusted", - reason: - "the pull request is from a fork; auto-resolve refuses to bind " + - "the workflow's Coder identity to a fork-PR author", - }; - } - } - - // Only read `author_association` from `comment` and `review`: those - // are the only payload fields where the association describes the - // event sender rather than the resource author. `issue` and - // `pull_request` `author_association` describe the opener, which is - // frequently NOT the sender (a MEMBER labeling an issue, an assignee - // receiving an assignment, etc.). - const associations: Array<{ source: string; value: unknown }> = [ - { source: "comment", value: context.payload.comment?.author_association }, - { source: "review", value: context.payload.review?.author_association }, - ]; - for (const { source, value } of associations) { - if (typeof value !== "string" || value.length === 0) { - continue; - } - if (TRUSTED_AUTHOR_ASSOCIATIONS.has(value)) { - return { - kind: "trusted", - reason: `${source}.author_association is ${value}`, - }; - } - return { - kind: "untrusted", - reason: - `${source}.author_association is ${value}, which lacks ` + - "repository write access", - }; - } - - return { kind: "no-signal" }; -} - export class CoderAgentChatAction { constructor( private readonly coder: CoderClient, private readonly octokit: Octokit, private readonly inputs: ActionInputs, - private readonly context: ActionContext, private readonly clock: Clock = defaultClock, ) {} @@ -307,14 +112,21 @@ export class CoderAgentChatAction { throw new Error("Missing GitHub URL"); } - const match = this.inputs.githubURL.match(GITHUB_URL_REGEX); - if (!match) { - throw new Error(`Invalid GitHub URL: ${this.inputs.githubURL}`); + const parsed = parseGithubItemURL(this.inputs.githubURL); + if (!parsed) { + throw new Error( + `Invalid \`github-url\` input "${this.inputs.githubURL}". ` + + "Expected `https://github.com///issues/` or " + + "`https://github.com///pull/`. The action " + + "rejects non-github.com hosts so a workflow that templates " + + "user-controlled content into this input cannot redirect the " + + "action to an attacker-chosen repository.", + ); } return { - githubOrg: match[1], - githubRepo: match[2], - githubIssueNumber: parseInt(match[3], 10), + githubOrg: parsed.owner, + githubRepo: parsed.repo, + githubIssueNumber: parsed.number, }; } @@ -322,7 +134,7 @@ export class CoderAgentChatAction { * Generate chat URL. */ generateChatUrl(chatId: ChatId): string { - return `${normalizeBaseUrl(this.inputs.coderURL)}/chats/${chatId}`; + return `${normalizeBaseUrl(this.inputs.coderURL)}/agents/${chatId}`; } // Post or update the success comment on the linked issue or pull @@ -374,17 +186,6 @@ export class CoderAgentChatAction { }); } - /** - * Warn loudly when the user opts in to inputs whose runtime behavior - * is not yet wired. The schema accepts these so the contract is stable; - * the warning prevents silent no-ops for workflow authors who explicitly - * opt in. - */ - warnUnwiredInputs(): void { - // All v0 inputs are now wired. The helper remains for the test - // suite import and future unwired inputs. - } - /** * Build a rich ActionOutputs from a Chat response. */ @@ -531,7 +332,7 @@ export class CoderAgentChatAction { * Throw when a terminal chat ended in `error`; pass `waiting` and * `completed` through unchanged. The `api_error` kind is coarse: * a workflow branching on it cannot distinguish chat-level failures - * from polling-transport failures. CODAGT-290 will refine the + * from polling-transport failures. Future work may refine the * mapping by inspecting `last_error`. */ private throwOnChatError(chat: CoderChat): CoderChat { @@ -581,183 +382,6 @@ export class CoderAgentChatAction { } } - /** - * Resolve the Coder username to run as. Resolution order, high to low: - * - * 1. `coder-username` input. - * 2. `github-user-id` input. - * 3. `context.payload.sender.id` (issue, pull request, comment, and most - * webhook-driven events that carry the triggering user under `sender`). - * 4. `context.actor` for events whose payload lacks a usable `sender.id` - * (partial sender objects, bot dispatches, custom dispatch chains). - * Resolved to a numeric id via `octokit.rest.users.getByUsername`, - * then to a Coder user. - * - * `schedule` events are refused before any auto-resolve source: their - * `actor` is the workflow file's last editor, not a triggering identity. - * - * Throws naming both inputs when no source resolves. Intermediate - * failures are wrapped to name the auto-resolved source, preserve the - * upstream error, and recommend `coder-username` as the bypass. - * - * Before sources 3 and 4, a trust gate (`classifyAutoResolveTrust`) - * refuses auto-resolve for fork pull requests and for triggering - * identities whose `comment.author_association` or - * `review.author_association` lacks repository write access (anything - * other than `OWNER`, `MEMBER`, `COLLABORATOR`). This prevents a - * hostile-trigger attack where an attacker who happens to have a - * Coder identity could open a fork PR or drop a comment to bind - * their Coder identity to the workflow and execute - * attacker-controlled prompts under the workflow's Coder session - * token. Setting `coder-username` or `github-user-id` bypasses the - * trust gate: the workflow author has explicitly chosen the identity. - * - * Returns `{ username, user? }`. `user` is set when the identity path - * fetched a `CoderSDKUser` (sources 2-4); the explicit `coder-username` - * path (source 1) always now also fetches the user via - * `getCoderUserByUsername` so `user.id` is available for the - * idempotency-by-label per-user scope. - * `resolveOrganizationID` reuses `user` to read `organization_ids` - * without a redundant lookup. - */ - async resolveCoderUsername(): Promise<{ - username: string; - user: CoderSDKUser; - }> { - if (this.inputs.coderUsername) { - core.info(`Using provided Coder username: ${this.inputs.coderUsername}`); - // Fetch the full user so `user.id` is available downstream for - // the `coder-agents-chat-action-user` per-user reuse scope. - let coderUser: CoderSDKUser; - try { - coderUser = await this.coder.getCoderUserByUsername( - this.inputs.coderUsername, - ); - } catch (err) { - // Symmetric with the named-org 404 wrap in `resolveOrganizationID`. - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new ActionFailureError( - "user_not_found", - `Coder user '${this.inputs.coderUsername}' not found. ` + - "Check the `coder-username` input value.", - undefined, - { cause: err }, - ); - } - throw err; - } - return { username: coderUser.username, user: coderUser }; - } - if (this.inputs.githubUserID !== undefined) { - core.info( - `Looking up Coder user by GitHub user ID: ${this.inputs.githubUserID}`, - ); - const coderUser = await this.coder.getCoderUserByGitHubId( - this.inputs.githubUserID, - ); - return { username: coderUser.username, user: coderUser }; - } - - // Refuse before any auto-resolve source so the exclusion is semantic, - // not an artifact of source ordering. Today's `schedule` payloads - // omit `sender`, but a future shape that delivered it would still - // describe the underlying webhook trigger, not the cron run. - if (this.context.eventName === "schedule") { - throw new Error( - "Cannot auto-resolve a GitHub identity for `schedule` events: " + - "`github.context.actor` for cron-triggered runs is the workflow " + - "file's last editor, not the triggering user. " + - "Set the `coder-username` input to a Coder username, or set " + - "`github-user-id` to the GitHub numeric user id of the user the " + - "chat should run as.", - ); - } - - // Trust gate: before auto-resolving from `sender.id` or `actor`, - // refuse if the triggering identity comes from a fork PR or carries a - // low-trust `author_association`. Without this gate, an attacker who - // happens to have a Coder identity could open a fork PR or drop an - // issue comment to bind their Coder identity to the workflow and - // execute attacker-controlled prompts under the workflow's Coder - // token. Explicit `coder-username` and `github-user-id` inputs are - // handled above and bypass this gate by design. - const trust = classifyAutoResolveTrust(this.context); - if (trust.kind === "untrusted") { - throw new Error( - "Refusing to auto-resolve a GitHub identity: " + - `${trust.reason}. ` + - "Set the `coder-username` input to a Coder username, or set " + - "`github-user-id` to the GitHub numeric user id of the user " + - "the chat should run as.", - ); - } - if (trust.kind === "trusted") { - core.info(`Auto-resolve trust check passed: ${trust.reason}`); - } - - // Prefer `sender.id` over `actor`: it's already numeric, no extra - // API call. The guard mirrors `z.number().int().positive()` on the - // `github-user-id` input. - const senderId = this.context.payload?.sender?.id; - if ( - typeof senderId === "number" && - Number.isInteger(senderId) && - senderId > 0 - ) { - core.info( - `Auto-resolving Coder user from github.context.payload.sender.id: ${senderId}`, - ); - try { - const coderUser = await this.coder.getCoderUserByGitHubId(senderId); - return { username: coderUser.username, user: coderUser }; - } catch (err) { - throw new Error( - `Failed to resolve Coder user from github.context.payload.sender.id (${senderId}): ${describeError(err)}. ` + - "Set the `coder-username` input to bypass auto-resolution.", - ); - } - } - - // Actor fallback for events whose payload lacks a usable `sender.id`. - // `workflow_dispatch` payloads do include `sender.id`, so source 3 - // handles it; this branch covers partial sender objects, bot - // dispatches, and custom dispatch chains. - const actor = this.context.actor; - if (actor) { - core.info( - `Auto-resolving Coder user from github.context.actor: ${actor}`, - ); - let actorId: number; - try { - const { data } = await this.octokit.rest.users.getByUsername({ - username: actor, - }); - actorId = data.id; - } catch (err) { - throw new Error( - `Failed to resolve GitHub user id for github.context.actor (${actor}): ${describeError(err)}. ` + - "Set the `coder-username` input to bypass auto-resolution.", - ); - } - try { - const coderUser = await this.coder.getCoderUserByGitHubId(actorId); - return { username: coderUser.username, user: coderUser }; - } catch (err) { - throw new Error( - `Failed to resolve Coder user for github.context.actor (${actor}, GitHub user id ${actorId}): ${describeError(err)}. ` + - "Set the `coder-username` input to bypass auto-resolution.", - ); - } - } - - throw new Error( - "Could not auto-resolve a GitHub identity from the workflow context. " + - "Set the `coder-username` input to a Coder username, or set " + - "`github-user-id` to the GitHub numeric user id of the user the " + - "chat should run as.", - ); - } - /** * Resolve the organization id to send on createChat. Resolution order: * @@ -765,23 +389,18 @@ export class CoderAgentChatAction { * `GET /api/v2/organizations/{name}`. Recommended when the user * belongs to more than one organization, since the fallback choice * is non-deterministic; a `core.warning` is emitted in that case. - * 2. The resolved Coder user's `organization_ids[0]`. When identity was - * resolved via the GitHub-id path the user object is reused; the - * `coder-username` path looks the user up here via - * `getCoderUserByUsername`. + * 2. The Coder user's `organization_ids[0]`. The action calls + * `users/me` once in `runInner` and threads the result here, so this + * helper never refetches. * * Throws `ActionFailureError("org_not_found")` when `coder-organization` - * names an org that does not exist (HTTP 404) or the resolved user has no - * org memberships. Throws `ActionFailureError("user_not_found")` when only - * `coder-username` is set and the user is missing (HTTP 404). Other API - * errors propagate as `CoderAPIError`. The original error is attached via - * `options.cause` on every wrap; `run()`'s `handleFailure` re-classifies - * the failure into the failure-path comment. + * names an org that does not exist (HTTP 404) or the Coder user has + * no org memberships. Other API errors propagate as `CoderAPIError`. The + * original error is attached via `options.cause` on every wrap; + * `run()`'s `handleFailure` re-classifies the failure into the + * failure-path comment. */ - async resolveOrganizationID( - coderUsername: string, - resolvedUser: CoderSDKUser | undefined, - ): Promise { + async resolveOrganizationID(user: CoderSDKUser): Promise { if (this.inputs.coderOrganization) { core.info( `Resolving Coder organization by name: ${this.inputs.coderOrganization}`, @@ -807,28 +426,6 @@ export class CoderAgentChatAction { } } - // Default to the user's first org membership. Fetch the user lazily - // when only `coder-username` was provided; wrap a 404 into - // `user_not_found` symmetrically with the named-org 404 above. - let user: CoderSDKUser; - if (resolvedUser) { - user = resolvedUser; - } else { - try { - user = await this.coder.getCoderUserByUsername(coderUsername); - } catch (err) { - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new ActionFailureError( - "user_not_found", - `Coder user '${coderUsername}' not found. ` + - "Check the `coder-username` input value.", - undefined, - { cause: err }, - ); - } - throw err; - } - } const orgID = user.organization_ids[0]; if (!orgID) { throw new ActionFailureError( @@ -840,8 +437,8 @@ export class CoderAgentChatAction { } if (user.organization_ids.length > 1) { // `organization_ids` is server-built via `array_agg` with no - // `ORDER BY`, so the choice is non-deterministic across vacuums and - // restarts. Recommend pinning via `coder-organization`. + // `ORDER BY`, so the choice is non-deterministic across vacuums + // and restarts. Recommend pinning via `coder-organization`. core.warning( `Coder user '${user.username}' has ${user.organization_ids.length} organization memberships; ` + `defaulting to ${orgID}. ` + @@ -885,9 +482,16 @@ export class CoderAgentChatAction { // the failure-path output contract is uniform. private async handleFailure(error: unknown): Promise { // `detail` is the comment-body shape; `failure` is the thrown shape. - // Classify first so spend-exceeded fields land in the comment body - // for both raw-Error and ActionFailureError inputs. - const detail: FailureDetail = classifyError(error); + // Pass an ActionFailureError's `kind` through to the body so the + // posted comment matches `chat-error-kind`. `classifyError` lives + // in `comment.ts` and cannot import `ActionFailureError` (cycle). + // `spend_exceeded` carries extra fields on the body variant and is + // only ever produced by `classifyError`, never thrown as an + // ActionFailureError, so the narrow is exhaustive. + const detail: FailureDetail = + error instanceof ActionFailureError && error.kind !== "spend_exceeded" + ? { kind: error.kind, message: error.message } + : classifyError(error); const failure = error instanceof ActionFailureError ? error @@ -920,7 +524,7 @@ export class CoderAgentChatAction { deriveCommentKey({ ...this.inputs, workflow }), ); const body = buildFailureCommentBody(detail, { - chatsUrl: buildDeploymentChatsUrl(this.inputs.coderURL), + agentsUrl: buildDeploymentAgentsUrl(this.inputs.coderURL), marker, chatUrl: failure.chatUrl, chatStatus: failure.chat?.status, @@ -937,16 +541,21 @@ export class CoderAgentChatAction { } private async runInner(): Promise { - this.warnUnwiredInputs(); - - const { username: coderUsername, user: resolvedUser } = - await this.resolveCoderUsername(); - const { githubOrg, githubRepo, githubIssueNumber } = this.parseGithubURL(); core.info(`GitHub owner: ${githubOrg}`); core.info(`GitHub repo: ${githubRepo}`); core.info(`GitHub item number: ${githubIssueNumber}`); - core.info(`Coder username: ${coderUsername}`); + + // The chat owner on POST /api/experimental/chats is always the + // `coder-token` holder; the API has no owner override. The action + // fetches users/me once for the org pick and the `coder-username` + // output. The resulting username also tells the workflow author + // from the run log which Coder identity the chat ran as. + const tokenOwner = await this.coder.getAuthenticatedUser(); + const coderUsername = tokenOwner.username; + core.info( + `Resolved Coder user from \`coder-token\` (users/me): ${coderUsername}`, + ); // If an existing chat ID is provided, send a message to it if (this.inputs.existingChatId) { @@ -970,22 +579,16 @@ export class CoderAgentChatAction { } // Chat reuse: the action reuses the most recent non-archived chat - // scoped to this `gh-target`, the resolved Coder user, and the - // workflow name (when `GITHUB_WORKFLOW` is set), so re-runs and - // follow-up triggers converge on one chat per target/user/workflow. + // scoped to this `gh-target` and the workflow name (when + // `GITHUB_WORKFLOW` is set). All chats are owned by the token + // holder so the per-user reuse label is not part of the scope. // `force-new-chat` skips the lookup; `idempotency-key` shards // further so two workflow runs with the same scope can maintain - // distinct chats. - const sanitizedKey = this.inputs.idempotencyKey - ? sanitizeLabelKey(this.inputs.idempotencyKey) + // distinct chats. Workflows that want per-actor separation can + // set `idempotency-key: ${{ github.actor }}` themselves. + const sanitizedIdempotency = this.inputs.idempotencyKey + ? sanitizeLabelToken(this.inputs.idempotencyKey) : undefined; - if (sanitizedKey && RESERVED_LABEL_KEYS.has(sanitizedKey)) { - throw new Error( - `idempotency-key sanitizes to a reserved chat-label key ("${sanitizedKey}"). ` + - `Reserved keys: ${[...RESERVED_LABEL_KEYS].join(", ")}. ` + - "Choose a different idempotency-key value.", - ); - } const ghTarget = `${githubOrg}/${githubRepo}#${githubIssueNumber}`; const workflow = process.env.GITHUB_WORKFLOW || undefined; @@ -994,9 +597,8 @@ export class CoderAgentChatAction { } else { const follow = await this.findReuseMatch( ghTarget, - resolvedUser.id, workflow, - sanitizedKey, + sanitizedIdempotency, ); if (follow) { core.info(`Reusing existing chat: ${follow.id}`); @@ -1016,21 +618,13 @@ export class CoderAgentChatAction { // and resolving eagerly would fire an extra API call and a spurious // `org_not_found` failure for users with no org memberships. core.info("Creating new agents chat..."); - const organizationID = await this.resolveOrganizationID( - coderUsername, - resolvedUser, - ); + const organizationID = await this.resolveOrganizationID(tokenOwner); const req: CreateChatRequest = { organization_id: organizationID, content: [{ type: "text", text: this.inputs.chatPrompt }], workspace_id: this.inputs.workspaceId, model_config_id: this.inputs.modelConfigId, - labels: this.buildChatLabels( - ghTarget, - resolvedUser.id, - workflow, - sanitizedKey, - ), + labels: this.buildChatLabels(ghTarget, workflow, sanitizedIdempotency), }; const createdChat = await this.coder.createChat(req); @@ -1166,8 +760,13 @@ export class CoderAgentChatAction { /** * Most-recent non-archived chat matching the reuse scope, or undefined. - * Scope: gh-target + coder-user; workflow when GITHUB_WORKFLOW is set; - * sanitized idempotency-key when set. Warns on multiple matches. + * Scope: gh-target; workflow when GITHUB_WORKFLOW is set; sanitized + * idempotency-key when set. Warns on multiple matches. + * + * All chats this action creates are owned by the `coder-token` holder + * (the chats API has no owner override), so the reuse scope does not + * include a per-actor label. Workflows that want per-actor separation + * pass `idempotency-key: ${{ github.actor }}` themselves. * * The label set must stay in sync with `buildChatLabels`: a key the * lookup queries but the create branch doesn't write (or vice versa) @@ -1176,20 +775,18 @@ export class CoderAgentChatAction { */ private async findReuseMatch( ghTarget: string, - coderUserId: string, workflow: string | undefined, - sanitizedKey: string | undefined, + sanitizedIdempotency: string | undefined, ): Promise { const labels: string[] = [ `${ACTION_LABEL_KEYS.marker}:true`, `${ACTION_LABEL_KEYS.target}:${ghTarget}`, - `${ACTION_LABEL_KEYS.user}:${coderUserId}`, ]; if (workflow) { labels.push(`${ACTION_LABEL_KEYS.workflow}:${workflow}`); } - if (sanitizedKey) { - labels.push(`${sanitizedKey}:true`); + if (sanitizedIdempotency) { + labels.push(`${ACTION_LABEL_KEYS.idempotency}:${sanitizedIdempotency}`); } let chats: CoderChat[]; try { @@ -1233,9 +830,11 @@ export class CoderAgentChatAction { } /** - * Labels written on chat creation. Three are always written; the - * workflow label is added when GITHUB_WORKFLOW is set; the sanitized - * idempotency-key is added when set. + * Labels written on chat creation. The marker and gh-target labels + * are always written; the workflow label is added when + * `GITHUB_WORKFLOW` is set; the sanitized idempotency-key is added + * under the fixed `coder-agents-chat-action-idempotency` key when + * set. * * The label set must stay in sync with `findReuseMatch`: a key the * create branch writes but the lookup doesn't query (or vice versa) @@ -1244,29 +843,18 @@ export class CoderAgentChatAction { */ private buildChatLabels( ghTarget: string, - coderUserId: string, workflow: string | undefined, - sanitizedKey: string | undefined, + sanitizedIdempotency: string | undefined, ): Record { - // Defense in depth: `runInner` rejects collisions before any API - // call; this guards direct callers. - if (sanitizedKey && RESERVED_LABEL_KEYS.has(sanitizedKey)) { - throw new Error( - `idempotency-key sanitizes to a reserved chat-label key ("${sanitizedKey}"). ` + - `Reserved keys: ${[...RESERVED_LABEL_KEYS].join(", ")}. ` + - "Choose a different idempotency-key value.", - ); - } const labels: Record = { [ACTION_LABEL_KEYS.marker]: "true", [ACTION_LABEL_KEYS.target]: ghTarget, - [ACTION_LABEL_KEYS.user]: coderUserId, }; if (workflow) { labels[ACTION_LABEL_KEYS.workflow] = workflow; } - if (sanitizedKey) { - labels[sanitizedKey] = "true"; + if (sanitizedIdempotency) { + labels[ACTION_LABEL_KEYS.idempotency] = sanitizedIdempotency; } return labels; } diff --git a/src/coder-client.test.ts b/src/coder-client.test.ts index 2df5fbd..7bed088 100644 --- a/src/coder-client.test.ts +++ b/src/coder-client.test.ts @@ -6,9 +6,6 @@ import { } from "./coder-client"; import { mockUser, - mockUserList, - mockUserListEmpty, - mockUserListDuplicate, mockChat, mockChatMessageResponse, mockOrganization, @@ -27,147 +24,6 @@ describe("CoderClient", () => { global.fetch = mockFetch as unknown as typeof fetch; }); - describe("getCoderUserByGitHubId", () => { - test("returns the user when found", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserList)); - const result = await client.getCoderUserByGitHubId( - mockUser.github_com_user_id, - ); - expect(mockFetch).toHaveBeenCalledWith( - expect.stringMatching( - new RegExp( - `^https://coder\\.test/api/v2/users\\?q=.*github_com_user_id%3A${mockUser.github_com_user_id}.*$`, - ), - ), - expect.objectContaining({ - headers: expect.objectContaining({ - "Coder-Session-Token": "test-token", - }), - }), - ); - expect(result.id).toBe(mockUser.id); - expect(result.username).toBe(mockUser.username); - }); - - test("throws when multiple users found", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserListDuplicate)); - expect( - client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), - ).rejects.toThrow(CoderAPIError); - }); - - test("throws when no user found", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserListEmpty)); - expect( - client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), - ).rejects.toThrow(CoderAPIError); - }); - - test("sends only the github_com_user_id filter (no status filter)", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserList)); - await client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0); - const calledUrl = mockFetch.mock.calls[0]?.[0] as string; - const rawQuery = decodeURIComponent(calledUrl.split("?q=")[1] ?? ""); - expect(rawQuery).toContain( - `github_com_user_id:${mockUser.github_com_user_id}`, - ); - // `status:` would over-filter dormant and suspended users. - expect(rawQuery).not.toContain("status:"); - }); - - test("returns the live user when a soft-deleted user shares the github id", async () => { - const liveUser = { ...mockUser }; - const deletedUser = { - ...mockUser, - id: "770e8400-e29b-41d4-a716-446655440002", - username: "olddeleteduser", - deleted: true, - }; - mockFetch.mockResolvedValue( - createMockResponse({ users: [deletedUser, liveUser] }), - ); - const result = await client.getCoderUserByGitHubId( - mockUser.github_com_user_id ?? 0, - ); - expect(result.id).toBe(liveUser.id); - expect(result.username).toBe(liveUser.username); - }); - - test("keeps a user with explicit deleted: false (locks in three-state semantics)", async () => { - const liveUser = { ...mockUser, deleted: false }; - mockFetch.mockResolvedValue(createMockResponse({ users: [liveUser] })); - const result = await client.getCoderUserByGitHubId( - mockUser.github_com_user_id ?? 0, - ); - expect(result.id).toBe(liveUser.id); - expect(result.username).toBe(liveUser.username); - }); - - test("errors with user_ambiguous kind when two live users share the github id", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserListDuplicate)); - let caught: unknown; - try { - await client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0); - } catch (err) { - caught = err; - } - expect(caught).toBeInstanceOf(CoderAPIError); - expect((caught as CoderAPIError).kind).toBe("user_ambiguous"); - }); - - test("errors with user_not_found kind when all matching users are soft-deleted", async () => { - const deletedUser = { - ...mockUser, - id: "770e8400-e29b-41d4-a716-446655440003", - username: "olddeleteduser", - deleted: true, - }; - mockFetch.mockResolvedValue(createMockResponse({ users: [deletedUser] })); - let caught: unknown; - try { - await client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0); - } catch (err) { - caught = err; - } - expect(caught).toBeInstanceOf(CoderAPIError); - expect((caught as CoderAPIError).kind).toBe("user_not_found"); - }); - - test("errors with user_not_found kind when the response is empty", async () => { - mockFetch.mockResolvedValue(createMockResponse(mockUserListEmpty)); - let caught: unknown; - try { - await client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0); - } catch (err) { - caught = err; - } - expect(caught).toBeInstanceOf(CoderAPIError); - expect((caught as CoderAPIError).kind).toBe("user_not_found"); - }); - - test("throws on 401 unauthorized", async () => { - mockFetch.mockResolvedValue( - createMockResponse( - { error: "Unauthorized" }, - { ok: false, status: 401, statusText: "Unauthorized" }, - ), - ); - expect( - client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), - ).rejects.toThrow(CoderAPIError); - }); - - test("throws when GitHub user ID is 0", async () => { - await expect(client.getCoderUserByGitHubId(0)).rejects.toBeInstanceOf( - CoderAPIError, - ); - await expect(client.getCoderUserByGitHubId(0)).rejects.toThrow( - "GitHub user ID cannot be 0", - ); - expect(mockFetch).not.toHaveBeenCalled(); - }); - }); - describe("createChat", () => { test("normalizes a trailing slash on serverURL so the API URL has no double slash", async () => { const clientWithSlash = new RealCoderClient( @@ -281,16 +137,16 @@ describe("CoderClient", () => { }); }); - describe("getCoderUserByUsername", () => { - test("returns the user when found", async () => { + describe("getAuthenticatedUser", () => { + test("returns the user behind the configured token", async () => { mockFetch.mockResolvedValueOnce(createMockResponse(mockUser)); - const result = await client.getCoderUserByUsername(mockUser.username); + const result = await client.getAuthenticatedUser(); expect(result.id).toBe(mockUser.id); expect(result.username).toBe(mockUser.username); expect(mockFetch).toHaveBeenCalledWith( - `https://coder.test/api/v2/users/${mockUser.username}`, + "https://coder.test/api/v2/users/me", expect.objectContaining({ headers: expect.objectContaining({ "Coder-Session-Token": "test-token", @@ -299,41 +155,23 @@ describe("CoderClient", () => { ); }); - test("encodes the username in the URL path", async () => { - mockFetch.mockResolvedValueOnce(createMockResponse(mockUser)); - - await client.getCoderUserByUsername("user with space"); - - expect(mockFetch).toHaveBeenCalledWith( - "https://coder.test/api/v2/users/user%20with%20space", - expect.anything(), - ); - }); - - test("throws CoderAPIError on empty username without making a request", async () => { - await expect(client.getCoderUserByUsername("")).rejects.toThrow( - CoderAPIError, - ); - expect(mockFetch).not.toHaveBeenCalled(); - }); - - test("throws CoderAPIError with statusCode 404 on missing user", async () => { + test("propagates 401 as CoderAPIError", async () => { mockFetch.mockResolvedValueOnce( createMockResponse( - { error: "Not Found" }, - { ok: false, status: 404, statusText: "Not Found" }, + { error: "Unauthorized" }, + { ok: false, status: 401, statusText: "Unauthorized" }, ), ); let caught: unknown; try { - await client.getCoderUserByUsername("missing"); + await client.getAuthenticatedUser(); } catch (e) { caught = e; } expect(caught).toBeInstanceOf(CoderAPIError); - expect((caught as CoderAPIError).statusCode).toBe(404); + expect((caught as CoderAPIError).statusCode).toBe(401); }); }); diff --git a/src/coder-client.ts b/src/coder-client.ts index f581e92..15b1ff7 100644 --- a/src/coder-client.ts +++ b/src/coder-client.ts @@ -8,11 +8,13 @@ import { normalizeBaseUrl } from "./url"; export const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; export interface CoderClient { - getCoderUserByGitHubId( - githubUserId: number | undefined, - ): Promise; - - getCoderUserByUsername(username: string): Promise; + /** + * Resolve the Coder user the configured `coder-token` belongs to via + * `GET /api/v2/users/me`. The chat owner on `POST /api/experimental/chats` + * is always the token holder (the API has no owner override), so this is + * the Coder identity the chat runs as. + */ + getAuthenticatedUser(): Promise; getOrganizationByName(name: string): Promise; @@ -98,68 +100,12 @@ export class RealCoderClient implements CoderClient { return response.json() as Promise; } - async getCoderUserByGitHubId( - githubUserId: number | undefined, - ): Promise { - if (githubUserId === undefined) { - throw new CoderAPIError("GitHub user ID cannot be undefined", 400); - } - if (githubUserId === 0) { - // Defense in depth: the input schema rejects 0 upstream. Throw - // CoderAPIError so `instanceof` checks and classifyError routing - // downstream stay sound. - throw new CoderAPIError("GitHub user ID cannot be 0", 400); - } - // coderd's GetUsers SQL hardcodes `users.deleted = false`, so the - // response is already filtered server-side. The client-side - // filter below is forward-compatible defense in depth in case - // `codersdk.User` later starts serializing `deleted`. - const filter = `github_com_user_id:${githubUserId}`; - const endpoint = `/api/v2/users?q=${encodeURIComponent(filter)}`; - const response = await this.request(endpoint); - const userList = CoderSDKGetUsersResponseSchema.parse(response); - const liveUsers = userList.users.filter((u) => !u.deleted); - if (liveUsers.length === 0) { - throw new CoderAPIError( - `No Coder user found with GitHub user ID ${githubUserId}`, - 404, - undefined, - "user_not_found", - ); - } - if (liveUsers.length > 1) { - throw new CoderAPIError( - `Multiple Coder users found with GitHub user ID ${githubUserId}`, - 409, - undefined, - "user_ambiguous", - ); - } - return CoderSDKUserSchema.parse(liveUsers[0]); - } - - async getCoderUserByUsername(username: string): Promise { - if (!username) { - throw new CoderAPIError("Coder username cannot be empty", 400); - } - const endpoint = `/api/v2/users/${encodeURIComponent(username)}`; - try { - const response = await this.request(endpoint); - return CoderSDKUserSchema.parse(response); - } catch (err) { - // Re-throw 404 with the `user_not_found` kind so `classifyError` - // routes a typo in `coder-username` to the helpful failure - // comment rather than a generic `api_error`. - if (err instanceof CoderAPIError && err.statusCode === 404) { - throw new CoderAPIError( - `No Coder user found with username "${username}"`, - 404, - err.response, - "user_not_found", - ); - } - throw err; - } + async getAuthenticatedUser(): Promise { + // `users/me` resolves the session token to its owning user. No + // caching here; callers memoize when they need to reference the + // result more than once per run. + const response = await this.request("/api/v2/users/me"); + return CoderSDKUserSchema.parse(response); } async getOrganizationByName(name: string): Promise { @@ -224,8 +170,8 @@ export const ChatIdSchema = z.string().uuid().brand("ChatId"); export type ChatId = z.infer; // `deleted` is parsed leniently: `codersdk.User` does not currently -// serialize it, but we declare it so the filter in -// `getCoderUserByGitHubId` keeps working if the API exposes it later. +// serialize it. The field is retained for forward compatibility with a +// future server schema that exposes it. export const CoderSDKUserSchema = z.object({ id: z.string().uuid(), username: z.string(), @@ -236,13 +182,6 @@ export const CoderSDKUserSchema = z.object({ }); export type CoderSDKUser = z.infer; -export const CoderSDKGetUsersResponseSchema = z.object({ - users: z.array(CoderSDKUserSchema), -}); -export type CoderSDKGetUsersResponse = z.infer< - typeof CoderSDKGetUsersResponseSchema ->; - // Organization schema. Returned by `GET /api/v2/organizations/{name}` and // used to resolve the `coder-organization` input to a UUID for createChat. export const CoderOrganizationSchema = z.object({ @@ -345,33 +284,17 @@ export type CreateChatMessageResponse = z.infer< typeof CreateChatMessageResponseSchema >; -// Full enum for the `chat-error-kind` action output. This client only -// raises `user_not_found` and `user_ambiguous`; the rest are populated -// downstream when API errors are mapped to outputs. -export const ChatErrorKindSchema = z.enum([ - "user_not_found", - "user_ambiguous", - "org_not_found", - "spend_exceeded", - "api_error", - "timeout", -]); -export type ChatErrorKind = z.infer; - /** * CoderAPIError carries the status code and raw response body from a Coder - * API failure plus an optional `kind` discriminator. The kind is the - * structural link from this client to the failure-path classifier in - * comment.ts: classifying on `kind` rather than `err.message` regex - * means a string reword in the error message cannot silently degrade the - * `chat-error-kind` output to `api_error`. + * API failure. The body is preserved verbatim so the failure-path + * classifier in `comment.ts` can pattern-match structured shapes (e.g. + * the spend-exceeded 409) without rerunning the request. */ export class CoderAPIError extends Error { constructor( message: string, public readonly statusCode: number, public readonly response?: unknown, - public readonly kind?: ChatErrorKind, ) { super(message); this.name = "CoderAPIError"; diff --git a/src/comment.test.ts b/src/comment.test.ts index 7e30b5e..4f80fd3 100644 --- a/src/comment.test.ts +++ b/src/comment.test.ts @@ -2,15 +2,17 @@ import { describe, expect, mock, test } from "bun:test"; import { CoderAPIError } from "./coder-client"; import { buildCommentMarker, - buildDeploymentChatsUrl, + buildDeploymentAgentsUrl, buildFailureCommentBody, buildSuccessCommentBody, type ChatErrorKind, classifyError, deriveCommentKey, + DETAIL_BLOCK_MAX_CHARS, type FailureDetail, findCommentByPredicate, normalizeBaseUrl, + renderDetailBlock, type SuccessCommentContext, } from "./comment"; @@ -63,12 +65,16 @@ describe("deriveCommentKey", () => { ).toBe("owner/repo#42"); }); - test("handles enterprise GitHub URLs", () => { + test("falls back to the raw URL for non-github.com hosts (host validation)", () => { + // The regex anchors to github.com so an enterprise host (or + // attacker-chosen host) does not parse out a usable owner/repo. + // The marker still collapses identical URLs across re-runs, but + // does not pretend to know the target. expect( deriveCommentKey({ githubURL: "https://code.acme.com/owner/repo/issues/42", }), - ).toBe("owner/repo#42"); + ).toBe("https://code.acme.com/owner/repo/issues/42"); }); test("appends workflow suffix to the derived per-target key", () => { @@ -125,29 +131,6 @@ describe("classifyError", () => { }); }); - test("maps the user-not-found error from getCoderUserByGitHubId", () => { - const err = new CoderAPIError( - "No Coder user found with GitHub user ID 12345", - 404, - undefined, - "user_not_found", - ); - const result = classifyError(err); - expect(result.kind).toBe("user_not_found"); - expect(result.message).toContain("No Coder user found"); - }); - - test("maps the multi-user error from getCoderUserByGitHubId", () => { - const err = new CoderAPIError( - "Multiple Coder users found with GitHub user ID 12345", - 409, - undefined, - "user_ambiguous", - ); - const result = classifyError(err); - expect(result.kind).toBe("user_ambiguous"); - }); - test("falls back to api_error for unknown CoderAPIError shapes", () => { const err = new CoderAPIError("Coder API error: Bad Gateway", 502); const result = classifyError(err); @@ -196,32 +179,13 @@ describe("classifyError", () => { expect(result.kind).toBe("api_error"); expect(result.message).toBe("connection refused"); }); - - // errorCode takes precedence over the spend-exceeded body shape so the - // classifier never silently misclassifies a user-lookup error that - // happens to ride a 409 with a spend-shaped body. - test("errorCode takes precedence over a spend-shaped 409 body", () => { - const err = new CoderAPIError( - "Multiple Coder users found with GitHub user ID 12345", - 409, - JSON.stringify({ - message: "unrelated", - spent_micros: 1, - limit_micros: 2, - resets_at: "", - }), - "user_ambiguous", - ); - const result = classifyError(err); - expect(result.kind).toBe("user_ambiguous"); - }); }); describe("buildFailureCommentBody", () => { const marker = ""; - const chatsUrl = "https://coder.test/chats"; + const agentsUrl = "https://coder.test/agents"; - test("spend_exceeded body includes kind, dollar amounts, deployment chat URL, and marker", () => { + test("spend_exceeded body includes kind, dollar amounts, deployment agents URL, and marker", () => { const detail: FailureDetail = { kind: "spend_exceeded", message: "Chat usage limit exceeded.", @@ -229,34 +193,11 @@ describe("buildFailureCommentBody", () => { limitMicros: 5000000, resetsAt: "2026-05-01T00:00:00Z", }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); + const body = buildFailureCommentBody(detail, { agentsUrl, marker }); expect(body).toContain("chat-error-kind=spend_exceeded"); expect(body).toContain("$1.23"); expect(body).toContain("$5.00"); - expect(body).toContain(chatsUrl); - expect(body.endsWith(marker)).toBe(true); - }); - - test("user_not_found body names both identity inputs and ends with marker", () => { - const detail: FailureDetail = { - kind: "user_not_found", - message: "No Coder user found with GitHub user ID 12345", - }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); - expect(body).toContain("chat-error-kind=user_not_found"); - expect(body).toContain("github-user-id"); - expect(body).toContain("coder-username"); - expect(body.endsWith(marker)).toBe(true); - }); - - test("user_ambiguous body suggests coder-username and ends with marker", () => { - const detail: FailureDetail = { - kind: "user_ambiguous", - message: "Multiple Coder users found with GitHub user ID 12345", - }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); - expect(body).toContain("chat-error-kind=user_ambiguous"); - expect(body).toContain("coder-username"); + expect(body).toContain(agentsUrl); expect(body.endsWith(marker)).toBe(true); }); @@ -265,7 +206,7 @@ describe("buildFailureCommentBody", () => { kind: "api_error", message: "Coder API error: Bad Gateway", }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); + const body = buildFailureCommentBody(detail, { agentsUrl, marker }); expect(body).toContain("chat-error-kind=api_error"); expect(body).toContain("Coder API error: Bad Gateway"); expect(body.endsWith(marker)).toBe(true); @@ -279,7 +220,7 @@ describe("buildFailureCommentBody", () => { kind: "org_not_found", message: "Coder user has no organization memberships", }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); + const body = buildFailureCommentBody(detail, { agentsUrl, marker }); expect(body).toContain("chat-error-kind=org_not_found"); expect(body).toContain("coder-organization"); expect(body.endsWith(marker)).toBe(true); @@ -295,9 +236,9 @@ describe("buildFailureCommentBody", () => { "after 600s waiting for a terminal status", }; const chatUrl = - "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000"; + "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000"; const body = buildFailureCommentBody(detail, { - chatsUrl, + agentsUrl, chatUrl, marker, }); @@ -325,9 +266,9 @@ describe("buildFailureCommentBody", () => { "connection reset by peer", }; const chatUrl = - "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000"; + "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000"; const body = buildFailureCommentBody(detail, { - chatsUrl, + agentsUrl, chatUrl, marker, }); @@ -353,9 +294,9 @@ describe("buildFailureCommentBody", () => { message: "Anthropic 429 rate limit", }; const chatUrl = - "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000"; + "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000"; const body = buildFailureCommentBody(detail, { - chatsUrl, + agentsUrl, chatUrl, chatStatus: "error", marker, @@ -380,10 +321,10 @@ describe("buildFailureCommentBody", () => { kind: "api_error", message: "Coder API error: Bad Gateway", }; - const body = buildFailureCommentBody(detail, { chatsUrl, marker }); + const body = buildFailureCommentBody(detail, { agentsUrl, marker }); expect(body).toContain("**Coder Agents Chat: failed to start**"); expect(body).toContain("while running the action"); - expect(body).toContain(chatsUrl); + expect(body).toContain(agentsUrl); expect(body.endsWith(marker)).toBe(true); }, ); @@ -408,19 +349,58 @@ describe("normalizeBaseUrl", () => { }); }); -describe("buildDeploymentChatsUrl", () => { - test("appends /chats to a clean base URL", () => { - expect(buildDeploymentChatsUrl("https://coder.test")).toBe( - "https://coder.test/chats", +describe("buildDeploymentAgentsUrl", () => { + test("appends /agents to a clean base URL", () => { + expect(buildDeploymentAgentsUrl("https://coder.test")).toBe( + "https://coder.test/agents", ); }); test("normalizes trailing slash, query, and fragment before appending", () => { - expect(buildDeploymentChatsUrl("https://coder.test/?x=1")).toBe( - "https://coder.test/chats", + expect(buildDeploymentAgentsUrl("https://coder.test/?x=1")).toBe( + "https://coder.test/agents", ); - expect(buildDeploymentChatsUrl("https://coder.test/#a")).toBe( - "https://coder.test/chats", + expect(buildDeploymentAgentsUrl("https://coder.test/#a")).toBe( + "https://coder.test/agents", + ); + }); +}); + +describe("renderDetailBlock", () => { + test("wraps a plain message in a 4-backtick fenced block", () => { + // Attacker-influenced strings flowing through `detail.message` + // must not break out of the markdown list context. The body now + // renders inside a 4-backtick fence. + const body = renderDetailBlock("plain message"); + expect(body).toBe("- Detail:\n````\nplain message\n````"); + }); + + test("neutralizes a markdown-injection attempt with backtick fences", () => { + // An adversarial chat.last_error containing a 3-backtick block + // would close the surrounding fence and inject markdown after. + // 4-backtick fences keep the 3-backtick content inside the code + // block. + const body = renderDetailBlock("````\nclose-then-inject\n````"); + // The 4-backtick run inside the message is downgraded to 3 so + // the surrounding 4-backtick fence stays the only sequence that + // closes the block. + expect(body).toBe("- Detail:\n````\n```\nclose-then-inject\n```\n````"); + }); + + test("strips control bytes other than newline and tab", () => { + // CR-only line endings or ANSI escapes flow through agent error + // wrappers; stripping them keeps the comment renderer + // predictable and avoids terminal-escape leakage if an operator + // pipes the comment body to a terminal. + const body = renderDetailBlock("a\u0000b\u0007c\nd\te"); + expect(body).toBe("- Detail:\n````\nabc\nd\te\n````"); + }); + + test("caps the message at DETAIL_BLOCK_MAX_CHARS", () => { + const body = renderDetailBlock("x".repeat(DETAIL_BLOCK_MAX_CHARS * 2)); + // Header + fence open + cap + fence close + 3 newlines. + expect(body.length).toBe( + "- Detail:\n````\n".length + DETAIL_BLOCK_MAX_CHARS + "\n````".length, ); }); }); @@ -511,7 +491,7 @@ describe("findCommentByPredicate", () => { describe("buildSuccessCommentBody", () => { const marker = ""; const chatUrl = - "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000"; + "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000"; test( "wait=complete + completed body shows chat URL, status, PR URL, and " + diff --git a/src/comment.ts b/src/comment.ts index bad4fbe..fbb7aff 100644 --- a/src/comment.ts +++ b/src/comment.ts @@ -1,12 +1,8 @@ import * as core from "@actions/core"; import type { getOctokit } from "@actions/github"; -import { - type ChatErrorKind, - type ChatStatus, - CoderAPIError, -} from "./coder-client"; -import { sanitizeLabelKey } from "./sanitize-label-key"; -import type { ActionInputs } from "./schemas"; +import { type ChatStatus, CoderAPIError } from "./coder-client"; +import { sanitizeLabelToken } from "./sanitize-label-token"; +import type { ActionInputs, ChatErrorKind } from "./schemas"; import { normalizeBaseUrl } from "./url"; // Re-export so `action.ts` and tests keep their existing import sites. @@ -14,15 +10,49 @@ export { normalizeBaseUrl } from "./url"; type Octokit = ReturnType; -// Shared regex for GitHub issue and PR URLs. Used by `deriveCommentKey` and -// `parseGithubURL` so adding another path (e.g. `/discussions/`) is one edit. -// Anchored at the tail so URLs with extra path segments after the number -// (e.g. `.../issues/123/files`) are rejected rather than silently truncated. -// The `(?:[?#].*)?` group keeps the anchor tolerant of query strings and -// fragments that real-world `github-url` inputs can carry (e.g. a URL copied -// while viewing a specific comment). -export const GITHUB_URL_REGEX = - /([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)\/?(?:[?#].*)?$/; +// Anchored regex for a GitHub issue or PR URL on `github.com`. Anchored +// at both ends so a non-github host or extra path segments +// (e.g. `.../issues/123/files`, `https://attacker.example/owner/repo/issues/1`) +// are rejected rather than silently truncated. The `(?:[?#].*)?` group keeps +// the anchor tolerant of query strings and fragments that real-world +// `github-url` inputs can carry (e.g. a URL copied while viewing a specific +// comment). +const GITHUB_URL_REGEX = + /^https:\/\/github\.com\/([^/]+)\/([^/]+)\/(?:issues|pull)\/(\d+)\/?(?:[?#].*)?$/; + +/** + * Parsed components of a `github-url` input. Returned by + * `parseGithubItemURL` after host and path validation. + */ +export interface GithubItemURL { + owner: string; + repo: string; + number: number; +} + +/** + * Validate `input` as a `https://github.com///(issues|pull)/` + * URL and return its components, or `undefined` if it does not match. The + * host is anchored to `github.com` so a workflow that templates user- + * controlled content into `github-url` cannot coerce the action into + * commenting on an arbitrary attacker-chosen owner/repo. + */ +export function parseGithubItemURL( + input: string | undefined, +): GithubItemURL | undefined { + if (!input) { + return undefined; + } + const match = input.match(GITHUB_URL_REGEX); + if (!match) { + return undefined; + } + return { + owner: match[1], + repo: match[2], + number: Number.parseInt(match[3], 10), + }; +} // Discriminated union so spend-exceeded fields are only representable on the // spend-exceeded variant; the body builder reads them directly without a @@ -36,19 +66,14 @@ export type FailureDetail = resetsAt: string; } | { - kind: - | "user_not_found" - | "user_ambiguous" - | "org_not_found" - | "api_error" - | "timeout"; + kind: "org_not_found" | "api_error" | "timeout"; message: string; }; // chat-error-kind enum surfaced as the action's `chat-error-kind` output. -// Re-exported from `coder-client.ts`; this re-export keeps the name local -// to `comment.ts` callers and `index.ts` for backward source compatibility. -export type { ChatErrorKind } from "./coder-client"; +// Re-exported so callers can import the type from `comment.ts` next to +// `FailureDetail` / `classifyError` / `buildFailureCommentBody`. +export type { ChatErrorKind } from "./schemas"; const COMMENT_MARKER_PREFIX = ""; @@ -75,18 +100,18 @@ export function deriveCommentKey( }, ): string { if (inputs.idempotencyKey) { - return sanitizeLabelKey(inputs.idempotencyKey); + return sanitizeLabelToken(inputs.idempotencyKey); } - const match = inputs.githubURL.match(GITHUB_URL_REGEX); + const parsed = parseGithubItemURL(inputs.githubURL); let base: string; - if (!match) { + if (!parsed) { // The action validates githubURL upstream; if we get here the input is // malformed and the failure-path comment cannot find a stable target. // Fall back to the URL itself so re-runs at least collapse on identical // URLs, even if the marker is uglier. base = inputs.githubURL; } else { - base = `${match[1]}/${match[2]}#${match[3]}`; + base = `${parsed.owner}/${parsed.repo}#${parsed.number}`; } if (inputs.workflow) { return `${base}:${inputs.workflow}`; @@ -96,10 +121,7 @@ export function deriveCommentKey( // Map a thrown error to a FailureDetail. // -// Classification keys on explicit signals so a message reword cannot demote -// a kind to `api_error`: -// - `kind` on CoderAPIError (set by the client) marks user-lookup -// failures. +// Classification: // - 409 with the spend-exceeded body shape (`spent_micros`, `limit_micros`, // `resets_at`) becomes `spend_exceeded`. // - Anything else becomes `api_error`. The message is the body's `message` @@ -107,12 +129,6 @@ export function deriveCommentKey( // falls back to `err.message` only when the body is empty. export function classifyError(err: unknown): FailureDetail { if (err instanceof CoderAPIError) { - // Check the explicit error-code discriminator first so a client error - // can never be misclassified by an unrelated 409 body shape. - const code = mapErrorCodeToKind(err.kind); - if (code) { - return { kind: code, message: err.message }; - } const spend = parseSpendExceededBody(err.response); if (err.statusCode === 409 && spend) { return { @@ -134,18 +150,6 @@ export function classifyError(err: unknown): FailureDetail { return { kind: "api_error", message: String(err) }; } -function mapErrorCodeToKind( - code: ChatErrorKind | undefined, -): "user_not_found" | "user_ambiguous" | undefined { - switch (code) { - case "user_not_found": - case "user_ambiguous": - return code; - default: - return undefined; - } -} - interface SpendExceededFields { message: string; spentMicros: number; @@ -212,8 +216,34 @@ function formatMicrosAsDollars(micros: number): string { return `$${dollars.toFixed(2)}`; } +/** + * Render `detail.message` (or any externally-influenced string) inside a + * fenced code block so markdown syntax in the message body cannot break + * out of the failure-comment list. The fence is 4 backticks; any literal + * 4-or-more backtick run in the message is downgraded to 3 so the + * surrounding fence stays closable. Control bytes other than newline and + * tab are stripped so adversarial content cannot inject CR-only line + * endings or other terminal escapes. The result is capped at + * `DETAIL_BLOCK_MAX_CHARS` to bound comment size against runaway messages. + */ +export const DETAIL_BLOCK_MAX_CHARS = 4000; + +export function renderDetailBlock(message: string): string { + const stripped = message.replace( + // biome-ignore lint/suspicious/noControlCharactersInRegex: stripping C0 controls is the point. + /[\x00-\x08\x0B-\x1F\x7F]/g, + "", + ); + const capped = + stripped.length > DETAIL_BLOCK_MAX_CHARS + ? stripped.slice(0, DETAIL_BLOCK_MAX_CHARS) + : stripped; + const safe = capped.replace(/`{4,}/g, "```"); + return `- Detail:\n\`\`\`\`\n${safe}\n\`\`\`\``; +} + export interface FailureCommentContext { - chatsUrl: string; + agentsUrl: string; marker: string; // Chat-specific URL when the failure surfaced after the chat existed // (timeout, error-state terminal, polling-network blip). Flips the @@ -243,7 +273,7 @@ export function buildFailureCommentBody( const lines: string[] = [heading, ""]; const linkLine = ctx.chatUrl ? `View the chat in the Coder deployment: ${ctx.chatUrl}` - : `View chats in the Coder deployment: ${ctx.chatsUrl}`; + : `View agents in the Coder deployment: ${ctx.agentsUrl}`; switch (detail.kind) { case "spend_exceeded": lines.push( @@ -259,37 +289,13 @@ export function buildFailureCommentBody( } lines.push("", linkLine); break; - case "user_not_found": - lines.push( - "No Coder user could be resolved for this run. Adjust either " + - "the `github-user-id` input (the GitHub identity is not linked " + - "to a Coder user) or pass `coder-username` directly.", - "", - `- chat-error-kind=${detail.kind}`, - `- Detail: ${detail.message}`, - "", - linkLine, - ); - break; - case "user_ambiguous": - lines.push( - "Multiple Coder users matched the GitHub identity. Set the " + - "`coder-username` input to the specific account this workflow " + - "should run as.", - "", - `- chat-error-kind=${detail.kind}`, - `- Detail: ${detail.message}`, - "", - linkLine, - ); - break; case "org_not_found": lines.push( - "The resolved Coder user has no matching organization. Set the " + + "The Coder user has no matching organization. Set the " + "`coder-organization` input or grant the user a membership.", "", `- chat-error-kind=${detail.kind}`, - `- Detail: ${detail.message}`, + renderDetailBlock(detail.message), "", linkLine, ); @@ -298,7 +304,7 @@ export function buildFailureCommentBody( lines.push(apiErrorPhrase(runPhase, ctx), ""); lines.push( `- chat-error-kind=${detail.kind}`, - `- Detail: ${detail.message}`, + renderDetailBlock(detail.message), ); if (ctx.chatStatus === "error") { lines.push( @@ -317,7 +323,7 @@ export function buildFailureCommentBody( "`wait-timeout-seconds`.", "", `- chat-error-kind=${detail.kind}`, - `- Detail: ${detail.message}`, + renderDetailBlock(detail.message), "", linkLine, ); @@ -543,8 +549,8 @@ export async function upsertCommentByMarker(args: { }); } -// Deployment-level chats URL for the "view chats" link in the failure body. +// Deployment-level agents URL for the "view agents" link in the failure body. // We use the deployment list because a creation failure has no chat ID. -export function buildDeploymentChatsUrl(coderURL: string): string { - return `${normalizeBaseUrl(coderURL)}/chats`; +export function buildDeploymentAgentsUrl(coderURL: string): string { + return `${normalizeBaseUrl(coderURL)}/agents`; } diff --git a/src/index.test.ts b/src/index.test.ts deleted file mode 100644 index 4a81d3d..0000000 --- a/src/index.test.ts +++ /dev/null @@ -1,58 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { parseGithubUserID } from "./index"; - -describe("parseGithubUserID", () => { - test("returns undefined when input is empty", () => { - expect(parseGithubUserID("")).toBeUndefined(); - }); - - test("parses a plain decimal integer", () => { - expect(parseGithubUserID("123")).toBe(123); - }); - - test("returns NaN for trailing non-numeric characters", () => { - // The original #16 bug: parseInt would return 123 here and the - // schema would happily resolve to user 123. - expect(parseGithubUserID("123abc")).toBe(Number.NaN); - }); - - test("returns NaN for hex literals", () => { - // `Number("0x1F")` is 31, which would pass `int().positive()`. - // The regex gate must reject every non-decimal numeric form so - // non-decimal input can never silently resolve to a user. - expect(parseGithubUserID("0x1F")).toBe(Number.NaN); - }); - - test("returns NaN for binary literals", () => { - expect(parseGithubUserID("0b101")).toBe(Number.NaN); - }); - - test("returns NaN for octal literals", () => { - expect(parseGithubUserID("0o7")).toBe(Number.NaN); - }); - - test("returns NaN for scientific notation", () => { - expect(parseGithubUserID("1e3")).toBe(Number.NaN); - }); - - test("returns NaN for decimals", () => { - // GitHub user IDs are integers. Rejecting at the parser keeps - // the runtime guard's shape aligned with the schema's - // `.int()` constraint. - expect(parseGithubUserID("12.5")).toBe(Number.NaN); - }); - - test("returns NaN for negative numbers", () => { - expect(parseGithubUserID("-1")).toBe(Number.NaN); - }); - - test("returns NaN for whitespace-wrapped input", () => { - // `Number(" 123 ")` is 123. Whitespace tolerance was never - // intentional behavior. The regex rejects it. - expect(parseGithubUserID(" 123 ")).toBe(Number.NaN); - }); - - test("returns NaN for purely non-numeric input", () => { - expect(parseGithubUserID("abc")).toBe(Number.NaN); - }); -}); diff --git a/src/index.ts b/src/index.ts index 08d3159..bd38911 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,24 +5,8 @@ import { RealCoderClient } from "./coder-client"; import { setActionOutputs, setFailureOutputs } from "./outputs"; import { ActionInputsSchema } from "./schemas"; -// Convert the `github-user-id` workflow input to a number, or return -// undefined when unset. Returns NaN for anything that isn't a plain -// decimal integer literal so it fails schema parse instead of silently -// resolving to the wrong Coder user. `Number()` alone is too permissive: -// it accepts hex (`"0x1F"` -> 31), binary (`"0b101"` -> 5), octal -// (`"0o7"` -> 7), and scientific notation (`"1e3"` -> 1000), all of -// which would pass `z.number().int().positive()`. The bare regex gate -// rejects every non-decimal form. See #16. -export function parseGithubUserID(raw: string): number | undefined { - if (!raw) return undefined; - if (!/^\d+$/.test(raw)) return Number.NaN; - return Number(raw); -} - async function main() { try { - const githubUserID = parseGithubUserID(core.getInput("github-user-id")); - const inputs = ActionInputsSchema.parse({ coderURL: core.getInput("coder-url", { required: true }), coderToken: core.getInput("coder-token", { required: true }), @@ -30,8 +14,6 @@ async function main() { coderOrganization: core.getInput("coder-organization") || undefined, githubURL: core.getInput("github-url", { required: true }), githubToken: core.getInput("github-token", { required: true }), - githubUserID, - coderUsername: core.getInput("coder-username") || undefined, workspaceId: core.getInput("workspace-id") || undefined, modelConfigId: core.getInput("model-config-id") || undefined, existingChatId: core.getInput("existing-chat-id") || undefined, @@ -51,12 +33,7 @@ async function main() { core.debug("Clients initialized"); - const action = new CoderAgentChatAction( - coder, - octokit, - inputs, - github.context, - ); + const action = new CoderAgentChatAction(coder, octokit, inputs); const outputs = await action.run(); setActionOutputs(outputs); diff --git a/src/outputs.test.ts b/src/outputs.test.ts index 1e84dac..d705618 100644 --- a/src/outputs.test.ts +++ b/src/outputs.test.ts @@ -10,7 +10,7 @@ import { mockChat } from "./test-helpers"; const baseOutputs: ActionOutputs = { coderUsername: "u", chatId: "990e8400-e29b-41d4-a716-446655440000", - chatUrl: "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000", + chatUrl: "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000", chatCreated: true, }; @@ -270,14 +270,14 @@ describe("setFailureOutputs", () => { const cap = captureSetOutput(); try { const err = new ActionFailureError("timeout", "Timed out", mockChat); - err.chatUrl = "https://coder.test/chats/abc"; + err.chatUrl = "https://coder.test/agents/abc"; err.coderUsername = "testuser"; setFailureOutputs(err); expect(cap.calls).toContainEqual([ "chat-url", - "https://coder.test/chats/abc", + "https://coder.test/agents/abc", ]); expect(cap.calls).toContainEqual(["coder-username", "testuser"]); } finally { diff --git a/src/sanitize-label-key.test.ts b/src/sanitize-label-key.test.ts deleted file mode 100644 index 5be6412..0000000 --- a/src/sanitize-label-key.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { - ACTION_LABEL_KEYS, - RESERVED_LABEL_KEYS, - sanitizeLabelKey, -} from "./sanitize-label-key"; - -describe("sanitizeLabelKey", () => { - test("lowercases and replaces disallowed characters with '-'", () => { - expect(sanitizeLabelKey("My Custom Key!")).toBe("my-custom-key-"); - }); - - test("preserves the four punctuation classes the platform allows", () => { - expect(sanitizeLabelKey("a.b_c/d-e")).toBe("a.b_c/d-e"); - }); - - test("falls back to 'key' when the input sanitizes to empty", () => { - expect(sanitizeLabelKey("!@#$%")).toBe("key"); - expect(sanitizeLabelKey("")).toBe("key"); - }); - - test("trims leading non-alphanumeric characters before returning", () => { - expect(sanitizeLabelKey(".foo")).toBe("foo"); - expect(sanitizeLabelKey("---bar")).toBe("bar"); - expect(sanitizeLabelKey("/baz")).toBe("baz"); - }); - - test("truncates to 64 bytes", () => { - const seventy = "a".repeat(70); - const result = sanitizeLabelKey(seventy); - expect(result).toHaveLength(64); - expect(result).toBe("a".repeat(64)); - }); -}); - -describe("RESERVED_LABEL_KEYS", () => { - test("includes the per-user scope key that prevents cross-user hijack", () => { - // Without this entry, a sanitized idempotency-key value of - // "coder-agents-chat-action-user" would silently overwrite the - // per-user label and let any user impersonate any other on the - // same target. - expect(RESERVED_LABEL_KEYS.has("coder-agents-chat-action-user")).toBe(true); - }); - - test("includes the per-workflow scope key that prevents reuse-scope hijack", () => { - // Without this entry, a sanitized idempotency-key value of - // "coder-agents-chat-action-workflow" would silently overwrite the - // per-workflow label and break per-workflow reuse isolation. - expect(RESERVED_LABEL_KEYS.has("coder-agents-chat-action-workflow")).toBe( - true, - ); - }); - - test("is derived from ACTION_LABEL_KEYS so neither table can drift", () => { - // `findReuseMatch` and `buildChatLabels` both reference - // `ACTION_LABEL_KEYS` for the action-owned label keys; the reserved - // set must reserve every one of them. If a developer adds an entry - // to `ACTION_LABEL_KEYS` without updating the reserved set, an - // `idempotency-key` value matching the new key could silently - // overwrite it. - for (const key of Object.values(ACTION_LABEL_KEYS)) { - expect(RESERVED_LABEL_KEYS.has(key)).toBe(true); - } - }); -}); diff --git a/src/sanitize-label-token.test.ts b/src/sanitize-label-token.test.ts new file mode 100644 index 0000000..944032b --- /dev/null +++ b/src/sanitize-label-token.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from "bun:test"; +import { sanitizeLabelToken } from "./sanitize-label-token"; + +describe("sanitizeLabelToken", () => { + test("lowercases and replaces disallowed characters with '-'", () => { + expect(sanitizeLabelToken("My Custom Key!")).toBe("my-custom-key-"); + }); + + test("preserves the four punctuation classes the platform allows", () => { + expect(sanitizeLabelToken("a.b_c/d-e")).toBe("a.b_c/d-e"); + }); + + test("falls back to 'key' when the input sanitizes to empty", () => { + expect(sanitizeLabelToken("!@#$%")).toBe("key"); + expect(sanitizeLabelToken("")).toBe("key"); + }); + + test("trims leading non-alphanumeric characters before returning", () => { + expect(sanitizeLabelToken(".foo")).toBe("foo"); + expect(sanitizeLabelToken("---bar")).toBe("bar"); + expect(sanitizeLabelToken("/baz")).toBe("baz"); + }); + + test("truncates to 64 bytes", () => { + const seventy = "a".repeat(70); + const result = sanitizeLabelToken(seventy); + expect(result).toHaveLength(64); + expect(result).toBe("a".repeat(64)); + }); +}); diff --git a/src/sanitize-label-key.ts b/src/sanitize-label-token.ts similarity index 54% rename from src/sanitize-label-key.ts rename to src/sanitize-label-token.ts index 1934dc7..85079d5 100644 --- a/src/sanitize-label-key.ts +++ b/src/sanitize-label-token.ts @@ -8,25 +8,17 @@ export const ACTION_LABEL_KEYS = { marker: "coder-agents-chat-action", target: "gh-target", - user: "coder-agents-chat-action-user", workflow: "coder-agents-chat-action-workflow", + idempotency: "coder-agents-chat-action-idempotency", } as const; /** - * Reserved label keys on chats this action creates. A sanitized - * `idempotency-key` matching one of these is rejected upstream so the - * user input cannot overwrite an action-owned label. + * Coerce an arbitrary string into a chat-label token the platform accepts. + * The platform applies the same regex to label keys and label values: + * `^[a-zA-Z0-9][a-zA-Z0-9._/-]*$`, max 64 bytes. Empty results fall back + * to `"key"`. */ -export const RESERVED_LABEL_KEYS: ReadonlySet = new Set( - Object.values(ACTION_LABEL_KEYS), -); - -/** - * Coerce an arbitrary string into a chat-label key the platform accepts. - * Platform regex: `^[a-zA-Z0-9][a-zA-Z0-9._/-]*$`, max 64 bytes. Empty - * results fall back to `"key"`. - */ -export function sanitizeLabelKey(input: string): string { +export function sanitizeLabelToken(input: string): string { const lowered = input.toLowerCase(); const replaced = lowered.replace(/[^a-z0-9._/-]/g, "-"); const trimmed = replaced.replace(/^[^a-z0-9]+/, ""); diff --git a/src/schemas.test.ts b/src/schemas.test.ts index a0f0860..d144bfc 100644 --- a/src/schemas.test.ts +++ b/src/schemas.test.ts @@ -15,7 +15,6 @@ const actionInputValid: ActionInputs = { chatPrompt: "test prompt", githubURL: "https://github.com/owner/repo/issues/123", githubToken: "github-token", - githubUserID: 12345, commentOnIssue: true, wait: "none", waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, @@ -32,7 +31,6 @@ describe("ActionInputsSchema", () => { expect(result.chatPrompt).toBe(actionInputValid.chatPrompt); expect(result.githubURL).toBe(actionInputValid.githubURL); expect(result.githubToken).toBe(actionInputValid.githubToken); - expect(result.githubUserID).toBe(actionInputValid.githubUserID); }); test("accepts optional workspace-id", () => { @@ -97,13 +95,6 @@ describe("ActionInputsSchema", () => { expect(result.coderURL).toBe(url); } }); - - test("accepts both github-user-id and coder-username unset", () => { - const { githubUserID: _, ...withoutGithubUserID } = actionInputValid; - const result = ActionInputsSchema.parse(withoutGithubUserID); - expect(result.githubUserID).toBeUndefined(); - expect(result.coderUsername).toBeUndefined(); - }); }); describe("Invalid Input Cases", () => { @@ -159,83 +150,28 @@ describe("ActionInputsSchema", () => { }; expect(() => ActionInputsSchema.parse(input)).toThrow(); }); - }); - - describe("User Identification (Mutual Exclusion)", () => { - test("accepts input with only githubUserID", () => { - const result = ActionInputsSchema.parse(actionInputValid); - expect(result.githubUserID).toBe(12345); - expect(result.coderUsername).toBeUndefined(); - }); - - test("accepts input with only coderUsername", () => { - const { githubUserID: _, ...withoutGithubUserID } = actionInputValid; - const input = { ...withoutGithubUserID, coderUsername: "testuser" }; - const result = ActionInputsSchema.parse(input); - expect(result.coderUsername).toBe("testuser"); - expect(result.githubUserID).toBeUndefined(); - }); - - test("rejects input with both githubUserID and coderUsername", () => { - const input = { - ...actionInputValid, - coderUsername: "testuser", - }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); - test("rejects input with both existingChatId and forceNewChat", () => { - const input = { + test("rejects existing-chat-id combined with force-new-chat", () => { + const result = ActionInputsSchema.safeParse({ ...actionInputValid, - existingChatId: "00000000-0000-0000-0000-000000000001", + existingChatId: "550e8400-e29b-41d4-a716-446655440000", forceNewChat: true, - }; - expect(() => ActionInputsSchema.parse(input)).toThrow( - /existing-chat-id and force-new-chat/, + }); + expect(result.success).toBe(false); + if (result.success) { + return; + } + expect(result.error.issues).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + path: ["forceNewChat"], + message: expect.stringMatching( + /existing-chat-id and force-new-chat/, + ), + }), + ]), ); }); - - test("rejects githubUserID of 0", () => { - const input = { - ...actionInputValid, - githubUserID: 0, - }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); - - test("rejects negative githubUserID", () => { - const input = { - ...actionInputValid, - githubUserID: -1, - }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); - - // The schema's `.int().positive()` already rejected NaN before - // this PR. We pin it here so the schema cannot silently relax - // to admit `NaN`, which is what the parser produces for any - // non-decimal input (see src/index.test.ts). - test("rejects NaN githubUserID", () => { - const input = { - ...actionInputValid, - githubUserID: Number.NaN, - }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); - - test("rejects non-integer githubUserID", () => { - const input = { - ...actionInputValid, - githubUserID: 12.5, - }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); - - test("rejects empty coderUsername", () => { - const { githubUserID: _, ...withoutGithubUserID } = actionInputValid; - const input = { ...withoutGithubUserID, coderUsername: "" }; - expect(() => ActionInputsSchema.parse(input)).toThrow(); - }); }); describe("Wait mode", () => { @@ -326,7 +262,7 @@ describe("ActionOutputsSchema", () => { const minimalOutputs = { coderUsername: "testuser", chatId: "990e8400-e29b-41d4-a716-446655440000", - chatUrl: "https://coder.test/chats/990e8400-e29b-41d4-a716-446655440000", + chatUrl: "https://coder.test/agents/990e8400-e29b-41d4-a716-446655440000", chatCreated: true, }; diff --git a/src/schemas.ts b/src/schemas.ts index a50f847..976f2eb 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -4,10 +4,6 @@ import { z } from "zod"; // in sync if either changes. export const DEFAULT_WAIT_TIMEOUT_SECONDS = 600; -// Mutual exclusion of github-user-id and coder-username is enforced by -// the wrapper schema below. Both identity inputs are optional at the -// object level so the runtime can later auto-resolve from the workflow -// context. const ActionInputsObjectSchema = z.object({ chatPrompt: z.string().min(1), coderToken: z.string().min(1), @@ -15,8 +11,6 @@ const ActionInputsObjectSchema = z.object({ coderOrganization: z.string().min(1).optional(), githubURL: z.string().url(), githubToken: z.string().min(1), - githubUserID: z.number().int().positive().optional(), - coderUsername: z.string().min(1).optional(), workspaceId: z.string().uuid().optional(), modelConfigId: z.string().uuid().optional(), existingChatId: z.string().uuid().optional(), @@ -32,13 +26,6 @@ const ActionInputsObjectSchema = z.object({ }); export const ActionInputsSchema = ActionInputsObjectSchema.refine( - (data) => - !(data.githubUserID !== undefined && data.coderUsername !== undefined), - { - message: "Cannot set both github-user-id and coder-username; choose one.", - path: ["coderUsername"], - }, -).refine( (data) => !(data.existingChatId !== undefined && data.forceNewChat === true), { message: "Cannot set both existing-chat-id and force-new-chat; choose one.", @@ -52,8 +39,6 @@ export type ActionInputs = z.infer; // branch on this enum without parsing the human-readable message. export const ChatErrorKindSchema = z.enum([ "spend_exceeded", - "user_not_found", - "user_ambiguous", "org_not_found", "api_error", "timeout", diff --git a/src/test-helpers.ts b/src/test-helpers.ts index 811a7c8..130a071 100644 --- a/src/test-helpers.ts +++ b/src/test-helpers.ts @@ -6,7 +6,6 @@ import { } from "./coder-client"; import type { CoderSDKUser, - CoderSDKGetUsersResponse, CoderChat, CoderOrganization, CreateChatRequest, @@ -17,7 +16,6 @@ import type { import type { Clock } from "./action"; import type { ActionInputs } from "./schemas"; import { DEFAULT_WAIT_TIMEOUT_SECONDS } from "./schemas"; -import type { ActionContext } from "./action"; /** * Fake clock that records every sleep duration and treats sleeps as @@ -49,25 +47,6 @@ export const mockUser: CoderSDKUser = { github_com_user_id: 12345, }; -export const mockUserList: CoderSDKGetUsersResponse = { - users: [mockUser], -}; - -export const mockUserListEmpty: CoderSDKGetUsersResponse = { - users: [], -}; - -export const mockUserListDuplicate: CoderSDKGetUsersResponse = { - users: [ - mockUser, - { - ...mockUser, - id: "660e8400-e29b-41d4-a716-446655440001", - username: "testuser2", - }, - ], -}; - // User with no organization memberships. export const mockUserNoOrgs: CoderSDKUser = { ...mockUser, @@ -142,7 +121,6 @@ export function createMockInputs( coderOrganization: undefined, githubToken: "github-token", githubURL: "https://github.com/test-org/test-repo/issues/123", - githubUserID: 12345, commentOnIssue: true, wait: "none", waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, @@ -155,10 +133,6 @@ export function createMockInputs( * Mock CoderClient for testing */ export class MockCoderClient implements CoderClient { - public mockGetCoderUserByGithubID = mock(); - public mockGetCoderUserByUsername = mock((_username: string) => - Promise.resolve(mockUser), - ); public mockGetOrganizationByName = mock((_name: string) => Promise.resolve(mockOrganization), ); @@ -168,13 +142,10 @@ export class MockCoderClient implements CoderClient { public mockListChats = mock((_opts?: ListChatsOptions) => Promise.resolve([] as CoderChat[]), ); + public mockGetAuthenticatedUser = mock(() => Promise.resolve(mockUser)); - async getCoderUserByGitHubId(githubUserId: number): Promise { - return this.mockGetCoderUserByGithubID(githubUserId); - } - - async getCoderUserByUsername(username: string): Promise { - return this.mockGetCoderUserByUsername(username); + async getAuthenticatedUser(): Promise { + return this.mockGetAuthenticatedUser(); } async getOrganizationByName(name: string): Promise { @@ -201,21 +172,6 @@ export class MockCoderClient implements CoderClient { } } -/** - * Build a minimal `ActionContext` shaped like `@actions/github`'s Context. - * Tests populate only the fields they exercise. - */ -export function createMockContext( - overrides?: Partial, -): ActionContext { - return { - eventName: "", - actor: "", - payload: {}, - ...overrides, - }; -} - /** * Mock Octokit for testing. Includes a `paginate` mock so tests for code * paths that walk every comment with `octokit.paginate(listComments, ...)`