Skip to content

Follow-up to #268: cover external and bot comment authors, validate --id - #270

Merged
schpetbot merged 1 commit into
mainfrom
oss-pr-lander/268
Aug 31, 2026
Merged

schpetbot merged 1 commit into
mainfrom
oss-pr-lander/268

Conversation

@schpetbot

Copy link
Copy Markdown
Collaborator

Follow-up to #268. Draft until #268 lands — this branch currently contains @leonardsellem's commits too, and will be rebased down to just the delta once #268 is merged.

#268 adds user.id and editedAt to issue comment list --json, and the shape is right: it passes the raw connection straight through, so GraphQL field names and nesting are preserved per CLAUDE.md. This fills in the cases it stops short of.

externalUser has the same problem user does

It didn't get the same treatment, and the schema is explicit about why it needs it — ExternalUser.displayName "Can match the display name of an actual user". So a consumer could disambiguate two workspace members from each other and still not tell a member from an external commenter with the same name. It now carries id too.

Integration-authored comments had no author identity at all

This is the bigger gap. A comment posted by GitHub, Slack, or a workflow has user and externalUser both null, so it arrived in the JSON with nothing identifying its author — the exact problem #268 sets out to solve, for a whole class of comment it doesn't reach.

Selecting botActor gives those comments type (non-null, and the reliable key here — ActorBot is not a Node and its id is nullable), plus subType, name, and id.

botActor.userDisplayName is available and deliberately left out: it names a person in an external system and is display-only, so it isn't worth the exposure to solve an identity problem the other fields already solve. Easy to add if you disagree.

A rendering bug we were one field away from

Because the query never asked who the bot was, every GitHub/Slack/workflow comment printed as @Unknown. Now that we fetch botActor, it renders the bot's name (falling back to its type when name is null):

- @Unknown commented 1/15/2024 [comment-uuid-1]
+ @GitHub commented 1/15/2024 [comment-uuid-1]

The author fallback was duplicated for root comments and replies; it's now one helper, with botActor checked last so a comment carrying both a user and a bot actor still renders the human. Every pre-existing snapshot is byte-identical — that's the regression assertion.

--id was forwarded unvalidated

--id <uuid:string> is a cliffy string, so a typo reached the API and came back as a raw GraphQL error. CLAUDE.md asks for an immediate, actionable error instead. Linear documents CommentCreateInput.id as "The identifier in UUID v4 format", so this checks v4 specifically — the shared isLinearUuid() is deliberately lax (it's used to tell UUIDs apart from names elsewhere, a different job), so this validation is local:

✗ Failed to add comment: Invalid comment ID: nope
  --id must be a v4 UUID, like 123e4567-e89b-42d3-a456-426614174000.

The flag stays hidden, now with a comment recording why it exists (caller-supplied ids make retries idempotent).

Verification

565 tests pass; deno task check, deno lint, deno fmt --check clean; deno task codegen produces no further diff. Manually exercised: --id absent from comment add --help, invalid and non-v4 values rejected with exit 1 and no stack trace, a valid v4 passing validation through to the issue lookup.

#268 adds `user.id` and `editedAt` to `issue comment list --json`, which is the
right shape -- the raw connection is passed straight through, so GraphQL field
names and nesting are preserved. This fills in the cases it stops short of.

`externalUser` did not get the same treatment as `user`, but it has the same
problem and the schema is explicit about why: ExternalUser.displayName "can
match the display name of an actual user". So a consumer could disambiguate two
workspace members from each other and still be unable to tell a member from an
external commenter with the same name. It now carries `id` too.

Integration-authored comments were the bigger gap. They have `user` and
`externalUser` both null, so they arrived in the JSON with no author
information at all -- the exact problem #268 sets out to fix, for a whole class
of comment it does not reach. Selecting `botActor` gives them `type` (non-null,
the reliable key, since ActorBot is not a Node and its `id` is nullable) plus
`subType`, `name` and `id`. `userDisplayName` is available but left out: it
names a person in an external system and is display-only, so it is not worth
the exposure to solve an identity problem the other fields already solve.

That also fixes a rendering bug we were one field away from: every GitHub,
Slack and workflow comment printed as `@Unknown`, because the query never asked
who the bot was. The author fallback was duplicated for root comments and
replies; it is now one helper, with botActor checked last so a comment carrying
both a user and a bot actor still renders the human, and every existing
snapshot stays byte-identical.

Finally, `--id` was forwarded to the API unvalidated. The repo already has
`isLinearUuid`, and CLAUDE.md asks for an immediate, actionable error when
user-supplied input is malformed rather than a raw GraphQL failure, so a
non-UUID is now rejected before the request with a message showing the expected
shape. The flag stays hidden, with a comment recording why it exists.
@schpetbot
schpetbot marked this pull request as ready for review August 31, 2026 20:36
@schpetbot
schpetbot merged commit 8b187e6 into main Aug 31, 2026
15 checks passed
@schpetbot
schpetbot deleted the oss-pr-lander/268 branch August 31, 2026 20:39
@schpetbot

Copy link
Copy Markdown
Collaborator Author

Shipped in v2.6.0.

Follow-up to #268.

  • issue comment list no longer shows @Unknown for comments posted by an integration or bot — the query now asks for botActor and renders the bot's name, falling back to its type.
  • --json exposes botActor (id, name, type, subType) alongside user.id and externalUser.id.
  • issue comment add --id rejects a value that is not a v4 UUID with an actionable error, instead of forwarding it and surfacing a raw API error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants