Skip to content

fix(provider-grok): Grok health checks no longer start Grok's updater - #18075

Merged
juliusmarminge merged 8 commits into
pingdotgg:mainfrom
bompus:fix/grok-probe-no-autoupdate-20261007
Oct 16, 2026
Merged

juliusmarminge merged 8 commits into
pingdotgg:mainfrom
bompus:fix/grok-probe-no-autoupdate-20261007

Conversation

@bompus

@bompus bompus commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

T3 checks each Grok instance's health in the background: grok --version, grok models, grok inspect --json (skills) and an ACP initialize. Grok checks for a newer CLI every time it launches, so those probes could start Grok's npm updater with nobody watching. On Windows that showed up as a blank console window (Grok -> npm -> vp) while T3 was idle.

Change

Grok documents GROK_DISABLE_AUTOUPDATER=1 as process-scoped update suppression. Every health probe now runs Grok with that variable set on a copy of the caller's environment, via one helper in packages/provider-grok/src/server/probeEnvironment.ts:

  • checkGrokProviderStatus uses it for --version, models and the ACP initialize probe.
  • discoverGrokSkills applies it itself, so the per-workspace skill probe the driver runs for each project (snapshotForCwd) is covered too. The original version of this PR missed that path.

Sessions, commit/title text generation and grok update keep the caller's environment unchanged, so normal use and manual updates still update as configured. Auth detection still reads the caller's environment.

Scope and approval

A focused fix for an unwanted side effect of an existing health check. Related: #2537 and the closed #14442. Replaces #16879, which was closed only for a missing model/harness line.

Verification

  • apps/server/src/provider/GrokProvider.test.ts: the fake Grok CLI records an update attempt whenever it runs without the flag, and the caller passes GROK_DISABLE_AUTOUPDATER=0. The probe records none, leaves the caller's environment object unchanged, and still reports ready status, auth and models. It fails without the change. 33 passed.
  • packages/provider-grok/src/server/skills.test.ts: the direct workspace skill probe spawns with the flag set and the caller's other variables intact. 6 passed.
  • vp run typecheck in packages/provider-grok and apps/server, targeted lint and format: clean.
  • Two independent review rounds (Claude Opus 5.5 and GPT-6.1 Sol) found no remaining issues.
  • Not reproduced on Windows here. The flag and its semantics come from Grok's own docs (~/.grok/docs/user-guide/14-headless-mode.md, "Update Check Suppression") and are present in the installed Grok 1.0.52 binary.

Original work by @bompus, taken over and finished by maintainers.

Claude Opus 5.5 and Claude Fable 5.1 in Claude Code, with review by GPT-6.1 Sol in Codex.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d4ffcd9

Macroscope's review found this PR approvable — This is a narrowly scoped, tested bug fix that disables Grok’s unintended startup updater only during health checks while preserving caller environments and normal session/update behavior. It does not change product settings, schemas, deployment, or static-analysis configuration.

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

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35e86850-229a-4073-af97-3b45ae949c91
📥 Commits

Reviewing files that changed from the base of the PR and between e73ebc7 and 8570d6d.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a12ceeec-e998-4b05-af2e-c99f44818dd1
📥 Commits

Reviewing files that changed from the base of the PR and between 289f3ae and e73ebc7.

📒 Files selected for processing: 4
  • packages/provider-grok/src/server/probeEnvironment.ts
  • packages/provider-grok/src/server/skills.test.ts
  • packages/provider-grok/src/server/skills.ts
  • packages/provider-grok/src/server/status.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The Grok status and skill probes now disable the auto-updater in a copy of the supplied environment. Tests check that the probes do not change the supplied environment or run the updater during readiness checks.

Changes: Grok probes

Layer / File(s) Summary
Create probe environment
packages/provider-grok/src/server/probeEnvironment.ts
Adds grokProbeEnvironment, which copies the supplied environment and sets GROK_DISABLE_AUTOUPDATER to "1".
Apply probe environment
packages/provider-grok/src/server/status.ts, packages/provider-grok/src/server/skills.ts, packages/provider-grok/src/server/skills.test.ts, apps/server/src/provider/GrokProvider.test.ts
Status and skill probes use the copied environment for Grok commands and ACP initialization. Tests check that other environment values are preserved, the caller’s environment remains unchanged, and readiness probing does not attempt an update.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e73eb

Grok health and skill probes suppress automatic updates without changing the environment used by normal sessions or manual updates. No merge-blocking risk remains, so the change is ready to merge.

Security architecture risk: ⚪ Minimal · up to e73eb

The change confines update suppression to background probes without changing executable selection, credentials, permissions, or the caller’s environment. No material security risk was found to be introduced or worsened.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed behavior is bounded to Grok probe subprocesses and their inherited environment. Configured binary paths, PATH-based selection, workspace cwd, and existing credentials remain inputs as before; the PR adds no execution principal, network endpoint, secret destination, or permission grant.

Trust Boundaries and Controls

  • inferred — The new flag is a cooperative CLI behavior control, not a sandbox or authorization boundary. Existing configured executables still receive the caller’s environment. The change does not increase their authority; it asks probe launches to avoid the updater side effect.

