shuke987 commented on PR #67635:
URL: https://github.com/apache/doris/pull/67635#issuecomment-5579014464
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67635
commit: b0f5c36b59149f6a3debec38d446fbce4f174a5b
base: 685e91f962a068ce098280d1ca84ddb6bc782c1d
reviewed_at: 2026-09-08T04:03:13+00:00
reviewer: shuke987
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 1, nit: 0}
rounds: 1
converged: true
```
**Result:** PASS with one non-blocking minor finding. This records the
AI-assisted local review of the commit above using the repository's code-review
checklist. Minor findings do not block the local review PASS gate.
**Minor / P2 — synchronize stderr before detecting refresh-token reuse.** At
[code-review-runner.yml:737–764](https://github.com/apache/doris/blob/b0f5c36b59149f6a3debec38d446fbce4f174a5b/.github/workflows/code-review-runner.yml#L737),
the unqualified `wait` does not synchronize the `tee` used in the external
Codex command's process-substitution redirection. If reuse is reported only on
stderr and its file write is delayed, failure inspection can run before the
diagnostic reaches the file. The step then omits `auth_invalid_reason`, so the
invalid-auth recording step is skipped and the credential can remain eligible.
This was reproduced by executing the actual workflow shell with fake
Codex/OSS and a controlled gate before tee writes: the shell exited without the
invalid-auth output while the gate was closed; releasing it produced the
complete reuse diagnostic. A direct-to-file stderr control detected the same
error. Suggested follow-up: explicitly manage and synchronize the log consumer,
or write stderr directly to the file before inspecting it, and add a regression
test controlling this ordering. The finding remains open and is recorded as
minor.
**Coverage and validation**
- Checked credential selection, token fingerprinting, independent marker
persistence, concurrent marker/context updates, credential replacement, failure
reporting, and existing auth writeback.
- Re-ran all 18 quarantine test methods successfully. They do not control
tee scheduling; the additional gated probe exposed the minor finding above.
- YAML parsing and diff checks passed. The same commit previously passed the
full 90-test script suite and syntax checks for all 22 workflow shell blocks.
- No live credentials, OSS writes, or model calls were used by the test
harness. Previously selected concurrent runs and failures before marker
persistence remain the documented boundaries.
<!-- doris-repo-review:v1:end -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
