Skip to content

feat(forge): add Linear provider with hybrid fallback to GitHub#716

Open
timeleft-- wants to merge 3 commits intocluesmith:mainfrom
MachineWisdomAI:feature/linear-forge-provider
Open

feat(forge): add Linear provider with hybrid fallback to GitHub#716
timeleft-- wants to merge 3 commits intocluesmith:mainfrom
MachineWisdomAI:feature/linear-forge-provider

Conversation

@timeleft--
Copy link
Copy Markdown
Contributor

Summary

  • Fix fallback bug: buildPresetFromScripts now skips concepts without scripts instead of setting null, enabling hybrid providers that implement only a subset of concepts (e.g., issues only) while letting unimplemented concepts fall through to the GitHub default.
  • Add Linear provider: 6 POSIX sh scripts (curl+jq) for issue-oriented concepts — auth-status, user-identity, issue-view, issue-list, issue-comment, recently-closed. PR concepts fall through to GitHub.
  • Pass forge config env vars: Non-concept keys in forge config (e.g., linear-team) are exported as CODEV_LINEAR_TEAM to scripts.
  • Widen issue identifier types: Accept alphanumeric identifiers like ENG-123 throughout agent-farm CLI and type system.

Test plan

  • TypeScript compiles without errors (pnpm run build)
  • All 2560 existing tests pass (pnpm test)
  • Manual: verify LINEAR_API_KEY=<key> sh scripts/forge/linear/issue-view.sh with CODEV_ISSUE_ID=ENG-123 returns valid JSON
  • Manual: verify afx spawn ENG-123 --protocol spir parses the identifier correctly
  • Manual: verify codev doctor shows linear as a known provider

🤖 Generated with Claude Code

Linear handles issue concepts (view, list, comment, recently-closed,
auth-status, user-identity) while PR concepts fall through to GitHub.

Key changes:
- Fix buildPresetFromScripts to skip missing scripts instead of setting
  null, enabling hybrid providers that only implement a subset of concepts
- Register linear provider in getProviderPresets
- Pass forge config keys as CODEV_ env vars (e.g. linear-team → CODEV_LINEAR_TEAM)
- Widen issueNumber types to accept alphanumeric identifiers (ENG-123)
- Add 6 POSIX sh scripts for Linear GraphQL API

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@waleedkadous
Copy link
Copy Markdown
Contributor

Younes, this is a genuinely thoughtful contribution — thank you for taking this on. The insight that drives the whole PR is excellent: a "hybrid" forge model where Linear handles issues and GitHub handles PRs is exactly right, and the way you unlocked it via buildPresetFromScripts (omitting missing concepts so they fall through to defaults, instead of writing null) is a clean architectural improvement that benefits gitlab/gitea too. The buildForgeEnv env-passthrough is also a nice generalizable primitive. I'd love to land this.

A few suggestions that I think would make it even stronger:

1. The DB layer needs to come along for the ride. The widening of Builder.issueNumber to number | string is great, but the persistence layer didn't get updated:

  • packages/codev/src/agent-farm/db/schema.ts:43issue_number INTEGER
  • packages/codev/src/agent-farm/db/types.ts:37issue_number: number | null
  • The two migration paths in db/index.ts:231,318

SQLite is flexible enough that storing "ENG-123" in an INTEGER column probably won't crash, but dbBuilderToBuilder returns it typed as number, which any downstream numeric comparison will trip over. Could you migrate the column to TEXT and widen DbBuilder.issue_number? Happy to point you at examples of how other migrations are structured in db/index.ts.

2. One spot in porch silently swallows alphanumeric IDs. packages/codev/src/commands/porch/prompts.ts:31-37 does parseInt(projectId, 10) and only calls fetchIssue when !isNaN(...). So porch would never look up titles for ENG-123 — it would always fall through to the spec file. The good news is fetchIssue already accepts string | number (from spec 603 cleanup), so dropping the parseInt guard and passing projectId directly should just work.

3. Two small Linear API issues in the scripts:

  • recently-closed.sh uses orderBy: completedAt — Linear's PaginationOrderBy enum only supports createdAt and updatedAt. updatedAt is probably the closest substitute.
  • issue-view.sh will throw when an issue isn't found (nodes[0].state.name on an empty array). A small jq guard or an explicit "issue not found" exit would make failures much easier to debug.

4. Could you add a few unit tests? The headline fix in buildPresetFromScripts deserves a test (provider with only some scripts → missing concepts are omitted, not null), and the ENG-123 CLI parsing path is worth one too. Three or four tests here would lock in the new behavior. The existing forge.test.ts and agent-farm/__tests__/ patterns are good models to copy from.

5. One process note: the spec/plan files are numbered 695-linear-forge-provider.md, but #695 is an existing merged PR for review-blocking work (we use the GitHub issue number as the file prefix). Could you open a fresh GitHub issue describing this hybrid-forge feature and renumber the spec/plan to match? It also gives us a place to discuss the design with future readers.

The hybrid forge concept is a real contribution — once these pieces land, this opens the door to all sorts of combos (Jira+GitHub, GitHub+GitLab during migrations, etc.). Looking forward to merging.

…ripts, tests

1. Migrate issue_number column from INTEGER to TEXT (schema, types,
   migration v8) so alphanumeric identifiers like "ENG-123" persist
   correctly through the DB layer.

2. Drop parseInt guard in porch getProjectSummary so alphanumeric
   project IDs route through fetchIssue instead of silently skipping.

3. Fix Linear scripts: recently-closed.sh uses updatedAt (valid enum
   value) instead of completedAt; issue-view.sh guards against empty
   result set with explicit error.

4. Add unit tests for Linear provider preset behavior (omission
   fallthrough, not null) and alphanumeric issue identifiers in
   spawn validation and mode detection.

5. Renumber spec/plan from 695 to 719 to match GitHub issue cluesmith#719.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@waleedkadous
Copy link
Copy Markdown
Contributor

Beautiful work on the revisions, Younes — the migration v8, the jq guard in issue-view.sh, and the new test coverage are all really nicely done. One small loose end: CI's Unit Tests step is failing on two stale assertions in src/__tests__/project-summary.test.ts that were locked in to the old prompts.ts behavior:

  1. returns GitHub issue title when issue exists — asserts fetchIssue was called with 126 (number). Now that we pass projectId through directly, it should be '126' (string).
  2. returns null for non-numeric project IDs with no spec file — asserts fetchIssue was not called. Now it correctly is called (that's the whole point of the fix). The test can be rewritten to just assert the final return value is null.

Both are quick test updates. Once those go green, this is good to merge from my end.

@timeleft--
Copy link
Copy Markdown
Contributor Author

Addressed in 4938cf7: updated the stale project-summary assertions so numeric-looking project IDs are passed through as strings, and non-numeric IDs still call fetchIssue before returning null.\n\nValidation: pnpm --filter @cluesmith/codev test -- src/__tests__/project-summary.test.ts (full Vitest suite ran: 128 files, 2569 passed, 13 skipped).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants