Skip to content

Set a new issue's board status to Backlog when gh:create files it - #3550

Merged
malibio merged 2 commits into
mainfrom
issue-3549-gh-create-board-status
Oct 4, 2026
Merged

malibio merged 2 commits into
mainfrom
issue-3549-gh-create-board-status

Conversation

@malibio

@malibio malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Closes #3549.

bun run gh:create added a new issue to the project board and left its Status empty, so the issue sat on the board in no column and read back with a blank status.

What changed

  • createIssue now sets the new item's Status to Backlog, using the project item id the add returns. The board is not listed to find it.
  • The status write is one private method, setItemStatus, shared by createIssue and updateIssueStatus. gh:status behaves as before.
  • If the issue is added to the board but the status write fails, the command still returns the issue and warns that the status was not set. If the add itself fails, the existing warning is unchanged and no status write is attempted.

Tests

Three tests added to scripts/gh-utils.test.ts:

  • creating an issue writes Backlog to the item the add returned, with no board listing;
  • a failed status write still returns the issue and warns once;
  • a failed board add attempts no status write.

The first two fail against the previous github-client.ts and pass with the change. bun run test:changed (scripts tier) and bun run quality:fix pass.

Docs

development/issue-workflow.md in nodespace-docs now says gh:create files a new issue as Backlog and that Todo means queued.

🤖 Generated with Claude Code

@malibio

malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review

Review Type: Initial Review (no prior reviews or comments on the PR)

Summary

A small, atomic change that does what the issue asks. createIssue now keeps the project item id the add returns and writes Backlog to it; the mutation moved into a private setItemStatus that updateIssueStatus also calls. All three paths behave correctly, including the scheduled-workflow path through findOrCreateTrackingIssue. No blocking findings.

Requirements Check

# Criterion Notes
1 Issue created with gh:create is on the board with Status Backlog ✅ createIssue calls setItemStatus(itemId, NEW_ISSUE_STATUS) after a successful add. Checked against a stubbed Octokit, not the live board.
2 Status set with the item id the add returned, no second board listing ✅ itemId comes from addIssueToProject; the test asserts no GetProjectItems query is sent.
3 Add succeeds, status write fails: command succeeds, prints number and URL, warns ✅ The failure is caught and warned; the issue is returned, so NodeSpaceGitHubManager.createIssue still prints number and URL.
4 Add fails: existing warning unchanged, no status write ✅ Warning text is untouched; the status write is in the else branch. The test asserts exactly one GraphQL call and one warning.
5 gh:status behaves as before and shares the status write ✅ updateIssueStatus calls setItemStatus; the mutation text and variables are identical to the removed inline copy.
6 Tests cover the create path and both failure cases ✅ Three new tests; bun test scripts/gh-utils.test.ts gives 21 pass, 0 fail.
7 development/issue-workflow.md says a new issue is filed as Backlog ✅ nodespace-docs e318cfd, lines 26-27: "Backlog is where gh:create files a new issue: filed, not queued." The docs checkout is not ahead of origin/main.
8 Code passes bun run quality:fix Not verified I did not run it (it rewrites files). The diff has no any and no lint suppression.
9 Tests pass (bun run test:changed) Partly verified I ran only the affected file (21 pass), not test:changed.

Path analysis

  • Add succeeds, status write succeeds: one add, one update on the returned item id, no warning.
  • Add succeeds, status write fails: one warning naming the issue and gh:status as the fix; addedToProject stays true, which is accurate.
  • Add fails (the scheduled-workflow case, where GITHUB_TOKEN has no Projects v2 write scope): itemId is null, the existing warning prints, and no status write is attempted. Scheduled runs make the same single GraphQL call and log the same single warning as before.
  • A malformed add response (item: null) throws inside addIssueToProject and is caught into the add-failed branch.

Code Review Findings

No Critical or Important findings.

🟢 Suggestion: the status-failure warning drops the cause
📁 scripts/github-client.ts:498-507

const statusSet = await this.setItemStatus(itemId, NEW_ISSUE_STATUS)
  .then(() => true)
  .catch(() => false);

