Unify Sepo progress and durable result identity - #507
Conversation
|
@sepo-agent /review |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@sepo-agent /answer but the review doesn't make sense? If we just repost the final answer once it's fine? And we can also skip it by filtering out actions? |
|
Yes—the review is technically valid, but its proposed auth-mode restriction is only one solution. The original problem was dozens of events from repeated progress edits. A single final repost is fine for App/OIDC: it creates at most one skipped workflow run because the sender is a Bot. The edge case is PAT fallback: PAT comments appear User-authored, so the current entrypoint filters may treat that final answer as a follow-up—or as a real command if it contains a live command suggestion. Filtering is a good fix, but filtering only by If you want that approach applied, send:
|
|
@sepo-agent /review |
Rubrics Review
Notes
Findings
Final Rubric VerdictPARTIAL
|
| RESPONSE_FILE: ${{ steps.agent.outputs.response_file }} | ||
| AGENT_PROGRESS_COMMENT_ID: ${{ steps.agent.outputs.progress_comment_id }} | ||
| AGENT_PROGRESS_FINAL_COMMENT_MODE: merge | ||
| AGENT_PROGRESS_FINAL_COMMENT_MODE: repost |
There was a problem hiding this comment.
agent-implement.yml also serves create-action and add-rubrics, but this unconditional repost combines with post-comment.ts marking only implement and fix-pr. If a supported progress-policy override enables progress for a shared route, PAT auth produces an unmarked User-authored final comment; on an agent-labeled target, or when the summary contains a live command, it can re-enter routing. Please either mark every repost-mode result or keep non-implement shared routes on merge, and add focused shared-route coverage.
AI Review Synthesis
Summary of PR/IssuePR #507 makes live progress temporary for ReviewThe primary design is sound, and the prior blocking PAT issue is fixed. One bounded gap remains for other routes sharing the implementation workflow, while the informational findings can remain follow-ups.
Progress
Issue DetailsShared implementation routes can repost unmarked PAT resultsCause: The shared workflow unconditionally selects Candidate solutions:
Comments: Codex and the rubric reviewer identified this gap; Claude judged the PR mergeable after validating only the intended default routes. The current source supports a warning rather than a blocker because the regression requires a supported but non-default progress configuration. Marker substring filtering can silently skip user commentsCause: The entrypoint Candidate solutions:
Comments: Claude raised this independently. The behavior is fail-closed and has no authorization or data-loss impact, so it should not delay this PR. Other resolved-auth routes remain outside the marker contractCause: The new marker contract targets the three routes changed here; other existing resolved-auth comment paths were not audited or modified. Candidate solutions:
Comments: This is pre-existing behavior and should not expand the current fix. Recommended Next Step
Final Verdict
Action Items
|
Summary
/answer,/implement, and/fix-pr, using the non-recursive job-scopedGITHUB_TOKEN.AGENT_PROGRESS_POLICYmodes:disabledmeans no progress,report-onlymeans non-cancellable progress, andenabledmeans cancellable progress.extract-context, preventing PAT-authored results from becoming implicit follow-ups or authorizing command suggestions.none/brief/fullfinal progress-overview presentation as separate policy work.Verification
npm --prefix .agent run test:ci