fix(plugin): close saved review tasks by ID - #1036
mldangelo-oai wants to merge 7 commits into
Conversation
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. |
![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: 1d4aeb7987
ℹ️ 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 key not in reopened_candidates: | ||
| if relative == "parent" or relative in current_results: | ||
| resolved.setdefault(key, disposition) |
There was a problem hiding this comment.
When a Standard worker successfully advances checkpoint-head.json to a terminal checkpoint but replacing result.json then fails, that selected checkpoint is included in current_drafts, yet this branch recognizes outcomes only from the parent or current_results. Consequently, a finding or rejected/not-applicable disposition recorded only in the selected checkpoint never enters resolved; the later superseded-checkpoint logic discards that outcome and can retain the older candidate as deferred, losing a confirmed security result during stopped-scan recovery. Treat the checkpoint named by selected_observations as an authoritative current outcome, subject to the existing source-order checks.
Useful? React with 👍 / 👎.
mldangelo-oai
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e224f0093
ℹ️ 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".
| ]; | ||
| } | ||
| return []; | ||
| return stable; |
There was a problem hiding this comment.
When a progress draft omits the optional surface id, buildCoverage generates one, but a later draft that repeats the same risk area and label without that generated ID now has no identity here. The saved surface is consequently retained as a separate row and the new surface receives a suffixed ID; if the old row was needs_follow_up and the new row reports or rejects it, the stale row still makes coverage partial and misrepresents the final result. Preserve the label/risk-area fallback, or otherwise reconcile generated IDs before merging.
Useful? React with 👍 / 👎.
| .update(JSON.stringify(item)) | ||
| .digest("hex") | ||
| .slice(0, 16)}`; | ||
| let id = baseId; |
There was a problem hiding this comment.
For an allowed ID-less generic deferral, hashing the entire row means that enriching or abbreviating its notes, paths, or other metadata in a later checkpoint produces a new ID. Reconciliation then treats the saved task and its updated observation as independent work, retaining both; the generated ID is also absent from the tool response, so a caller that omitted it initially cannot reliably copy it into resolvedDeferred. Match the observation to saved deferred work before minting a new identity rather than deriving identity from mutable payload content.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
Terminal Standard and diff drafts can explicitly close saved review tasks. Recovery preserves reopened work, candidate evidence, and worker ownership through interrupted writes and retries.
Changes
coverage.resolvedDeferredentries containing the saved task ID and completion reason. Surface updates also use saved IDs.Testing
Risk and rollout
The schema addition is optional; no CLI changes. Callers must copy saved IDs when closing tasks or updating surfaces. Candidate findings still require explicit outcomes, and ambiguous ordering keeps work pending. Tests use synthetic fixtures and simulated workers; no live scan was run. Merge #1040 first, then retarget this PR to main.
Public disclosure review