The add-failure warning can name a likely cause (token scope). A status write that fails after a successful add has no such obvious cause: the likeliest is a stale statusFieldId or option id, which the comment on statusOptions says has happened before. Catch the error and append its message to the warning, so a stale id is diagnosable from the log. Principle: error messages should aid debugging.

🟢 Suggestion: pin the option id on the gh:status path
📁 scripts/gh-utils.test.ts:274-303
The option-id lookup moved from updateIssueStatus into setItemStatus, but the two updateIssueStatus tests assert only vars.itemId. If the helper resolved the wrong option, only the create-path test would notice, and only for Backlog. Add expect(update!.vars.value).toEqual({ singleSelectOptionId: "58fb1205" }) to the "Done" test, so criterion 5 ("behaves as before") is pinned by a test.

🟢 Nit: caller comments now understate what createIssue warns about
📁 scripts/gh-utils.ts:151-153, 359-363
Both say GitHubClient.createIssue warns "when the board add fails". It now also warns when the status write fails. A one-clause update keeps them accurate.

🟢 Nit: the option id is looked up twice on the gh:status path
📁 scripts/github-client.ts:246-250, 541
statusOptionId in updateIssueStatus is now used only for the validity check; setItemStatus looks it up again. Harmless. Leave it, or rename the local so it reads as validation only.

What is good

  • NEW_ISSUE_STATUS is named for its role and typed through keyof typeof this.statusOptions, so a typo fails to compile.
  • The new tests would fail without the change: the first expects exactly one updateProjectV2ItemFieldValue call, where the old code made none.
  • The failure tests assert call counts (one query, one warning), not only outcomes, so an accidental extra write or a double warning is caught.
  • Comments explain why, at the density of the surrounding file, and carry no issue numbers.
  • Security: no new input surface, no secrets, and the warnings print only the issue number and status name.

Recommendation

APPROVE. The change is a net improvement and correct on every path. The two suggestions and two nits are optional.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

@malibio

malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review addressed

All four findings from the review were suggestions or nits, and all four are addressed in the follow-up commit. None was skipped.

Finding Change
Status-failure warning drops the cause The warning now includes the error's message, so a stale status field or option id can be told apart from an outage. The failed-status test asserts the cause appears in the warning.
Option id not pinned on the gh:status path The updateIssueStatus test for Done now asserts the option id it writes.
Caller comments understate what createIssue warns about Both comments in scripts/gh-utils.ts now mention the status write.
Option id looked up twice updateIssueStatus validates with status in this.statusOptions; the lookup happens once, in setItemStatus.

bun run test:changed (scripts tier) and bun run quality:fix pass.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The review approved with no critical or important findings. The follow-up changes a warning's text, a validation expression with the same behaviour, two comments and two test assertions. No control flow or public behaviour changed.

@malibio

malibio commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Merge queue: #3550 was taken out of the queue — the merge gate failed on it, on top of main. Reproduce by merging or rebasing origin/main and running bun run test:changed, fix, push, and re-run bun run merge 3550.

▶ Merge gate: full pyramid on the rebased PR.
  stage logs: /var/folders/yj/_2dcl_l16cl6dr1bp7h1j6hw0000gn/T/nodespace-gate-logs/_gate-merge-2026-10-04T21-27-15-814Z

▶ quality:scripts:check (scripts/ lint + typecheck) (timeout 10m)
  ✓ quality:scripts:check (scripts/ lint + typecheck) 3s
▶ quality:design-tokens (design-token drift) (timeout 5m)
  ✓ quality:design-tokens (design-token drift) 0s
▶ check-version-sync (app version drift) (timeout 5m)
  ✓ check-version-sync (app version drift) 0s
▶ check-pro-boundary (ADR-081 code-boundary ratchet) (timeout 5m)
  ✓ check-pro-boundary (ADR-081 code-boundary ratchet) 1s
▶ check-node-type-literals (ADR-086 chain-aware type rules) (timeout 5m)
  ✓ check-node-type-literals (ADR-086 chain-aware type rules) 0s
▶ check-lifecycle-reads (ADR-087 one participation check) (timeout 5m)
  ✓ check-lifecycle-reads (ADR-087 one participation check) 0s
▶ skill installer script (tsc) (timeout 5m)
  ✓ skill installer script (tsc) 0s
▶ types:check (generated TypeScript drift) (timeout 15m)
▶ test (frontend, Happy-DOM) (timeout 15m)

$ bun run scripts/gen-types.ts --check
sccache: error: Server startup failed: File exists (os error 17)
Run with SCCACHE_LOG=debug SCCACHE_NO_DAEMON=1 to get more information
error: could not compile `serde` (lib)
Caused by:
  process didn't exit successfully: `/Users/malibio/nodespace/nodespace-core/.tools/bin/sccache /Users/malibio/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rustc --crate-name serde --edition=2021 /Users/malibio/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.228/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no --cfg 'feature="default"' --cfg 'feature="derive"' --cfg 'feature="serde_derive"' --cfg 'feature="std"' --check-cfg 'cfg(docsrs,test)' --check-cfg 'cfg(feature, values("alloc", "default", "derive", "rc", "serde_derive", "std", "unstable"))' -C metadata=aaee17beacbba064 -C extra-filename=-7f77c2b06d88ef17 --out-dir /Users/malibio/nodespace/nodespace-core/.claude/worktrees/_gate/target/debug/deps -C strip=debuginfo -L dependency=/Users/malibio/nodespace/nodespace-core/.claude/worktrees/_gate/target/debug/deps --extern serde_core=/Users/malibio/nodespace/nodespace-core/.claude/worktrees/_gate/target/debug/deps/libserde_core-4a78b979e22553ab.rmeta --extern serde_derive=/Users/malibio/nodespace/nodespace-core/.claude/worktrees/_gate/target/debug/deps/libserde_derive-56b5e4d6c10dbed6.dylib --cap-lints allow --cfg if_docsrs_then_no_serde_core --check-cfg 'cfg(feature, values("result"))' --check-cfg 'cfg(if_docsrs_then_no_serde_core)' --check-cfg 'cfg(no_core_cstr)' --check-cfg 'cfg(no_core_error)' --check-cfg 'cfg(no_core_net)' --check-cfg 'cfg(no_core_num_saturating)' --check-cfg 'cfg(no_diagnostic_namespace)' --check-cfg 'cfg(no_serde_derive)' --check-cfg 'cfg(no_std_atomic)' --check-cfg 'cfg(no_std_atomic64)' --check-cfg 'cfg(no_target_has_atomic)'` (exit status: 2)
✗ cargo run -q … failed (exit 101)
error: script "types:check" exited with code 1
[stage exited with code 1]

  full output: /var/folders/yj/_2dcl_l16cl6dr1bp7h1j6hw0000gn/T/nodespace-gate-logs/_gate-merge-2026-10-04T21-27-15-814Z/types-check-generated-typescript-drift.log

✗ types:check (generated TypeScript drift) failed — merge blocked.
  Fix the failure, push, and re-run: bun run merge <PR#>

malibio and others added 2 commits October 4, 2026 23:33
…oses #3549)

createIssue added the issue to the project board and left its Status
empty, so it sat in no column. It now writes Backlog using the item id
the add returns. The status write is shared with updateIssueStatus.
A failed status write warns and still returns the issue.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… pin the option on the gh:status path

- The status-failure warning includes the error message, so a stale
  status field or option id is diagnosable from the log.
- The gh:status test asserts the option id it writes, now that the
  lookup lives in the shared helper.
- updateIssueStatus validates the status without a second lookup.
- Two caller comments say createIssue also warns on a failed status write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@malibio
malibio force-pushed the issue-3549-gh-create-board-status branch from b2bed40 to 1700c66 Compare October 4, 2026 21:33
@malibio
malibio merged commit 5d50bf0 into main Oct 4, 2026
@malibio
malibio deleted the issue-3549-gh-create-board-status branch October 4, 2026 21:33
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.

Set a new issue's board status to Backlog when gh:create files it

1 participant