Support optional project assignment in issue create#129
Conversation
There was a problem hiding this comment.
QA Review — issue #128 / PR #129
Decision: APPROVED
What I verified
- PR is on branch
issues/orfe-128✅ andRef: #128is the first line of the PR body ✅ - All 319 tests pass (
npm test— full run, no failures) ✅ npm run lint— clean ✅npm run typecheck— clean ✅npm run build— clean ✅- Greg's verification claims are substantiated by local re-run.
Scope and correctness
The implementation matches issue #128 acceptance criteria fully:
-
Explicit opt-in only —
isProjectAssignmentRequested()gates all project config resolution;resolveProjectAssignmentRequest()returnsnullunless one ofadd_to_project,project_owner,project_number,status_field_name, orstatusis present. Project defaults from config are only read when explicitly opted in — matching Jelena's product direction exactly. -
Backward compatibility — test #223 (
preserves create-only behavior when repo config has a default project but no project assignment was requested) fires no project API calls when opts are absent. ✅ -
Create-only — test #69 (CLI) / #220 (core). ✅
-
Create + project add — tests #70 / #221 with
add_to_project: true. ✅ -
Create + project add + status — test #222 with
status: 'Todo'only (noadd_to_project: true), confirming that providingstatusalone is enough to trigger the full pipeline. ✅ -
Partial failure: project_add stage — tests #71 / #224 confirm the error has the correct code (
auth_failed), message that names the created issue, anddetails.stage: 'project_add'withcreated_issueidentity in the structured payload. ✅ -
Partial failure: project_status stage — test #225 covers the case where project add succeeds but status mutation fails; error has
details.stage: 'project_status'withcreated_issue,project_owner,project_number,status_field_name, andrequested_status. ✅ -
Config validation fails fast —
resolveProjectAssignmentRequest()is called before thetryblock that creates the issue. If the config is missing required project coords,resolveProjectCommandConfigthrows withinvalid_usagebefore any REST call is made. No orphan issue created in that path. -
Error re-wrapping is clean — project assignment errors thrown as
OrfeErrorpass throughmapIssueCreateError'sinstanceof OrfeErrorguard unchanged; no double-wrapping. -
Architecture invariants — generic runtime boundary preserved; core has no OpenCode-specific dependencies; project operations use GraphQL as required; structured JSON contract maintained for success and error envelopes;
error.detailsfield is additive and optional so callers can tolerate its absence. -
docs/orfe/spec.mdupdated with full project-assignment rules,project_assignmentsuccess shape, and partial-failure behavior contract. ✅ -
Plugin schema —
add_to_projectcorrectly added asboolean().optional();project_owner,project_number,status_field_name, andstatuswere already in the plugin args from prior project commands. ✅ -
Help output — definition tests confirm
--add-to-projectand--status <value>appear in the usage string; the CLI "renders leaf help for every agreed V1 command" test includesissue createand passes. ✅
Blockers
None.
Important (non-blocking, follow-up recommended)
successDataExample in definition.ts does not include project_assignment — the command definition's successDataExample only shows the minimal create-only response shape. Agents discovering the command via orfe help --command-name "issue create" will see a success example without project_assignment, which may cause them to silently drop or misinterpret that field. The spec.md and test coverage are correct, but the in-tool help is the primary runtime discovery surface for agents. Recommend a follow-up to add a second annotated example or update the successDataExample to reflect the optional field.
Nice-to-have (no follow-up required unless desired)
matchesProjectByOwnerAndNumberandmatchesProjectAddItemhelper functions are duplicated verbatim betweentest/core.test.tsandtest/cli.test.ts. Extracting them to a shared test utility would reduce drift risk.- No test for "project assignment requested but no
projects.defaultconfigured and no explicit coordinates" — the behavior is correct (throwsinvalid_usagebefore creating the issue) but is an untested code path. - The
getGitHubClient()call insideapplyProjectAssignmentis a second invocation after the issue is already created. In a degenerate case where token renewal fails between the two calls, the partial-failure error would go throughmapIssueCreateErrorwithout a stage-aware payload. Extremely unlikely in practice; noted for completeness.
Docs / invariants
docs/orfe/spec.mdupdated ✅. No new ADR required (existing command-surface extension precedent applies). No new debt introduced. Invariants preserved.
Ref: #128
Summary
issue createthroughadd_to_project, project inputs, and default project resolution only when opted inVerification
npm test✅npm run lint✅npm run typecheck✅npm run build✅Docs / ADR / debt
docs/orfe/spec.md; existing invariants and ADRs already cover this command-surface extension and no new durable debt was introducedRisks / follow-ups
error.details, so callers should tolerate that additional structured field