Skip to content

fix(canvas): persist model on Save+Restart for runtime-bearing workspaces - #2426

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/canvas-model-save-runtime-config
May 1, 2026
Merged

HongmingWang-Rabbit merged 1 commit into
stagingfrom
fix/canvas-model-save-runtime-config

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

The Model dropdown writes to config.runtime_config.model whenever a runtime is selected (hermes, claude-code, etc.), but handleSave was only diffing against top-level config.model — so for any runtime-bearing workspace, PUT /workspaces/:id/model never fired and MODEL_PROVIDER never landed in workspace_secrets.

Symptom (2026-04-30, hongmingwang Hermes Agent)

User reports: "config is not saving, because each time i save and restart, later I bump into same default config".

Trace:

  1. Pick minimax/MiniMax-M2.7-highspeed from the dropdown
  2. Click Save & Restart
  3. Save reports success; restart fires
  4. New EC2 boots with HERMES_DEFAULT_MODEL empty (because MODEL_PROVIDER was never persisted)
  5. install.sh falls back to nousresearch/hermes-4-70b
  6. hermes-agent errors "No LLM provider configured" on every chat turn

Fix

handleSave now reads the effective model from runtime_config.model first and falls back to top-level model for legacy no-runtime workspaces. Same logic for the old-value diff so a no-op save still skips the PUT.

Tests

ConfigTab.hermes.test.tsx — 2 new tests:

  • PUTs /model when the dropdown changed runtime_config.model on a hermes workspace
  • Does NOT PUT when the value is unchanged from GET /model

Test plan

  • npx vitest run src/components/tabs/__tests__/ConfigTab.hermes.test.tsx — 7/7 pass
  • After deploy: hongmingwang user picks minimax model, Save&Restart, hermes chat returns LLM response (not "No LLM provider configured")

🤖 Generated with Claude Code

…aces

The Model dropdown's onChange writes to config.runtime_config.model
whenever a runtime is set (hermes, claude-code, etc.), and only
falls back to top-level config.model when no runtime is selected.
But handleSave used to diff the new value against top-level
nextSource.model only — so for any runtime-bearing workspace, the
PUT /workspaces/:id/model never fired and MODEL_PROVIDER never
landed in workspace_secrets.

Symptom (2026-04-30, hongmingwang Hermes Agent
32993ee7-840e-4c02-8ca8-cb9d75d112a5):
  - User picks minimax/MiniMax-M2.7-highspeed from the dropdown
  - Hits Save & Restart
  - Save reports success; restart fires
  - The new EC2 boots with HERMES_DEFAULT_MODEL empty
  - install.sh defaults to nousresearch/hermes-4-70b
  - hermes-agent errors "No LLM provider configured" on every chat
    turn because no NOUS_API_KEY / OPENROUTER_API_KEY is set
  - Reload Config tab → model field reverts to whatever
    GET /workspaces/:id/model returns (i.e. empty / template default)

handleSave now reads the effective model from runtime_config.model
first and falls back to top-level model for legacy no-runtime
workspaces. Same change for the old-value diff so a no-op Save
still skips the PUT.

Tests pin both branches: PUTs /model when the dropdown changed
runtime_config.model on a hermes workspace; does NOT PUT when
the value is unchanged from what GET /model returned.
@HongmingWang-Rabbit
HongmingWang-Rabbit added this pull request to the merge queue May 1, 2026
Merged via the queue into staging with commit 6cca4c5 May 1, 2026
20 checks passed
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/canvas-model-save-runtime-config branch May 1, 2026 01:46
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
…an explicit waitFor before asserting mockRFBConstructor arguments.\nThe noVNC client is loaded via dynamic import inside a useEffect, so in\nCI the assertion could race ahead of the async init and fail with\n'Number of calls: 0'. This is the flake blocking CI/all-required on #2426.\n\nCo-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
Serialized merge by gitea-merge-queue after current-main, genuine approvals, and required CI checks were green.
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
The status-pagination bug (RCA, #2440-family): merge/verify status readers
fetched only the FIRST page of a commit's statuses. On high-churn PRs Gitea
caps the combined GET /commits/{sha}/status `statuses` array at the default
page size (~30) and pushes older-but-still-current required-context rows past
it. A reader of that truncated view records the required context as ABSENT
(missing) even though its current SUCCESS row exists — wrongly blocking, or
mis-reading the gate. Confirmed on #2448/#2426/#2438/#2331/#2259/#2055/#2032
(reviewers had to manually paginate to verify gates this whole session). Live
proof on PR #2331 head: combined /status returns 30 rows; exhaustive
/statuses returns 50 rows across 20 distinct contexts.

Two verify-by-state readers consumed that capped combined view for
required-context decisions and are fixed here to page the dedicated
/commits/{sha}/statuses list to EXHAUSTION (until a short/empty page), then
collapse to newest-row-per-context:

- prod-auto-deploy.py (wait-ci gate): replaced the single combined /status
  fetch with fetch_all_statuses() (paginated). A required context past page 1
  no longer reads "missing" forever and times out a legitimate prod deploy.
  latest_status_for_context now selects newest-by-id so the oldest-first
  /statuses ordering can't let a stale run shadow the current one.
- audit-force-merge.sh: replaced the single combined /status fetch with a
  page loop over /commits/{sha}/statuses, accumulating all rows before the
  newest-wins CHECK_STATE collapse. A required SUCCESS past the cap no longer
  reads "missing" and emits a false-positive incident.force_merge.

gitea-merge-queue.py already paginates /statuses to exhaustion
(get_combined_status + api_paginated) — left unchanged; it is the reference
behavior this change brings the other two readers in line with.

STRENGTHENING ONLY — fail-closed preserved, NO fail-open path introduced:
- prod-auto-deploy: a genuinely-absent required context appears on NO page,
  so ci_context_state() still returns "missing", context_is_satisfied()
  rejects it, and the gate never greens (times out). Any page that errors or
  is not a list raises (fetch_all_statuses/_api_json_list) — a partial list
  never passes as complete.
- audit-force-merge: any non-200 page or non-array body aborts with exit 1;
  an absent required context has no CHECK_STATE entry so `${...:-missing}`
  keeps it not-green and the audit still fires.

Tests (mutation-resistant): added regressions that (a) place a required
SUCCESS on page 2+ behind a full page of churn and assert the reader FINDS it,
and (b) make a required context genuinely absent on all pages and assert the
reader STILL fail-closes (missing/never-satisfied → blocks/times out). Mocks
the paginated HTTP responses. Also locks newest-wins collapse, short-page
stop, full-page continue, and page-error propagation.

Refs: status-pagination RCA, #2440-family.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant