Skip to content

0.7.0 shipped [directive] as unreachable: commander's [] default makes the nullish fallback to configured authors dead code #484

Description

@MongLong0214

Released in 0.7.0, whose headline change was that [directive] became reachable. It did not.

Reproduction against the released artefact

Fresh repository, one authored record, commitlore.trustedAuthor written the way init writes it:

$ node dist/commitlore.mjs inject --path pricing.ts
  [claim]      r-trust99  …  shared Redis cache | ops refuses another stateful dependency

$ node dist/commitlore.mjs inject --path pricing.ts --trusted-author owner@example.invalid
  [directive]  r-trust99  …  shared Redis cache | ops refuses another stateful dependency

The first form is what CLAUDE_HOOK_COMMAND runs. Every record on every install still graded [claim] — the exact condition #415 was opened about.

Cause

src/commands/inject.ts declares the option with a default:

.option('--trusted-author <author>', '…', collect, [])

so options.trustedAuthor is [], not undefined, when the flag is absent. The fix for #415 then wrote:

const trustedAuthors = options.trustedAuthor ?? configuredTrustedAuthors(cwd);

[] is not nullish. The fallback never fired. #415's defect, reintroduced one layer up by the fix for it.

Why the tests passed

test/trusted-authors.test.ts drove buildInjection with options assembled by hand:

buildInjection({ path, cwd: repo, noIndex: true, trustedAuthors: configuredTrustedAuthors(repo) })

That asserts the grading layer works given correct options. It cannot see whether the CLI ever produces them, and the CLI did not.

The file's own header had already written the warning, one layer down:

"a unit test of gradeRecord would have passed throughout the entire period this bug existed"

The same sentence applied to buildInjection and was not heard.

Fix

Resolve on length rather than nullishness — an explicit flag is always non-empty, an absent flag is always empty, whichever shape commander hands over:

const flagged = options.trustedAuthor ?? [];
const trustedAuthors = flagged.length > 0 ? flagged : configuredTrustedAuthors(cwd);

Four new cases in test/trusted-authors.test.ts spawn the built CLI rather than importing into it. They fail against the 0.7.0 build.

Also in the patch

package-lock.json declared 0.1.0 while both manifests read 0.7.0 — stale since the first release.

Process failures worth recording

  • The promotion was merged and tagged before the review verdict arrived. The verdict was FAIL, and it named this defect.
  • The evidence submitted for that review contained two miscounts: 132 commits (it was 137) and seven install checks in RELEASE-GATE.md §4 (it lists six).

0.7.0 is not retracted; its release notes now carry the defect at the top and point at 0.7.1.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions