Repository navigation
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-01a117b5-1db2-733b-84cf-e5743d04bfc4 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a117b5-1db2-733b-84cf-e5743d04bfc4 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed and why
Allow users to delete
local_agentcomments 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:
The browser spec saves reproducible screenshots to
tmp/comment-actions-{light,dark}.pngandtmp/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
The API uses the comment ID, not the thread ID:
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 rspecagainst 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:689scroll-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.