fix/forge core#309
Conversation
namastex888
commented
Dec 11, 2025
namastex888
commented
- chore: remove orphaned root scripts and stale docs
- chore: remove dead upstream asset sync script
- feat: use crates.io codex deps via local forge-core
- chore: sync versions to 0.8.7-rc.27
- fix: make check-versions.sh resilient to missing forge-core submodule
- chore: release v0.8.7-rc.28
- chore: release v0.8.7-rc.29
- fix: skip forge-core sync when PAT_TOKEN unavailable
- chore: release v0.8.7-rc.30
- fix: clone forge-core in CI build, skip forge-core sync without PAT
- fix: use crates.io deps by default, dev-core patches crates-io
- chore: release v0.8.7-rc.31
- fix: dev-core-off now deletes forge-core directory
- feat: absorb forge-extensions into forge-core-services
Sync main to dev (rc.24)
- MASTER_RC_PLAN.md: stale planning document - automagik-forge.service: production-specific config - gh-build.sh: replaced by GitHub Actions publishing - local-build.sh: replaced by scripts/build/build.sh - test-npm-package.sh: replaced by scripts/test/test-package.sh - check-both.sh: unused rust-analyzer config - scripts/publish-automagik.sh: depended on deleted gh-build.sh - Remove publish/beta Makefile targets (use Actions UI)
The sync-upstream-assets.js script referenced a non-existent upstream/frontend/public/ide directory. The destination frontend/public/ide already contains the required IDE icons, making this sync unnecessary.
- Update workspace to use codex crates from crates.io (v0.63.0): codex-protocol, codex-app-server-protocol, codex-mcp-types - Remove git patches for codex crates - Use path deps for forge-core crates (built with crates.io codex) - This enables faster builds by avoiding git fetch for codex Published crates: - codex-mcp-types, codex-utils-cache, codex-utils-git, codex-utils-image, codex-protocol, codex-app-server-protocol
- Remove forge-core clone step from CI (CI uses crates.io, not local paths) - Change workspace deps from path to crates.io versions (0.8.7-rc.30) - Update .cargo/config.toml to patch crates-io instead of git URL - Update Makefile dev-core-off to handle [patch.crates-io] This fixes the architecture: - Default mode: crates.io dependencies (for CI/production) - dev-core mode: local paths via [patch.crates-io] (for local development) CI should NEVER need forge-core directory.
- Add rm -rf forge-core/ to dev-core-off target - Remove broken Dockerfile.builder symlink (pointed to forge-core) This ensures make dev-core-off returns to a clean state: 1. Comments out [patch.crates-io] section 2. Deletes forge-core/ directory 3. Uses crates.io dependencies
- 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
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 project's dependency management, build processes, and module structure. The primary goal is to simplify development workflows by standardizing on 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}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and beneficial refactoring of the project's dependency management and structure. Key changes include migrating forge-core dependencies to use crates.io by default, absorbing the forge-extensions into forge-core-services, and removing a substantial amount of stale documentation, build scripts, and Android-specific code. These changes simplify the development workflow, improve maintainability, and align the project with modern release practices by moving release logic to GitHub Actions. The updates to Makefile and check-versions.sh also make the local development environment more robust and resilient. Overall, this is a solid set of improvements that streamlines the codebase.
| @sed -i 's/^\[patch\.crates-io\]/# [patch.crates-io]/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-db = { path/# forge-core-db = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-services = { path/# forge-core-services = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-server = { path/# forge-core-server = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-deployment = { path/# forge-core-deployment = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-local-deployment = { path/# forge-core-local-deployment = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-executors = { path/# forge-core-executors = { path/g' .cargo/config.toml | ||
| @sed -i 's/^forge-core-utils = { path/# forge-core-utils = { path/g' .cargo/config.toml |
There was a problem hiding this comment.
These sed commands are quite repetitive and could be consolidated into a single, more maintainable command. This would make the Makefile cleaner and less prone to errors if the format of .cargo/config.toml changes.
For example, you could use a single sed command with multiple expressions or a more general regex to target all forge-core- lines at once.
A possible implementation could look like this:
@sed -i -e 's/^\[patch\.crates-io\]/# [patch.crates-io]/' -e '/^forge-core-/s/^/#/' .cargo/config.tomlThis would require adjusting the corresponding dev-core target to uncomment the lines correctly, but would make both targets much more concise.
- 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.
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.
|
@codex review this pr |
1 similar comment
|
@codex review this pr |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
|
@codex review this pr |
![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".
| [workspace.package] | ||
| version = "0.8.7-rc.24" | ||
| version = "0.8.7-rc.38" |
There was a problem hiding this comment.
The workspace package version was bumped to 0.8.7-rc.38 while the npm manifests (package.json, frontend/package.json, npx-cli/package.json) remain at 0.8.7-rc.31, so the repository now ships two different release numbers. Running scripts/check-versions.sh currently fails with a mismatch warning, which will block the release/version consistency checks that the workflow relies on. This regression was introduced by this commit (previously all files were on the same 0.8.7-rc.24 version).
Useful? React with 👍 / 👎.
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.
|
@codex review this pr |
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".
| # Look for version in [workspace.package] section using sed range | ||
| sed -n '/^\[workspace\.package\]/,/^\[/p' "$file" 2>/dev/null | grep -E '^version\s*=' | head -1 | sed 's/.*"\([^"]*\)".*/\1/' | ||
| sed -n '/^\[workspace\.package\]/,/^\[/p' "$file" 2>/dev/null | grep -E '^version\s*=' | head -1 | sed 's/.*"\([^"]*\)".*/\1/' || echo "" |
There was a problem hiding this comment.
The new sed pattern used to extract [workspace.package] versions includes a literal ", " (sed 's/.*"\([^"]*", "\)".*/\1/'), which does not match standard lines like version = "0.8.7-rc.31". As a result get_workspace_version returns the entire line or an empty string, so WORKSPACE_VERSION/FORGE_CORE_VERSION never equal the npm version and the script now exits with errors even when versions are aligned (see the mismatch checks at lines 96–106 and exit at 205). This breaks make/CI runs that invoke scripts/check-versions.sh in otherwise consistent repos. The regex should capture just the quoted version value.
Useful? React with 👍 / 👎.
ace3c2b to
110995e
Compare
GitHub Actions
Inactive
1 participant
