fix: cancel selected-workspace PTC across processes - #196
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: a937bffbaf
ℹ️ 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".
a937bff to
a75dfad
Compare
a75dfad to
e45a407
Compare
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
beb5ad7 to
503de80
Compare
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 503de80c55
ℹ️ 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".
503de80 to
09180cd
Compare
09180cd to
aa5882a
Compare
danny-avila
changed the title
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
aa5882a to
01749d1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d017f3ed06
ℹ️ 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".
d017f3e to
dd3d293
Compare
|
@codex review exact head dd3d293968c748e704c2abfe9dca5f63e9fb555d |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd3d293e41
ℹ️ 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".
|
@codex review exact head 9adede8. Please verify the four prior distributed-cancellation findings: external cancellation now wakes the original waiter through the process-wide registry; retryable cancellation bodies stay abortable; cancellation targets are attached before enqueue so failed admission cannot orphan runnable work; and reconnect reconciliation retries with bounded backoff. Also verify the bounded credential-refresh settlement drain. |
9adede8 to
c8876d1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8876d1306
ℹ️ 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".
|
@codex review Please review exact current head 5c512a7 and confirm the reviewed commit. All four latest findings are addressed as one outcome-lifecycle fix. Enqueue, subscription and completion failures recover a durable winning result. Late Stop retains its principal-bound target tombstone through the existing TTL. Subscriber shutdown disconnects without awaiting an offline startup. Commitment checks the absolute producer deadline atomically with Redis TIME, instead of retrospectively rejecting a successful commit whose acknowledgement arrived late; both delayed-dispatch and delayed-ack tests pass. Result/marker publication uses one MSET write to avoid partial OOM publication. The wider audit also clamps worker budgets to the producer absolute deadline across config drift, owns completion rejections before subscription awaits, and memoizes cancellation fencing so cleanup does not duplicate a successful Redis write. Focused real Redis tests cover the transitions and a local LibreChat + native SRT worker acceptance run is being repeated on this exact head with physical PID and delayed-write Stop assertions. Please inspect the shared invariants and any remaining alternate failure path. |
|
Final local acceptance passed on 5c512a7 after rebuilding Code API. Real LibreChat + local Agents SDK integration + Redis/Mongo + two native SRT workers, all on isolated ports; deterministic model only, actual tools and filesystem effects. Verified: physical create/read/edit persistence, Ask/Reject/Accept edits/Full access, reload persistence, workspace-bound PTC and private skill inputs, worker isolation, and offline fail-closed behavior. Both command and PTC Stop tests first verified a live shell PID, clicked the real UI Stop control, verified that PID disappeared within five seconds, waited beyond the delayed mutation time and confirmed no late file was written, then reused the workspace successfully. One focused E2E scenario passed in about 1.3 minutes. No hosted deployment or existing services were changed. The earlier real-SRT speculative replay canary also passed with exactly one network POST and one workspace write. A separate real Redis check registered 64 targets with one subscriber, cancelled 32 and interrupted none of the others (an isolation check, not a production-capacity claim). Full suites are left to CI; local checks were focused. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c512a7181
ℹ️ 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".
|
@codex review Please review exact head a5b528f and confirm the reviewed commit. Both latest findings are addressed by deepening the shared outcome interface. Fencing now returns a discriminated cancelled/expired/completed outcome, with the winning result in the same atomic Redis response. Enqueue, subscription, completion and disconnect recovery consume that snapshot without follow-up GETs. The normal Stop route requests only the small decision, not the result body. Initial worker result recovery uses one MGET snapshot as well. Jobs carry the producer cancellation retention requirement; completion uses the larger local/producer retention. Late Stop atomically renews completion evidence to cover the renewed request tombstone, using millisecond precision to avoid rounded-TTL gaps. Focused real Redis tests cover connection loss after the decision, immediate recovery without a lost queue event, timeout configuration drift, and subsecond retention renewal. Local end-to-end verification will be repeated on this exact head. The previous immutable outcome/deadline and lost-reply invariants remain covered. Please audit any remaining lifecycle boundary as a whole rather than suggesting independent signal snapshots after a durable outcome has already won. |
|
Final local acceptance passed again on exact head a5b528f. Real LibreChat + locally linked Agents SDK + Code API + Redis/Mongo + native SRT workers, isolated non-default ports, deterministic model responses only. Physical create/read/edit persistence, approvals, workspace-bound PTC/private skills, worker isolation and offline fail-closed checks passed. Both command and PTC Stop verified a live shell PID, killed it within five seconds, prevented the delayed write after waiting past its scheduled time, and allowed workspace reuse. One focused E2E scenario passed in 1.3 minutes; temporary services were cleaned up. No hosted deployment or existing workspaces changed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5b528f8ab
ℹ️ 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".
|
@codex review Please review exact head b75bbe1. Confirm that commit in your result. All three findings are fixed together at the outcome boundary. Selected native workspace jobs now map and durably commit their result inside the existing bridge sessionResultFinalizer, before the bridge clears the mutation fence, regardless of whether egress restoration is needed. Failure or ambiguity there leaves the root quarantined. A successful committed handoff remains successful through later Stop, egress revocation, or bridge cleanup failure. No separate post-handoff signal check can overwrite that result. Attached request tombstones no longer renew on Stop at all, removing the split renewal window; admission retention bounds the mapping. Deterministic result decoding/validation is outside the Redis transport retry loop. Real Redis regressions cover handoff completion versus cancellation/quarantine, missing and corrupt results with exactly one attempt, and interrupted split renewal. 26 focused Redis/bridge tests plus 23 registry tests pass. The actual local LibreChat + Code API + native SRT worker acceptance just passed again, including physical command and PTC Stop/no-late-write checks, files, approvals and private skills. Please audit these shared invariants and remaining alternate transitions. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b75bbe11d3
ℹ️ 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".
|
Final post-commit local verification passed on b75bbe1 after rebuilding Code API. The real LibreChat + SDK integration + native SRT worker acceptance passed again in 1.3 minutes, including verified live PIDs before UI Stop, PID termination within five seconds, no delayed file writes, and subsequent workspace reuse for both commands and PTC. Files, approvals, private skills, isolation, reload and offline fail-closed checks passed. Separate real-SRT speculative-network regression passed too. All services used disposable identities and non-default ports; existing deployments/workspaces were untouched. |
|
@codex review Please review exact head ad14f28 and confirm the reviewed commit. The stalled-job redelivery finding is fixed at admission as well as settlement. Before any sandbox work, one atomic Redis operation returns the cached committed result or acquires a once-only execution claim. Duplicate processors and ambiguous/lost claim replies cannot authorize another execution; the bounded claim is retained through the producer/local cancellation horizon. The claim shares the cancellation decision transaction, so cancel-before-start fails closed. commitJobResult now distinguishes committed, cancelled and already_completed. An existing completion cannot be attributed to a second mutation handoff: the native bridge finalizer throws and quarantines that root. The original durable result is never overwritten. Real Redis tests cover concurrent claims (exactly one execution), lost claim ACK, pre-cancellation, missing cached payload, duplicate handoff quarantine and preserving the first result. 23 outcome tests plus 30 registry/request tests pass; service build passes with only existing unrelated warnings. Final local LibreChat/native-SRT acceptance is being repeated on this head. Please inspect remaining lifecycle invariants. |
|
Final local acceptance and real-SRT speculative replay canary both pass on ad14f28. Real LibreChat, locally linked SDK, Code API, Redis/Mongo and native SRT workers on disposable non-default ports. Both command and PTC Stop verified live shell PIDs, termination within five seconds, absence of delayed writes after their scheduled time, and workspace reuse. File persistence, approval modes, private skills, worker isolation, reload and offline failure checks pass. Temporary services cleaned up; no hosted deployment or existing workspace changed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad14f28ad0
ℹ️ 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".
|
@codex review Please re-review exact unchanged head ad14f28. All valid findings are fixed, CI is fully green, and final real local LibreChat/Code API/native-SRT acceptance passes. The last finding is disproved by source and real Redis failure injection. In enqueueForActiveIncarnation, workspaceQuarantineKey is KEYS[7], created by SET key assignmentId with NO TTL. Assignment/queue/deadline/receipt expiry does not expire that fence. A failed SET that would relabel it quarantined leaves the original persistent pending fence blocking reuse. The test acknowledged and fulfilled a native assignment, injected failure into the quarantine eval, waited beyond the deadline, and obtained PTTL=-1 both before and after; a new dispatch was rejected WORKSPACE_QUARANTINED. Detailed evidence is in discussion_r4006669482. No code change was needed. Please confirm the exact-head result after accounting for that persistent fence, and report only additional actionable findings with a demonstrated lifecycle path. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
ad14f28 to
e8b7b34
Compare
1 participant
Summary
Scale and reliability
Verification
Depends on #195.