Resilience and Maintainability Implications

  • observed — Probe failures remain bounded by existing timeout and scoped execution paths. Skill discovery retains typed spawn, timeout, exit, and decoding failures; status checking retains degraded metadata fallbacks. The override adds no shared mutable environment state requiring cleanup or recovery.
Pre-merge checks: Passed 3 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive The description references related issues and explains why this is a focused obvious-bug fix, but explicit maintainer approval remains unresolved. Obtain and record explicit maintainer approval for the scope, or confirm that the repository policy exemption for a focused obvious-bug fix applies.
Passed Passed checks: 3 passed
Check name Status Explanation
Description check Passed The description includes all required sections and clearly explains the problem, implementation, scope, approval rationale, focused verification, and known testing limitations.
Out of Scope Changes check Passed The changes are limited to Grok health-probe environments, related tests, and the shared helper. Normal sessions, authentication detection, and manual updates remain unchanged.
Title check Passed The title clearly and concisely describes the main change: Grok health checks no longer start the Grok updater.
✨ Finishing Touches: 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The focused tests show health probes suppressing startup updates while normal sessions retain them. The PR and #16879 still leave approval of that behavior unresolved. Can a maintainer confirm whether health checks should always disable Grok's updater under the scope policy? Leaving this open for that decision.

@bompus

bompus commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

On the approval question: I think this fits the "very small, focused fix for an obvious bug" exception. The change makes the status probe stop starting the CLI's npm updater. It doesn't change the defaults for normal sessions or manual updates.

The evidence is the Windows process chain captured earlier (Grok -> npm -> vp, Grok 1.0.46), which matches the brief blank console popup in #2537. One limit: in an isolated run I could not reproduce the updater launch on the baseline, so the case rests on that earlier capture. A maintainer can confirm whether health checks should always disable the updater.

If you'd rather treat this as a behavior change, say so and I'll move it to an Ideas discussion.

@bompus bompus changed the title fix(server): disable Grok startup updates during health checks fix(server): Grok health checks no longer start Grok's updater Oct 12, 2026
@juliusmarminge juliusmarminge self-assigned this Oct 15, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Note

Claude Opus 5.5 responding on behalf of Julius.

Taking over this one. I will push follow-up commits to this branch on top of yours and merge main in rather than rebasing.

juliusmarminge and others added 3 commits October 15, 2026 12:58
checkGrokProviderStatus set GROK_DISABLE_AUTOUPDATER=1 for the probes it
runs itself, but the driver's per-workspace snapshotForCwd calls
discoverGrokSkills directly with the instance environment, so `grok
inspect --json` could still start Grok's launch-time updater.

Derive the probe environment in one place (grokProbeEnvironment) and
apply it inside discoverGrokSkills as well as the status check, so every
probe spawn suppresses the updater and no caller has to remember to.
Sessions, text generation, and `grok update` keep the caller's
environment.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ls path

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juliusmarminge juliusmarminge changed the title fix(server): Grok health checks no longer start Grok's updater fix(provider-grok): Grok health checks no longer start Grok's updater Oct 15, 2026
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Oct 15, 2026
@juliusmarminge
juliusmarminge merged commit 2a7bd16 into pingdotgg:main Oct 16, 2026
27 checks passed
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Oct 16, 2026
…pingdotgg#18075)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 2a7bd16)
aorwall added a commit to aorwall/t3code that referenced this pull request Oct 19, 2026
Merges `pingdotgg/t3code` `main` at `3962ae8586` (96 commits since
`e536bfd33b`) into the fork. One merge commit; the tracker entry in
`docs/fork/upstream-merge-log.md` has the full decision list.

## Conflicts (8)

