Skip to content

Collapse old fix-pr status comments#162

Merged
lolipopshock merged 2 commits into
mainfrom
agent/implement-issue-68/codex-25487660362
May 7, 2026
Merged

Collapse old fix-pr status comments#162
lolipopshock merged 2 commits into
mainfrom
agent/implement-issue-68/codex-25487660362

Conversation

@sepo-agent-app
Copy link
Copy Markdown

@sepo-agent-app sepo-agent-app Bot commented May 7, 2026

Summary

  • Add a generated marker and legacy matcher for fix-pr status comments.
  • Collapse previous fix-pr status comments before posting a new one when AGENT_COLLAPSE_OLD_REVIEWS is enabled.
  • Forward the cleanup toggle from agent-fix-pr.yml and update docs/tests.

Tests

  • npm run build
  • node --test .agent/dist/__tests__/review-summary-minimize.test.js .agent/dist/__tests__/response.test.js .agent/dist/__tests__/post-comment-cli.test.js .agent/dist/__tests__/envelope.test.js
  • git diff --check

Closes #68

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

- name: Post status comment
if: always() && steps.pr.outputs.cross_repo != 'true' && steps.pr.outputs.pr_state == 'OPEN'
env:
AGENT_COLLAPSE_OLD_REVIEWS: ${{ vars.AGENT_COLLAPSE_OLD_REVIEWS }}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wires cleanup into the normal open same-repo fix-pr status path, but the Post unsupported status step below still posts with gh pr comment, so repeated closed or cross-repo fix-pr runs will not collapse older unsupported comments or include the new marker. Please route that path through post-comment.ts with STATUS=unsupported or run the same minimizer before posting.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

Sepo pushed fixes for this PR. Branch: agent/implement-issue-68/codex-25487660362. Requested by @sepo-agent.

  • Routed unsupported fix-pr status posting through the shared post-comment.js formatter/cleanup path with STATUS=unsupported.
  • Added focused coverage for unsupported status cleanup, marker formatting, legacy matching, and workflow wiring.
  • Verified with npm run build, targeted node --test, and git diff --check.

@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

Sepo is dispatching follow-up automation.

Source Next Target Round Status
fix-pr review PR #162 5 / 40 Dispatched

Reason: agent planner selected review: fix-pr succeeded after addressing the prior review synthesis action items, so one automatic review pass is warranted to validate the updated PR before stopping.

@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

Rubrics Review

Total Score Verdict Rubrics Scored
100 PASS 7
Dimension Rubric Result Score Evidence
coding_workflow / generic Reuse existing code pass 8/8 The PR extends the existing .agent/src/review-summary-minimize.ts cleanup path with collapsePreviousFixPrComments and routes posting through .agent/src/cli/post-comment.ts instead of adding separate workflow-local API logic.
coding_workflow / generic Keep docs in sync pass 7/7 The diff updates .agent/docs/architecture/supported-workflows.md and .agent/docs/customization/configuration-list.md so AGENT_COLLAPSE_OLD_REVIEWS now documents fix-pr status cleanup.
coding_workflow / generic Make user-facing automation idempotent pass 7/7 Adds <!-- sepo-agent-fix-pr-status -->, legacy text matching in .agent/src/fix-pr-status.ts, same-agent minimization in .agent/src/review-summary-minimize.ts, and pre-post cleanup in .agent/src/cli/post-comment.ts.
communication / generic Keep status comments compact pass 6/6 The new identifier is a hidden HTML marker, and unsupported/success comments remain concise user-facing status text rather than exposing cleanup details.
coding_workflow / generic Comment on accepted workflow stops pass 6/6 The unsupported fix-pr path still posts a visible stop reason, and now does so through post-comment.js with STATUS: unsupported in .github/workflows/agent-fix-pr.yml.
coding_workflow / generic Make surgical changes pass 8/8 The diff is narrowly scoped to fix-pr status markers, cleanup wiring, focused tests, one workflow pass-through, and matching docs for issue #68.
coding_style / generic Prefer simple implementations pass 7/7 The implementation is a small marker plus regex matcher and reuse of the existing GraphQL minimization helper, with focused coverage in post-comment-cli, response, review-summary-minimize, and workflow envelope tests.

Notes

  • The linked issue For the fix-pr, new fix-pr should collapse previous fix-pr comments? #68 asks whether new fix-pr runs should collapse previous fix-pr comments; the PR directly implements that behavior.
  • The latest diff also addresses the prior edge-path concern by routing Post unsupported status through the shared post-comment.js formatter/cleanup path.
  • No rubric-specific follow-up needed.

Findings

  • INFO: The applicable selected rubrics are satisfied. Delegated route authorization, one-shot orchestration defaults, preview contracts, merge strategy, and rubric-writing guidance were not scored because this PR does not change those behaviors.

Final Rubric Verdict

PASS

@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

Review

The current reviewer outputs converge on SHIP: the prior WARNING has been addressed, rubrics pass, and no remaining required branch change was identified. One reviewer suggested extra direct marker assertions for three formatter cases, but the current behavioral coverage already exercises marker detection and posting paths, so that is optional test hardening rather than an actionable blocker.

