Repository navigation
Conversation
The grace round that runs after the tool-request budget is exhausted dropped the result of its task_done call: a model that spent all its rounds on context tools and then called task_done(DONE) in the grace round was still recorded as incomplete, so callers kept the misleading "main_task did not complete before stopping" failure instead of a reusable checkpoint. The grace round now mirrors the main loop's terminal semantics: - task_done(DONE) completes the run; comments issued alongside it in the same round still land. - task_done(FAILED) surfaces as a task failure error and returns immediately without executing the rest of the round. - Transient LLM errors are logged and swallowed so budget exhaustion stays the recorded stop cause. - Cancellation wins over completion at every window and surfaces as ctx.Err() so callers classify the item as FailureCancelled instead of FailureBudget: before the grace call, inside the LLM call, after the response arrives, between the round's tool calls, and before returning completion. Regression coverage includes the completion propagation, same-round ordering for DONE and FAILED, the swallowed LLM error, all five cancellation windows, and cross-protocol propagation over the OpenAI chat-completions, Anthropic messages, and OpenAI responses formats.
Only the tool-request budget stop turns a grace-round task_done(DONE) into completion; a token-budget stop must stay budget-stopped.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-84961 | undici |
8.10.0 |
8.10.2 |
🔶 High · 9 findings
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-69152 | brace-expansion |
5.0.8 |
5.0.9 |
| CVE-2026-102276 | brace-expansion |
5.0.8 |
5.0.10 |
| CVE-2026-84445 | google.golang.org/grpc |
v1.83.1 |
1.83.2 |
| CVE-2026-85152 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-67213 | nanoid |
3.3.16 |
3.3.18 |
| CVE-2026-84933 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-19534 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-85014 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-102278 | brace-expansion |
5.0.8 |
5.0.11 |
🟡 Medium · 9 findings
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-102277 | brace-expansion |
5.0.8 |
5.0.12 |
| CVE-2026-84890 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-86818 | fast-uri |
4.1.4 |
4.1.5 |
| CVE-2026-84947 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-85024 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-82417 | qs |
6.15.3 |
6.16.0 |
| CVE-2026-18149 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-85008 | undici |
8.10.0 |
8.10.2 |
| CVE-2026-86472 | fast-uri |
4.1.4 |
4.1.5 |
🟢 Low · 2 findings
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-82562 | qs |
6.15.3 |
6.16.0 |
| CVE-2026-18540 | undici |
8.10.0 |
8.10.2 |
View full analysis in Upwind Console
Scan completed in 10s
Scan history (1 scan)
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
595dd80 < |
2026-10-05 02:07 UTC | +21 | 0 | +21 |
Last scanned: 595dd80 · 2026-10-05 02:07 UTC
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
595dd80 < |
2026-10-05 02:07 UTC | 0 | 0 | 0 |
Last scanned: 595dd80 · 2026-10-05 02:07 UTC
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 595dd801e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(calls) == 0 { | ||
| return | ||
| return false, nil |
There was a problem hiding this comment.
When cancellation occurs while the grace-round request is in flight but the client still returns a successful response without tool calls, this early return runs before the new ctx.Err() check. RunMainTask therefore reports the original budget stop instead of context.Canceled; in scan mode, the item can be recorded as budget-failed and the command can return the generic all-scans-failed error rather than the cancellation. Check the context before the zero-call return.
Useful? React with 👍 / 👎.
| // Scenario A (the regression): the model spends all its rounds on | ||
| // context tools (file_read), then — given one last chance in the grace | ||
| // round — correctly calls task_done with state DONE. The completion must | ||
| // propagate: RunMainTask reports completed=true with StopNone, so callers | ||
| // record a reusable checkpoint instead of translating the run into |
There was a problem hiding this comment.
This block is representative of extensive comments in the new tests that narrate the fixture and expected assertions already expressed by the test name, response construction, and checks. The root AGENTS.md requires comments to be sparse and limited to non-obvious reasons, explicitly excluding comments that restate or narrate the code; retain only explanations of genuinely subtle constraints.
AGENTS.md reference: AGENTS.md:L37-L40
Useful? React with 👍 / 👎.
|
| File | Line | Type | Value |
|---|---|---|---|
cmd/opencodereview/provider_tui_test.go |
1947 | generic-api-key | sk-s••••abcd (+1 more) |
examples/github_actions/README.md |
525 | pem-private-key | -----BEGIN PRIVATE KEY----- *** -----END PRIVATE KEY----- (+5 more) |
internal/llm/bedrock_test.go |
332 | generic-api-key | sk-t••••-key |
internal/llm/resolver.go |
73 | generic-api-key | ANTH••••OKEN |
scripts/github-actions/action-contract.test.js |
1275 | generic-api-key | conf••••inel |
scripts/github-actions/action-contract.test.js |
1300 | generic-api-key | conf••••SIST |
scripts/github-actions/action-contract.test.js |
1335 | generic-api-key | conf••••inel |
scripts/github-actions/action-contract.test.js |
1378 | generic-api-key | prot••••inel (+1 more) |
scripts/github-actions/action-contract.test.js |
1529 | generic-api-key | lega••••inel |
scripts/github-actions/action-contract.test.js |
1555 | generic-api-key | extr••••inel |
scripts/github-actions/action-contract.test.js |
1588 | generic-api-key | retr••••inel |
🔶 PII · 12 findings
| File | Line | Type | Value |
|---|---|---|---|
cmd/opencodereview/git_test.go |
19 | ••••@test.com (+5 more) |
|
cmd/opencodereview/misc_helpers_test.go |
43 | ••••@bar.com |
|
cmd/opencodereview/rules_cmd_test.go |
26 | ••••@t.co (+4 more) |
|
extensions/frontend/package-lock.json |
2871 | ••••@izs.me (+2 more) |
|
internal/diff/git_resolve_test.go |
168 | ••••@github.com |
|
internal/diff/git_resolve_test.go |
170 | ••••@github.com |
|
internal/diff/git_resolve_test.go |
178 | ••••@host.com |
|
internal/diff/git_resolve_test.go |
178 | ••••@c.git |
|
internal/llm/client_test.go |
1089 | City | •••••••• (+1 more) |
internal/session/manifest_test.go |
669 | hu••••@github.com |
|
internal/viewer/hostguard.go |
44 | IPv6 | •••••••• |
scripts/publish/publish.sh |
163 | ••••@open-code-review.local |
A secret that has been pushed remains in the repository's history even after the line is removed.
Scan completed in 6s.
Scan history (1 scan)
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
595dd80 < |
2026-10-05 02:07 UTC | +23 | 0 | +23 |
Last scanned: 595dd80 · 2026-10-05 02:07 UTC
2 participants
Carry of upstream alibaba#1019 (by @AllenMuu, fixes alibaba#1018) on the
sendbirdbuild branch until it lands upstream.alibaba#1019 conflicts with current upstream main, so this is its commit (author preserved) rebased onto main:
RunPerFile→RunMainTask/newPath→taskKeyrename.Why we need it: with per-file fan-out, a lane that exhausts its 50-round budget and closes with task_done in the grace round was reported as failed, which failed the whole file (5 files on CCO alibaba#948).
🤖 Generated with Claude Code
https://claude.ai/code/session_016fFEYAkexJ1sSe5eeLxxuH