Skip to content

fix(comments): allow deleting agent comments owned by the current user - #247

Draft
alexbbt wants to merge 2 commits into
block:mainfrom
alexbbt:alexbbt/delete-own-agent-comments
Draft

alexbbt wants to merge 2 commits into
block:mainfrom
alexbbt:alexbbt/delete-own-agent-comments

Conversation

@alexbbt

@alexbbt alexbbt commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

What changed and why

Allow users to delete local_agent comments attributed to their account, through the browser or API. Ownership uses the user's ID, not the posting token's ID, so another token for the same account can delete the comment.

Other users' comments remain protected, including from plan authors and admins. Editing stays limited to the owner's undeleted human comments. Cloud-persona and system comments remain protected because their author IDs do not identify user accounts.

The browser shows Delete, but no Edit action or editor, for the owner's agent comments. The served agent guides describe the new permission. API deletion also refreshes the document-comment list for connected viewers, so deleted text and empty-thread links do not remain there.

Evidence

JavaScript-enabled browser checks passed in light and dark themes. Inspected screenshots show:

  • Edit and Delete on the owner's human comment.
  • Delete only on the owner's agent comment.
  • Neither action on another account's human or agent comment.
  • A "Comment deleted" placeholder after deletion, with the other comments intact.

The browser spec saves reproducible screenshots to tmp/comment-actions-{light,dark}.png and tmp/comment-actions-deleted-{light,dark}.png. Public artifact uploads are disabled in the authoring environment, so images are not attached to this PR.

How to try it

  1. Open a plan with a human comment and an API-posted agent comment attributed to your account.
  2. Open the discussion. Both comments offer Delete, but only the human comment offers Edit.
  3. Cancel the agent comment's deletion, then confirm it. Cancellation keeps the comment. Confirmation replaces it with "Comment deleted" when replies remain.
  4. Open the same discussion as another account. Neither comment offers Edit or Delete.

The API uses the comment ID, not the thread ID:

curl -H "Authorization: Bearer $TOKEN" -X DELETE \
  "$BASE_URL/api/v1/plans/$PLAN_ID/comments/$COMMENT_ID/delete"

An owned human or local-agent comment returns 200 with deleted_at. Another account's comment returns 403. Repeated deletion remains idempotent.

Testing

  • bundle exec rspec spec/policies/coplan/comment_policy_spec.rb spec/requests/comments_spec.rb spec/requests/api/v1/comments_spec.rb spec/requests/agent_instructions_spec.rb spec/system/comment_actions_spec.rb spec/system/comment_composer_spec.rb — 127 examples, 0 failures.

  • bin/rubocop — 557 files, no offenses.

  • Full bundle exec rspec against a dedicated local MySQL database on the final commit — 2,526 examples, 0 failures.

  • Started the development app with synthetic data and exercised the documented API deletion command and browser deletion successfully. Verified the browser's soft deletion in the database.

  • Local Ruby is 3.4.5. CI uses the repository's pinned Ruby 3.4.7.

  • Two independent adversarial reviews covered server authorization and UI/broadcast behavior. The authorization review found no bypass. The UI review found the stale document-comment list after API deletion. Added browser tests that reproduced it in both themes, then passed after the broadcast fix.

  • CI on the final commit: MySQL, lint, Semgrep, zizmor, and DCO passed. PostgreSQL ran 2,526 examples with one failure in the unchanged spec/system/inline_editor_spec.rb:689 scroll-position test (five-second timeout at line 707). The new comment tests passed. That test performs no comment deletion, and the changed rendering preserves its human-comment behavior. A rerun needs a maintainer: GitHub rejected the author's failed-job rerun with "Must have admin rights to Repository." The PR remains in draft pending that check. Failed run.

  • Added or updated real JavaScript-enabled browser specs in spec/system/ for changed UI behavior, covering the user interaction and visible result; or explained why no browser UI is involved.

  • Added model, service, or request specs where needed for domain behavior, authorization, or API cases.

  • Ran bundle exec rspec, or explained why it could not run and listed the specs that did run.

  • Tested affected UI behavior in both light and dark themes, or explained why theme testing does not apply.

  • Attached a video or images demonstrating the change, if this PR adds or changes a feature or fixes a UI bug.

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.

1 participant