fix: full version sync in sync-forge-core-tag workflow#321
fix: full version sync in sync-forge-core-tag workflow#321namastex888 wants to merge 32 commits into
Conversation
- Move omni service to forge-core-services::services::omni - Move forge_config to forge-core-services::services::forge_config - Remove Android support (android.rs, jni deps) - Delete forge-extensions/ directory - Update all imports to use forge_core_services - Fix prepare-commit-msg hook to use --no-verify for automated sync
- pre-push hook now auto-pushes forge-core first if unpushed commits - pre-push hook auto-disables patches and amends commit - No more manual dev-core-off needed - git push handles everything - make dev-core re-enables patches when needed
- Change `super::super::omni::` to `crate::services::omni::` for cleaner imports - Document forge-core-only changes edge case in DUAL_REPO_WORKFLOW.md - Add warning about uncommitted changes before dev-core-off Council review feedback applied (simplifier recommendation).
The pre-push hook now handles everything automatically: - Pushes forge-core first (if unpushed commits) - Disables Cargo [patch] overrides - Regenerates Cargo.lock - Amends commit with config changes Updated: - AGENTS.md Amendment #11: "Fully Automatic" instead of manual steps - docs/DUAL_REPO_WORKFLOW.md: Removed dev-core-off from normal workflow `make dev-core-off` is no longer part of normal workflow.
- Updated AGENTS.md Amendment #11: simplified to 3-step workflow - Rewrote forge-core-workflow.md spell: ONLY command is `make dev-core` - Updated docs/DUAL_REPO_WORKFLOW.md: removed all legacy edge case notes - Fixed pre-push hook: error message no longer mentions dev-core-off - Simplified forge-core blocker hooks: cleaner workflow instructions The pre-push hook now handles everything automatically: 1. Push forge-core first (if unpushed commits) 2. Auto-disable Cargo [patch] overrides 3. Regenerate Cargo.lock with git deps 4. Amend commit with config changes 5. Allow push to proceed No manual intervention required. Just: make dev-core → edit → commit → push
## Changes ### forge-core - Enable RC version publishing to crates.io (tag pattern: v0.8.7-rc.30) - Add rollback safety: yank published crates on failure - Auto-chain publish-crates.yml from pre-release.yml - Add workflow_call trigger for programmatic invocation - Add prerelease flag to GitHub releases ### forge - Fix sync race condition: query specific workflow run ID - Use timestamp-based filtering to find triggered workflow - Increase timeout and add better error handling ## Workflow PR merge with "rc" label → forge release → forge-core sync → crates.io publish (7 crates) → GitHub release Zero manual intervention required.
GitHub's on.push.tags uses glob patterns, not regex. - [0-9]+ only matches single digit + literal plus sign - [0-9]* matches any digit sequence (correct glob) Added documentation about this common pitfall.
…hing - Remove multi-platform binary builds (not needed for crates.io) - Remove npx-cli packaging - Remove macOS code signing - Reduce pre-release.yml from 463 to 125 lines - Update test.yml to use [self-hosted, Linux, X64] runner labels - Add workspace cleanup step for self-hosted runners - Update toolchain to nightly-2025-12-04 New flow: workflow_dispatch → bump-version → publish-crates.yml Build time: 10+ min → <3 min
Downloaded from crates.io - all 7 crates now published and verified.
The pnpm/action-setup creates ~/setup-pnpm which can become stale on self-hosted runners, causing ENOTEMPTY errors on rmdir.
The examples were using the old `forge_core_services` path which no longer exists. Updated to use `services` module path instead.
The Node.js self-installer fails with ENOENT when the runner state is corrupted. Standalone mode downloads a prebuilt binary instead.
Major changes: - Fix global profile cache race condition by removing set_cached() calls (4 locations: task_attempts.rs, execution_runs.rs, tasks.rs) - Rename DEFAULT → GENIE in profile code and default_profiles.json - Add variant field to ExecutionRun for profile alignment - Simplify tasks.rs to kanban-only (exclude agent-related endpoints) - Add profile_loader service with per-workspace caching via ProfileCacheManager Thread-safe architecture: - ProfileCacheManager now caches profiles per-workspace - No more global state mutation with ExecutorConfigs::set_cached() - Concurrent project access is now safe Migration: - 20251212100000_add_variant_to_execution_runs.sql adds variant column
- Fix ESLint warning: add useWorktree to useCallback deps in TaskFormDialog - Remove unused types: MarkdownLinkProps, MarkdownDivProps from markdown.ts - Configure Knip ignoreIssues for VirtualizedList.tsx false positives Quality gates now pass: - ESLint: 0 warnings - Knip: 0 dead code warnings - TypeScript: 0 errors - Build: Success
In WSL2, closed ports timeout instead of returning "connection refused". Without an explicit timeout, isPortAvailable() hangs indefinitely. Added 1s timeout to net.createConnection and handle timeout event.
Build Task struct directly from row data instead of calling Task::find_by_id() for each row in the loop. This reduces query count from O(n+1) to O(1) for kanban board loading.
- Update sync-forge-core-tag.yml to handle crates.io version strings - Add repository_dispatch notification to forge-core publish-crates.yml - Fix sed patterns to update workspace dependencies (not git tags) - Update forge-core workspace deps to 0.8.7-rc.38 The workflow now: 1. forge-core publishes to crates.io 2. Sends repository_dispatch to automagik-forge 3. sync-forge-core-tag.yml updates all 7 workspace dependencies 4. Creates PR automatically - zero manual version editing
Root cause: setup-node action references $PNPM_VERSION but it was undefined in test.yml and playwright-e2e.yml. This caused pnpm to fall back to auto-detection, which fails on stale cache. Fix: Define PNPM_VERSION=9.15.4 (matches lockfileVersion 9.0)
pnpm/action-setup@v4 rejects having both `version` parameter AND `packageManager` in package.json. Removed version parameter from composite action to let it auto-detect from package.json.
Critical fixes: - Extract test module from services/mod.rs (1098→641 lines) to comply with 1000-line limit - Fix pre-push hook silent failures - proper error handling for cargo fetch and git amend - Port OmniClient HTTP tests to forge-core-services (10 tests from deleted forge-extensions) - Add circuit breaker to Omni notification worker (MAX_CONSECUTIVE_FAILURES=10) Important fixes: - Fix incorrect exclude_agents comments in router.rs - Fix misleading health check message - Remove dev-core-off from Makefile (fully automated via hooks now) - Update stale zerocopy pin comment in Cargo.toml - Add ::warning:: annotations for PAT_TOKEN missing in release workflow Suggestions: - Remove magic number comment (37 agent profiles) - Fix HeaderValue unwrap panic risk with safe fallback
When forge-core publishes to crates.io, sync ALL versions: - npm package.json files (root, npx-cli, frontend) - Cargo.toml workspace version - Cargo.toml dependencies (existing) - Validates with check-versions.sh before commit Prevents version desync between Cargo and npm packages.
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 significantly refactors the development and release process by introducing a fully automated 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
|
|
Duplicate - changes should go to a fresh branch |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and impressive overhaul of the forge-core development workflow, making it fully automatic and much more developer-friendly. The new pre-push hook that handles forge-core pushes, patch disabling, and commit amending is a fantastic piece of automation that will prevent many common errors. The extensive refactoring to move forge-extensions logic into forge-core and streamline the forge-app crate is a great architectural improvement. The documentation updates are thorough and clearly explain the new, simplified process. I've also noted several smaller but important reliability and correctness fixes throughout the codebase. Overall, this is an excellent set of changes that dramatically improves the developer experience.
| let mut consecutive_failures = 0u32; | ||
| const MAX_CONSECUTIVE_FAILURES: u32 = 10; | ||
|
|
||
| loop { | ||
| match process_next_omni_notification(&pool, &config).await { | ||
| Ok(true) => { | ||
| // Processed at least one item, immediately attempt next | ||
| // Processed at least one item, reset failure counter | ||
| consecutive_failures = 0; | ||
| continue; | ||
| } | ||
| Ok(false) => { | ||
| // Queue empty → short backoff | ||
| // Queue empty → reset counter and short backoff | ||
| consecutive_failures = 0; | ||
| sleep(Duration::from_secs(10)).await; | ||
| } | ||
| Err(err) => { | ||
| tracing::error!("Omni notification worker error: {err:?}"); | ||
| consecutive_failures += 1; | ||
| tracing::error!( | ||
| consecutive_failures = consecutive_failures, | ||
| "Omni notification worker error: {err:?}" | ||
| ); | ||
|
|
||
| if consecutive_failures >= MAX_CONSECUTIVE_FAILURES { | ||
| tracing::error!( | ||
| "Omni worker disabled after {} consecutive failures", | ||
| consecutive_failures | ||
| ); | ||
| break; | ||
| } | ||
|
|
||
| sleep(Duration::from_secs(15)).await; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a great reliability improvement for the Omni notification worker. Adding a circuit breaker pattern with consecutive_failures prevents the worker from getting stuck in a loop and spamming logs if there's a persistent issue with processing notifications. Disabling the worker after MAX_CONSECUTIVE_FAILURES is a sensible approach to fail gracefully.
| let content_type = HeaderValue::from_str(mime.as_ref()) | ||
| .unwrap_or_else(|_| HeaderValue::from_static("application/octet-stream")); | ||
| response | ||
| .headers_mut() | ||
| .insert(header::CONTENT_TYPE, content_type); |
There was a problem hiding this comment.
This is a good defensive programming improvement. Replacing .unwrap() with .unwrap_or_else() prevents a potential panic if HeaderValue::from_str fails for some reason, making the static file serving more robust.
| function isPortAvailable(port) { | ||
| return new Promise((resolve) => { | ||
| const sock = net.createConnection({ port, host: "localhost" }); | ||
| const sock = net.createConnection({ port, host: "localhost", timeout: 1000 }); | ||
| sock.on("connect", () => { | ||
| sock.destroy(); | ||
| resolve(false); | ||
| resolve(false); // Port is in use | ||
| }); | ||
| sock.on("error", () => resolve(true)); // Port is available | ||
| sock.on("timeout", () => { | ||
| sock.destroy(); | ||
| resolve(true); // Timeout means port is available (WSL2 behavior) | ||
| }); | ||
| sock.on("error", () => resolve(true)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Adding a timeout to the net.createConnection call is a smart fix for improving the reliability of port checking, especially in environments like WSL2 where network behavior can be unpredictable. This prevents the script from hanging indefinitely and makes the development environment startup more robust.
| // Agent button exists (use getByText with exact to avoid matching "agent" in description) | ||
| const agentSection = page.getByText('Agent', { exact: true }).locator('..'); | ||
| await expect(agentSection.getByRole('button')).toBeVisible(); // Agent selector exists |
There was a problem hiding this comment.
This is a good improvement to the test's robustness. Using getByText('Agent', { exact: true }) makes the selector more specific and less prone to flakiness if the word "agent" were to appear elsewhere in the component's text content. It's a great example of writing resilient E2E tests.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Check if there are changes | ||
| if git diff --quiet; then | ||
| echo "⚠️ No changes detected - forge-core tag may already be up to date" | ||
| echo "⚠️ No changes detected - forge-core version may already be up to date" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The Create branch for Cargo.toml update step exits when git diff --quiet finds no changes, but the job still runs the later cargo-update/commit/PR steps. If the dispatched version already matches the repository (e.g., rerunning after a previous sync), this early exit leaves no branch and no staged files, so the subsequent git commit fails with "nothing to commit" and the workflow errors out despite there being nothing to update. Gate the downstream steps on the diff check or end the job when no changes are detected.
Useful? React with 👍 / 👎.
1 participant
Summary
sync-forge-core-tag.ymlto sync ALL versions when forge-core publishesChanges
When forge-core publishes to crates.io, the workflow now syncs:
[workspace.package]versionpackage.json(root)npx-cli/package.jsonfrontend/package.jsoncheck-versions.shbefore commitTest plan
sync-forge-core-tag.ymlwith a test versioncheck-versions.shvalidation runs before commit