Skip to content

fix(server): keep the native Grok default model - #10025

Open
saphid wants to merge 386 commits into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:fix/v2-grok-default-model-20260905
Open

fix(server): keep the native Grok default model#10025
saphid wants to merge 386 commits into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:fix/v2-grok-default-model-20260905

Conversation

@saphid

@saphid saphid commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The Grok V2 adapter keeps the session’s native model when the default grok-build product entry is selected, while explicit native selections still switch models. ACP setup metadata follows successful same-session changes so switching A → B → A applies the final switch.

Current head dc4ab339f includes current V2 base 7dd9873ed and the shared test-only prerequisite #10865. Merge #10865 first; the topic itself changes 3 files. This rebuild removes the obsolete merged-base history from the PR diff.

Validation: 180 focused tests pass across the affected feature and foundation persistence coverage. Scoped server typecheck passes; targeted lint, formatting and diff checks pass. New-head CI is pending.

Earlier live-provider adapter probes reproduced the original issue and verified the feature patch on combined candidate fa6a1ab04. Those are historical adapter evidence, not fresh rendered-client or live-provider verification of this rebuilt head.

Independent cross-provider review was unavailable: the direct Claude Opus 5 high launcher in this audit exited 1 with expired OAuth before inference. No Claude review is claimed.

Targets Orchestrator V2 in #2829. Updated with GPT-6 in Codex/T3.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 5, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This changes runtime handling of the built-in Grok default model, including session setup and subsequent model switches. The scope is focused and well tested, but default-model behavior warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@t3-code t3-code Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed diff and relevant surrounding code at 0ab3cb4150b0243e98340928e06e1435b2432ccd. no blocking findings.

the flavor hook is called during session configuration and the existing grok helper suppresses the product slug while retaining explicit model switching. reviewed the added constructor-to-acp cases and helper/caller context. no code changes needed. i did not rerun the adapter suite or a live grok session.

ci snapshot: no failing or pending checks; some checks are skipped. this is a comment review, not an approval or merge.

saphid added a commit to saphid/t3code that referenced this pull request Sep 7, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All clear

Posted via Macroscope — Effect Service Conventions

@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 2 times, most recently from 9242b91 to 3d81172 Compare September 8, 2026 03:31
@saphid

saphid commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Ran an independent automated first-pass review (Amp/Astra, read-only) on fecd670438: no correctness, Effect-convention, or performance defects in the PR's own change.

One real finding surfaced, pre-existing but on the path this PR codifies: same-session model switches compare against stale session-setup metadata, so A → B → A skipped the final session/set_model and silently left the session on B. Addressed in 0e4a315 (configureSession now persists the applied model into activeSessionSetup), with a Grok regression test that fails without the fix. Focused suites pass (145 tests), typecheck clean.

@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies.

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Sep 8, 2026
juliusmarminge and others added 18 commits September 7, 2026 23:12
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Re-route steering that races completion into idempotent follow-up dispatches
- Preserve scheduled-task attribution and provider ownership history across clients
Port main's pull-request discovery, active thread ordering, async question dismissal, settlement fixes, provider-session import, attachment context, and provider correctness changes into orchestration v2.

Keep the branch's intentional composer and subagent behavior while adopting main's web and mobile fixes. Prevent headless setup terminals from hanging on the color probe, and move the v2 migration block to 050-061 after main claimed 048-049.
Restore the failed-before-start timer guard, align two server fixtures with the reconciled behavior, and remove dead files, exports, and dependencies surfaced by Knip.

Drop the temporary branch push trigger now that the PR is mergeable, so each update runs the pull-request workflow once.
Keep main's batch machine-update banner and update action while preserving the v2 runtime-based environment lock used by draft load balancing.
Adopt TypeScript 7 and Effect rc.112 across orchestration v2, including the TaggedError API migration and updated Effect-aware tests. Restore main's composer-aware scroll-to-end clearance while retaining selected-model settings sync, preview recording transfer, image galleries, desktop context menus, and layout hit targets. Regenerate the lockfile on the upgraded dependency baseline.
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch from efd9994 to 6102d00 Compare September 8, 2026 06:23
juliusmarminge and others added 5 commits September 8, 2026 14:26
Same-runtime model switches compare against the stored session-setup
metadata, which is never refreshed after a switch. Switching A -> B -> A
therefore skipped the final set_model and silently left the session on B.

configureSession now records the model the session actually runs on into
activeSessionSetup after applying a selection, for the generic path and
flavor-provided applyModelSelection alike, and a Grok regression test
covers the return trip against the mock ACP agent.
Advertise bounded socket snapshots and authoritative dispatch validation, omit raw command output and inline file bodies at the wire boundary, and preserve compact status metadata across web and mobile. Add transport-budget coverage for snapshots, resume, commands, legacy import, and projection maintenance.
@saphid
saphid force-pushed the fix/v2-grok-default-model-20260905 branch from 8f65bb5 to bde7c4d Compare September 9, 2026 00:06
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). labels Sep 9, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All clear

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

This comment has been minimized.

@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch from 8187b17 to ada72ce Compare September 9, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants