Skip to content

[quality] Two unreachable defensive sub-expressions permanently cap source region coverage #700

Description

@hivecommons-hive

Finding

Two sub-expressions in the shared lib/ helpers are provably unreachable. No
test can ever cover them, so they permanently cap --check-source-regions below
100% and will keep showing up as "uncovered regions" in every future audit.

Raw merged V8 region coverage (node tests/tools/coverage-report.mjs, full
node --test suite, TZ=UTC, at b54cf81) reports:

src/lib/profile-links.mjs         25/26 regions   uncovered line: 66
scripts/lib/project-assets.mjs    54/55 regions   uncovered line: 69

Every other uncovered source region in the repository is already claimed by an
open PR (#678, #680, #686, #688, #693). These two are the remainder, and they
are not a testing gap — they are dead code.

1. src/lib/profile-links.mjs:66 — if (!url.hostname) return null;

  if (!ALLOWED_PROTOCOLS.has(url.protocol)) return null;
  // A userinfo component is only ever used here to disguise the real host.
  if (url.username || url.password) return null;
  if (!url.hostname) return null;

ALLOWED_PROTOCOLS is {'http:', 'https:'}. Both are WHATWG special schemes,
and the URL parser rejects a special-scheme URL with an empty host outright —
it never yields a parsed URL whose hostname is ''. Probed:

"http:"        -> THROWS        "http://"      -> THROWS
"http:/"       -> THROWS        "https://:80/" -> THROWS
"http://?q"    -> THROWS        "https://#f"   -> THROWS
"http://:@"    -> THROWS        "https:////"   -> THROWS
"http:///x"    -> hostname "x"  "https:/foo"   -> hostname "foo"
"http:foo"     -> hostname "foo" "http://."    -> hostname "."

So by the time control reaches line 66, url.hostname is always non-empty.

Exact replacement — delete line 66 and the blank line above it is kept:

  if (!ALLOWED_PROTOCOLS.has(url.protocol)) return null;
  // A userinfo component is only ever used here to disguise the real host.
  if (url.username || url.password) return null;

  return url.href;

If the guard is deliberately kept as belt-and-braces against a future protocol
being added to ALLOWED_PROTOCOLS, say so in a comment and widen
ALLOWED_PROTOCOLS in the same change so the branch becomes reachable — an
unreachable guard that no test can pin is not defence, it is noise.

2. scripts/lib/project-assets.mjs:69 — the || null in return file || null;

  const file = segments[segments.length - 1];
  const extension = file.slice(file.lastIndexOf('.')).toLowerCase();
  if (!file.includes('.') || !MIRRORABLE_ARTWORK_EXTENSIONS.has(extension)) {
    return null;
  }
  if (!name || file === name) return file || null;

For || null to be taken, file must be falsy, i.e. file === '' (it is always
a string, from String.prototype.split). But ''.includes('.') is false, so
an empty file has already returned null two lines earlier. Confirmed against
every trailing-empty-segment shape: "/", "//", "a/", "a//", "///",
"x/y/" all return null from the extension guard, plus a 4096-combination
sweep over segment shapes that never reaches it.

Exact replacement for line 69:

  if (!name || file === name) return file;

Recommendation

Apply both replacements above in a single PR. Combined effect: source region
coverage for these two files goes to 55/55 and 26/26, and — once #678, #680,
#686, #688 and #693 land — the repository reaches 100% source region coverage,
which makes the --check-source-regions gate proposed in #674 meaningful rather
than permanently short of its own ceiling.

  • Delete if (!url.hostname) return null; from src/lib/profile-links.mjs
  • Change return file || null; to return file; in scripts/lib/project-assets.mjs

Existing tests in tests/profile-links.test.mjs and
tests/project-assets.test.mjs already pin the surrounding behaviour and should
pass unchanged; no test edit is required.

This change is production code, so it is outside the quality lane's PR
mandate — it needs a human or a lane that may touch src/ and scripts/ to
land.
The replacement text above is exact, so applying it is mechanical.

Evidence and provenance

  • Suite: TZ=UTC node --test (full unit suite), NODE_V8_COVERAGE collected raw,
    merged through this repository's own tests/tools/coverage-report.mjs
    collect() / summarizeRegions() (per-character max merge, not offset-keyed).
  • Revision: b54cf81 (origin/main), clean npm ci.
  • Reachability is established by control-flow analysis and direct probing, not by
    absence of coverage, so no end-to-end evidence is needed or claimed: an
    unreachable expression cannot be covered by any suite.

Priority

  • Impact: medium
  • Effort: low

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5n31 | SHA: b54cf81

— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88

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

    agent/qualityApproved by a Hive merger/owner for auto-merge on green CIhive/hosted-available-lke648397-260827-5n31Approved by a Hive merger/owner for auto-merge on green CIqualityApproved by a Hive merger/owner for auto-merge on green CItestingApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions