Website - #1
Closed
lukemarsden wants to merge 24 commits into
Closed
Conversation
… how to patch the python code for sd-scripts
lukemarsden
added a commit
that referenced
this pull request
Sep 17, 2025
CRITICAL: Fix PIN promise destruction causing pairing failures at stage #1 - Remove premature event handler unregistration in HTTP server startup - Add comprehensive debug logging to trace event handler lifecycle - ROOT CAUSE: Event handler was being unregistered immediately after server start - FIX: Keep PairSignal event handler registered for server lifetime 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 8, 2025
**Issue #1-3: WolfLobbyID Handling** - Add WolfLobbyID to SessionMetadata (was missing) - Save WolfLobbyID when creating external agent session - Fix token response to return lobby ID instead of PIN **Issue #5: Moonlight Credentials** - Add api.credentials = 'helix' in MoonlightStreamViewer - Matches moonlight-web-config/config.json setting **Documentation**: - docs/STREAMING_ISSUES_FOUND.md - Complete review findings - 12 issues documented (3 critical fixed, 2 need action, 7 minor/future) Remaining: Wolf host pairing needed before streaming works 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 10, 2025
FINAL WORKING SOLUTION after 8 attempts: Fix #1: Duplicate pause guard (Wolf) - Prevents multiple EOS events - Session count stays correct - CONFIRMED working in logs Fix #2: Prevent auto-leave on pause (Wolf + Helix) - Lobbies don't auto-leave when Wolf-UI pauses - Wolf-UI session stays connected to lobby even when disconnected - Lobby never becomes empty - No stale buffer accumulation - Agents keep running Test pattern: 1→2→3→1 should now work without rejoin hang! 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 16, 2025
CRITICAL: Fix PIN promise destruction causing pairing failures at stage #1 - Remove premature event handler unregistration in HTTP server startup - Add comprehensive debug logging to trace event handler lifecycle - ROOT CAUSE: Event handler was being unregistered immediately after server start - FIX: Keep PairSignal event handler registered for server lifetime 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 16, 2025
**Issue #1-3: WolfLobbyID Handling** - Add WolfLobbyID to SessionMetadata (was missing) - Save WolfLobbyID when creating external agent session - Fix token response to return lobby ID instead of PIN **Issue #5: Moonlight Credentials** - Add api.credentials = 'helix' in MoonlightStreamViewer - Matches moonlight-web-config/config.json setting **Documentation**: - docs/STREAMING_ISSUES_FOUND.md - Complete review findings - 12 issues documented (3 critical fixed, 2 need action, 7 minor/future) Remaining: Wolf host pairing needed before streaming works 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 16, 2025
FINAL WORKING SOLUTION after 8 attempts: Fix #1: Duplicate pause guard (Wolf) - Prevents multiple EOS events - Session count stays correct - CONFIRMED working in logs Fix #2: Prevent auto-leave on pause (Wolf + Helix) - Lobbies don't auto-leave when Wolf-UI pauses - Wolf-UI session stays connected to lobby even when disconnected - Lobby never becomes empty - No stale buffer accumulation - Agents keep running Test pattern: 1→2→3→1 should now work without rejoin hang! 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Oct 29, 2025
New rule at top of list: Everything we ship must work on fresh installs without manual intervention. NEVER rely on dev-only setup steps or manually copied files. Build all required files into container images. This was triggered by Wolf template issue - init script looked for template that didn't exist in production, causing infinite loop on fresh installs.
lukemarsden
added a commit
that referenced
this pull request
Nov 14, 2025
THREE CRITICAL BUGS found causing HTTPS deadlock within 16 hours: BUG #1: GStreamer Thread-Safety Violation (PRIMARY ROOT CAUSE) - gst_element_send_event() called from HTTPS thread (wrong context!) - Must be called from pipeline's g_main_loop_run() thread - HTTPS thread blocks on GStreamer internal mutex (0x70537c0062b0) - Located in streaming.cpp:124, 132, 176, 184, 401, 524 - FIX: Use g_main_loop_quit() instead (thread-safe) BUG #2: NVIDIA Driver Mutex Deadlock (SECONDARY) - Multiple GStreamer pipelines compete for NVIDIA mutex (0x705580003b80) - Circular deadlock: HTTPS→GStreamer→NVIDIA→? - Core dump shows 2 threads stuck on same NVIDIA mutex - Inside proprietary libEGL_nvidia.so.0 (no symbols) - FIX: Separate CUDA contexts per pipeline OR remove NVIDIA from SSL BUG #3: HTTPS Connection Leak (CONTRIBUTING FACTOR) - custom-https.cpp error handler doesn't close sockets - 17 leaked connections in 16 hours (~1/hour leak rate!) - Connections stuck in CLOSE_WAIT forever - From: external browsers, moonlight-web, localhost - FIX: Add socket->close() in error handler COMPLETE DEADLOCK CHAIN: 1. HTTPS request fires StopStreamEvent (endpoints.hpp:484) 2. Event handler runs in HTTPS thread (synchronous dispatch) 3. Calls gst_element_send_event() - WRONG THREAD (Bug #1) 4. Blocks on GStreamer mutex 5. GStreamer holds mutex, waiting on NVIDIA 6. NVIDIA mutex held by another operation 7. ALL new HTTPS requests block on continue_lock() 8. System appears completely hung for HTTPS EVIDENCE: - HTTP (port 47989) still works perfectly - HTTPS (port 47984) completely hung - Core dump shows exact mutex addresses and call stacks - 17 leaked CLOSE_WAIT connections - Thread 99 stuck in gst_element_send_event from wrong context CRITICAL FIX: Replace all gst_element_send_event(eos) with g_main_loop_quit() in event handlers at streaming.cpp:124,132,176,184,401,524
lukemarsden
added a commit
that referenced
this pull request
Nov 19, 2025
BUG #1: Inconsistent NVIDIA runtime detection pattern - Line 811 used 'grep -i nvidia' (too broad, matches image names) - Line 779 used 'grep -i "runtimes.*nvidia"' (correct, matches runtime only) - Fixed line 811 to use consistent pattern - Prevents false positives when nvidia/cuda images are present BUG #2: Race condition after Docker installation - Docker daemon takes 1-3 seconds to initialize after systemctl start - check_docker_sudo() was called immediately, could fail if daemon not ready - Added 30-second wait loop checking 'docker ps' readiness - Applied to both Ubuntu/Debian and Fedora installation paths - Prevents intermittent failures: "Docker is not running" after fresh install Both fixes are defensive and prevent edge cases without changing behavior for correctly configured systems.
3 tasks
chocobar
added a commit
that referenced
this pull request
Feb 11, 2026
…ation - Fix silently swallowed Exec() error in migration (bug #1) - Fix WHERE condition: LENGTH(name) > 255 instead of OCTET_LENGTH > 2704 (bug #2) - Add Go-level name truncation in CreateSession, UpdateSession, UpdateSessionMeta, and UpdateSessionName to prevent cryptic GORM errors - Add 6 unit tests covering truncation for ASCII, multibyte (CJK), and boundary cases across all session name write paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lukemarsden
added a commit
that referenced
this pull request
Mar 18, 2026
Issue #1 (stuck "Starting Desktop"): - Add defer in StartDesktop to clear external_agent_status on any error - Give waitForDesktopBridge its own 90s context decoupled from dockerCtx Issue #4 (status not cleared on stop): - StopDesktop unconditionally clears external_agent_status and status_message Issue #5 (no restart button in Starting state): - Frontend: show Stop button in "starting" state in both screenshot and stream modes - Show "may have failed to start" message after 2-minute timeout Issue #10a (duplicate sessions per spectask): - Re-read task from DB before CreateSession; skip if PlanningSessionID already set Issue #10b (scanner targets wrong sessions): - processPendingPromptsForIdleSessions now filters to canonical planning_session_id only Issue #2 (duplicate message sends): - Add ClaimPromptForSending() atomic store method (UPDATE WHERE status IN pending/failed) - Both interrupt and any-pending delivery paths use claim before send Issue #7 (promotion race gives empty zvol): - resolveDockerDataDir: acquire read lock before fresh zvol creation; re-check after Issue #3: Already handled by existing open_thread on agent_ready reconnect Issue #6: Fixed in merged PR #1947 (RecoverStaleBuilds 60s retry) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Spec-Ref: helix-specs@04b515c3c:001588_read-helixs-design2026
chocobar
pushed a commit
that referenced
this pull request
Apr 22, 2026
The Design Review UI made two sequential API calls on "Approve Design": 1. submitReviewMutation (marks review record approved) 2. v1SpecTasksApproveSpecsCreate (approves the spec task) If #1 succeeded but #2 failed, the review showed "approved" but the spec task stayed in spec_review with SpecApproval == nil — creating the inconsistent state that led to the infinite loop. Fix: move spec task approval into submitDesignReview's "approve" case (matching the existing pattern where "request_changes" already updates the spec task). Remove the redundant second API call from the frontend. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Spec-Ref: helix-specs@560941003:001869_bug-report-spec-tasks
11 tasks
4 tasks
lukemarsden
added a commit
that referenced
this pull request
May 13, 2026
When Helix changes the hardcoded definition of a Helix-owned context_server (e.g. PR #2418 switched chrome-devtools from `npx chrome-devtools-mcp@latest` to `/usr/bin/chrome-devtools-mcp`), the daemon's deep-merge in mergeSettings was treating the on-disk OLD entry as a "user override" and letting it win. This pinned the broken `npx` config in long-running containers' persisted settings.json forever and re-produced the 180s `chrome-devtools context server failed to start: Context server request timeout` errors that PR #2418 was meant to fix — even on containers running the new image and new API binary. Bug observed in https://meta.helix.ml/orgs/helix/projects/prj_01kg02vqqyg178c1n2ydscn5fb/tasks/spt_01kqc4ev5rt9rknk6g8dbkzj9a shortly after PR #2418 merged: chrome-devtools / drone-ci / github all showed "Context server request timeout" in Zed, despite the container running helix-ubuntu:6de75e (built post-merge with the new global MCP binaries) and helix-api running the new zed_config.go. Fix: introduce HELIX_OWNED_CONTEXT_SERVERS = {chrome-devtools, helix-session, helix-desktop} — the set of context_server names hardcoded in api/pkg/external-agent/zed_config.go. Two corresponding behavior changes in api/cmd/settings-sync-daemon/main.go: 1. mergeSettings: skip user-side context_server entries whose name is in HELIX_OWNED_CONTEXT_SERVERS so Helix's hardcoded definition unconditionally wins. Also strip helix-owned names from the "user-only" branch so a stale on-disk entry can't survive even when the API temporarily emits no context_servers. 2. extractUserOverrides: never capture helix-owned names as user overrides (otherwise the stale entry would round-trip back to the API and force the next sync to re-write the OLD value to disk, permanently nullifying the force-overwrite from #1). User-configured MCPs (e.g. drone-ci, github, custom servers from project skills or app config) are NOT in the helix-owned set — those legitimately can be edited by the user in their on-disk settings.json and must round-trip. Tests: - TestMergeSettings_HelixOwnedContextServersWin (4 sub-tests): force-overwrite chrome-devtools and helix-session when user has stale entries, allow user-configured drone-ci to win, strip helix-owned names even when helix has no servers. - TestExtractUserOverrides_SkipsHelixOwnedContextServers (2 sub-tests): stale on-disk helix-owned entries are not captured as user overrides; non-helix user overrides still round-trip. All sub-tests verified to FAIL when both guards are commented out (by replacing `if HELIX_OWNED_CONTEXT_SERVERS[name] {` with `if false && HELIX_OWNED_CONTEXT_SERVERS[name] {` and re-running). Full diagnosis: design/2026-05-13-mcp-cache-contention-and-duplicate-claude-spawn.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chocobar
added a commit
that referenced
this pull request
May 29, 2026
Reported on SaaS: activating a trial via the admin dashboard with "0 credits" produced a $200 wallet balance. Two bugs stacked: 1. admin_trial_handlers.go:171 silently overrode body.Credits <= 0 to defaultTrialCredits (100). Admin types 0, backend writes 100. 2. The Stripe trial subscription's first invoice.paid event still fires handleInvoicePaymentPaidEvent, which credits the wallet by product.metadata.credits (also 100 on SaaS). Stacks with #1 -> $200. This PR addresses (1) only: form credit value is sent verbatim to the backend, with no defaulting in either direction. The Stripe webhook layer is the standard "subscription gives monthly credits" mechanic and is left alone -- if we want to suppress that too, it's a separate conversation. Changes: - Backend default of body.Credits <= 0 -> 100 removed. 0 stays 0. - Backend now rejects negative credits with 400 (previously they were also silently overridden to 100). - defaultTrialCredits constant deleted (unused). - Swagger doc updated to describe the new semantics. - Frontend DEFAULT_CREDITS dialog default lowered from 100 to 0 so admins aren't prefilled with a value they didn't choose. Out of scope: - Disabling the Stripe trial invoice.paid -> wallet credit. The product.metadata.credits flow still adds the subscription's standard monthly allotment on trial start; separate decision.
This was referenced Jun 9, 2026
chocobar
added a commit
that referenced
this pull request
Jun 15, 2026
chocobar
added a commit
that referenced
this pull request
Jun 15, 2026
…t, simpler demand)
Second-pass review caught 7 CONFIRMED bugs that my first ultrareview
fixup introduced. Most centered on tightening isReady to require
Status=online, which cascaded into multiple new failure modes. Plus
an indefinite Deprovision retry storm and a SpecTemplate dependency
that always read the fallback value.
Split isReady into two predicates (the root fix for most issues):
- isReadyState (broad: ComputeState=Ready, any Status). Used by
D4 candidate selection and idleSince pruning. Tolerates
heartbeat flap; lets Ready+offline orphans be reclaimed.
- isReadyAndOnline (narrow: Ready AND online). Used by D3 capacity
and demand math. Excludes phantom offline capacity.
Cascaded fixes:
1. Heartbeat-flap disables D3 (CONFIRMED)
Original: `readyCount >= Floor` where readyCount required online.
One host briefly offline -> readyCount drops -> D3 disabled.
Fix: gate D3 on `readyOnlineCount > 0` (any reachable host gives
demand signal). Independent of Floor satisfaction; survives flap.
2. Ready+offline orphan rot (CONFIRMED)
Crashed-host rows accumulated forever - excluded from D3 headroom
AND D4 candidates, no other reclaim path.
Fix: D4 candidate set uses isReadyState (broad). Offline+idle
rows shed naturally. Deprovision may fail (upstream gone); the
bounded-retry below caps the noise.
3. Idle timer reset on heartbeat flap (CONFIRMED)
prune loop dropped idleSince entries when row left ready-and-online
set. Flap -> entry deleted -> next cycle re-arms idle-since at
NOW -> chronic-idle host never crosses IdleTimeout.
Fix: prune loop uses isReadyState. Flap preserves accumulated time.
4. SpecTemplate-based ceil-div always used fallback 20 (CONFIRMED)
bootstrap doesn't populate SpecTemplate (always zero in production),
so defaultMaxSandboxes returned 20 always - under-provisioned 4x
on smaller-capacity hosts.
Fix: simplified demandNeed to
min(MaxConcurrentProvisions, max(1, slotsShort))
No SpecTemplate dependency. Slot-shortage bounded by what
MaxConcurrentProvisions allows.
5. Floor=0 cold-boot still over-provisioned (CONFIRMED)
readyCount(0) >= Floor(0) was trivially true -> D3 fired at boot
with no demand signal.
Fix: same as #1 - gate on readyOnlineCount > 0. Floor=0 with no
Ready hosts means no signal; D3 stays quiet. Operators wanting
true cold-start scale-on-request need either Floor>=1 or
event-driven provisioning (future work).
6. Permanent Deprovision-retry infinite loop (CONFIRMED, critical)
Ultrareview-1 fixup's orphan-prevention swung too far the other
way: indefinite retry on a permanently-broken upstream (404 on
retry, IAM revoked, region offline) created a stuck phantom
Ready row forever, suppressing legitimate scale-up.
Fix: maxDeprovisionRetryAge (15m) bounds the retry budget.
deprovisionFailingSince map tracks first-failed time per
candidate. Within budget, retry. Past budget, give up: log at
error level (with provider_id for manual cleanup), Deregister
the row, move on. Better to acknowledge a possible upstream
orphan than to wedge the Helix-side row forever.
Side cleanups:
- deprovisionFailingSince is pruned alongside idleSince so the
tracker doesn't leak entries for hosts that left Ready state
while a retry was in flight.
- The bounded-retry comment supersedes the earlier "rollbackStuckRow
is the safety net" comment - clarifies that D4 owns its own
cleanup, not the rollback path.
Tests added (4 new):
- TestD4BoundedDeprovisionRetryEventuallyGivesUp
- TestD3SurvivesHeartbeatFlap
- TestD4ShedsReadyOfflineOrphans
- TestD4IdleTimerSurvivesHeartbeatFlap
Updated test:
- TestD3BatchesDemandNeedUpToMaxConcurrentProvisions now expects
4 hosts (min of MaxConcurrentProvisions=4 and slotsShort=20)
rather than 2 (the previous ceil-div-by-SpecTemplate result).
chocobar
added a commit
that referenced
this pull request
Jun 15, 2026
…loor=0)
Second-pass review caught three CONFIRMED bugs in the previous D3
fixup, all in computeNeeded.
1. Heartbeat-flap disables D3 (CONFIRMED)
The previous gate `readyCount >= Floor` required Status=online.
A single Floor-host briefly flickering offline -> readyCount
drops below Floor -> D3 silently disables, exactly when scale-up
is most needed.
Fix: gate on `readyOnlineCount > 0`. As long as ANY reachable
host exists, demand pressure can fire. Floor satisfaction is
irrelevant to D3's "is there spare capacity right now" check.
2. SpecTemplate-based ceil-div always used fallback 20 (CONFIRMED)
bootstrap doesn't populate cfg.SpecTemplate (it's zero-valued
in production), so defaultMaxSandboxes returned the fallback 20
on every call. On hosts with MaxSandboxes=5, demandNeed
under-provisioned 4x: needed 4 hosts to cover a 20-slot deficit,
computed 1.
Fix: simplified demandNeed to
min(MaxConcurrentProvisions, max(1, slotsShort))
No SpecTemplate dependency. Slot-shortage bounded by what the
per-cycle provision cap allows; the outer Max-room check still
caps total owned.
3. Floor=0 cold-boot still over-provisioned (CONFIRMED)
`readyCount(0) >= Floor(0)` was trivially true -> D3 fired at
boot with no demand signal.
Fix: same as #1 - gate on `readyOnlineCount > 0`. Floor=0 with
no Ready hosts means no signal; D3 stays quiet. Operators
wanting "true cold-start scale on first request" need either
Floor>=1 or an event-driven provisioning path (future work).
Tests:
- TestD3BatchesDemandNeedUpToMaxConcurrentProvisions updated to
expect 4 hosts (min(MaxConcurrentProvisions=4, slotsShort=20))
instead of 2 (the previous ceil-by-SpecTemplate result, which
was 4x under-provisioning in production).
- TestD3SurvivesHeartbeatFlap added: 2 Floor hosts, one online
one offline, demand for more slots; verifies D3 continues to
provision rather than disabling on the flap.
- seedReadyRowOffline helper added.
chocobar
added a commit
that referenced
this pull request
Jul 10, 2026
…edup, self-heal) Implements the remaining project.go review findings in the org runtime layer (no shared git-schema change), each covered by a unit test: - Orphan cleanup (#2): if AttachRepoToProject fails after CreateGitRepo, the just-created repo is deleted so a retry doesn't leak it (and doesn't create `<worker>-2` beside the orphan). New DeleteGitRepo on ProjectService. - Cross-process race dedup (#1): CreateGitRepo auto-increments the name on collision rather than erroring; a returned name != requested means another replica (which repoEnsureMu can't serialise) won the create race. Delete the duplicate and error so the caller retries instead of silently keeping `<worker>-2`. - Deleted-repo self-heal (#4): the fast path and the ensureWorkerRepo re-check now validate the repo still exists (new GetGitRepo + ErrRepoNotFound); a DefaultRepoID/state repo deleted out-of-band is re-provisioned instead of handed back dead. Transient (non-not-found) read errors never trigger a recreate, to avoid duplicates. ProjectService gains GetGitRepo / DeleteGitRepo, wired in the in-proc adapter to the existing get/delete git-repository handlers. Test fakes updated.
This was referenced Aug 13, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
NOTE: must delete keycloak db otherwise everything will be fucked