-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(clients): honor selected models in Pi and Aside catalogs #3698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
3ed95a4
docs: plan Grok catalog and Codex patch stabilization
invalid-email-address b8010ae
fix(clients): honor provider selections in exported catalogs
invalid-email-address e2845df
Merge remote-tracking branch 'origin/dev' into codex/grok-pi-filter-5598
invalid-email-address c3f9c4b
Merge remote-tracking branch 'origin/dev' into codex/grok-pi-filter-5598
invalid-email-address b4e535e
fix(clients): reload selection after initial model discovery
invalid-email-address 67d0104
test(responses): keep replay credentials stable across clock ticks
invalid-email-address f63eac9
Merge remote-tracking branch 'origin/dev' into codex/grok-pi-filter-5598
invalid-email-address File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
17 changes: 17 additions & 0 deletions
17
devlog/_plan/260906_grok_catalog_and_patch/000_research.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Grok catalog selection and Codex patch parity | ||
|
|
||
| Class C3, spec-satisfaction repair. The user clarified that filtering means enabled model visibility in Pi/Aside, not assistant output filtering. No output-filter changes are authorized by this unit. Prior Chat patch fixes must remain effective at every Codex Responses tool completion boundary. | ||
|
|
||
| Scope: export catalog selection, refresh of already-owned Pi/Aside integrations, native Responses custom-tool repair, focused regression tests, matching docs. No live user config changes, deployment, release, unrelated cleanup, local test suites or local typecheck. Push --no-verify and admin merge are explicitly authorized. Probes and hosted CI are authorized. | ||
|
|
||
| Evidence: src/clients/config-export/constants.ts selects openai-completions for Pi; Aside uses the same builder. src/server/management/model-rows.ts:218 filters disabled only. src/cli/opencode.ts:373 likewise ignores selectedModels. src/codex/catalog/provider-fetch.ts:1992 is canonical for allowlist plus disabled and pending selection. src/server/management/model-routes.ts visibility writes converge Codex only. Both explicit sync paths refresh only MCode among owned file integrations. Live read-only snapshot: /v1/models and existing Aside managed file currently contain xai/grok-4.6 only; do not claim that the live snapshot reproduced a full catalog leak. Synthetic allowlist and stale-owned-file scenarios will establish the gaps. | ||
|
|
||
| Independent patch analysis: native custom exec is skipped by repairable||aliased in responses-custom-tool-repair.ts:206. Its item.done/input.done preserve raw patch while response.completed repairs it. Function apply_patch helper aliases can stream raw patch before compiling final JS. Existing arbitrary JS and foreign namespace boundaries stay byte-exact. | ||
|
|
||
| Dependencies: 010 export selection -> 020 owned file convergence -> 030 Codex patch completion parity. Each has its own PABCD and reviewable PR. The patch layer is a separate user-requested stabilization concern published after catalog layers in the requested stack. | ||
|
|
||
| Resource scope: existing GitHub repo credentials, read-only local configuration with no secret output; at most 24 synthetic live-provider calls, each <=120 seconds; six-hour execution window. No explicit token budget. Probe scripts stay ignored .tmp; public notes contain no credentials or private requests. DONE = regression probes, actual hosted exact-head test/typecheck CI, independent review, registered stack merged and fetched-dev ancestry. BLOCKED only for a persistent external dependency; no stopping on CI queueing. Each later P rechecks current source and carries earlier evidence. Escalation: reclaim failed delegated scope; new write delegation requires P amendment. | ||
|
|
||
| ## Baseline probes | ||
|
|
||
| `bun .tmp/grok-stabilization/catalog-probe.ts` exit 0: management/CLI × pi/aside each emitted grok-4.3, grok-4.5, grok-4.6 despite selectedModels=[grok-4.6]; full management roster was three. `bun .tmp/grok-stabilization/patch-probe.ts` exit 0: native custom exec emitted one raw delta, input.done uncompiled and item.done uncompiled; function apply_patch alias emitted one raw preview despite compiled final. Both are observation probes before repair, not passing acceptance assertions. `python3 .tmp/grok-stabilization/verify-roadmap.py` exit 0 checks numbered roadmap artifacts and actual existing source target paths. No local suite or typecheck was run. |
10 changes: 10 additions & 0 deletions
10
devlog/_plan/260906_grok_catalog_and_patch/010_export_selection.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| # 010 Export catalog visibility | ||
|
|
||
| Loop: spec-satisfaction repair; trigger: selectedModels ignored by export projection. Goal: Pi/Aside export obeys the same provider allowlist, blocklist, pending selection as routed catalog. No change to full management catalog or routing authorization. | ||
|
|
||
| MODIFY src/server/management/model-rows.ts: import filterCatalogVisibleModels. In loadExportModels compute visible routed row identities from filterCatalogVisibleModels(rows.filter(row => !row.native), config); return only !row.disabled and (row.native || visible set contains row), then map toExportModel. Preserve native visibility semantics. | ||
| MODIFY src/cli/opencode.ts: use same canonical filter once for rows with non-native provider/id identity, then exclude those not retained before seen.add. Do not infer provider identities for legacy rows missing them; keep existing disabled and Direct-native checks. Preserve order, custom/combo aliases and per-row metadata; do not duplicate allowlist matching. | ||
| MODIFY tests/server/management-client-config-route.test.ts and tests/cli/cli-export-command.test.ts: fixtures with xai selectedModels=[grok-4.6], full three-model roster, blocklist override, empty allowlist, slash-bearing ids, disabled duplicate. Render both pi and aside through production loader and CLI projection. A nonempty allowlist retains only selected IDs; a ready provider with an empty allowlist retains its full otherwise-visible roster; pending initial selection keeps routed rows hidden. Management still offers all IDs. CLI consumers must reload their configured state after discovery because the request can persist the initial selection. | ||
| MODIFY docs-site/src/content/docs/guides/integrations.md: explain selected list applies to generated catalogs. | ||
|
|
||
| Verifier: standalone synthetic imports of loadExportModels/exportModelsFromProxyRows plus Pi/Aside serializers; no bun:test. CI runs existing focused regressions, typecheck and full platform suite. Before C record exact source SHA and probe output. Stop after export boundaries agree; next cycle refreshes old owned files. |
17 changes: 17 additions & 0 deletions
17
devlog/_plan/260906_grok_catalog_and_patch/020_owned_refresh.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # 020 Converge already-owned Pi/Aside catalogs | ||
|
|
||
| Depends on 010 filtered loader. Loop spec-satisfaction repair. Goal: a model visibility/selection change and explicit sync refresh existing connected Pi/Aside files. No adoption of unowned/manual files, no recreation of removed blocks, no override of drift. | ||
|
|
||
| NEW src/integrations/catalog-refresh.ts: bounded helper refreshOwnedCatalogIntegrations(input) iterates explicit client list [pi, aside] (sync may include mcode), passes lazy cached models loader to refreshOwnedIntegration, catches per-client errors and returns existing outcome shape. Use existing ownership store, mutation flight and coordinated writer; never bypass fingerprints. | ||
| MODIFY src/server/management/model-routes.ts: local async convergence helper calls existing convergeCodexCatalog then new owned refresh for pi/aside with port from URL/config and lazy loadExportModels(config); attach clientIntegrations outcome to disabled-models, model-visibility, selected-models and model-preset writes. Keep successful config persistence even when one file refuses refresh; return warning outcome. | ||
| MODIFY src/server/management/config-routes.ts and src/cli/dispatch.ts: expand current MCode-only owned refresh to mcode/pi/aside via helper; preserve native Grok/Desktop gates and refused-sync behavior. | ||
| MODIFY existing tests/clients/sync-client-integrations.test.ts and tests/server/management-integration-routes.test.ts: fake IO/store or isolated home seeds owned pi/aside with two models, refresh with selected one, assert hidden row removed and other provider fields preserved. Prove unowned, removed and drifted configs untouched; one failure does not block other client. Add route-driven visibility refresh coverage using injected convergence. | ||
| UPDATE structure/09_client-integrations.md and owning docs page with ownership/refusal semantics. | ||
|
|
||
| Verification: standalone isolated writer probe using synthetic models and temp homes, then exact-head hosted CI. C4 care for automatic owned-file writes: independent review must confirm ownership/no-clobber and per-client failure boundaries. Final enforcement is existing coordinated writer; refresh helper is an early caller, not a permission boundary. Known bypass: manually calling writer with explicit adoption; no such call in this unit. Stop when file projection converges or produces truthful refusal. | ||
|
|
||
| ## Audit amendment: overlapping refreshes | ||
|
|
||
| The existing constant refresh mutation-flight key incorrectly joins different model selections. MODIFY src/integrations/owned-refresh.ts to use a unique per-refresh operation key (crypto.randomUUID), making overlapping refreshes explicitly busy rather than reporting another desired catalog as success. Implicit refresh never joins an explicit HTTP mutation. Add controlled overlap with distinct old/new rosters: second call reports integration_mutation_busy; first result describes only its own write. Subsequent retry applies the new roster. Return per-client failures; never retry stale snapshots automatically. | ||
|
|
||
| Add a ManagementApiDeps refreshOwnedCatalogIntegrations seam for route verification, defaulting to the real helper. Creation: exported helper/deps type; consumption: model routes and explicit sync. No serialization/deserialization: runtime-only dependency injection. Tests use fake IO/store or temporary home, never actual user-owned files. | ||
13 changes: 13 additions & 0 deletions
13
devlog/_plan/260906_grok_catalog_and_patch/030_responses_patch.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # 030 Codex native Responses patch completion parity | ||
|
|
||
| Depends on recorded export layers for stack delivery; runtime independent. Class C3, spec-satisfaction repair. Goal: same repaired executable input at deltas/input.done/item.done/response.completed for complete patches misrouted as exec. No arbitrary JavaScript rewriting. | ||
|
|
||
| MODIFY src/server/responses-custom-tool-repair.ts: register same-name routed custom calls in addition to aliases. Track original wire name and target; hold custom input deltas when code-mode exec may be a patch envelope or when helper alias requires compilation. Accumulate under TranslatorBudget, release on done/dispose. Run restoreRoutedCustomCalls for same-name custom items, and use existing resolveCodeModeHelperName/compileCodeModeHelperInput at input.done. Do not place exec in repairNames. Preserve ordinary JavaScript streaming where monotonic; once raw prefix would diverge, withhold to authoritative completion. Suppress function helper-alias progressive previews rather than emitting raw patch before compiled JS. | ||
| MODIFY tests/responses/responses-custom-tool-repair.test.ts: native custom exec raw/wrapped complete patch, fragmented marker, input.done and output_item.done plus terminal snapshots; function apply_patch wrapper alias; invalid/incomplete envelopes and valid JS remain exact; flat catalogs and foreign namespaces do not retarget; cancellation frees retained buffers. | ||
| UPDATE existing patch compatibility docs and structure/11_compatibility-contracts.md to describe completion-boundary parity. | ||
|
|
||
| Verifier: pure standalone synthetic SSE-block imports, compare outputs at each lifecycle edge and execute generated JS against a recording tools.apply_patch stub (no filesystem writes). Probe must assert monotonic preview or held preview, one call, exact canonical patch data. CI runs added regressions and existing bridge/native compatibility tests plus full suite/typecheck. Complete only after independent review and exact-head CI; register requested stack and admin merge after verified heads. Fetch dev and prove every merge SHA ancestor. D records parity inventory and public PR links. | ||
|
|
||
| ## Audit amendment | ||
|
|
||
| Executable repair is limited to authorized code-mode exec and recognized helper aliases; unrelated same-name native custom tools keep raw input byte-for-byte. Explicit negative: render_diagram input JSON string {"input":"literal"} is not unwrapped. Separate scenarios cover missing input.done, terminal-only completion, failed/incomplete after held deltas, and disposal. Authoritative completion wins over previews. Failure never synthesizes successful completion. All retained buffers release. One simulated execution means choose the client-consumed completed item once, not execute every redundant lifecycle representation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.