[1892 by GPT-5.6 Sol] Make /use switch the durable PR model label and queued provider - #1897
[1892 by GPT-5.6 Sol] Make /use switch the durable PR model label and queued provider#1897propr-dev[bot] wants to merge 27 commits into
Conversation
…label and qu Implemented by ProPR AI using gpt-5.6-sol model. Implementation completed successfully.
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 4d5856a Summary of ChangesFixed the PR’s
Validation passed:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
Fixed the PR’s `max-lines` lint failure. - Extracted provider-limit retry supersession logic into [prCommentAgentUtils.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T11-50-47/src/jobs/prCommentAgentUtils.ts:265). - Simplified [processPullRequestCommentJob.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T11-50-47/src/jobs/processPullRequestCommentJob.ts:155) while preserving behavior. Validation passed: - Root lint - TypeScript build - Fast unit suite: 251 tests - Focused routing/comment suite: 76 tests - `git diff --check` No commit was created. PR: #1897 Comment by: @github-actions[bot] (ID: 5292959913) Model: gpt-5.6-sol
|
✅ Applied the requested follow-up changes in commit dc6bd57 Summary of ChangesUpdated test/slashCommandsBlock.test.ts to assert Validation:
🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…r/worktrees/integry/propr/pr-1897-followup-2026-08-14T11-57-07/test/slashCommandsBlock.test.ts:52) to assert `/use` documents durable model switching and follow-up execution, replacing the obsolete single-run override expectation. Updated [test/slashCommandsBlock.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T11-57-07/test/slashCommandsBlock.test.ts:52) to assert `/use` documents durable model switching and follow-up execution, replacing the obsolete single-run override expectation. Validation: - Targeted suite: 7/7 passed - `git diff --check`: passed - No commit created, as requested. PR: #1897 Comment by: @github-actions[bot] (ID: 5293002305) Model: gpt-5.6-sol
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
|
✅ Applied the requested follow-up changes in commit 4718a87 Summary of ChangesFixed the full-suite failure by adding the new core resolver to the merge-conflict test mock in processMergeConflictJob.test.ts. Validation passed:
No commit created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…r to the merge-conflict test mock in [processMergeConflictJob.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T12-09-15/test/processMergeConflictJob.test.ts:176). Fixed the full-suite failure by adding the new core resolver to the merge-conflict test mock in [processMergeConflictJob.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T12-09-15/test/processMergeConflictJob.test.ts:176). Validation passed: - Targeted test: 10/10 - TypeScript build - `git diff --check` No commit created. PR: #1897 Comment by: @github-actions[bot] (ID: 5293116167) Model: gpt-5.6-sol
|
Independent audit of exact head 1. Model-label transition can delete unrelated concurrent labels
Make the model-label mutation preserve every unrelated label that exists at mutation time and verify the final canonical model-label invariant before enqueue. It must not overwrite concurrently added/removed AI, security, release, or workflow labels. Use a bounded live-read/mutate/verify/retry strategy (or another GitHub-safe approach) that converges to exactly one canonical configured model label while preserving unrelated live labels. If convergence/verification fails, do not enqueue or acknowledge success. Add deterministic concurrency tests where unrelated labels are added and removed between reads/writes, including an issue-comment event and a stale review-comment payload. 2. Provider-limit retry reconstruction loses picked/batched commentsPending pickup reads then deletes Redis and appends comments only to local There is a second loss interleaving: a delayed provider retry can become active after the label transition and before the handler removes it. The worker returns Persist/restore the full claimed comment set exactly once before any provider-limit requeue or superseded exit. Deduplicate by stable comment identity including type/revision as appropriate, preserve order and code context, and retain the explicit selected agent/model/label. A crash/retry or webhook redelivery must not lose or duplicate work. Add focused tests for:
Also update the stale internal Run the focused routing/batching/retry suites, full tests, build, typecheck, lint, and |
|
✅ Applied the requested follow-up changes in commit a3ab4e9 Summary of ChangesImplemented both release-blocking race fixes.
Validation:
No commit, push, PR, review, publish, merge, or deployment actions were performed. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented both release-blocking race fixes. - Model-label transitions now use bounded live-read/mutate/verify retries with targeted label operations, preserving unrelated concurrent labels and refusing enqueue/acknowledgement without verified convergence. - Claimed comments are durably persisted before retries or superseded exits, with atomic Redis claims/restoration and identity deduplication across comment type and revision. - Provider retry reconstruction preserves complete ordered comments, code context, and explicit routing. - Updated the stale `UseCommandMeta` description. - Added deterministic concurrency, removal/enqueue failure, crash replay, redelivery, mixed issue/review, and idempotency tests. Validation: - Focused suites: passed. - Build: passed. - Typecheck: passed. - Lint: passed with one existing-style `max-lines` warning in [processPullRequestCommentJob.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T12-28-52/src/jobs/processPullRequestCommentJob.ts). - `git diff --check`: passed. - Full suite: 285/286 runs passed; only `llmMetrics.test.ts` timed out because Redis was unavailable and the environment has neither `redis-server` nor Docker. UI: 43 files / 341 tests passed. No commit, push, PR, review, publish, merge, or deployment actions were performed. PR: #1897 Comment by: @integry (ID: 5293286649) Model: gpt-5.6-sol
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 75e7a4d Summary of ChangesFixed the build-check failure without changing runtime behavior.
Validated successfully:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…vior. Fixed the build-check failure without changing runtime behavior. - Moved local job types into [prCommentJobTypes.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T13-01-02/src/jobs/prCommentJobTypes.ts). - Reduced [processPullRequestCommentJob.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T13-01-02/src/jobs/processPullRequestCommentJob.ts) below the 400-line ESLint limit. Validated successfully: - Root lint and TypeScript build - Shared, core, and API builds/lint - Docs typecheck/build - Fast unit suite: 251 passed - `git diff --check` No commit was created. PR: #1897 Comment by: @github-actions[bot] (ID: 5293577166) Model: gpt-5.6-sol
|
/merge llm-codex-gpt56-sol |
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
…-use-switch-the-durab-20260814-1122-q78
|
/review llm-codex-gpt55 |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex-gpt55Overall EvaluationThis PR is close, but not ready to merge. It substantially improves ✅ Live label convergence — The new convergence path reads live issue labels, mutates only managed model labels, and verifies the final single-model-label invariant. ✅ Retry durability — Provider-limit retry reconstruction now dedupes and preserves the full claimed comment set plus explicit agent/model/label routing. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Ordinary jobs bypass the durable label
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Cover label-change-before-runAdd a regression test for a normal queued follow-up that observes label A at enqueue time, then has the PR switched to label B before execution. This is optional after the code fix, but it would lock in the intended distinction between explicit ScoreThe core implementation is strong and current checks are green, but the remaining routing regression directly conflicts with the durable-label source-of-truth behavior this PR is meant to enforce. 🤖 Review Details
🤖 Review by ProPR |
|
Please fix the confirmed F1 routing provenance regression, narrowly and without changing the already-audited label-race or retry-durability machinery. At exact head Required correction:
Commit only through ProPR's deterministic finalizer. Do not merge the pull request. |
|
✅ Applied the requested follow-up changes in commit a9af5ef Summary of ChangesImplemented the narrow F1 provenance fix.
Changed: Validation:
No commit or merge was performed. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented the narrow F1 provenance fix. - Ordinary jobs retain enqueue-time `llm` fallback but omit `agentAlias`, `modelName`, and `modelLabel`. - Explicit `/use` and `/switch` selections still serialize durable provenance. - Added an A→B regression proving worker-time live-label routing. - Existing `/use` batching, retry, supersession, and restoration regressions remain unchanged and pass. Changed: - [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T13-50-53/packages/core/src/webhook/commentEventHandler.ts:912) - [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T13-50-53/test/commentEventHandler.switch-use.test.ts:1054) Validation: - Focused routing/label/pending/retry suite: 346 passed - Hosted fast-unit suite locally: 264 passed - Typecheck, builds, root lint, changed-core lint: passed - `git diff --check`: passed - Exact-head hosted PR checks: all green - Local full-suite Redis bootstrap was unavailable because Docker/Redis are absent; the exact-head hosted Full Test Suite passed. No commit or merge was performed. PR: #1897 Comment by: @integry (ID: 5294070848) Model: gpt-5.6-sol
|
/review llm-codex-gpt55 |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex-gpt55Overall EvaluationThis PR substantially implements durable ✅ Explicit provenance narrowed — ✅ Label transition preserves unrelated labels — the new convergence path in ✅ Delayed retry recovery covered — delayed provider-limit retries are restored into pending comments before removal, with idempotent Redis restoration. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F2: 🔴 Active ordinary retries are not superseded
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Preserve retry comment chronologyWhen a delayed retry is superseded, the selected ScoreThe PR fixes the main provenance regression and has strong targeted coverage, but the remaining active ordinary provider-limit race can still defeat the core 🤖 Review Details
🤖 Review by ProPR |
|
/use codex-gpt56-sol Fix only the remaining F17 publication interleaving on exact head a88bc90. Independently reproduced executable race: older /use completes its monotonic marker claim and label convergence, then stalls before the durable queue insertion. A newer /use then claims the newer marker, converges the label, and queues. When the older delivery resumes it still queues a second job with stale explicit routing. The final label is the newer model, but two durable jobs exist and the older one carries the old explicit agent/model. Extend the existing per-PR transition protocol so freshness claim, label convergence and verification, and durable queue-or-pending publication are serialized as one per-PR operation with ownership-safe finally release and no cross-PR global lock. Alternatively, an equivalent atomic protocol must revalidate marker ownership at publication and prevent the older explicit job from being inserted, while preserving its comment exactly once as pending where appropriate. A mere second non-atomic marker read is insufficient. Add the exact barrier regression: older transition succeeds and its queue insertion is held; newer delivery completes; older resumes; assert the durable label and the only executable routing are the newer selection, with no lost or duplicated comment. Retain the existing concurrent pre-transition, same-timestamp revision, undefined/captured-empty rollback, unrelated-label, provider-retry, pending durability, and custom-pattern cases. Run the focused suites, strict root/core lint and typecheck, and git diff --check. Do not address suggestions or broaden scope. |
|
✅ Applied the requested follow-up changes in commit a41a41e Summary of ChangesImplemented the F17 publication interleaving fix without committing.
Validation passed:
Changed four scoped files, including commentEventHandler.ts and its focused regression. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…mmitting. Implemented the F17 publication interleaving fix without committing. - Extended the identity-bound per-PR lease across freshness claim, label convergence/verification, fencing, and durable queue-or-pending publication. - Added ownership checks immediately before durable mutations and token-safe release. - Added the held-queue-insertion regression proving one executable job, newer routing, and both comments exactly once. - Preserved per-PR isolation and existing rollback/retry/custom-pattern behavior. Validation passed: - Focused suites: 105 tests - Root/core lint - Root/core typecheck - `git diff --check` Changed four scoped files, including [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T19-28-43/packages/core/src/webhook/commentEventHandler.ts) and its focused regression. PR: #1897 Comment by: @integry (ID: 5297329034) Model: gpt-5.6-sol
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR correctly redesigns ✅ Canonical routing is persisted — Aliases and standard full labels resolve to a configured agent, model, and canonical label, all of which are carried through batching and retries. ✅ Pending work is substantially more durable — Revision-aware identities, atomic claims, idempotent restoration, and retry reconstruction preserve comments and routing across common crash and retry paths. ✅ Unrelated labels are preserved — Exclusive convergence mutates only managed model labels and verifies or restores the resulting label set. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F20: 🔴 Retry publication races with
|
|
✅ Applied the requested follow-up changes in commit fdb39cb
Summary of ChangesImplemented only F20 and F21.
Verification passed:
No commit was created. 🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
Implemented only F20 and F21. - Provider-limit retries now acquire the same per-PR lease as `/use`, recheck live routing, and publish while holding it. - Label convergence and restoration verify lease ownership before and after every mutation and between attempts. - Added retry-publication race and mid-mutation lease-loss tests. Verification passed: - 76 `/use`/`/switch` tests - 10 label-operation tests - TypeScript typecheck - Diff check and targeted lint (warnings only) No commit was created. PR: #1897 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol
|
✅ Applied the requested follow-up changes in commit 5005769 Summary of ChangesFixed the sole full-suite failure in ultrafixLabelTransition.ts. Lease acquisition errors now retain the original Redis error message while remaining typed Validation:
🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
…on.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T19-58-05/packages/core/src/utils/ultrafixLabelTransition.ts:65). Fixed the sole full-suite failure in [ultrafixLabelTransition.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T19-58-05/packages/core/src/utils/ultrafixLabelTransition.ts:65). Lease acquisition errors now retain the original Redis error message while remaining typed `LabelTransitionLeaseError`. Validation: - 108 focused routing, lease, label, and `/ultrafix` tests passed - Core typecheck passed - Core lint passed with one pre-existing warning - `git diff --check` passed - No commit created PR: #1897 Comment by: @github-actions[bot] (ID: 5297512148) Model: gpt-5.6-sol
|
✅ Applied the requested follow-up changes in commit 112a3f3 Summary of ChangesFixed the two CI lint failures without changing runtime behavior:
Validation passed:
Changes remain uncommitted as requested. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
…avior: Fixed the two CI lint failures without changing runtime behavior: - Grouped label-restoration state to satisfy `max-params` in [labelOperations.ts](/home/node/workspace/packages/core/src/utils/github/labelOperations.ts:47). - Compacted exports to satisfy `max-lines` in [prCommentJobUtils.ts](/home/node/workspace/src/jobs/prCommentJobUtils.ts:453). Validation passed: - Strict root and changed core-file lint - Core typecheck and build - Root build - 25 focused routing/label-transition tests - `git diff --check` Changes remain uncommitted as requested. PR: #1897 Comment by: @github-actions[bot] (ID: 5297601788) Model: gpt-5.6-sol
|
ℹ️ Analyzed the follow-up request by @github-actions[bot] Analysis SummaryNo additional edits were needed. Current head
Verified:
No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/merge llm-codex-gpt56-sol |
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
…-use-switch-the-durab-20260814-1122-q78
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
|
/use codex-gpt56-sol One final narrowly scoped durability blocker is executable on exact merged head b2b2969. processCommentEvent claims pr-comment-processed before dispatching a slash command. handleModelSelectionCommand catches LabelTransitionLeaseError and returns success, so the outer catch never deletes that claim. A transient first lease acquisition failure followed by redelivery of the identical /use event produces zero jobs both times: the second delivery is suppressed as already processed. This silently loses the command. Fix only this command-loss path. A label-transition lease acquisition/ownership failure must propagate through processCommentEvent failure handling (or equivalently release the exact slash-command claim and surface retryable failure) so the delivery is not acknowledged as accepted and a redelivery can retry. Preserve idempotence if failure occurs after partial label convergence or queue publication: same revision, deterministic job identity, no duplicate comment/job/acknowledgement. Add a committed regression with first lease acquisition failing transiently and the identical redelivery successfully converging and enqueueing exactly once; also cover ownership loss after a partial irreversible step as appropriate. Update the existing no-enqueue/no-ack lease-failure expectations to require the retryable failure, and retain the complete 76 switch/use, 10 label, 22 Ultrafix, provider-retry, rollback, strict lint/typecheck, and merged-main gates. Do not change unrelated behavior. |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR correctly reframes ✅ Canonical routing is explicit — ✅ Label publication is carefully fenced — The transition uses live reads, a per-PR lease, verification, rollback, and managed-label-only mutations, while preserving unrelated labels. ✅ Comment recovery is substantially stronger — Claimed comments and provider retries retain routing and revision identities, with atomic, idempotent Redis restoration. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F22: 🔴 Stale same-timestamp edits can win
F23: 🔴 Waiting-job handoff can strand
|
|
✅ Applied the requested follow-up changes in commit 642e156 Summary of ChangesImplemented the narrowly scoped durability fix.
Changed: All gates passed: 78 switch/use, 10 label, 22 Ultrafix, provider-retry/rollback coverage, core build, strict lint, root/core typecheck, diff-check, and merged-main ancestry. No commit created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented the narrowly scoped durability fix. - Lease acquisition/ownership errors now propagate to `processCommentEvent`, which releases the slash-command claim and surfaces retryable failure. - Identical redelivery recognizes an already-published command revision, preventing duplicate pending comments, jobs, or acknowledgements. - Added regressions for transient acquisition failure and ownership loss after label convergence plus queue publication. Changed: - [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T20-34-47/packages/core/src/webhook/commentEventHandler.ts:384) - [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1897-followup-2026-08-14T20-34-47/test/commentEventHandler.switch-use.test.ts:943) All gates passed: 78 switch/use, 10 label, 22 Ultrafix, provider-retry/rollback coverage, core build, strict lint, root/core typecheck, diff-check, and merged-main ancestry. No commit created. PR: #1897 Comment by: @integry (ID: 5297862473) Model: gpt-5.6-sol
|
🛑 Execution Cancelled The task processing was stopped by user request. You can post a new comment to restart processing. |
|
Superseded by minimal issue #1905 per the product decision: |
|
ℹ️ Analyzed the follow-up request by @integry Analysis SummaryNo changes made. PR #1897 remains intact as historical evidence and must not be merged. Verified the worktree is clean at No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
AI Implementation Summary
Closes #1892
Branch:
1892/gpt-5.6-sol-make-use-switch-the-durab-20260814-1122-q78Commits: ✅ Changes committed (f9c166e)
AI Processing Completed
Execution Details:
Summary:
Implemented GitHub issue #1892.
Key changes:
/usenow atomically switches to the canonical model label before queueing work;/switchremains compatible.Primary implementation: commentEventHandler.ts, modelLabelResolution.ts, processPullRequestCommentJob.ts.
Validation:
max-lineswarning.Detailed Logs:
01a00003-21a4-73c3-a6ed-15de8c65ec03Log files stored at:
/tmp/claude-logs/issue-1892-2026-08-14T11-48-03-576Z-conversation.json/tmp/claude-logs/issue-1892-2026-08-14T11-48-03-576Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #1892.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!