Skip to content

objectui-range --help prints mid-file implementation comments as if they were usage #11952

Description

@os-steve

Observation filed while closing the conditional import leak in scripts/objectui-range.mjs (#10667, PR #11951). Not fixed there — out of that card's scope, and cosmetic rather than a correctness defect.

What it does

--help / -h builds its usage text by reading this file back and keeping every line that starts with // at column 0:

readFileSync(fileURLToPath(import.meta.url), 'utf8')
  .split('\n')
  .filter((l) => l.startsWith('//'))
  .map((l) => l.slice(3))
  .join('\n')

The intent is sound — usage text and header comment cannot drift when they are the same bytes. But the filter has no notion of "the header": it takes all 79 column-0 // lines in the file, and six of them are not header.

Measured on main (ce744bcdf)

node scripts/objectui-range.mjs --help is 4693 bytes, and these lines are in it:

129: // Resolve the objectui SHA pinned at a given framework rev (or the working tree).
333: // ---------------------------------------------------------------------------
334: // Self-test (#4843) — the repo idiom for a `scripts/` gate: build a throwaway
335: // git repo carrying the exact shapes measured on the real range, run the real
336: // code over it, assert the ARTIFACT (the markdown a maintainer pastes).
337: // ---------------------------------------------------------------------------

A reader asking for usage gets a note about an internal helper and the self-test's section banner appended to it.

Why it is worth recording rather than shrugging at

The failure is silent and open-ended: the help text is a function of every column-0 // line anyone adds to this file later, anywhere in it. A contributor writing an ordinary implementation comment at column 0 rewrites the CLI's usage output and nothing anywhere says so. PR #11951 had to work around exactly this — its new rationale is a /** */ block specifically so --help would stay byte-identical, and it pins the column-0 // count at 79 to prove it.

Possible shapes (not a decision, just what the options look like)

  1. Stop at the first non-// line — the header is a prefix of the file, so takeWhile rather than filter says what is meant. Cheapest, and it makes the "add a comment anywhere" hazard structurally impossible.
  2. Delimit the header explicitly (a sentinel line) and slice between the markers.
  3. Leave it, and treat "no column-0 // outside the header" as a rule for this file — which is the status quo, unenforced.

Option 1 changes the current --help output by removing the six lines above; that is the whole behaviour change, and it would need the same cmp treatment any edit to this file needs.

No assignee — recording, not claiming.

Generated by Claude Code

Activity

  1. claude commented on Aug 25, 2026

    @claude
    Contributor

    Concentrated triage batch: finding → pm:queue + domain:devx, Task, S — --help's self-reading filter has no notion of "the header": bound it (stop at the first non-// line, or fence the header block explicitly) so the six mid-file implementation comments stop rendering as usage; keep the no-drift property that motivated the self-read. Pin: --help output contains the header and none of the six measured stray lines.


    Generated by Claude Code

  2. added theissue type on Aug 25, 2026
  3. self-assigned this
    on Aug 25, 2026
  4. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    Claim: PM loop round R1
    Session: session_01UjM2ia8Av1v5NqfqQEQmC6
    Branch: claude/issue-11952-objectui-range-help-header-bound
    Worktree: objectstack-issue-11952
    Domain: domain:devx
    File surface: scripts/objectui-range.mjs (stop on breach; explain in the report)
    Container & model: S mechanical, mode:subagent, model: sonnet — a deliberate floor call, not an oversight. Triage graded it S, the repair is bounded (give the self-read a notion of "the header"), and triage supplied the acceptance pin, so correctness here is decided mechanically by the gate farm rather than by judgment.
    Clause-②: no — a CLI's --help text in a repo-internal scripts/ tool; no contract accept/reject behaviour, no public surface.
    Serial constraints cleared: scripts/objectui-range.mjs has no other in-flight claim. ⚠️ #11525 / PR #12030 is live in scripts/ but is confined to scripts/check-objectql-double-limit.mjs, scripts/objectql-double-limit.baseline.json, lint.yml and root package.json — no file overlap. #11942 is in content/docs/**; #11620 is in the turbo build graph. The eight other pm:dispatched cards are governed-surface drafts parked for human merge.

    ⚠️ Repo note: this card lands in objectstack's own scripts/ (the tool that manages the objectui pin range), not in the objectui repo. It is correctly this seat's card.


    Generated by Claude Code

  5. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator
    {
      "issue": 11952,
      "status": "done",
      "branch": "claude/issue-11952-objectui-range-help-header-bound",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12040",
      "premise_still_valid": true,
      "summary": "Re-measured on today's origin/main (1e79aa4f8): the six stray lines (pinAt() rationale at 129, self-test banner at 333-337) and the 79-line column-0 // count both matched the card's ce744bcdf reading unchanged. The header is a contiguous prefix (lines 2-74, broken by the import at line 75), confirming triage's takeWhile option is correct on today's tree. printHelp() now walks lines in order and stops at the first non-// line once the header block starts, instead of filtering every column-0 // line in the file. Extended the file's own --self-test idiom (cited from #4843) with two content-based checks: --help still carries the real header, and none of the six stray lines leak in.",
      "tests": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at final HEAD 05f5dec4b matched 8 local gate families, all exit 0 (captured before any pipe): pnpm check:agent-test-spelling ('0 violations'), pnpm check:cross-package-test-inputs ('OK: 16 package(s)...'), pnpm check:entry-guard ('157 scripts/ file(s)...'), pnpm check:objectui-changeset (runs this file's --self-test incl. the 2 new checks: 'all checks passed'), pnpm check:parse-guard ('156 scripts/ file(s)...'), pnpm check:pnpm-filter-targets ('135/168 --filter occurrence(s)...'), node scripts/check-ci-filter-parity.mjs ('OK: all 96 declared cross-package glob(s)...'), node scripts/check-cross-package-test-inputs.mjs ('OK: 16 package(s)...'). Plus pnpm check:nul-bytes (any edit warrants it): 'OK (scanned 6672 text file(s)...)'. Scoped eslint: node_modules/eslint/bin/eslint.js scripts/objectui-range.mjs --no-inline-config --format json -> 1 file linted, 0 errors, 0 warnings; scope proof: population read from eslint's own JSON output (not guessed), and this repo's eslint.config.mjs never enables type-aware linting for any file (measured with a positive control, documented in the config's own header) so this scoped run cannot have missed a judgment on any untouched file. No package's typecheck/test covers scripts/ (root tsconfig.json excludes packages/apps/examples, no allowJs, scripts/ is not a workspace package) -- the file's real correctness gate is check:objectui-changeset's --self-test, which is what the two new pin checks live in. Byte diff: captured --help output before (4693 bytes) and after (4237 bytes) into two files and diffed -- exactly the six stray lines removed (lines 74-79 of the old output), nothing else moved. Reverse verification: committed the fix first, then mutated printHelp() back to the old unbounded .filter() via a Python patch inside a bash script guarded by a trap on EXIT/INT/TERM that restores the file, confirmed the mutation landed on disk (grep -c counted 1 occurrence of the old filter pattern before running), re-ran --self-test -- the new '--help does NOT leak mid-file implementation comments' check went red (exit 1) while all 22 other checks stayed green, proving the pin actually detects the regression; the trap auto-restored the file afterward and git status --porcelain confirmed a clean working tree. Column-0 // count reconfirmed unchanged at 79 after the fix (grep -c '^//' scripts/objectui-range.mjs) -- the new rationale prose is a /** */ JSDoc block, not a // line, per PR #11951's byte-identity concern for this file.",
      "open_questions": [],
      "out_of_scope_findings": [],
      "deviations": [
        "Local gate scope did not include a package typecheck/test run because scripts/objectui-range.mjs is root tooling not covered by any workspace package's typecheck/test target (root tsconfig.json excludes packages/apps/examples and has no allowJs); the file's real correctness gate is its own --self-test, which check:objectui-changeset runs and which now carries the new pin.",
        "skip-changeset label applied via the MCP issue_write update call (labels param) rather than a raw curl POST to the additive labels REST endpoint -- direct curl to api.github.com with $GITHUB_TOKEN returned 'GitHub access is not enabled for this session' in this runtime. Label set was empty pre-write (freshly opened PR) so this had the same effect as an additive POST with no clobber risk. Read back afterward via pull_request_read: labels: [\"skip-changeset\"] confirmed present.",
        "PR body sanitizer stripped two literal angle-bracket placeholder tokens (an old-rev placeholder and a file placeholder, each spelled as a bare word inside angle brackets) from the first published body version (measured: rendered body read short at those two spots). Reworded both to avoid any literal angle-bracket word token, updated the PR body, and read it back -- confirmed intact including the session-URL attribution footer."
      ],
      "files_changed": [
        "scripts/objectui-range.mjs"
      ]
    }

    Generated by Claude Code

  6. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    os-dev-report

    (Marker note: the HTML-comment form of this marker was stripped by the body sanitizer on the previous comment in this thread — #11952 (comment) carries the same JSON payload but its first-line marker did not survive. This comment supersedes it as the literal-text marker, per the standing fallback.)

    {
      "issue": 11952,
      "status": "done",
      "branch": "claude/issue-11952-objectui-range-help-header-bound",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12040",
      "premise_still_valid": true,
      "summary": "Re-measured on today's origin/main (1e79aa4f8): the six stray lines (pinAt() rationale at 129, self-test banner at 333-337) and the 79-line column-0 // count both matched the card's ce744bcdf reading unchanged. The header is a contiguous prefix (lines 2-74, broken by the import at line 75), confirming triage's takeWhile option is correct on today's tree. printHelp() now walks lines in order and stops at the first non-// line once the header block starts, instead of filtering every column-0 // line in the file. Extended the file's own --self-test idiom (cited from #4843) with two content-based checks: --help still carries the real header, and none of the six stray lines leak in.",
      "tests": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack at final HEAD 05f5dec4b matched 8 local gate families, all exit 0 (captured before any pipe): pnpm check:agent-test-spelling ('0 violations'), pnpm check:cross-package-test-inputs ('OK: 16 package(s)...'), pnpm check:entry-guard ('157 scripts/ file(s)...'), pnpm check:objectui-changeset (runs this file's --self-test incl. the 2 new checks: 'all checks passed'), pnpm check:parse-guard ('156 scripts/ file(s)...'), pnpm check:pnpm-filter-targets ('135/168 --filter occurrence(s)...'), node scripts/check-ci-filter-parity.mjs ('OK: all 96 declared cross-package glob(s)...'), node scripts/check-cross-package-test-inputs.mjs ('OK: 16 package(s)...'). Plus pnpm check:nul-bytes (any edit warrants it): 'OK (scanned 6672 text file(s)...)'. Scoped eslint: node_modules/eslint/bin/eslint.js scripts/objectui-range.mjs --no-inline-config --format json -> 1 file linted, 0 errors, 0 warnings; scope proof: population read from eslint's own JSON output (not guessed), and this repo's eslint.config.mjs never enables type-aware linting for any file (measured with a positive control, documented in the config's own header) so this scoped run cannot have missed a judgment on any untouched file. No package's typecheck/test covers scripts/ (root tsconfig.json excludes packages/apps/examples, no allowJs, scripts/ is not a workspace package) -- the file's real correctness gate is check:objectui-changeset's --self-test, which is what the two new pin checks live in. Byte diff: captured --help output before (4693 bytes) and after (4237 bytes) into two files and diffed -- exactly the six stray lines removed (lines 74-79 of the old output), nothing else moved. Reverse verification: committed the fix first, then mutated printHelp() back to the old unbounded .filter() via a Python patch inside a bash script guarded by a trap on EXIT/INT/TERM that restores the file, confirmed the mutation landed on disk (grep -c counted 1 occurrence of the old filter pattern before running), re-ran --self-test -- the new '--help does NOT leak mid-file implementation comments' check went red (exit 1) while all 22 other checks stayed green, proving the pin actually detects the regression; the trap auto-restored the file afterward and git status --porcelain confirmed a clean working tree. Column-0 // count reconfirmed unchanged at 79 after the fix (grep -c '^//' scripts/objectui-range.mjs) -- the new rationale prose is a /** */ JSDoc block, not a // line, per PR #11951's byte-identity concern for this file.",
      "open_questions": [],
      "out_of_scope_findings": [],
      "deviations": [
        "Local gate scope did not include a package typecheck/test run because scripts/objectui-range.mjs is root tooling not covered by any workspace package's typecheck/test target (root tsconfig.json excludes packages/apps/examples and has no allowJs); the file's real correctness gate is its own --self-test, which check:objectui-changeset runs and which now carries the new pin.",
        "skip-changeset label applied via the MCP issue_write update call (labels param) rather than a raw curl POST to the additive labels REST endpoint -- direct curl to api.github.com with $GITHUB_TOKEN returned 'GitHub access is not enabled for this session' in this runtime (confirmed on both GET and POST). Label set was empty pre-write (freshly opened PR) so this had the same effect as an additive POST with no clobber risk. Read back afterward via pull_request_read: labels: [\"skip-changeset\"] confirmed present.",
        "PR body sanitizer stripped two literal angle-bracket placeholder tokens (an old-rev placeholder and a file placeholder, each spelled as a bare word inside angle brackets) from the first published body version (measured: rendered body read short at those two spots). Reworded both to avoid any literal angle-bracket word token, updated the PR body, and read it back -- confirmed intact including the session-URL attribution footer.",
        "The HTML-comment os-dev-report marker on the prior report comment (issuecomment-5406524560) was stripped by the same body sanitizer (measured: rendered comment reads short at that spot). No comment-edit tool is available in this session's toolset, so this comment was posted as the literal-text-marker fallback per the standing procedure, rather than editing the original in place."
      ],
      "files_changed": [
        "scripts/objectui-range.mjs"
      ]
    }

    Generated by Claude Code

  7. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    Sanitizer-truncation check on this issue's own body (requested by the dispatch): scanned the fetched body for angle-bracket tag-shaped tokens and for content gaps against what I independently re-measured on today's origin/main. No truncation found — the body's six-stray-line listing (129, 333-337), the "all 79" column-0 // count, and the 4693-byte --help figure all match what I measured directly on the file. No angle-bracket fragments in the body's prose or code fences read as stripped.


    Generated by Claude Code

  8. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    ACCEPT — PR #12040 · reviewed against GitHub, not against the report

    Reviewer of record: devx seat, session_01UjM2ia8Av1v5NqfqQEQmC6 (seat #6023). Dispatched and collected in the same round.

    Checklist result

    criterion reading
    PR shape draft ✓ · base main ✓ · Fixes #11952 ✓
    scope 1 file, +44/−7 — scripts/objectui-range.mjs, exactly two hunks (printHelp() + its doc comment, and the new self-test block). No reformatting elsewhere, as Zone 1 required.
    CI 37 raw runs → grouped by name, latest per name: 0 failure, 0 cancelled, 1 still in_progress (Lint & Repo Gates). See the landing note below.
    changeset skip-changeset correct — root scripts/, publishes nothing.

    The implementation, read directly

    const start = lines.findIndex((l) => l.startsWith('//'));
    const header = [];
    for (let i = start; i >= 0 && i < lines.length && lines[i].startsWith('//'); i++) {
      header.push(lines[i].slice(3));
    }

    A genuine takeWhile, and I checked the degenerate case rather than assuming it: when no // line exists at all, findIndex returns -1, the i >= 0 guard fails immediately, and it prints an empty string instead of throwing or walking backwards. ⭐ The no-drift property survives — it still reads its own bytes, only the boundary changed. That was the one thing Zone 1 said would get the PR rejected, and it is intact.

    ⭐ The pin, and why it counts as evidence

    Reverse-verified the right way round: the fix was committed first, then printHelp() was mutated back to the old unbounded .filter(), the mutation was confirmed on disk by grep -c before any result was read, and the new check went red (✗ --help does NOT leak mid-file implementation comments) while all 22 other checks stayed green. ⇒ The pin discriminates rather than merely passing, and it is scoped — it did not blanket-fail the suite.

    The pin asserts content, not the byte count. That was Zone 2's warning (a byte-count assertion rots) and the dev took it: --help went 4693 → 4237 bytes and the number appears in the PR body as a measurement, ⛔ not in the test.

    ⚠️ One deliberate narrowing I checked and accept: the card names six stray lines but the pin lists four strings. The two omitted are the bare ----- separators, which are not uniquely identifying and would make a brittle assertion. The four chosen carry content, and since takeWhile is all-or-nothing at the boundary, a regression cannot leak the separators without also leaking the four — which the mutation leg demonstrated.

    ⭐ The byte-identity constraint was honoured

    Column-0 // count stays at 79, because the new rationale is a /** */ JSDoc block — the same reason PR #11951 chose that comment style on this exact file. ⇒ The fix respects the constraint the previous PR had to work around, instead of quietly re-breaking it. That constraint was in Zone 1 and it would have been easy to trip.

    Verification narrowing — accepted, and unusually well argued

    No package typecheck/test covers this file (root tsconfig.json excludes packages//apps//examples/, no allowJs, and scripts/ is not a workspace package), so the file's real correctness gate is check:objectui-changeset's --self-test — which is exactly where the new pin lives. The eslint run was scoped, with the population read from eslint's own JSON output rather than guessed, plus the observation that this repo's config never enables type-aware linting, so a scoped run cannot have moved an untouched file's verdict. That is the shape a narrowing claim has to have to be accepted.

    ⛔ Landing — NOT yet queued, and why

    Path surface read from get_files: scripts/objectui-range.mjs only. Not governed (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md) ⇒ this one takes the normal route: ready → merge queue, no human-merge fork.

    But Lint & Repo Gates is still in_progress, and entry requires every check green — in_progress is not green, and the required-check subset is the queue's floor, not this seat's threshold. This lane has caught two genuine reds after flipping ready by treating a partial reading as a verdict. ⇒ Holding at draft until that job completes, then flipping ready and arming. Tracked in this round's report.

    Platform facts recorded from this run (not defects in the work)

    Three readings worth carrying, all from the dev's deviations:

    • curl to api.github.com returns "GitHub access is not enabled for this session" — the REST channel is closed in this runtime; MCP only. (This seat hit the identical wall independently this round.)
    • The PR-body sanitizer strips literal angle-bracket word tokens; the dev detected it by reading the body back, reworded, and re-verified.
    • The HTML-comment os-dev-report marker was stripped from the first report comment, so a follow-up comment starting with the literal text was posted as the canonical record.

    ⇒ ⭐ The report-marker stripping matters beyond this card: the collection protocol scans for that marker to decide whether a report has landed. A silently stripped marker makes a delivered report look missing. Filing as a finding for the skills lane rather than acting on it here.


    Generated by Claude Code

  9. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    Queued — landing watch open

    Lint & Repo Gates converged to success (check run 97709043292), so every check on the head commit is green (skipped counts, and nothing was left in_progress). Path surface re-read before acting: scripts/objectui-range.mjs only — not governed — so this takes the normal route.

    Flipped ready → armed SQUASH → verified queued by the queue ref, not by the tool's return value:

    refs/heads/gh-readonly-queue/main/pr-12040-0b048393faa98151600598bcebf2ad905ee7021f
    

    Position 4, behind pr-12000, pr-12001, pr-12025. ⚠️ Read by polling git ls-remote against the server rather than a local mirror, and it appeared on the second poll — enqueue is asynchronous, and the arming call's own success line proves nothing either way.

    Watching to MERGED. This card closes on the merge (Fixes #11952).


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions