feat: add --issue flag to factory ceo/run for direct issue targeting#210
feat: add --issue flag to factory ceo/run for direct issue targeting#210akashgit wants to merge 3 commits into
Conversation
Adds the ability to point the factory at a GitHub or GitLab issue directly, fetching the issue content and using it as the build spec. - New module factory/issue.py with IssueSpec dataclass, parse_issue_ref, infer_remote, fetch_issue, and format_issue_as_spec - --issue flag on both ceo and run subparsers - Mutual exclusion with --prompt, --focus, --no-github, --mode interactive/research - Issue spec persisted to .factory/strategy/current.md (same as --prompt) - Issue number/URL passed through to _build_ceo_task for CEO directive - 25 tests covering parsing, remote inference, formatting, fetching, and CLI validation Closes #209 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
==========================================
- Coverage 87.01% 86.94% -0.07%
==========================================
Files 51 52 +1
Lines 7300 7447 +147
==========================================
+ Hits 6352 6475 +123
- Misses 948 972 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes ordering bug in cmd_run where --issue validation happened after the network call to fetch the issue. Also moves --no-github check in cmd_ceo to early validation block. Adds missing cmd_run mutual exclusion tests (4 new tests). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Factory Review: KEEPVerdict: KEEP Experiment: #54 Score Comparison
Guard Checks
Real Integration Test
Reviewer Notes
Posted by Factory CEO — PR left open for human review |
akashgit
left a comment
There was a problem hiding this comment.
PR #210 Review: feat: add --issue flag to factory ceo/run for direct issue targeting
+561 / -8 across 3 files — new factory/issue.py module, CLI integration in factory/cli.py, and 25 tests in tests/test_issue.py.
Overview
Adds --issue to factory ceo and factory run, allowing users to point the factory at a GitHub/GitLab issue by number, URL, or shorthand (owner/repo#N). The issue is fetched via gh/glab CLI, formatted as a markdown spec, persisted to .factory/strategy/current.md, and injected into the CEO task with a directive and finalize-tracking section.
Well-structured feature. The parsing is solid, forge detection is smart, and the test coverage is thorough. A few things to fix:
Issues
1. Missing --issue + --mode research mutual exclusion in cmd_run (bug)
cmd_ceo blocks this combination (line 1377), but cmd_run does not. The run subparser accepts --mode research, so a user can run factory run /path --issue 42 --mode research and bypass the guard. The test suite (TestCLIMutualExclusion) also doesn't cover this case for run.
factory/cli.py:2199 — add after the no_github check:
if issue_ref and mode == "research":
print("Error: --mode research and --issue are mutually exclusive. ...")
return 1But note: mode isn't parsed until line 2215, after the issue fetch at line 2205. So either move the mode parsing up, or place this check after line 2221. Placing it after means the issue would already be fetched before the error fires — wasteful but not broken. Alternatively, move the mode extraction before the fetch block.
2. Duplicated issue-handling code between cmd_ceo and cmd_run (quality)
The following block appears nearly identically in both functions:
- Three mutual exclusion checks (
--prompt,--focus,--no-github) fetch_issue+format_issue_as_speccallstrategy_dircreation andcurrent.mdwriteissue_number/issue_urlextraction
Extract this into a helper like _resolve_issue(issue_ref, project_path) -> tuple[str, int, str] that returns (context, number, url), and a _check_issue_exclusions(issue_ref, prompt_file, focus, no_github) or similar. This keeps both command paths in sync and avoids the already-demonstrated gap (the missing research check in cmd_run).
3. issue_number=issue_number, issue_url=issue_url crammed on one line
factory/cli.py:1472 — the surrounding kwargs are one-per-line. This breaks the pattern:
interactive_idea=interactive_idea,
research_ideation=research_ideation,
messages=pending,
issue_number=issue_number, issue_url=issue_url, # ← two on one lineMinor Notes
- Nested GitLab groups:
parse_issue_refcapturesowner/repowith[^/]+/[^/]+, which won't handlegroup/subgroup/project. Edge case, not blocking. --issue+--loop: No guard against this. In the heartbeat loop, every cycle gets the same issue spec. Probably fine for multi-part issues, but the behavior could surprise users. Consider at least a log warning.- Loop re-detection at line 2292 uses
bool(prompt_file or context)withoutissue_ref— works becausecontextwas set from the issue, but fragile if future changes clearcontext. Consider addingissue_reffor consistency.
What's Good
issue.pyis clean, well-separated, and easy to test independently- Forge auto-detection from git remote (SSH + HTTPS) is thorough
- Error messages are clear and actionable ("CLI not found", "Cannot parse issue reference")
- The
## Issue Trackingsection in the CEO task wiring the--issueflag tofactory finalizeis a nice end-to-end touch - Test coverage is excellent — 25 tests covering parsing, remote inference, formatting, subprocess mocking, error handling, and CLI mutual exclusion
Verdict
Fix issue #1 (the missing research mutual exclusion in cmd_run) before merge. Issue #2 (duplication) is the right follow-up — the gap in #1 is a direct consequence of the duplication, so extracting the shared logic prevents this class of bug.
Addendum: Consider expanding
|
|
Closing in favor of a reworked PR that expands --focus instead of adding --issue. See #211 for the design rationale. |
Summary
factory/issue.pymodule withIssueSpecdataclass, URL/ref parsing, remote inference, and issue fetching viagh/glabCLI--issueflag added to bothceoandrunsubparsers with mutual exclusion against--prompt,--focus,--no-github,--mode interactive, and--mode research.factory/strategy/current.md, and passed through_build_ceo_taskwith directive and finalize tracking sectionsCloses #209
Test plan
pytest tests/test_issue.py -v— 25 tests passpytest -v— full suite (355 tests) passesruff check .— cleanmypy factory/issue.py factory/cli.py— clean🤖 Generated with Claude Code