- **`ChatView.tsx`** (`converged`): upstream pingdotgg#19869 replaced
`useCallback` with plain functions plus `useStableCallback`. I took
upstream's file and re-applied each fork delta in that style: the
sandbox start for scripts, the draft-thread create and host-run path in
`runProjectScript`, Workspace script saves, the sandbox surface, file
retargeting, the announced preview and terminal effects, the
sandbox-commands banner, the PR-surface gate and the thread-fork dialog.
**This file is the one to review closely.**
- **`rightPanelStore.ts`** / **`FilePreviewPanel.tsx`**: the fork's
`retargetFile` sits beside upstream's new `navigateFolder` and
`folderTrail` (pingdotgg#15045).
- **`ci.yml`**: upstream folded the `typecheck` job into `build`
(pingdotgg#19633). The Moatless spec check now follows `build`'s final
`Typecheck` step.
- **`Sidebar.tsx`, `ProjectSettingsPanel.tsx`**: upstream's code plus
the fork's additions.
- **`index.html`**: kept the Moatless title. Added a `branding-shell`
path-policy entry for it and `branding.ts`.
- **`source-control-github/package.json`**: took upstream's new
`generate` script and kept `tsc --noEmit` (the `tsc-rs` tripwire).
- **`pnpm-lock.yaml`**: took upstream's copy, stripped `tsc-rs`, and
re-derived it with a real install.

Merge stats: 738 files landed against 736 in the upstream range. The one
upstream file that didn't land is `index.html`, whose only upstream
change was the title. The three extra landed files are fork docs. The
fork delta is 815 files.

## Contract
- Added `UnsupportedMethodError` to
`orchestration.subscribeCommandOutput` and `server.getDatabaseStorage`.
Both are new upstream methods Moatless doesn't dispatch.
- Moatless now serves 11 more `pullRequests.*` reads (moatless pingdotgg#1161,
pingdotgg#1164, pingdotgg#1169). They share `PullRequestRpcError` with methods it still
refuses, so the union keeps `UnsupportedMethodError`. The 11 are
declared as `pull-requests-*-shared-union` exceptions in
`inventory.json`, which retire together once the rest of the family is
served.

## Also in this PR
`sandboxSurfaces.test.ts` had been red on `main` since `95dcaca54f` made
the `pull-request` surface sandbox-independent without updating the
test. The test and module doc now match the code.

## Verification
Duplicate-adds, tripwires, resolution-check, unsupported-methods,
lockfile, format, lint, typecheck and the production build are all
green.

The full test pass hit its one-hour deadline, so I ran the remaining
packages one at a time. These still fail when run alone, in the same
files the 2026-10-15 merge logged as sandbox failures:
- `shared`: `AgentScope`
- `t3`: `ServerBrowserPage`, `AcpAdapterV2`
- `provider-opencode`: `OpenCodeServerLedger`
- `ssh`: `tunnel.test.ts`

`t3`'s `OrchestratorReplayFixtures` `claude_result_is_error` fails only
because this sandbox sets `CLAUDE_CONFIG_DIR`; it passes with that
unset. Every other package passes.

## Upstream features

### Usable as-is
- Folder breadcrumbs navigate to parent folders (pingdotgg#15045).
- Chat view, composer and sidebar no longer keep old renders alive
(pingdotgg#19869, pingdotgg#19852).
- Thread list cache saves without re-encoding every thread (pingdotgg#19915).
- React Compiler runs natively (pingdotgg#20114).
- Linked pull requests are virtualized (pingdotgg#20107).
- A linked PR can be dragged into the composer as a context chip
(pingdotgg#20102).
- HTML renders show a skeleton and open images in the image viewer
(pingdotgg#19654, pingdotgg#20036).
- Model picker search lists the newest models first (pingdotgg#18494).
- Wrapped split diffs stop jumping while scrolling (pingdotgg#20022).
- Text selection continues under the turn minimap (pingdotgg#16748).
- Sent queued messages say "from queue" (pingdotgg#19849).
- Settings stay compact with many connected machines (pingdotgg#19744).
- Middle-click and a setting to open PR links on GitHub (pingdotgg#19854,
pingdotgg#19719).
- Text generation no longer falls back to the flagship model (pingdotgg#19722).
- Pending context handoff is hidden on threads with no sent message
(pingdotgg#20033).
- Mobile and desktop changes don't affect the fork's web client.
- Not taken: the Beta branding (pingdotgg#20032).

### Unsupported in Moatless / needs implementation
- **Live command output** (pingdotgg#15094):
`orchestration.subscribeCommandOutput`, shown in
`CommandOutputPanel.tsx` behind `capabilities.commandOutputStreaming`.
Covered by the new union entry.
- **Database storage notice and `t3 doctor` reclaim** (pingdotgg#19436, pingdotgg#19437):
`server.getDatabaseStorage`, shown in `DatabaseStorageNotice.tsx` behind
`capabilities.databaseStorage`. Covered by the new union entry.
- **Delete a branch after its PR merges or closes** (pingdotgg#19257): a
`pullRequests.runAction` option. The action is refused here.
- **Device setup through a remote device host only** (pingdotgg#20077): behind
`FEATURES.deviceHub`.

### Backend behavior to consider reproducing in Moatless
- Resumed Claude subagents keep updating their own thread and pick up
the parent's renamed worktree branch (pingdotgg#20061, pingdotgg#20079).
- Steering no longer ends running subagents (pingdotgg#19210).
- Worktrees shared by subagents are cleaned up, and orphaned ones are
reported (pingdotgg#19748, pingdotgg#19749).
- Superseded history is compacted in the background (pingdotgg#19435).
- A run whose provider stream ends without a terminal event fails
instead of hanging (pingdotgg#19131).
- One malformed ACP session update no longer ends the session (pingdotgg#20015).
- A dropped HTTP/2 connection no longer crashes the server (pingdotgg#18378).
- PR refreshes are bounded by tracker reads (pingdotgg#19403).
- Cursor threads in Auto use Cursor's Auto-review (pingdotgg#19712).
- Grok health checks no longer start its updater (pingdotgg#18075).
- Expanding a wide ignored folder no longer classifies every child
(pingdotgg#20062).
- OTLP exports name the machine and build they came from (pingdotgg#19370).

Recorded in `docs/fork/gaps.md` under "Runtime fixes upstream made to
its own server".

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/d6f425f2-8234-412e-8189-1c71b309c04a
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:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants