feat: add github_issue_id field to tasks for issue tracking #280
Conversation
Summary of ChangesHello @jmanhype, 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 introduces a new mechanism for enhanced traceability by linking tasks to their corresponding GitHub issues. By adding a Highlights
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 github_issue_id field to tasks, which is a valuable addition for traceability. The changes are well-contained, touching the OpenAPI specification, API handlers, and a runtime database migration. My review focuses on improving the implementation's robustness and maintainability. I've identified a correctness issue where an API endpoint returns stale data, along with opportunities to reduce code duplication and improve the efficiency of database operations. Addressing these points will make the new feature more reliable and easier to maintain.
forge-app/src/router.rs
Outdated
| // Set github_issue_id if provided (Forge extension for issue tracking) | ||
| if let Some(github_issue_id) = payload.github_issue_id { | ||
| sqlx::query( | ||
| "UPDATE tasks SET github_issue_id = ?, updated_at = datetime('now') WHERE id = ?", | ||
| ) | ||
| .bind(github_issue_id) | ||
| .bind(task_id) | ||
| .execute(&deployment.db().pool) | ||
| .await?; | ||
| } |
There was a problem hiding this comment.
This approach of creating a task and then immediately updating it with github_issue_id in a separate query is inefficient as it requires two database round-trips for a single logical operation. It's also not atomic; if the UPDATE query fails, the task will exist in an inconsistent state without the associated issue ID.
A more robust and efficient solution would be to pass github_issue_id to the Task::create function and handle it within a single INSERT statement. This would likely involve modifying db::models::task::CreateTask and Task::create. If modifying the upstream db crate is not feasible, this workaround is understandable, but it's worth considering a refactor for long-term maintainability and performance.
forge-app/src/router.rs
Outdated
| // Set github_issue_id if provided (Forge extension for issue tracking) | ||
| if let Some(github_issue_id) = payload.github_issue_id { | ||
| sqlx::query( | ||
| "UPDATE tasks SET github_issue_id = ?, updated_at = datetime('now') WHERE id = ?", | ||
| ) | ||
| .bind(github_issue_id) | ||
| .bind(task_id) | ||
| .execute(&deployment.db().pool) | ||
| .await?; | ||
| } |
There was a problem hiding this comment.
This block of code for updating github_issue_id is a duplicate of the logic in the forge_create_task function (lines 179-188). To improve maintainability and avoid potential inconsistencies in the future, consider extracting this logic into a private helper function. This function could take the database pool, task_id, and github_issue_id as arguments.
Split the monolithic 1680-line router.rs into modular route files: - routes/state.rs: ForgeAppState and related implementations - routes/tasks.rs: Task creation, listing, and WebSocket streaming - routes/attempts.rs: Task attempt handlers with forge overrides - routes/config.rs: Global forge configuration endpoints - routes/projects.rs: Project-specific settings and profiles - routes/omni.rs: Omni service status and notifications - routes/agents.rs: Forge agents and neuron management - routes/frontend.rs: Static file serving and documentation The main router.rs is now a thin ~145-line composition layer that imports and composes these modules. Also added build.rs to create dummy frontend/dist directory for compilation. Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
- Enhanced ThemeProvider with localStorage persistence for immediate theme application on page load - Added resolvedTheme property to handle system preference resolution - Created ThemeToggle dropdown component with Light/Dark/System options - Integrated ThemeToggle into navbar for easy access - Added listener for system theme changes when in SYSTEM mode Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Implements a CLI command to clean up orphaned worktrees that are no longer referenced by active task attempts in the database. Features: - Scans temp directory for worktree directories - Queries database for active task_attempts with worktree paths - Identifies orphans (on disk but not in DB) - Shows orphan list with directory sizes - --force flag to delete orphans and show space recovered Usage: forge cleanup # Dry run - shows what would be deleted forge cleanup --force # Actually delete orphan worktrees Changes: - forge-app/src/bin/cleanup.rs: New cleanup binary - forge-app/Cargo.toml: Register new binary - npx-cli/bin/cli.js: Route cleanup subcommand - scripts/build/build.sh: Build and package cleanup binary Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
- Replace flatten() with explicit error handling for directory reads
- Refactor size_human() to static format_size() method
- Use to_string_lossy() instead of unwrap_or("unknown") for names
- Add TODO comment about duplicated database URL logic
- Make CLI argument parsing more robust by searching args array
Addresses high and medium severity feedback from code review.
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Adds github_issue_id field to tasks to enforce the "No Wish Without Issue" rule from AGENTS.md. This links tasks to their originating GitHub issues for traceability. Changes: - Add runtime migration to add github_issue_id column to tasks table - Update ForgeCreateTask struct with github_issue_id field - Update forge_create_task to persist github_issue_id - Update forge_create_task_and_start with ForgeCreateAndStartTaskRequest - Update OpenAPI spec with github_issue_id documentation Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
jmanhype
force-pushed
the
claude/add-github-issue-id-016zEn965BVzWggfSJcMGa3K
branch
from
e222198 to
940a93e
Compare
Wrap frequently re-rendered list item components with React.memo: - TaskCard (renders 10-50+ times in Kanban board) - DisplayConversationEntry (renders for every message in conversation) - DiffCard (renders once per changed file) - ProjectCard (renders in project grid) - TaskRelationshipCard (renders in relationship lists) Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Replace lodash imports (isEqual, cloneDeep, merge) with native JavaScript alternatives in ProjectSettings.tsx and GeneralSettings.tsx: - Add frontend/src/utils/object.ts with native implementations: - isEqual: deep equality comparison - deepClone: uses structuredClone API - deepMerge: custom deep merge implementation - Remove lodash from root package.json - Remove @types/lodash from frontend/package.json This reduces the bundle size by ~40KB. Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
- Add WeakMap tracking to prevent stack overflow on circular objects - Fix Set deep equality to use deep comparison instead of shallow - Addresses gemini-code-assist critical safety issues gemini-code-assist review identified these critical bugs: 1. Missing circular reference handling could crash app 2. Set comparison used shallow equality instead of deep Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Fixes gemini-code-assist correctness issue where forge_create_task_and_start returned stale task data after updating github_issue_id field. - Change task to mut and re-fetch after UPDATE - Ensures returned task object has the updated github_issue_id gemini-code-assist review identified: API endpoint returns stale data after update Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Adds typeof window check before accessing window.matchMedia to prevent crashes during server-side rendering. Fixes gemini-code-assist critical SSR safety issue that could crash the application during SSR when theme is SYSTEM mode. - Check if window exists before matchMedia call - Default to 'light' theme during SSR Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Fixed two compilation issues discovered during dogfood testing: 1. Wrong API method (lines 111, 387): - Changed Task::get_one() to Task::find_by_id() - Added proper Option handling with ok_or() 2. Missing struct field (lines 210, 526): - Added attempt_count field to TaskWithAttemptStatus initializers - Line 210: attempt_count = 0 (list tasks) - Line 526: attempt_count = 1 (just created first attempt) These fixes ensure the code compiles and runs correctly with the merged PR automagik-dev#280 (GitHub Issue Tracking) changes. Related: Issue automagik-dev#289 (Task serialization follow-up) Tested: Dogfood testing on testing/all-gemini-fixes-merged branch 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
|
@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".
forge-app/src/routes/tasks.rs
Outdated
| let task_title_id = git_branch_id(&task.title); | ||
| let short_id = short_uuid(&task_attempt_id); | ||
| let branch_name = format!("forge/{}-{}", short_id, task_title_id); | ||
|
|
||
| let mut task_attempt = TaskAttempt::create( |
There was a problem hiding this comment.
The create‑and‑start endpoint now ignores the use_worktree flag: the request struct no longer accepts it and the handler unconditionally builds a forge/... branch and persists use_worktree as true. Frontend flows such as the Genie chat still send use_worktree: false to run directly on the base branch and hide the task via forge_agents, but those requests will now always create a worktree attempt and surface the task on the kanban board. This is a regression from the previous implementation that honoured use_worktree and marked non‑worktree tasks as agents.
Useful? React with 👍 / 👎.
|
@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".
| router = router.merge(config::forge_config_router().with_state::<ForgeAppState>(dep_clone.clone())); | ||
| router = | ||
| router.merge(containers::router(deployment).with_state::<ForgeAppState>(dep_clone.clone())); | ||
| router = | ||
| router.merge(projects::router(deployment).with_state::<ForgeAppState>(dep_clone.clone())); | ||
| router.merge(upstream_projects::router(deployment).with_state::<ForgeAppState>(dep_clone.clone())); |
There was a problem hiding this comment.
The refactored upstream_api_router now merges config, containers, projects, drafts, tasks, attempts, etc., but it no longer merges server::routes::tags, so the /api/tags endpoints are never registered. The frontend still calls /api/tags via tagsApi (see frontend/src/lib/api.ts), so listing or editing tags will now return 404s after this change.
Useful? React with 👍 / 👎.
Resolves two regressions identified by Codex in PR automagik-dev#280: 1. Missing tags router (404s): Added tags::router import and merger to upstream_api_router. Tags routes were accidentally omitted during router modularization, causing 404 errors for tag endpoints. 2. use_worktree flag ignored: Restored use_worktree parameter handling in forge_create_task_and_start. The flag was hardcoded to true, breaking the ability to create tasks that run on base branch without worktrees (e.g., Genie chat). Technical changes: - Added tags import and router merge in router.rs:forge-app/src/router.rs:127 - Added use_worktree field to ForgeCreateAndStartTaskRequest - Updated branch name logic to respect use_worktree flag - Fixed database insert to use payload.use_worktree instead of hardcoded true 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
3 participants
Adds github_issue_id field to tasks to enforce the "No Wish Without Issue" rule from AGENTS.md. This links tasks to their originating GitHub issues for traceability.
Changes: