chore(tools): validate a created ticket against the identifier orca reported - #610
Conversation
…eported Creating a Linear ticket and validating it are two commands, and every caller so far joined them by typing the identifier into the second one. That identifier is a guess. Measured 2026-07-25: a ticket was created as ORB-87 while the validation ran against ORB-85. ORB-85 exists, belongs to something else, and passed. So the defective ticket read as verified, and the defect (a dependency named in prose with no blockedBy relation) was only caught by re-running the checker by hand. new-ticket.mjs forwards every flag to orca linear create, which already does the creating properly (--body-file - for stdin, project and labels by exact name, --json). The only thing it adds is the link: the identifier is parsed out of orca's own JSON and passed to check-ticket.mjs, so the check cannot target a different issue. A created-but-defective ticket exits 1 and prints both the identifier and the problems, rather than exiting 0 on someone else's ticket. Not reimplemented, deliberately: orca owns creation and this stays a wrapper, per the tools contract's 'prefer delegating over reimplementing'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #610
Recommendation: NEEDS WORK
Summary
tools/new-ticket.mjs is a thin, well-motivated wrapper over orca linear create that closes a real gap (an identifier typed by hand can target the wrong ticket). The delegation design is sound. The problem is verification: this PR's own change to tools/test-tools.mjs (the harness execution gate) adds only the trivial no-args usage-error case for the new script. Every substantive decision path the tool exists to make has zero execution coverage, unlike the same file's existing orcaEnv(...)-stubbed cases for launch-worker.mjs, nudge-worker.mjs, and wave-plan.mjs. That is exactly the failure mode rubric dimension 15 exists to catch: a harness cannot certify itself by being read.
Findings
[High] tools/new-ticket.mjs's core decision paths have zero execution coverage in the harness gate
- dimension: 15 (Harness changes need EXECUTED evidence)
- location:
tools/test-tools.mjs:270-297(gateCases) andtools/new-ticket.mjs:52-108 - issue: the script has five distinct outcome branches beyond bare usage: (1) orca
linear createsucceeds + the created issue passescheck-ticket.mjs→ exit 0, (2) succeeds but the issue is DEFECTIVE → exit 1, (3)linear createitself throws → exit 3, (4) orca reports success but its JSON carries no parseable identifier → exit 3, (5) argv non-empty but missing--title→ exit 2 (a different branch than the argv.length===0 case). The PR's only test-tools.mjs addition is oneINVALID_INPUTrow (argv: [], status: 2), which drives only the argv.length===0 branch. There is nogateCases["new-ticket.mjs"]entry at all, unlike every comparable orca-driven tool in the same file. - risk: the tool's entire reason for existing — validating the ticket orca actually created, using the identifier orca reported — never executes in CI. A regression in the identifier-extraction fallback chain, the exit-code mapping, or the handoff to
check-ticket.mjswould pass Harness Execution and merge undetected. - fix: add
gateCases["new-ticket.mjs"]using the sameorcaEnv([...])pattern aswave-plan.mjs/launch-worker.mjs, stubbing bothlinear createand the nestedlinear issue <id>call (from thecheck-ticket.mjssubprocess it shells out to, which inherits the same env). Cover: success+valid → 0, success+defective → 1, orca failure → 3, missing-identifier → 3. - reference:
.claude/skills/pr-review/rubric.mddimension 15;tools/CONVENTIONS.md"The gate"
[Medium] Identifier extraction guesses across four un-verified JSON shapes
- dimension: 5 (No-workaround / root-cause)
- location:
tools/new-ticket.mjs:76-80 - issue:
parsed?.result?.issue ?? parsed?.issue ?? parsed?.result ?? parsedtries four envelope shapes;check-ticket.mjs's own orca-JSON parsing needs only two (parsed.result ?? parsedthen.issue ?? parsedResult). The extra fallbacks aren't motivated by a comment or the PR body, which never actually ran the create+validate path against real orca. - risk: if the real envelope doesn't match any guessed shape, the tool reports "no identifier" (exit 3) even though creation succeeded — recoverable (raw output printed, tells the caller to check Linear before retrying) but exactly the gap the missing gateCases coverage above would catch.
- fix: confirm the real shape once and collapse to check-ticket.mjs's two-level pattern, or add a one-line WHY if multiple shapes are genuinely possible.
- reference: root CLAUDE.md rule 1
Subagents
parity-checker / i18n-syncer / contract-aligner / security-reviewer / design-reviewer: all N/A — diff is tools/ only, touches no apps/*, orbit-api, or shared contract types.
Validation
Lint, Type Check, Harness Execution all SUCCESS per this PR's own CI run (gh pr view --json statusCheckRollup); Unit Tests was still in progress at review time. Local /validate execution wasn't possible in this review session's sandbox (Bash there blocks any filesystem-writing invocation, including running test-tools.mjs itself), so CI's own run is cited in its place.
Deferred
Dimensions 8-14 are N/A (no apps/*, orbit-api, shared-types, or feature-surface changes in this diff). All three changed files (README.md, new-ticket.mjs, test-tools.mjs) received a verdict.
What's good
Genuine delegation (no reimplementation of orca linear create), the identifier can only come from parsed JSON never a typed arg (the actual bug this closes), distinct exit codes per failure class documented in both the header and --help, correct stdio: ["inherit","pipe","pipe"] so --body-file - still works, and the README catalog entry lands in the same PR.
Recommendation
Add the missing gateCases["new-ticket.mjs"] entry before merge so the create→validate round trip, the defective-ticket path, and the orca-failure path all execute under CI. While there, confirm the real orca linear create --json shape and drop the unverified fallback branches (or justify them with a WHY). Everything else in the diff is clean.
PR #610 review, both findings. [High] The wrapper's whole reason for existing - validating the issue orca actually created - had no execution coverage. Adds gateCases with both orca legs stubbed (linear create, and the nested linear issue that check-ticket makes), driving all five outcomes: valid, defective, orca failure, success with no identifier, and the usage error. [Medium] Collapses the four guessed JSON envelope shapes to the two-level unwrap check-ticket.mjs already uses, confirmed against the live envelope. Also makes --project required. A project-less ticket is invisible to /orchestrate, which is project-scoped by default, so it is created and then never worked on - measured on ORB-86, which /bug created with no project. /bug now routes the ticket into a scope project, with Backlog as the default home, and creates through this wrapper instead of the create-then-validate pair that put a guessed identifier in the loop.
|
There was a problem hiding this comment.
Code Review: PR #610
Scope: PR #610 in thomasluizon/orbit-ui-mobile (chore(tools): validate a created ticket against the identifier orca reported)
Recommendation: APPROVE
Summary
A pure tools/ change: tools/new-ticket.mjs wraps orca linear create and immediately
re-validates the issue it just created using the identifier orca itself reported (parsed
from --json output), closing a real measured failure mode where a hand-typed identifier let
a defective ticket (ORB-87) read as verified against the wrong, unrelated issue (ORB-85).
.claude/skills/bug/SKILL.md and tools/README.md are updated to point at the new wrapper,
and tools/test-tools.mjs gets full coverage of the wrapper's decision paths (success,
defective-but-created, orca failure, success-with-no-identifier, missing --project). The
implementation is a thin, faithful pass-through (execFileSync with an argv array, no shell
interpolation) and every claimed exit code in the PR body matches the code exactly.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
[Info] Established repo pattern followed correctly
· dimension: 5 (no-workaround / root-cause)
· location: orbit-ui-mobile/tools/new-ticket.mjs:1-115
· issue: n/a — praise. The tool does not reimplement `orca linear create`; every flag is
forwarded verbatim, and the JSON-envelope unwrap (`parsed.result ?? parsed`, then
`.issue ?? result`) mirrors the same two-level unwrap already used in `check-ticket.mjs`,
so the two tools stay consistent rather than drifting.
· risk: none.
· fix: none needed.
· reference: CLAUDE.md rule 1; tools/CONVENTIONS.md ("prefer delegating over reimplementing")
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/** or apps/mobile/** file changed |
| i18n-syncer | N/A — no user-facing string / i18n json changed |
| contract-aligner | N/A — no packages/shared/src/types/* / endpoints.ts / orbit-api change |
| security-reviewer | N/A — no orbit-api code changed |
| design-reviewer | N/A — no apps/* or landing-page UI file changed |
Validation
| Check | Result |
|---|---|
| Lint | PASS (CI: Lint job, SUCCESS on this PR's head) |
| Type check | PASS (CI: Type Check job, SUCCESS) |
| Tests | PASS (CI: Unit Tests job, SUCCESS) |
| Build (api) | N/A — orbit-api not touched |
Harness Execution (tools/test-tools.mjs + .claude/hooks/test-hooks.mjs) |
PASS (CI: Harness Execution job, SUCCESS) |
Note: this review's session sandbox blocked local execution of node tools/test-tools.mjs
and gh pr checks; the above PASS verdicts are read from this exact PR head's own
statusCheckRollup via gh pr view --json statusCheckRollup (all 20+ non-review checks
SUCCESS, including Harness Execution, Lint, Type Check, Unit Tests, Cross-Platform Parity,
Contract Drift, Dash Ban, Copy Register, Skill and Agent Frontmatter, Suppressions Ratchet),
not independently re-run by this session. Stated per the verification protocol's honesty
clause rather than claimed as a local execution that did not happen.
Deferred — N/A dimensions & files not verdicted
- Dimension 8 (DESIGN.md/AI-slop), 9 (parity), 10 (i18n), 11 (contract drift), 12
(security, API side), 13 (backend hard rules), 14 (FEATURES.md parity): all N/A, the diff
touches only.claude/skills/bug/SKILL.md,tools/README.md,tools/new-ticket.mjs,
tools/test-tools.mjs— none of those dimensions' surfaces. - All 4 changed files (
tools/new-ticket.mjs,tools/test-tools.mjs,tools/README.md,
.claude/skills/bug/SKILL.md) got a verdict; nothing changed-and-unlooked-at. - Not verified: whether other skills that still call
orca linear createdirectly
(feature/SKILL.md,audit-tests/SKILL.md,audit-performance/SKILL.md,
audit-security/SKILL.md,audit-code-quality/SKILL.md,prod-readiness/SKILL.md,
_shared/audit-to-tickets.md) should also migrate tonew-ticket.mjs. None of those files
are in this diff, so per the rubric's "focus on changed code, not pre-existing issues"
this is out of scope for this review, not a finding — worth a fast-follow ticket, but not
a blocker here.
What's good
- Root-causes a real, measured failure (dated, with the exact wrong-identifier pair) rather
than adding a reminder or a checklist step — the fix makes the wrong behavior structurally
impossible instead of relying on discipline. - Stays a thin wrapper per
tools/CONVENTIONS.md: no reimplementation oforca linear create, every flag forwarded verbatim viaexecFileSync(array argv, not a shell string,
so no injection surface). - Every claimed exit code (0/1/2/3) in the PR body is exactly what the code does, verified
line-by-line againsttools/new-ticket.mjs. - Full decision-path coverage lands in the same PR (
tools/test-tools.mjs): the created+valid
path, created+defective path, orca-failure path, success-with-no-identifier path, and the
missing---projectrefusal — satisfying rubric dimension 15 and the CONVENTIONS.md gate
contract in the same change, not as a follow-up. .claude/skills/bug/SKILL.mdandtools/README.mdare updated in the same PR so the new
tool is actually reachable from the workflow that needed it, not left orphaned.
Recommendation
Approve as-is. Optional, non-blocking fast-follow: point the other ticket-creating skills
(feature, the four audit-* skills, prod-readiness, _shared/audit-to-tickets.md) at
tools/new-ticket.mjs too, since they carry the same guessed-identifier risk this PR just
closed for /bug — worth its own ticket rather than scope creep here.
1 participant
What
tools/new-ticket.mjs, a thin wrapper overorca linear createthat validates the issue it just created.Why
Creating a ticket and validating it are two commands, and every caller so far joined them by typing the identifier into the second one. That identifier is a guess.
Measured 2026-07-25: a ticket was created as
ORB-87while the validation ran againstORB-85. ORB-85 exists, belongs to something else, and passed. The defective ticket read as verified. Its real defect (a dependency named in prose with noblockedByrelation) surfaced only because the checker happened to be re-run by hand.That is the failure mode worth closing: not a check that fails, a check that passes against the wrong subject.
What it does not do
It does not reimplement creation.
orca linear createalready handles--body-file -for stdin, project and labels by exact name,--state,--estimateand--json, and every flag is forwarded verbatim. The tools contract says prefer delegating over reimplementing, so this stays a wrapper.The only thing added is the link: the identifier is parsed out of orca's own JSON and handed to
check-ticket.mjs. It cannot be typed.Behaviour
check-ticket.mjsExit 3 on a missing identifier is deliberate: if there is nothing to validate, saying so beats exiting 0 on an unchecked ticket.
Verification
Coverage entry added in the same change, so the structural guard from #606 stays satisfied.
🤖 Generated with Claude Code