Conversation
HEY's seen, unseen and moves endpoints scope the requested ids to the identity's postings and answer success whatever matched, and there is no per-posting read to check against, so the CLI cannot answer not_found for a wrong-kind id there. The skill said it does, which is what let an agent trust a week of no-op marks (hey-cli#396). Say what actually happens and how to confirm, until HEY reports the non-match.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The recommended verification command omits its required box argument and only displays help.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates agent guidance to accurately describe silent no-ops when incorrect IDs are passed to posting commands.
Changes:
- Documents affected commands and verification guidance.
- Clarifies which commands still return
not_found.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
skills/hey/SKILL.md |
Corrects ID-error behavior guidance. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c6e4f83f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Box names: `imbox`, `feedbox`, `trailbox`, `asidebox`, `laterbox`, `bubblebox` | ||
|
|
||
| **Response format:** `hey box view --json` returns the box itself — `id`, `kind`, `name`, `app_url`, `next_history_url`, `next_page` — with a `postings` array of the email threads in it. Each posting has: `id` (box item ID), `topic_id` (thread ID), `name` (subject), `seen` (read status), `created_at`, `contacts`, `summary`, `app_url`, `visible_entry_count`. Use `id` for `hey seen`, `hey unseen`, `hey move`, `hey label add`, `hey label remove`, `hey trash`, `hey spam`, `hey ignore`, and `hey stop-ignoring`, and `topic_id` for `hey thread read`, `hey reply`, `hey forward`, `hey share` and `hey attachment list`. A box item `id` passed to `hey thread read` answers `not_found`, and so does a `topic_id` passed to `hey move`. | ||
| **Response format:** `hey box view --json` returns the box itself — `id`, `kind`, `name`, `app_url`, `next_history_url`, `next_page` — with a `postings` array of the email threads in it. Each posting has: `id` (box item ID), `topic_id` (thread ID), `name` (subject), `seen` (read status), `created_at`, `contacts`, `summary`, `app_url`, `visible_entry_count`. Use `id` for `hey seen`, `hey unseen`, `hey move`, `hey label add`, `hey label remove`, `hey trash`, `hey spam`, `hey ignore`, and `hey stop-ignoring`, and `topic_id` for `hey thread read`, `hey reply`, `hey forward`, `hey share` and `hey attachment list`. A box item `id` passed to `hey thread read` answers `not_found`. The reverse is not caught yet: `hey seen`, `hey unseen` and `hey move` given a `topic_id` (or any id that is not one of your box items) answer success and change nothing, because HEY's endpoints do not report a non-match — so confirm with `hey box view --json` (`seen`, or the thread's box) rather than trusting the envelope. `hey trash`, `hey spam` and the label commands do answer `not_found`. |
There was a problem hiding this comment.
For a posting outside the box's first page, this confirmation cannot establish whether the mutation worked: hey box view --json has no mailbox argument, so boxCommand.run only prints help, and even hey box view <box> --json reads just one page. Direct the agent to re-read the source/destination box with --all or the cursor for the page containing the posting; otherwise the new warning can still lead it to misreport a successful operation as missing or leave a no-op unverified.
AGENTS.md reference: AGENTS.md:L393-L399
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in b231f73 (this review ran against the first commit): the check now names the box and reads every page — hey box view <box> --json --all --jq '.data.postings[] | select(.id == <id>) | .seen', and the destination box for a move.
| Box names: `imbox`, `feedbox`, `trailbox`, `asidebox`, `laterbox`, `bubblebox` | ||
|
|
||
| **Response format:** `hey box view --json` returns the box itself — `id`, `kind`, `name`, `app_url`, `next_history_url`, `next_page` — with a `postings` array of the email threads in it. Each posting has: `id` (box item ID), `topic_id` (thread ID), `name` (subject), `seen` (read status), `created_at`, `contacts`, `summary`, `app_url`, `visible_entry_count`. Use `id` for `hey seen`, `hey unseen`, `hey move`, `hey label add`, `hey label remove`, `hey trash`, `hey spam`, `hey ignore`, and `hey stop-ignoring`, and `topic_id` for `hey thread read`, `hey reply`, `hey forward`, `hey share` and `hey attachment list`. A box item `id` passed to `hey thread read` answers `not_found`, and so does a `topic_id` passed to `hey move`. | ||
| **Response format:** `hey box view --json` returns the box itself — `id`, `kind`, `name`, `app_url`, `next_history_url`, `next_page` — with a `postings` array of the email threads in it. Each posting has: `id` (box item ID), `topic_id` (thread ID), `name` (subject), `seen` (read status), `created_at`, `contacts`, `summary`, `app_url`, `visible_entry_count`. Use `id` for `hey seen`, `hey unseen`, `hey move`, `hey label add`, `hey label remove`, `hey trash`, `hey spam`, `hey ignore`, and `hey stop-ignoring`, and `topic_id` for `hey thread read`, `hey reply`, `hey forward`, `hey share` and `hey attachment list`. A box item `id` passed to `hey thread read` answers `not_found`. The reverse is not caught yet: `hey seen`, `hey unseen` and `hey move` given a `topic_id` (or any id that is not one of your box items) answer success and change nothing, because HEY's endpoints do not report a non-match — so confirm with `hey box view --json` (`seen`, or the thread's box) rather than trusting the envelope. `hey trash`, `hey spam` and the label commands do answer `not_found`. |
There was a problem hiding this comment.
When a call contains both valid posting IDs and an unmatched topic ID, these endpoints mutate the valid postings rather than “change nothing”; more seriously, because the arguments are untyped integers, a topic ID that numerically equals another posting's ID mutates that unrelated thread. The skill should say that unmatched values are silently ignored while every identity-scoped posting ID that happens to match is changed, so an agent does not treat a partially successful batch—or an unintended move—as a complete no-op.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed — "change nothing" was only true of an all-unmatched call, and the numeric collision is the sharper hazard. 05acdbb rewrites both notes: unmatched ids are silently ignored while every id in the call that is one of your box items is changed, so a mixed batch is a partial success reported as a whole one, and a topic_id that equals another of your box item ids marks or moves that unrelated thread.
There was a problem hiding this comment.
🟡 Changes recommended
The verification recipe can still misclassify capped listings and ID collisions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
Refs #396 (does not close it — the fix that issue asks for is on HEY's side)
What is wrong
The skill says a
topic_idpassed tohey moveanswersnot_found, and that passing the wrong kind of id to any posting command answersnot_found. Neither is true forhey seen,hey unseenorhey move: HEY's/postings/seen,/postings/unseenand/postings/movesendpoints scope the requested ids to the identity's postings and answer success whatever matched, and there is no per-posting read the CLI could check against.hey trash,hey spamand the label/folder commands do 404 when nothing matches, which is where the general claim came from. That false promise is what let an agent trust several days of silently no-op'd marks in #396.Change
Documentation only: the two ID notes in
skills/hey/SKILL.mdnow say which commands cannot report a non-match yet and how to confirm one (hey box view --json). No client-side pre-check is added — that is a server change (a404when none of the ids belong to the identity, and a per-id result for a partial batch), recorded against the API separately; once it lands the CLI surfaces it asnot_foundunchanged.Summary by cubic
Updates
skills/hey/SKILL.mdto stop promisingnot_foundfor atopic_idpassed tohey seen,hey unseen, orhey move. Those commands ignore unmatched ids, act on any that do match, and answer success regardless, so a mixed batch is a partial success reported as whole. Docs now show confirming a mark withhey box view <box> --json --all, reading every page (including the cursor and notice) so a posting past the first page isn't mistaken for a non-match. Docs only, no behavior change; cleans up the misleading promise that let an agent trust no-op'd marks (refs #396, does not close it).Written for commit 20dfe0d. Summary will update on new commits.