Skip to content

developer-experience: bare value-less CLI flags silently corrupt ledger/config state (issue #160) - #161

Draft
ruvnet wants to merge 2 commits into
mainfrom
dream/2026-10-09-developer-experience
Draft

ruvnet wants to merge 2 commits into
mainfrom
dream/2026-10-09-developer-experience

Conversation

@ruvnet

@ruvnet ruvnet commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Nightly Dream Cycle, 2026-10-09. DEEP=developer-experience, SCAN=cli,tui. Full report: docs/dream-cycle/2026-10-09-developer-experience-report.md. Issue: #160.

Hypothesis (frozen before implementation)

Given a dream-machine CLI invocation where a flag parseArgs represents as the boolean true (supplied with no following value) reaches a call site using the (flags.foo as string) || fallback idiom, when each such call site is converted to a new stringFlag() helper — returning fallback only when the flag is genuinely absent (undefined), throwing a clear usage error when it is present but not a string — then every bare, value-less instance of that flag should produce an immediate, clear error: --X requires a value at exit 1 instead of silently corrupting persisted state (the ledger, a generated config) or silently masking real state (an empty-ledger fallback reported as "OK"), while every already-passing invocation with a real string value stays byte-identical.

Candidate

packages/cli/src/index.ts (+1 helper, 9 call sites converted): init --repo/--out, compile --out, schedule --out, ledger <sub> --path, all 10 of ledger append's string fields, tui --path, and freshness stamp --id (added in a second pass after an independent critic caught the first-pass miss). packages/cli/src/index.test.ts: +15 regression tests, one per converted call site.

Evaluation Receipt

Real evaluator: npm test (vitest run && npm run test:governance).

Baseline (main@23a5770) Candidate
vitest 769 passed 784 passed (+15 new, 0 regressions)
governance 150 passed 150 passed (unchanged)
npm run typecheck clean clean
npm run lint clean clean

Live pre/post-fix, all exit 0 → exit 1 with a clear usage error, no state written:

  • ledger append --date (no value) wrote the literal cell true into LEDGER.md pre-fix.
  • ledger verify --path / tui --path (no value) silently reported a real, non-empty ledger as "✓ ledger OK — 0 rows" pre-fix.
  • init --repo (no value) baked "repo": true into the generated config pre-fix.
  • freshness stamp --id (no value) would have written policyId: true pre-fix.

Baseline

Parent commit 23a577082aad39750051dc7ef0ec9d0d72900adf (main). Note: this session's checkout is shallow; the first baseline run showed 3 unrelated governance failures from BASELINE_COMMIT being unreachable (already-tracked issue #149 / open PR #150) — resolved locally with git fetch --depth=200, no code change, before measuring the real 769+150 baseline above.

Darwin Lineage

DARWIN=not-applicable — single validation-shape fix, no evolvable population, same judgment as #103/#122/#141.

Evidence

OBSERVATION (grep + live repro ×4) → MEASUREMENT (769+150 → 784+150, 0 regressions) → INFERENCE (independent critic, separate subagent) → DECISION (one real scope miss found and fixed; one disclosed, not fixed) → DECISION (ACCEPT).

Reward-Hack Check

Independent adversarial critic (separate subagent, given only the diff, no authorship stake) returned APPROVE-WITH-FOLLOWUP: confirmed all repros genuine (reverted the fix, reproduced live) and genuinely fixed; confirmed the 15 new tests are non-tautological (fail on old code, pass on new); confirmed no gold/threshold/test-infra change; confirmed --no-color/--help/--version/--report/--merged correctly left untouched. It caught one real miss — freshness stamp --id used the identical unconverted pattern, mischaracterized by this candidate's first-pass "out of scope" writeup — fixed in a second pass, one more test added, full suite re-confirmed green. Its second finding (verify-entrypoint --cmd, a different pre-existing code shape) is a disclosed follow-up, not folded in.

Security Review

Pure in-memory argument-validation logic. No new I/O/credential/network surface. Strictly more restrictive than before — can only newly reject previously-silently-wrong input, never newly accept something previously rejected.

Regression Analysis

0 pre-existing tests modified or removed. 15 new vitest tests added. Lint/typecheck/build clean.

ADR

None — a CLI argument-validation consistency fix, not an architectural decision.

Gist

GIST=LOCAL — no gh CLI (invalid GH_TOKEN this session) or gist-creation MCP tool available. Full report committed at docs/dream-cycle/2026-10-09-developer-experience-report.md.

Issue

#160

Witness

report_sha256 : ef457ff97dd17817f1f1351031c35f923e3edec5dc4dc69b439606287d7973e0
session_commit: 23a577082aad39750051dc7ef0ec9d0d72900adf
witness       : c76e4b1238ea13ac44549c3b1c3af0351c5f75572bb37a405f477533c05c877a

Reproduce: node packages/cli/dist/bin.js witness verify-report docs/dream-cycle/2026-10-09-developer-experience-report.md --commit 23a577082aad39750051dc7ef0ec9d0d72900adf

Merge Policy

Human review required. This session never self-merges and never autonomously promotes candidate state. dream.config.json's autoMerge: false; no automerge-safe label applied.


🤖 Generated with Claude Code

https://claude.ai/code/session_01MeVxH3JkwJ2ySWc8TsMu3T


Generated by Claude Code

claude added 2 commits October 9, 2026 09:33
…er/config state (issue #160)

parseArgs represents a value-less --flag as the boolean true, not a
string. Nine call sites read an optional flag via `(flags.foo as string)
|| fallback`, where `true || fallback` is `true` -- the sentinel silently
survives as if it were the user's value. Live-reproduced: `ledger append
--date` (no value) wrote the literal cell "true" into LEDGER.md; `ledger
verify --path` / `tui --path` (no value) silently reported an empty
ledger as "OK"; `init --repo` (no value) baked "repo": true into the
generated config. All four exited 0, no error.

Adds `stringFlag()`, mirroring this file's own existing
parseMergedPrNumbers/parseOpenCandidateCount/parseSinceRow/
parseLegacyDigest precedent (fallback on absence, throw on any
non-string value), applied at every affected call site including
freshness stamp --id, found by an independent critic subagent after the
first pass missed it.

769+150 -> 784+150 tests (+15), 0 regressions. lint/typecheck clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MeVxH3JkwJ2ySWc8TsMu3T
Avoided literal | characters in the new row's text -- the finding
describes a code pattern containing ||, which would have tripped the
already-open, unfixed escapeCell/splitRow round-trip bug (issue #144 /
PR #145) and corrupted the row's column count. Verified clean against
CI's actual gate: `ledger verify --since-row 38 --legacy-digest ...` ->
51 rows OK.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MeVxH3JkwJ2ySWc8TsMu3T

ruvnet commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

CI: dependency-audit is failing, but it's not this PR's.

dependency-audit runs npm audit --audit-level=high against the full, dev-inclusive lockfile. It fails identically on both commits pushed so far (740491a, 3f4a62f) with one finding:

source-map-js  1.0.0 - 1.2.1  (severity: high, GHSA-68fv-2mgg-jv7q)

This PR never touches package.json or package-lock.json (git diff origin/main...HEAD -- package.json package-lock.json is empty) — source-map-js@1.2.1 is already pinned in main's own lockfile (pulled in transitively via vite→postcss and @vitest/coverage-v8→magicast), so this check is red on main too, not something introduced here.

Checked for an existing fix to port: open PR #159 (dependabot dev-dependency bump) does not touch source-map-js in its lockfile diff, so there's nothing to port yet.

Proposed patch (not applied in this PR, to avoid widening an unrelated-conceptual-change diff): npm audit fix resolves it with a pure lockfile re-resolution — the declared range ^1.2.1 already permits 1.2.2+ (the fixed version), so no package.json devDependency version bump is needed, just regenerating the lockfile. Recommend a dedicated small PR (or folding into #159) rather than here.

Not re-run (this is a deterministic finding, not a flake) — will keep watching.


Generated by Claude Code

This branch has not been deployed

No deployments
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