Issue Severity Description
No actionable issues remain INFO The unsupported-status cleanup gap is fixed, and current reviews report no BLOCKING findings. The remaining formatter-test suggestion is optional and does not require another automated fix-pr pass.

Progress

  • Issue For the fix-pr, new fix-pr should collapse previous fix-pr comments? #68 is narrow and has no broader parent/tracking context in the linked discussion.
  • The prior inline warning about unsupported fix-pr comments was addressed by routing Post unsupported status through post-comment.js with STATUS=unsupported.
  • Rubrics review now passes 7/7, including idempotent automation, docs sync, surgical change scope, and accepted workflow stop behavior.
  • Reviewer-reported verification includes npm ci, npm run build, targeted node --test, and git diff --check HEAD.

Recommended Next Step

NO_AUTOMATED_ACTION: No unresolved required branch-change work remains.

Final Verdict

SHIP

Action Items

  • No required branch-change work remains.

@sepo-agent-app sepo-agent-app Bot added priority/p2 Project management: medium priority effort/low Project management: low effort labels May 7, 2026
Copy link
Copy Markdown
Contributor

@lolipopshock lolipopshock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sepo-agent /answer can you check my comment?

ROUTE: fix-pr
STATUS: unsupported
TARGET_NUMBER: ${{ env.PR_NUMBER }}
run: node .agent/dist/cli/post-comment.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the comment will only be posted when the route is not supported?

@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

Yes, for that specific step. The Post unsupported status comment only runs when the PR cannot be handled by fix-pr: cross-repo or not open.

That does not mean all fix-pr comments only post in unsupported cases. The supported open same-repo path is handled by the step just above it, Post status comment, which posts success/no_changes/failed/etc. Both now route through post-comment.js:

- name: Post status comment
if: always() && steps.pr.outputs.cross_repo != 'true' && steps.pr.outputs.pr_state == 'OPEN'
env:
AGENT_COLLAPSE_OLD_REVIEWS: ${{ vars.AGENT_COLLAPSE_OLD_REVIEWS }}
APPROVAL_COMMENT_URL: ${{ inputs.approval_comment_url }}
BRANCH: ${{ steps.pr.outputs.head_ref }}
COMMENT_TARGET: pr
GH_TOKEN: ${{ steps.auth.outputs.token }}
PR_NUMBER: ${{ env.PR_NUMBER }}
REQUESTED_BY: ${{ inputs.orchestration_enabled == 'true' && (vars.AGENT_HANDLE || '@sepo-agent') || inputs.requested_by || github.actor }}
RESPONSE_FILE: ${{ steps.agent.outputs.response_file }}
RESUME_STATUS: ${{ steps.agent.outputs.resume_status }}
ROUTE: fix-pr
STATUS: ${{ (steps.commit.outcome == 'failure' || steps.push-head.outcome == 'failure') && 'failed' || steps.response.outputs.status || 'failed' }}
TARGET_NUMBER: ${{ env.PR_NUMBER }}
run: node .agent/dist/cli/post-comment.js
- name: Post unsupported status
if: steps.pr.outputs.cross_repo == 'true' || steps.pr.outputs.pr_state != 'OPEN'
env:
AGENT_COLLAPSE_OLD_REVIEWS: ${{ vars.AGENT_COLLAPSE_OLD_REVIEWS }}
APPROVAL_COMMENT_URL: ${{ inputs.approval_comment_url }}
COMMENT_TARGET: pr
GH_TOKEN: ${{ steps.auth.outputs.token }}
ROUTE: fix-pr
STATUS: unsupported
TARGET_NUMBER: ${{ env.PR_NUMBER }}
run: node .agent/dist/cli/post-comment.js

Small wording nuance: ROUTE: fix-pr is still supported here; STATUS: unsupported means the PR target is unsupported for automatic fixing. That status formats the “could not update this PR automatically” comment and still goes through fix-pr cleanup/marker handling:

repo/.agent/src/response.ts

Lines 115 to 123 in 950bdfb

case "unsupported":
return [
"**Sepo could not update this PR automatically.**",
"",
marker,
"",
"PR fix runs currently support open same-repository pull requests only.",
data.approvalCommentUrl ? `- Approval: ${data.approvalCommentUrl}` : "",
].filter(Boolean).join("\n");

@lolipopshock lolipopshock marked this pull request as ready for review May 7, 2026 16:34
@lolipopshock lolipopshock merged commit 635bbec into main May 7, 2026
14 checks passed
@sepo-agent-app sepo-agent-app Bot deleted the agent/implement-issue-68/codex-25487660362 branch May 7, 2026 16:35
@sepo-agent-app
Copy link
Copy Markdown
Author

sepo-agent-app Bot commented May 7, 2026

Rubrics Update

No changes were committed to agent/rubrics from PR #162.

no rubric changes

I found no trusted, durable new preference to encode. The only human PR input was a clarification question from lolipopshock; the substantive review guidance was agent-authored and already covered by active rubrics like idempotent-user-facing-automation, reuse-existing-code, compact-user-facing-status-comments, and comment-on-accepted-workflow-stops.

Rubrics checkout remains clean.

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

Labels

agent Handled by the agent effort/low Project management: low effort priority/p2 Project management: medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

For the fix-pr, new fix-pr should collapse previous fix-pr comments?

2 participants