perf: optimize Linux builds with Docker container (GitHub-hosted)#293
Closed
namastex888 wants to merge 66 commits into
Closed
perf: optimize Linux builds with Docker container (GitHub-hosted)#293namastex888 wants to merge 66 commits into
namastex888 wants to merge 66 commits into
Conversation
We currently have columns named "Archive" and "Cancelled" however tasks that are "Stopped" stays in Review instead of coing to cancelled. Also the Cancelled Icon should be the one being currently used in Archived column And the dot colourc in canceleld should be red (as is in Archived currently) AND archived Column should be some sort of grayish colour (the dot) AND have the icon of an small archive, the same icon we have when we hover the task card and we can actually archive shit
Adds ability to develop with local forge-core for debugging: - make dev-core: clones forge-core, enables path override, starts dev - make dev-core-off: switches back to git dependencies - Cargo config split into base (platform) and dev-core (with patch) - forge-core/ and .cargo/config.toml are gitignored This enables seamless debugging when forge-core updates break the app.
When user needs to work with forge-core (debugging, testing changes, understanding crates), instruct them to use make dev-core. Adds 12 new learnings (learn-194 through learn-205): - Recognition triggers for forge-core work - Response pattern with instructions - How it works technically - Anti-patterns to avoid Evidence: commit 7f42fa4 (make dev-core implementation)
Version sync to 0.8.4 across all packages. DX Improvements: - Makefile: BRANCH selection, true reset, cross-repo commands - dev-core safety: marker file, hooks, CI check - Scripts: pre-push hook, bump-forge-core helper - CI: check-release-intent workflow for smart RC triggers Breaking: Major version alignment from 0.7.5 to 0.8.4
Cross-repo version sync for RC testing.
Problem: Cargo [patch] sections break workspace inheritance when forge-core crates use `version.workspace = true`. Solution: Use file swapping instead of patching: - Root Cargo.toml excludes forge-app from workspace - forge-app becomes standalone with empty [workspace] - All workspace deps inlined in forge-app/Cargo.dev-core.toml - forge-extensions/config uses path dep for services Files: - Add Cargo.dev-core.toml (root, forge-app, forge-extensions/config) - Restore .cargo/config.toml (platform settings) - Delete .cargo/config.dev-core.toml (no longer needed) - Update Makefile to swap 3 files instead of patching - Add FORGE_WATCH_PATHS support to run-dev.sh
- e2e: remove sudo apt-get step (self-hosted runners don't have sudo) - android-apk: remove pull_request trigger (should only run on tags) - dead-code: delete unused slider.tsx and its dep @radix-ui/react-slider - knip: add @virtuoso.dev/message-list to ignoreDependencies (false positive)
- Makefile dev-core: use for loop to reduce code duplication - Makefile dev-core-off: use for loop with mv (more atomic than cp+rm) - run-dev.sh: use bash array for safer FORGE_WATCH_PATHS handling
- Increase Playwright webServer timeout from 120s to 600s (10 min) to accommodate Rust compilation on first run - Add SKIP_FRONTEND_BUILD env var support in run-dev.sh to avoid double-building frontend in CI - Change cargo clean to conditional (FORCE_CLEAN env var) to preserve incremental compilation by default - Apply cargo fmt fixes to version.rs and router.rs
- Fix API endpoint: /api/forge/tasks/create-and-start → /api/tasks/create-and-start - Fix payload structure: use nested task + executor_profile_id objects - Add use_worktree: false to register tasks as agent tasks in forge_agents table - Enable video recording for all tests (not just failures) - Upload both HTML report and test-results (videos, screenshots, traces) as artifacts
- Add tag verification with retry in bump-forge-core.sh - Add check-versions CI job to validate version consistency - Fix silent failures in Makefile dev-core targets - Add pre-flight validation for .dev-core.toml files - Create dev-core-helper.sh for comprehensive dev workflow - Create check-versions.sh for CI version validation - Create sync-versions.js for unified version management - Update forge-extensions/config in bump script
… tests
The page.on('websocket', ...) listener only captures NEW WebSocket
connections. The tests were setting up capture AFTER setupTasksView()
already navigated, missing the WebSocket connection entirely.
Fixed all 4 tests to:
1. Navigate to root first and skip onboarding
2. Get project ID via API
3. Set up WebSocket capture BEFORE navigating to tasks view
4. Then navigate to tasks view (WebSocket captured correctly)
This matches the working pattern in journey-realtime-events.spec.ts
which correctly sets up capture before navigation.
In CI, the database starts fresh with no projects. Tests were calling getFirstProject() which returned undefined, causing WebSocket to connect with project_id=undefined. Added ensureProjectExists() helper that: 1. Checks for existing projects 2. Creates a test project if none exists 3. Returns the project ID This fixes the websocket-task-filtering tests that were failing because they couldn't get a valid project ID.
Use git_repo_path and use_existing_repo fields as required by the /api/projects endpoint instead of incorrect path field.
Major refactor of dev-core workflow: 1. Cargo [patch] Migration - Use .cargo/config.toml [patch] section instead of file swapping - Simplified Makefile: 80+ lines → 50 lines - Delete obsolete .dev-core.toml files (3 files removed) - Update pre-push hook to detect active [patch] section 2. Auto Version Bump Workflow - New .github/workflows/auto-version-bump.yml - Triggers on PR merge with 'rc' or 'stable' label - 'rc' label: bump RC (0.8.4-rc.2 → 0.8.4-rc.3) - 'stable' label: promote to stable (0.8.4-rc.3 → 0.8.5) 3. Cross-Repo Sync Workflow - New .github/workflows/sync-forge-core-tag.yml - Auto-updates forge-core tag references when forge-core releases - Supports repository_dispatch and manual workflow_dispatch Benefits: - Single command experience: `make dev-core` does everything - No file swapping, uses native Cargo mechanism - Works in git worktrees (task worktrees use git deps automatically) - Label-based version control for releases
The check-dev-core CI step was falsely detecting dev-core mode because it matched commented-out [patch.] sections in .cargo/config.toml. Now filters out comment lines before checking for patches.
- Fix snapshot message detection to handle both { JsonPatch: [...] }
wrapped format and direct array format
- Add defensive handling for task creation response format variations
Remove the genie lamp widget from frontend - genie moves to separate repo.
Frontend removed:
- GenieMasterWidget (lamp button)
- Lamp icon component
- SubGenieContext (dead code)
- subGenieApi service
- Mobile genie nav tab
Backend removed:
- /api/forge/master-genie/{id}/neurons endpoint
- /api/forge/neurons/{id}/subtasks endpoint
- Neuron struct and handlers
Kept for generic agent runs:
- forge_agents table
- /api/forge/agents endpoints
Add "Use worktree" checkbox to TaskFormDialog for the "Create & Start" flow, matching the existing behavior in CreateAttemptDialog.
Resolve conflict in taskStatusMapping.ts - keep Archive icon for archived status
…ehensive docs - Add branch matching enforcement to dev-core target (prevents mismatched dev) - Enhance status command with cross-repo visibility (mode, branches, versions, changes) - Add comprehensive workflow guide at docs/DUAL_REPO_WORKFLOW.md (8-step process) - Link from DEVELOPER.md to workflow guide Implements phases 2, 3, 4 of dev-core automation plan. Addresses confusion around PR creation sequence and version sync. +512 lines of documentation and automation
Workspace improvements: - Add codex dependencies (protocol, app-server-protocol, mcp-types) from namastexlabs fork - Configure .cargo/config.toml patches for codex crates.io overrides - Add forge MCP server to .mcp.json for local task management - Add MASTER_RC_PLAN.md documenting RC release validation circuit These changes enable: - Codex integration without vendoring (git patches) - Local forge-core MCP server testing - RC release workflow documentation
- Add [workspace.package] section for forge-core crate compatibility - Add rmcp to workspace dependencies (required by forge-core/server) - Change BRANCH default to auto-detect current branch (not 'dev') - Implement smart branch sync: check if branch exists, create from dev if not - Add workspace compatibility pre-flight checks to prevent cryptic cargo errors - Add workspace-compat command to dev-core-helper.sh
The ReleaseNotesDialog was only calling modal.resolve() but not modal.hide(), which meant the promise would resolve but the modal would stay visible. This blocked E2E tests with overlay blocking pointer events. Changes: - Add modal.hide() after modal.resolve() in ReleaseNotesDialog - Improve closeReleaseNotes E2E helper with role-based selector - Add wait for WebSocket message delivery in filtering test
- Improve skipOnboarding to loop through up to 6 dialogs - Add comprehensive button regex for all onboarding steps - Add ensureNoModalOverlay helper for defensive modal cleanup - Update setupTasksView to call both helpers after navigation This addresses flaky E2E tests where multiple onboarding dialogs (DisclaimerDialog, OnboardingDialog, GitHubLoginDialog, PrivacyOptInDialog, ReleaseNotesDialog) could block test interactions.
The z-[9999] overlay from Dialog component was still blocking tests. Improved ensureNoModalOverlay helper to: - Wait for pending animations before checking - Check for both high z-index and regular overlays - Try clicking dismiss buttons before pressing Escape - Loop up to 5 attempts to dismiss stubborn modals
The CI screenshot revealed the PrivacyOptInDialog ("Help Us Build Better")
was blocking tests. Added button patterns for:
- "No thanks"
- "Yes, help improve..."
Updated both skipOnboarding and ensureNoModalOverlay helpers.
CI starts with an empty database with no projects. The setupTasksView helper was using getFirstProject which returns null when no projects exist. This caused tests to fail trying to create tasks for null project. Changed to ensureProjectExists which creates a project if none exist, ensuring tests can run in a fresh CI environment.
Use the same navigation pattern as the working websocket tests: - Navigate directly to tasks page with page.goto() - Only dismiss release notes modal (not full onboarding flow) - Add URL verification to ensure we're on tasks page - Remove redundant skipOnboarding call after navigation that may cause redirects
The E2E tests were failing because dialogs (especially PrivacyOptInDialog) can appear on any page navigation, not just the initial load. This caused modal overlays to block all test interactions. Changes: - helpers.ts: Add skipOnboarding after navigation in setupTasksView - websocket-task-filtering.spec.ts: Add skipOnboarding after navigation - journey-realtime-events.spec.ts: Add skipOnboarding after navigation and replace getFirstProject with ensureProjectExists for CI compatibility Pattern: After any page.goto() followed by waitForLoadState(), always call skipOnboarding() to dismiss any dialog that may have appeared.
Bug: Dialog modals (DisclaimerDialog, OnboardingDialog, GitHubLoginDialog, PrivacyOptInDialog, ReleaseNotesDialog) sometimes appeared more than once during a session. The validation mechanism for tracking whether a dialog has been shown was flawed. Root causes identified: 1. useEffect dependency on entire config object caused re-runs on every config update (90% confidence) 2. Component remount resets ref guard (85% confidence) 3. Loading state race condition (70% confidence) Fixes: - Add sessionStorage guard to persist onboarding completion across remounts - Use granular dependency array with specific config flags instead of entire config object - Stabilize updateAndSaveConfig callback identity using ref pattern
The loading state is used by AppContent to conditionally render routes. Setting loading=true during updateAndSaveConfig was causing all routes to unmount, which broke navigation (React Router loses current location). This fixes the navigation regression where E2E tests were stuck on the Projects page instead of the tasks view after onboarding dialog saves.
…tion The E2E helpers were pressing Escape as a fallback to close modals, but this triggered the useKeyExit hook in project-tasks.tsx which navigates back to /projects. This caused tests to fail by navigating away from the tasks view. - Remove Escape fallback from closeReleaseNotes() - rely on button clicks - Remove Escape fallback from ensureNoModalOverlay() - only use buttons - Add documentation warning about useKeyExit interaction
The Create Attempt modal shows different provider and branch values depending on the environment: - Provider: selected during onboarding (varies per environment) - Branch: based on project's actual base branch (not always 'dev') Changed assertions to check for button existence rather than specific values, making tests work across different CI/local environments.
- Add ci-test profile: inherits test, opt-level=0, reduced debug symbols - Add ci-build profile: inherits release, opt-level=2, thin LTO, codegen-units=16 - Preserves release profile unchanged (production optimization) - Enables faster CI builds with parallelization Part of Phase 1: Quick Wins for runner optimization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Standardize service vars (SERVICE_NAME, SERVICE_PORT, HEALTH_ENDPOINT, PM2_CONFIG) - Update PM2 name: 8887-forge → 8887: Forge - Add smart update detection and health check retry logic - Consistent with service standardization across all repos
The dev merge (31600cd) introduced 'needs: check-dev-core' but the check-dev-core job doesn't exist, causing: "The workflow must contain at least one job with no dependencies." Reset both workflow files to main version.
The main branch version has an 'Install build dependencies' step that uses sudo, which doesn't exist on the self-hosted runner. Restoring the branch's own workflow version that was successfully running tests (from commit 165ce4c).
…eritance fix: dev-core mode workspace inheritance issue
Problem: Self-hosted Linux runners don't have sudo installed, causing build failures with "sudo: command not found" error (exit code 127). Root Cause Analysis: - 🥇 Self-hosted runners lack sudo binary (90% confidence) - 🥈 Workflow assumed GitHub-hosted runners (90% confidence) - 🥉 Build tools should be pre-installed on runners (60% confidence) Solution: - Skip sudo apt-get install on self-hosted runners - Add verification step to check if build tools exist - Fail fast with clear error if tools missing Evidence: Applied debug spell methodology (7 hypotheses → 3 with confidence scores) Location: .github/workflows/build-all-platforms.yml:162-177 Regression Check: - Self-hosted runners: sudo step skipped, verification runs - GitHub-hosted runners: sudo install still works - Clear error if tools missing on self-hosted Related: RC release automation debugging 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add container configuration to build-all-platforms.yml to use the new high-performance forge-builder image for Linux x64 and ARM64 builds on CT 200 self-hosted runners. **Performance Target:** Reduce Linux build times from 5-12min → 2-3min Changes: - Add container: ghcr.io/namastexlabs/forge-builder:nightly-2025-05-18 to Linux build matrix entries - Skip Rust toolchain installation when using container (runner.os != 'Linux') - Skip Linux build tools installation when container provided - Skip cargo-zigbuild and Zig installation when container provided - Symlink Dockerfile.builder from forge-core (shared build image) All build tools (Rust, Node, pnpm, cargo-zigbuild, Zig) are pre-installed in the container, eliminating 3-5 minutes of setup time per build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ker container Switch Linux x64 and ARM64 builds from self-hosted runners to GitHub-hosted runners with forge-builder Docker container. Benefits: - Eliminates 3-5 minutes of Rust toolchain setup per build - More reliable (no self-hosted runner maintenance) - Consistent with forge-core configuration - Expected build time: 5-12min → 2-4min for Linux platforms Docker image: ghcr.io/namastexlabs/forge-builder:nightly-2025-05-18 Contains pre-installed: Rust toolchain, cargo-zigbuild, system dependencies
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Summary of ChangesHello @namastex888, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on significant performance improvements for Linux CI/CD builds by leveraging GitHub-hosted runners and Docker containers, leading to faster and more reliable build processes. It also introduces a sophisticated cross-repository development workflow, enhancing the developer experience for projects with shared core libraries. Concurrently, the PR refines internal agent structures with a new multi-persona advisory team, cleans up deprecated frontend code, and fortifies the E2E testing suite for increased stability and accuracy. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request is a significant step forward in optimizing the build process and developer experience. The move to a Docker-based Linux build, the introduction of CI-specific Cargo profiles, and the refined dev-core workflow are excellent improvements. The cleanup of the "Genie" agent configurations and UI components simplifies the codebase. The enhancements to the E2E testing setup, particularly in playwright.config.ts and the test helpers, will greatly improve test reliability. I have a couple of critical/high-severity comments regarding a potentially invalid Dockerfile and the robustness of the Makefile scripts, but overall this is a very strong set of changes.
Comment on lines
+757
to
+764
| @sed -i 's/^# \[patch\./[patch./g' .cargo/config.toml | ||
| @sed -i 's/^# db = /db = /g' .cargo/config.toml | ||
| @sed -i 's/^# services = /services = /g' .cargo/config.toml | ||
| @sed -i 's/^# server = /server = /g' .cargo/config.toml | ||
| @sed -i 's/^# deployment = /deployment = /g' .cargo/config.toml | ||
| @sed -i 's/^# local-deployment = /local-deployment = /g' .cargo/config.toml | ||
| @sed -i 's/^# executors = /executors = /g' .cargo/config.toml | ||
| @sed -i 's/^# utils = /utils = /g' .cargo/config.toml |
Contributor
There was a problem hiding this comment.
The sed commands here use patterns that are too broad (e.g., s/^# db = /db = /g). This could accidentally uncomment lines in other sections of .cargo/config.toml if they happen to match. To make this more robust, I recommend using more specific patterns that match the full line you intend to change. For example, for the db line, you could use a pattern like s/^# \(db = { path = "forge-core\\/crates\\/db" }\)/\1/g. This should be applied to all sed commands in this target.
Comment on lines
+785
to
+792
| @sed -i 's/^\[patch\./# [patch./g' .cargo/config.toml | ||
| @sed -i 's/^db = /# db = /g' .cargo/config.toml | ||
| @sed -i 's/^services = /# services = /g' .cargo/config.toml | ||
| @sed -i 's/^server = /# server = /g' .cargo/config.toml | ||
| @sed -i 's/^deployment = /# deployment = /g' .cargo/config.toml | ||
| @sed -i 's/^local-deployment = /# local-deployment = /g' .cargo/config.toml | ||
| @sed -i 's/^executors = /# executors = /g' .cargo/config.toml | ||
| @sed -i 's/^utils = /# utils = /g' .cargo/config.toml |
Contributor
There was a problem hiding this comment.
Similar to the dev-core target, the sed commands here use patterns that are too broad (e.g., s/^db = /# db = /g). This could accidentally comment out lines in other sections of .cargo/config.toml. To make this more robust, I recommend using more specific patterns. For example, for the db line, you could use a pattern like s/^\(db = { path = "forge-core\\/crates\\/db" }\)/# \1/g. This should be applied to all sed commands in this target.
Contributor
Author
|
Closing - this was a mistake. RC releases stay on dev branch, not merged to main. |
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.
Performance Optimization: Docker Container for Linux Builds
This PR migrates Linux builds from self-hosted runners to GitHub-hosted runners with a pre-built Docker container.
Changes
[self-hosted, Linux, X64]→ubuntu-latest[self-hosted, Linux, X64]→ubuntu-latestghcr.io/namastexlabs/forge-builder:nightly-2025-05-18containerBenefits
Docker Image
Image:
ghcr.io/namastexlabs/forge-builder:nightly-2025-05-18Contains:
Testing Plan
Related
🚀 Generated with Claude Code