Skip to content

fix: advance scanner past consumed --conditions value; drop invalid -C= form - #12

Closed
cldmv-bot[bot] wants to merge 7 commits into
nextfrom
fix/devcheck-condition-detection
Closed

fix: advance scanner past consumed --conditions value; drop invalid -C= form#12
cldmv-bot[bot] wants to merge 7 commits into
nextfrom
fix/devcheck-condition-detection

Conversation

@cldmv-bot

@cldmv-bot cldmv-bot Bot commented Aug 9, 2026 •
edited by Shinrai
Loading

Copy link
Copy Markdown
Contributor

Closing as diverged — superseded by #13.

This PR was auto-opened when a follow-up commit was pushed to fix/devcheck-condition-detection after that branch had already been squash-merged (as #10). Because the branch predates the v1.1.5 release + @cldmv/fix-headers bump now on next, its diff would revert package.json (1.1.5→1.1.4) and package-lock.json alongside the real scanner change.

The intended scanner fix (advance past a consumed --conditions/-C value; drop the invalid -C= form; add -C coverage) is redone cleanly, cut fresh from next, in #13 — diff there is only devcheck.mjs + the test. The stale branch will be deleted.

Shinrai added 7 commits August 8, 2026 08:51
@Shinrai
fix(devcheck): condition-only trigger + execArgv detection + scoped-i…
e2cd0f3 
…nstall guard

Backports the corrected devcheck from @cldmv/holdmytask (CLDMV/holdmytask#12);
addresses the bugs filed in #9. The `uuid-dev` namespacing was already correct;
the check logic around it was not.

- Trigger is now condition-only. Only `--conditions=uuid-dev` selects src/ (via
  the ./main export); NODE_ENV does not affect resolution, so the old
  `!nodeEnv || (!["","development"].includes(nodeEnv) && !hasUUIDDev)` was wrong
  both ways: NODE_ENV=development without the condition stayed silent while
  actually on dist/ (false negative), and the condition set without NODE_ENV
  nagged though correctly on src/ (false positive).
- Detect the condition in process.execArgv as well as NODE_OPTIONS. `node
  --conditions=uuid-dev file.mjs` puts it in execArgv, and that's how vitest
  passes it to workers (worker NODE_OPTIONS is undefined; execArgv carries the
  flag) - so the NODE_OPTIONS-only check missed the CLI/worker form.
- Skip when installed as a dependency, detecting a `node_modules` segment
  anywhere above the file so it works for the scoped `node_modules/@cldmv/uuid`
  layout (a "parent === node_modules" check would miss it). Guards against a
  git/tarball install exiting inside a consumer's app.
- Fix the copy-paste `@Project: @cldmv/slothlet` header to `@cldmv/uuid`.
- Add tests/DevCheck.test.vitest.mjs (9 cases): nags without the condition;
  silent with it via NODE_OPTIONS and via execArgv; NODE_ENV=development alone
  does not silence; still nags after a dist/ build; generic development
  condition rejected; skips in CI; skips when installed as a scoped dep; does
  nothing with no src/.

The nag remains intentional whenever src/ is present and the condition is unset
(even after a build): a source checkout should run from src/ via the condition,
so flagging that you're silently on dist/ is the point.
@Shinrai
fix(review): match --conditions values exactly, not by substring
c8e7c54 
Addresses PR #10 Copilot review: `flags.includes("uuid-dev")` would
false-positive on any flag value containing that substring (e.g.
`--conditions=not-uuid-dev`), spuriously silencing devcheck. Parse the actual
`--conditions` values from execArgv and NODE_OPTIONS (handling `=`/space/`-C`/
comma forms) and match `uuid-dev` exactly. Added regression tests: rejects a
substring-containing condition; accepts uuid-dev among comma-separated
conditions.
@Shinrai
fix: remove vestigial ./devcheck export (points at unshipped file)
6f46387 
The `./devcheck` -> `./devcheck.mjs` export pointed at a file not in the
published `files` allowlist (verified via npm pack: devcheck.mjs isn't in the
tarball), so `import "@cldmv/uuid/devcheck"` 404s for consumers. devcheck is an
internal dev-time guard that index.mjs loads via a relative import, not the
package export - nothing imports the subpath. Removing the dead export makes
package.json honest. Same cleanup as CLDMV/holdmytask.
@Shinrai
fix(review): parse --conditions as whole literal values; revert heade…
6b5a670 
…r; add test header

Addresses the second Copilot re-review on PR #10 (3 suppressed comments):

- devcheck.mjs: stop splitting condition values on `,`/`|`. Node treats each
  `--conditions` occurrence as ONE literal condition and does not split on comma
  or pipe (verified: `--conditions=uuid-dev,x` and `--conditions=uuid-dev|production`
  do NOT enable uuid-dev). The old split caused a false negative - e.g.
  `uuid-dev|production` would silence devcheck while Node actually resolved to
  dist/. Now collect each value whole and match exactly. Fixed the test that
  wrongly asserted comma-joined silences (now asserts it nags), added pipe-joined
  nag, space-separated (`--conditions uuid-dev`) and repeated-flag silent cases.
- devcheck.mjs: reverted the `@Project` header to `@cldmv/slothlet` to match the
  rest of the repo and tools/fix-headers.mjs (`projectName: @cldmv/slothlet`);
  a single-file change to `@cldmv/uuid` is an outlier that fix:headers would
  revert. The repo-wide @project correction (config + restamp of all files) is a
  separate, dedicated change.
- tests/DevCheck.test.vitest.mjs: added the standard project header block so it
  isn't an outlier vs the other test files.
@Shinrai
chore(headers): correct @project @cldmv/slothlet -> @cldmv/uuid repo-…
5ba1a01 
…wide

The header @project across the repo was a copy-paste leftover from the slothlet
template, and tools/fix-headers.mjs was hardcoded to projectName
"@cldmv/slothlet" so `npm run fix:headers` kept re-stamping it. Set projectName
to "@cldmv/uuid" and ran fix:headers, correcting @project on all files (and
re-stamping @Last-modified). This also makes devcheck.mjs's header consistent
rather than an outlier (the earlier single-file change was reverted for exactly
that reason). Folded into this PR per maintainer request.
@Shinrai
fix(headers): remove duplicate header in tsconfig.dts.jsonc
5805b4c 
The repo-wide fix:headers run added a second header block to
.configs/tsconfig.dts.jsonc: the file's pre-existing header used a legacy
single-star `/* ... */` block, which @cldmv/fix-headers does not recognize as
its managed header (it manages the JSDoc `/** ... */` form), so it added a new
`/** */` block on top instead of updating in place. Removed the legacy `/* */`
block, leaving a single recognized `/** */` header. Verified fix:headers is now
idempotent on the file (dry-run reports 0 updates). Addresses the suppressed
Copilot comment on PR #10.
@Shinrai
fix: advance scanner past consumed --conditions value; drop invalid -…
0140802 
…C= form

Syncs the parser fix made in CLDMV/holdmytask#12 (same devcheck scanner):

- The space-form branch (`--conditions x` / `-C x`) consumed tokens[i+1] as the
  value but did not advance the loop index, so a value that itself looks like a
  flag was double-processed. Now increment i past the consumed value token.
- Dropped the `-C=` branch: Node rejects `-C=value` ("bad option"), so it can
  never appear in execArgv/NODE_OPTIONS - dead code. Valid forms are
  `--conditions=x`, `--conditions x`, `-C x`.
- Added -C short-flag test coverage.
@cldmv-bot cldmv-bot Bot added ! fix → next v4 flow: fix contributor PR targeting the next integration branch area: core Touches core library / runtime source code area: tests Touches test files, fixtures, or test infrastructure type: dependencies Relates to dependency updates, version bumps, or package management labels Aug 9, 2026
@Shinrai Shinrai mentioned this pull request Aug 9, 2026
Merged
@Shinrai
Shinrai requested a lite review from Copilot August 9, 2026 03:34
Copilot AI reviewed Aug 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request tightens the devcheck.mjs detection logic for Node’s --conditions flag (including execArgv handling and rejecting invalid -C=<value>), removes an invalid published export for ./devcheck, and updates headers repo-wide to reflect the @cldmv/uuid project name.

Changes:

  • Fix devcheck.mjs to collect --conditions values as exact literals across process.execArgv and NODE_OPTIONS, correctly consuming space-form flag values.
  • Remove ./devcheck from package.json exports (it points at files not shipped in the published package).
  • Apply header normalization/renames across source, tests, scripts, and configs; add a new DevCheck Vitest suite.

Reviewed changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/lib/header-config.mjs Header metadata updated for renamed project.
tools/fix-headers.mjs Updates default projectName used by header-fix tooling.
tests/validators.test.vitest.mjs Header metadata updated for renamed project.
tests/uuid-coverage.test.vitest.mjs Header metadata updated for renamed project.
tests/ultra-short-variants.test.vitest.mjs Header metadata updated for renamed project.
tests/run-vitest.mjs Adds standardized header block to runner script.
tests/rfc-uuids.test.vitest.mjs Header metadata updated for renamed project.
tests/issuer-registry.test.vitest.mjs Header metadata updated for renamed project.
tests/integration.test.vitest.mjs Header metadata updated for renamed project.
tests/entropy-sources.test.vitest.mjs Header metadata updated for renamed project.
tests/DevCheck.test.vitest.mjs New test suite validating devcheck behavior via subprocess fixtures.
tests/default-string-behavior.test.vitest.mjs Header metadata updated for renamed project.
tests/custom-uuid.test.vitest.mjs Header metadata updated for renamed project.
tests/collision.test.vitest.mjs Header metadata updated for renamed project.
tests/bit-utils.test.vitest.mjs Header metadata updated for renamed project.
src/uuid.mjs Header metadata updated for renamed project.
src/lib/versions/timestamp/v2.mjs Header metadata updated for renamed project.
src/lib/versions/timestamp/v1.mjs Header metadata updated for renamed project.
src/lib/versions/timestamp/index.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v8.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v7.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v6.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v4.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v35.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/v1.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/utils.mjs Header metadata updated for renamed project.
src/lib/versions/rfc/index.mjs Header metadata updated for renamed project.
src/lib/versions/issuer/v1.mjs Header metadata updated for renamed project.
src/lib/versions/issuer/index.mjs Header metadata updated for renamed project.
src/lib/versions/index.mjs Header metadata updated for renamed project.
src/lib/validators.mjs Header metadata updated for renamed project.
src/lib/issuer-registry.mjs Header metadata updated for renamed project.
src/lib/entropy-sources.mjs Header metadata updated for renamed project.
src/lib/constants.mjs Header metadata updated for renamed project.
src/lib/bit-utils.mjs Header metadata updated for renamed project.
scripts/test-version-files.mjs Header metadata updated for renamed project.
scripts/test-timestamp.mjs Header metadata updated for renamed project.
scripts/demo-default-string.mjs Header metadata updated for renamed project.
scripts/demo-custom-uuids.mjs Header metadata updated for renamed project.
scripts/debug-timestamp.mjs Header metadata updated for renamed project.
package.json Removes invalid ./devcheck export mapping.
index.mjs Header metadata updated for renamed project.
index.cjs Header metadata updated for renamed project.
devcheck.mjs Fixes --conditions parsing/scan behavior; improves messaging and guards.
.configs/vitest.config.mjs Header metadata updated for renamed project.
.configs/vitest.ci.config.mjs Header corrected for project + copyright range.
.configs/tsconfig.dts.jsonc Header comment normalized to standard block style.
.configs/eslint.config.mjs Header metadata updated for renamed project.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread devcheck.mjs
@Shinrai

Shinrai commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Closing as diverged — superseded by #13.

This PR was auto-opened when a follow-up commit was pushed to fix/devcheck-condition-detection after that branch had already been squash-merged (as #10). Because the branch predates the v1.1.5 release + @cldmv/fix-headers bump now on next, its diff would revert package.json (1.1.5→1.1.4) and package-lock.json alongside the real scanner change.

The intended scanner fix (advance past a consumed --conditions/-C value; drop the invalid -C= form; add -C coverage) is redone cleanly, cut fresh from next, in #13 — diff there is only devcheck.mjs + the test. The stale branch will be deleted.

@Shinrai Shinrai closed this Aug 9, 2026
@Shinrai
Shinrai deleted the fix/devcheck-condition-detection branch August 9, 2026 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Shinrai Shinrai Shinrai left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

area: core Touches core library / runtime source code area: tests Touches test files, fixtures, or test infrastructure ! fix → next v4 flow: fix contributor PR targeting the next integration branch type: dependencies Relates to dependency updates, version bumps, or package management

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Shinrai