Remove redundant pr_comment set_status action - #1043
thisrohangupta merged 2 commits into
Conversation
resolve and unresolve already cover changing a PR comment thread's status without a body, so set_status duplicated them. Also drops the BodyFieldSpec enum field that only set_status used. Co-authored-by: Cursor <cursoragent@cursor.com>
vivek-kumar-harness
left a comment
There was a problem hiding this comment.
The cleanup makes sense: set_status duplicates resolve/unresolve, and keeping the two zero-body actions gives agents a simpler contract. I also verified the affected tests, typecheck, standards checks, and docs check pass.
Two small suggestions:
-
tests/tools/tool-handlers.test.tsnow has two tests for the sameresource_id→comment_idbehavior (around lines 1690 and 2376). Please keep the existing test at 1690, add the{ status: "resolved" }body assertion from the later test, and remove the duplicate. -
Consider leaving
BodyFieldSpec.enumin place. Although onlyset_statuscurrently uses it, it is useful agent-facing metadata exposed throughharness_describe, and removing it is separate from removing the redundant action. If we intentionally want to remove it as unused API surface, that would be clearer as a separate change.
The remaining README, server-instruction, test-plan, and set_status cleanup looks complete.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the review.
|
Summary
set_statusexecute action frompr_comment. It duplicatedresolveandunresolve, which hit the same status endpoint and need no body.enumfield fromBodyFieldSpec, which onlyset_statusused, and restores the related test comments.resolve/unresolve.Test plan
tsc --noEmitvitest run(only pre-existingip-address-security-libfailures from the installed dependency version)Made with Cursor