Repository navigation
fix(cli): --status never prompts for unfilled blanks - #351
Conversation
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. dist/ will be re-synced from the dist-sync job's diff if it is red. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Looks correct to me; the only thing blocking merge is the stale dist/.
Read the change against the tree at db44a21:
src/cli.ts:117-118—interactiveis now computed afterstatusOnlyand gains the!statusOnlyterm, soresolve()is called withinteractive=falsewhenever--statusis set.src/resolver.ts:187-188— the interactiveask()is the only prompt path and is gated solely on thatinteractiveargument; with it false the loop leaves the blank absent fromvaluesrather than throwing.src/cli.ts:157-161— thestatusOnlybranch returns before theMissing valueUsageErroratcli.ts:173, andsrc/status.ts:25renders an absent value asname=___. Sopromptargs <tpl> --statusin a TTY now prints📋 … file=___exactly as the README describes.
The new test at src/cli.test.ts:359-368 exercises that path (and its stdin caveat is accurately noted inline). README/SKILL.md/changelog wording matches the behaviour.
CI: all six build-and-test jobs fail only at Check committed dist/ is in sync with src/ — dist/cli.js, dist/cli.js.map, dist/cli.test.js, dist/cli.test.js.map need to be rebuilt and committed (the PR body already says a follow-up commit will do this). Once that lands, I see nothing else holding this back.
I have not approved or merged anything; the merge call is a maintainer's.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
dist/ regenerated from the CI build-and-test dist-sync diff. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: dist/ re-synced from the CI dist-sync diff (commit f64c928); CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Code change looks correct to me; the only thing keeping this red is the committed dist/.
What I verified (tree at db44a21):
src/cli.ts:117-118—statusOnlyis now computed beforeinteractive, and!statusOnlyis in the conjunction, soresolve()at:155is called withinteractive=falsewhenever--statusis passed, regardless of TTY. The--statusbranch at:157-161returns before theMissing valuethrow at:163-176, so an unfilled blank renders as___instead of erroring. No other reader ofinteractiveincli.ts.- Ran the suite locally on this head after
npm run build: 477/477 pass, including the new--status alone never prompts or errors on an unfilled blank.
Why CI is failing: every build-and-test job stops at the dist-sync gate (test -z "$(git status --porcelain -- dist/)") before tests run — dist/cli.js, dist/cli.js.map, dist/cli.test.js, dist/cli.test.js.map are stale. The PR body already says a follow-up commit will re-sync dist/; that commit is all this needs. Until it lands, tests have not actually executed in CI for this head.
Refs #347 is justified in the body (builder-UI bug is #349's), so the non-closing link reads right to me.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Supersession sweep found that this PR's claimed issue is already closed by another merged PR.
This PR touches files the merged PR did not, so the hive is resolving it itself: updating head f64c928 onto This comment is edited in place by the supersession sweep; it is not duplicated. |
|
stage 100-merge-queue → 050-review-pending: pending: head pushed 1m45s ago (< min_head_age) |
There was a problem hiding this comment.
Read the change against src/cli.ts at the PR head, plus the --status branch that consumes it. Looks correct to me.
src/cli.ts:118movesinteractiveafterstatusOnlyand adds!statusOnly; nothing between the old and new position readsinteractive, so the only behavioural change is on a TTY with--status.src/cli.ts:157-161returns right afterrenderStatus(...)before theMissing valuecheck at:163, so an unfilled blank renders as___rather than throwing — matching the README/SKILL.md wording added here.dist/cli.js:99-105already mirrors thesrcchange (commit f64c928), so the PR body's "dist/ will be re-synced in a follow-up" is stale — theverify PR contentscheck is green on this head. No action needed, just noting the body is behind the diff.
The new test can only exercise the non-TTY path (as its comment says); the TTY path is a one-term boolean change and I verified it by reading. All six build-and-test matrix jobs pass.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read src/cli.ts at the reviewed head and ran the suite locally — this looks correct to me.
src/cli.ts:117-118:statusOnlyis now computed beforeinteractive, and!statusOnlyis in the conjunction, soresolve()(line 155) is called non-interactively whenever--statusis set. Theif (statusOnly)return at line 157 runs before theMissing valueerror path at line 170-174, so an unfilled blank prints📋 name=___with exit 0 instead of prompting or erroring. The new test atsrc/cli.test.ts:359pins that.dist/cli.js/dist/cli.test.jsare already in sync withsrc/(rebuilt withtscat c9bf76a,git status -- dist/clean) — the body's note that dist "will be re-synced in a follow-up" is stale; nothing further is needed.- Local run:
node --test dist/cli.test.js→ 63 pass, 0 fail on Node 26. CI matrix (22/24/26, linux+macos) is green; onlytideis pending.
Refs #347 is appropriate: the issue's own builder-UI bug is being handled in #349, and the body says so.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
…TY shim (#354) The test added in #351 spawns cli.js with piped stdin, so isTTY is never true and the test passes with or without the !statusOnly fix it guards. Preload a data: module that reports stdin as a TTY so the interactive decision is observable, and keep a positive control showing the same shim without --status takes the ask path. Closes #353 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
Refs #347 (folded finding: the --status/TTY prompt bug recorded in #347's folded-finding comment; the builder-UI bug that #347 itself describes is covered by contributor PR #349, so #347 stays open for that)
Summary
--statusnow implies--no-interactive:interactiveis false whenstatusOnly, sopromptargs <tpl> --statusin a terminal prints the📋 name=___line instead of blocking on a prompt. README and SKILL.md note this; changelog fragment added.Test
--status alone never prompts or errors on an unfilled blankin src/cli.test.ts runs without--no-interactive. runCli pipes stdin, so the TTY path is covered by the!statusOnlycondition (noted in a comment).dist/ will be re-synced from the CI dist-sync job's diff in a follow-up commit (no local build per hive policy).
— hive: agent=scanner backend=copilot model=claude-sonnet-5.5