Merge api keys - #4
Merged
Merged
Conversation
…eem to work from the parent frame
…having to urlencode in the frontend)
lukemarsden
added a commit
that referenced
this pull request
Nov 14, 2025
PROVEN FACTS (from core dump + source analysis): Thread Flow: 1. Thread 99 = HTTPS server (wolf.cpp:187, port 47984) 2. Processing /cancel endpoint (endpoints::https::cancel) 3. Fires StopStreamEvent SYNCHRONOUSLY (event_bus.hpp:171) 4. Handler calls gst_element_send_event FROM HTTPS THREAD 5. GStreamer recursively traverses pipeline (frames #7→#5) 6. Blocks on mutex 0x70537c0062b0 in libgstbase-1.0.so.0 7. Thread 40 (audio pipeline owner) is HEALTHY in ppoll 8. Only Thread 99 waiting on this mutex - no contention GStreamer Analysis: - gst_element_send_event IS thread-safe (uses recursive STATE_LOCK) - Documented as "MT safe" - can be called from any thread - But empirically CAUSES DEADLOCK when called from HTTPS thread - GStreamer has both recursive (STATE/PAD) and NON-recursive (live_lock) mutexes The Mystery: - WHO holds mutex 0x70537c0062b0? NOT Thread 40, not any visible thread - Options: abandoned by crashed thread, corrupted, or race condition - Cannot prove exact mechanism without debugging symbols CONFIRMED FIXES: 1. HTTPS connection leak (100% certain) - add close() in error handler 2. Replace gst_element_send_event with g_main_loop_quit (80% confidence) - Eliminates cross-thread pipeline calls - g_main_loop_quit IS thread-safe (documented) - Even though gst_element_send_event claims to be safe, empirically fails Gaps in Evidence: - No debug symbols for libgstbase (can't see frame #4 function) - Core dump partially corrupted - Can't identify mutex owner - Need symbols + reproduction to prove exact mechanism
lukemarsden
added a commit
that referenced
this pull request
Mar 18, 2026
Spec-Ref: helix-specs@792cfa369:001588_read-helixs-design2026
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
lukemarsden
added a commit
that referenced
this pull request
Apr 30, 2026
Three pre-baked profiles for the customer's actual deployment, each sized to its hardware and using the current best open-weights models (April 2026 — DeepSeek-V4-Pro, GLM-4.7-Flash, Qwen3.6-35B-A3B, Qwen3.5-27B). Verified composeparse handles each unchanged. Profiles: - design/sample-profiles/customer-node1-4xA100.yaml 4× A100 80GB. 4 services on GPUs 0-2 (qwen3 embeddings sharing GPU 0, GLM-4.7-Flash 31B on GPU 1, Qwen3.6-35B-A3B MoE on GPU 2). GPU 3 reserved for desktops via Decision 15. A100 has no NVENC so video encoding falls back to libx264 software — fine for 1-2 sessions (verified live in cloud GPU campaign run #4). composeparse output: 4 services, GPUCount=3 → 4-GPU host has 1 GPU of explicit headroom. - design/sample-profiles/customer-node2to4-4xL40S.yaml 4× L40S 48GB. Same 4-services-on-3-GPUs shape as Node 1; sized for L40S's smaller VRAM (Qwen3.5-27B + Qwen3.6-35B-A3B FP8 fit single cards). Deployed identically to all three nodes (2, 3, 4) — the inference router round-robins. L40S has full NVENC + display engine → hardware-accelerated desktops on GPU 3. - design/sample-profiles/customer-node5-8xMI300X.yaml 8× MI300X 192GB = 1.5 TiB total VRAM. Runs DeepSeek-V4-Pro 862B FP8 with TP=8 across all 8 cards via rocm/vllm. **Inference-only** — MI300X CDNA-3 is compute-only, can't render desktops (Mesa radeonsi refuses graphics context — verified live in cloud GPU campaign run #5). The rocm/vllm image needs explicit `entrypoint: ["vllm", "serve"]` because unlike vllm/vllm-openai its default entrypoint is /bin/bash (also verified live). Wiring across the 4 surfaces: - api/pkg/runner/composeparse/sample_profiles_test.go — locked in: customer-node1 (4 services, 3 GPUs), customer-node2to4 (4, 3), customer-node5 (1 service, 8 GPUs). Future parser changes that break any of these will fail tests. - design/sample-profiles/README.md — table updated with all three plus a new section explaining the per-node deployment. - frontend/src/components/dashboard/profileBlocks.ts — three new curated entries in the Profile Gallery: "Customer Node 1 — 4×A100 80GB", "Customer Nodes 2-4 — 4×L40S 48GB (each)", and "Customer Node 5 — 8×MI300X big-iron (inference-only)". Each card has accurate pros/cons including the desktop-headroom story per node. - integration-test/gpucloud/matrix.yaml — the existing disabled node1-a100-4x / node2-l40s-4x / node3-l40s-4x / node4-l40s-4x / node5-mi300x-8x entries now point at the new richer customer-nodeN-... profiles instead of the generic placeholders they had before. UI flexibility audit (separate question from the user): the compose YAML field in EditRunnerProfile.tsx is a plain `<TextField multiline minRows={20}>` textarea — smart users have full flexibility to define arbitrary services with any Docker image (incl. custom builds + private registries), any env vars, any CLI args, any GPU pinning. Validation is server-side via composeparse on save (parses the YAML, extracts model list + GPU count) — there's no client-side allowlist or schema enforcement. Test results: TestParse_SampleProfiles green for all 9 profiles including the new 3. Frontend builds clean (39s). Harness dry-run shows the new entries are correctly disabled (so accidental cloud spend is impossible without flipping enabled: true). Spec-Ref: helix-specs@ac4cc3643:001959_we-need-to-replace-all
lukemarsden
added a commit
that referenced
this pull request
Apr 30, 2026
A100 has no NVENC and no display engine — the curated profile and test comments described "GPU 3 reserved for desktops" without flagging that desktops there are CPU-encoded via libx264 (verified live in run #4 of the cloud GPU campaign). L40S, by contrast, has full NVENC + display engine. - Test comments + curated description now lead with the software-encoded caveat so operators don't pick A100 expecting hardware desktop streaming. - Profile editor now links the NVIDIA Video Encode/Decode Support Matrix where vendor metadata is filled in, so operators can self- serve the per-SKU NVENC/NVDEC question. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Spec-Ref: helix-specs@ac4cc3643:001959_we-need-to-replace-all
lukemarsden
added a commit
that referenced
this pull request
May 15, 2026
Spec-Ref: helix-specs@969a88880:002021_investigate-notion
chocobar
added a commit
that referenced
this pull request
Jun 15, 2026
chocobar
added a commit
that referenced
this pull request
Jun 15, 2026
…review-3) Second-pass review caught a regression my own fix introduced. The previous commit (16cbaff) tried to be lenient about leading slashes by using strings.Trim (both sides) - "/ghcr.io" normalised to "ghcr.io" on the Go side and was accepted as valid. But the shell consumer (sandbox/04-start-dockerd.sh:231) does NOT normalise. It runs `sed "s|^[^/]*/|${HELIX_SANDBOX_REGISTRY}/|"` which substitutes the value verbatim. So "/ghcr.io" produced: Go side: "ghcr.io/helixml/helix-sandbox:tag" (works) Shell side: "/ghcr.io/helixml/helix-ubuntu:tag" (docker rejects) Exactly the cross-consumer divergence the FIRST ultrareview was about. My "lenient leading slash" call was wrong: silently normalising on one side while the other rejects creates a worse failure mode (succeeds at boot, fails hours later in a sandbox container). Fix: - Reject leading slash explicitly with an actionable error pointing at "likely a templating leak (/${REGISTRY_HOST})". - Reorder checks so internal-whitespace fires FIRST (most fundamental corruption), then URL, then leading-slash, then embedded-path. Adjacent typos now produce distinct diagnostics in the right order. - Drop strings.Trim (both sides) in favour of strings.TrimRight (trailing slash still tolerated; leading slash now goes through the explicit rejection branch). Tests: - Removed the two "/foo normalises" happy-path cases (they codified the regression by asserting the wrong behaviour). - Added three leading-slash rejection cases including the "/${REGISTRY_HOST}" templating-leak shape. - Added an iterated-whitespace+slash case "/ ghcr.io / " that confirms the whitespace check fires first. Also fixes config.go docstring drift (finding #4): the docstring no longer mentions the rejection rules. Restored the explicit list of rejected shapes including the double-org failure mode (the original text that the first fixup lost in a rewrite). Findings 5 (error precedence): addressed by the reorder. The whitespace check now fires first for "mirror.corp helixml"-shape typos.
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 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.
No description provided.