Simplify add-rubrics to proposal PR flow#382
Conversation
|
@sepo-agent /orchestrate review and fix any potential bugs |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I have enough evidence for the rubric score. The remaining tradeoff is not a bug: the route is proposal-only and well-guarded, but it still uses a sizable dedicated workflow and has generic fallback PR metadata when the model omits structured fields.## Rubrics Review
Notes
Findings
Final Rubric VerdictPARTIAL
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Rubrics Review
Notes
Findings
Final Rubric VerdictPASS
|
| required: false | ||
| default: agent/rubrics | ||
|
|
||
| permissions: |
There was a problem hiding this comment.
The model job still inherits the workflow-level write and OIDC permissions. Even though run-agent-task withholds GH_TOKEN, an approve-all model step can use the job OIDC request environment to mint a hosted App token through the same auth broker path, so proposal mode still has a direct write-token bypass before validated post-processing. Please scope permissions per job: keep add-rubrics read-only with no id-token: write, and grant write/OIDC permissions only to add-rubrics-post-process.
AI Review Synthesis
Summary of PR/Issue
ReviewThe PR still needs rework before merge. Claude and the rubrics review consider the split post-process architecture mostly sound, but Codex’s current blocker is supported by the live workflow permissions.
Progress
Issue DetailsModel Job Inherits Write/OIDC PermissionsCause: Workflow-level permissions apply to both Candidate solutions: Move permissions to job scope. Keep Comments: This is distinct from the older line 434 mutable-workspace thread; that issue was addressed by the second job. The remaining problem is the first job’s own authority. Trusted Copy Is Coupled To Current Rubrics File TypesCause: The trusted-copy CLI recursively scans the source Candidate solutions: Either document that Comments: This is a liveness hardening concern, not the merge blocker. Recommended Next StepFIX_PR: the unresolved permission boundary requires a concrete workflow change and is safe for an automated fix-pr pass. Final VerdictNEEDS_REWORK Action Items
|
|
Sepo is dispatching follow-up automation.
Reason: agent planner selected fix-pr: Latest review synthesis reported needs_rework with Recommended Next Step: FIX_PR, and the remaining item is a concrete workflow permissions bug suitable for an automated PR fix. Task for fix-pr:
Constraints: Ignore optional INFO notes, metadata-only polish, already-fixed findings, and human-judgment nits unless required by those action items. |
|
Sepo pushed fixes for this PR. Branch:
Verification
|
Summary
/add-rubricsandagent/add-rubricsrouting for rubric proposal requests.agent/rubricscheckout, validates YAML, and opens a draft PR targetingagent/rubrics.Verification
npm --prefix .agent run buildGITHUB_OUTPUT=/tmp/sepo-test-output node --test .agent/dist/__tests__/triage.test.js .agent/dist/__tests__/envelope.test.js .agent/dist/__tests__/response.test.js .agent/dist/__tests__/rubrics.test.js .agent/dist/__tests__/onboarding-check-cli.test.jsnode --test .agent/dist/__tests__/docs-validation.test.jsGITHUB_OUTPUT=/tmp/sepo-test-output node --test .agent/dist/__tests__/envelope.test.js .agent/dist/__tests__/triage.test.jsgit diff --checkCloses #381