chore: sync from forge - ESLint 9 upgrade session#39
Conversation
ESLint 9 flat config migration: - Migrate from .eslintrc.cjs to eslint.config.js flat config format - Upgrade eslint 8.57→9.39, @typescript-eslint/* 7.18→8.50 - Replace react-spring with @react-spring/web (eliminates 9 deprecated deps) - Fix 4 lint errors caught by stricter v9 rules Dependency improvements: - Eliminates 14 deprecated subdependencies from pnpm install warnings - Only 2 remain (from click-to-react-component and forge-inspector) Performance and cleanup: - Wrap TaskCard in memo() to prevent unnecessary re-renders - Remove debug console.logs from TaskKanbanBoard, TaskFollowUpSection - Simplify useTaskCompletionNotification (Set vs Map, clearer logic) - Fix useConversationHistory to emit entries when process completes - Extract primitive from branchStatus for stable useMemo deps
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 integrates a series of changes from an upstream forge, primarily focusing on critical performance enhancements and a database schema correction. The updates aim to reduce perceived latency for task initiation, optimize resource utilization during startup, and ensure data integrity within the 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 several significant performance improvements and a database migration fix. The changes to move long-running operations like starting a task attempt into background tasks will greatly improve API responsiveness. The pre-warming of the PATH resolution cache is also a smart optimization to reduce latency for the first task. The database migration correctly addresses a type mismatch for foreign keys. My only suggestion is to restore some logging that was removed during a refactoring in the shell utility, as it could be valuable for debugging user environment issues.
| ); | ||
| return None; | ||
| } | ||
| Ok(Err(_)) | Err(_) => return None, |
There was a problem hiding this comment.
The error handling for wait_with_output and timeouts has been simplified to return None without logging. While this is more concise, it removes valuable debugging information if PATH resolution fails for a specific shell. Consider reintroducing logging at the debug level to aid in troubleshooting user environment issues without being too noisy.
Ok(Err(err)) => {
tracing::debug!("Failed to get PATH from {}: {}", shell.display(), err);
return None;
}
Err(_) => {
tracing::debug!("Timed out getting PATH from {}", shell.display());
return None;
}
|
@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".
| }); | ||
|
|
||
| // Optimistically return true - attempt is starting in background | ||
| let is_attempt_running = true; |
There was a problem hiding this comment.
The endpoint now spawns start_attempt in the background but immediately sets is_attempt_running = true in the response. If start_attempt fails (e.g., container creation or git setup error), the client still gets a success response indicating an in-progress attempt with no way to detect the failure because the result is only logged inside the spawned task. Previously this flag reflected the start result; now it is always true, so the UI can show a running attempt that never starts when the background call errors.
Useful? React with 👍 / 👎.
The P0 performance optimization (42s -> <1s HTTP response) introduced silent failures: start_attempt() runs in tokio::spawn() with errors only logged, returning is_attempt_running=true before knowing if the attempt actually started. This fix: - Add `Initializing` status to ExecutionProcessStatus enum - Create ExecutionProcess with Initializing status BEFORE spawning - Transition to Running on success, Failed on error - SSE automatically pushes status changes to clients Also fixes logging: - main.rs: Log success/failure of PATH cache warming (not just "warmed") - shell.rs: Restore debug/warn logging for shell failures/timeouts
namastex888
force-pushed
the
fix/publish-version-check
branch
from
ac283f9 to
1ca961f
Compare
|
@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".
| run_reason: ExecutionProcessRunReason::CodingAgent, | ||
| }, | ||
| execution_process_id, | ||
| None, // before_head_commit captured during actual startup | ||
| ExecutionProcessStatus::Initializing, |
There was a problem hiding this comment.
New task creation now inserts execution processes with ExecutionProcessStatus::Initializing, but the execution_processes.status column is constrained to running/completed/failed/killed (see crates/db/migrations/20251127100000_execution_runs_and_commit_support.sql). With no migration to allow the new value, these inserts will fail with a CHECK constraint error, so create_task_attempt/create_task_and_start will 500 before a process is created. Update the schema or use an existing status before inserting.
Useful? React with 👍 / 👎.
Adds migration to include 'initializing' in the execution_processes status CHECK constraint. Required for the early ExecutionProcess creation pattern that surfaces startup failures to clients.
|
@codex review this pr |
1 similar comment
|
@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".
| // Start the actual execution | ||
| if let Err(start_error) = self | ||
| .start_execution_inner(&task_attempt, &execution_process, &executor_action) | ||
| .await | ||
| { |
There was a problem hiding this comment.
start_attempt_with_process returns immediately after start_execution_inner without running the log normalization and spawn_stream_raw_logs_to_db steps that start_execution performs, so execution processes launched via this new async path (now used by the task and task_attempt create endpoints) never persist executor output to the database or normalized feeds. Attempts started through those routes will run but their logs are dropped, leaving clients with missing history and SSE playback.
Useful? React with 👍 / 👎.
1 participant
Summary
Synced changes from automagik-forge during ESLint 9 upgrade session.
Related forge PR: automagik-dev/forge#325
Changes
.github/workflows/check-release-intent.yml- workflow updatescrates/db/migrations/- task attempt config type fixcrates/executors/src/executors/claude.rs- executor changescrates/local-deployment/src/container.rs- container updatescrates/server/src/main.rs- server updatescrates/server/src/routes/task_attempts.rs- route changescrates/server/src/routes/tasks.rs- route changescrates/utils/Cargo.toml- dependency updatescrates/utils/src/shell.rs- shell utility changesTest Plan