[improvement](ci) Refine Codex review goal completion prompt#64658
Conversation
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary: The Codex review workflow now enters goal mode through the runner's --goal invocation, so the review instructions should describe the completion criteria and convergence process instead of telling the model to create the goal itself. This change keeps the shared-ledger subagent process, final convergence requirement, GitHub review verification, xhigh reasoning setting, and Litefuse trace input path aligned with the validated workflow behavior.
### Release note
None
### Check List (For Author)
- Test: Manual test
- Verified the two committed files match fresh origin validation base commit ca89739.
- Ran workflow YAML parsing locally.
- Ran git diff --check for the committed files.
- Validated on origin PR #49 via workflow run 27895281133, which succeeded, submitted 8 inline review comments, and uploaded Litefuse trace c3078f147a04f9ead1ddb64ba02562fb with no missing I/O.
- Behavior changed: No
- Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review completed. I found two blocking issues: the workflow installs and runs an unpinned executable fetched from mutable OSS storage in a privileged CI context, and the generic code-review skill now mandates subagent/GitHub-review side effects for all review uses instead of scoping them to goal-mode GitHub reviews where those tools and permissions are available.
Critical checkpoint conclusions:
- Goal/test coverage: the PR targets Codex goal-mode review orchestration; the workflow changes implement that path, but the binary provenance and generic skill scoping issues are not covered by an enforceable validation.
- Scope/focus: the change is mostly focused on the review runner and review skill, but the skill change broadens behavior beyond this runner.
- Concurrency/lifecycle/config/compatibility: no Doris runtime concurrency, lifecycle, config, storage format, transaction, FE-BE protocol, or data-write path changes were found.
- Parallel paths: workflow_call and workflow_dispatch share the changed runner path; ordinary/local code-review skill uses are the problematic parallel path.
- Tests/results: no test result files are involved; I validated YAML parsing and diff whitespace locally, but did not run the workflow.
- Observability/performance: not applicable beyond the existing workflow logging and Litefuse recording.
User focus: no additional user-provided focus was present.
Subagent conclusions: the workflow-focused pass proposed the unpinned executable finding; the skill-focused pass proposed the generic mandatory side-effect/subagent finding. I independently verified both against the diff, existing review context, SECURITY.md, and threat-model.md. Final convergence round ended with both subagents reporting NO_NEW_VALUABLE_FINDINGS for this comment set.
github-actions
Bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
3 participants
What problem does this PR solve?
Problem Summary: I modified the codex to support the
codex exec --goalmode. The Codex review workflow now enters goal mode through the runner's --goal invocation, so the review instructions should describe the completion criteria and convergence process instead of telling the model to create the goal itself.see modification in https://github.com/zclllyybb/codex/tree/codex-exec-goal
Verification:
Before:
[feature](eager-agg) Bilateral push-down for eager aggregation #63690 got 1 comment when
/reviewNow:
[test](ci) Fresh Codex review prompt validation zclllyybb/doris#49 replicated the same PR, receiving 8 comments in a single
/review