Skip to content

fix(gsd): show dispatched model in widget after hook override#3418

Open
atikin19sh wants to merge 2 commits intogsd-build:mainfrom
atikin19sh:fix/widget-model-after-routing
Open

fix(gsd): show dispatched model in widget after hook override#3418
atikin19sh wants to merge 2 commits intogsd-build:mainfrom
atikin19sh:fix/widget-model-after-routing

Conversation

@atikin19sh
Copy link
Copy Markdown

@atikin19sh atikin19sh commented Apr 1, 2026

Summary

  • render the dashboard model from currentUnitModel (the actually dispatched unit model) with fallback to cmdCtx.model
  • move updateProgressWidget call to after hook model override handling in runUnitPhase
  • reset currentUnitModel at unit start to avoid stale carry-over on early failures
  • add regression assertion that widget update happens after hook override block

Why

When a hook-level model override is applied, the widget could still show the pre-override model because the widget was created earlier in the phase. This makes model observability misleading.

Verification

  • npm run typecheck:extensions
  • npm run test:compile
  • node --import ./scripts/dist-test-resolve.mjs --test dist-test/src/resources/extensions/gsd/tests/auto-loop.test.js

@github-actions github-actions bot added bug Something isn't working High Priority labels Apr 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

🔴 PR Risk Report — CRITICAL

Files changed 5
Systems affected 2
Overall risk 🔴 CRITICAL

Affected Systems

Risk System
🔴 critical Auto Engine
🟡 medium Web Mode
File Breakdown
Risk File Systems
🔴 src/resources/extensions/gsd/auto-dashboard.ts Auto Engine, Web Mode
🔴 src/resources/extensions/gsd/auto.ts Auto Engine
🔴 src/resources/extensions/gsd/auto/phases.ts Auto Engine
🔴 src/resources/extensions/gsd/auto/run-unit.ts Auto Engine
src/resources/extensions/gsd/tests/auto-loop.test.ts (unclassified)

⚠️ Critical risk — please verify: state persistence, auth token lifecycle, agent loop race conditions, RPC protocol compatibility.

@atikin19sh
Copy link
Copy Markdown
Author

Re-ran verification on this branch with a full unit sweep.

✅ Relevant checks for this PR

  • npm run typecheck:extensions — pass
  • npm run test:compile — pass
  • node --import ./scripts/dist-test-resolve.mjs --test dist-test/src/resources/extensions/gsd/tests/auto-loop.test.js — pass (includes the new regression assertion for widget update ordering after hook override)

ℹ️ Full suite status

  • npm run test:unit => 4541 passed, 12 failed, 5 skipped

The 12 failures are outside the scope of this PR and are in unrelated areas:

  • src/tests/app-smoke.test.js (initResources marker/sibling pruning)
  • src/tests/mcp-server.test.js (missing dist/mcp-server.js in dist-test env)
  • src/tests/rtk-session-stats.test.js and command rewrite RTK tests
  • src/tests/welcome-screen.test.js

No failures were observed in the GSD auto-loop/model-routing/widget code path changed here.

@Solvely-Colin Solvely-Colin self-assigned this Apr 1, 2026
Copy link
Copy Markdown
Collaborator

@Solvely-Colin Solvely-Colin left a comment

Choose a reason for hiding this comment

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

Blocking: this can make the progress widget show a model the unit is not actually running on. We now prefer \currentUnitModelfor display, but inrunUnit()we still fall back to the session default if restoring that model afternewSession()fails. In that failure path,currentUnitModelis never cleared, so the widget can keep showing the override/selected model even though execution continued on the default session model. Can we either clearcurrentUnitModelon restore failure or fall back tocmdCtx.model` after session creation? I’d also like a behavioral test for that path, since the current test only checks source ordering and wouldn’t catch this regression.

When pi.setModel returns false after newSession(), the unit continues
on the session default model but currentUnitModel was left pointing at
the override/selected model. This caused the progress widget to display
a model the unit was not actually running on.

Fix: null currentUnitModel in the restore-failed branch so the widget
falls back to cmdCtx.model (the actual session model).

Add behavioral regression test: verifies currentUnitModel is null after
a failed restore, which the prior source-ordering assertion did not cover.

Closes gsd-build#3418
@atikin19sh atikin19sh requested a review from Solvely-Colin April 3, 2026 22:03
@jeremymcs
Copy link
Copy Markdown
Collaborator

Friendly nudge — this PR has outstanding change requests that haven't been addressed yet. Please update when you get a chance so review can continue.

🤖 Automated PR audit — 2026-04-04

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working High Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants