Skip to content

lint.yml's "Build the ledgered packages' dependencies" step duplicates the closure refresh once #8330 lands #8331

Description

@os-zhuang

Restart-when: closed #8330, then any PR touches .github/workflows/lint.yml (carry comment-or-removal as a declared rider); or the gate filters and the workflow step filters are observed to diverge

Observation-class finding, filed unassigned from #8271's implementation (PR #8330). Not a defect and nothing is broken by it — recording it so it is not rediscovered.

Blocked-by: #8330 — this is only true once that PR merges.

What

.github/workflows/lint.yml carries a "Build the ledgered packages' dependencies" step immediately before the step that runs pnpm check:type-check-debt:

- name: Build the ledgered packages' dependencies
  run: pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'

- name: Re-measure the type-check DEBT / TEST_DEBT ledger
  run: pnpm check:type-check-debt

PR #8330 moves that build into the gate — refreshBuiltClosure() in scripts/check-type-check-coverage.mjs runs the same command with the same filters, because the local path had no equivalent and a stale dist/ there produced drift that was not in the source. Once it lands, the workflow step becomes a duplicate of work the gate now does itself.

Cost of leaving it

Measured: a closure build with everything already current is 9.5 s (70/70 turbo cache hits). So the duplicate is cache hits, not a rebuild — the reason this is filed as an observation rather than queued.

Why it might still be worth doing

The argument for removing it is not the 9.5 s, it is that a precondition owned in two places drifts: if the gate's filters and the workflow step's filters ever diverge, CI measures a different world than a local run, which is the exact class of defect #8271 was.

The argument against is that any lint.yml edit pulls in the workflow gate family (check:required-contexts, check:workflow-status-functions, check:node-version, check:shard-attestation, check:filter-alias-parity, check:changeset-gate-self-tests, check-changeset-no-major) for a change worth seconds — and that keeping the build visible in the workflow is documentation for a reader of lint.yml, who otherwise cannot see that the step happens at all.

Whoever picks this up should decide between removal and leaving a comment in lint.yml pointing at refreshBuiltClosure(); the second may well be the better trade.


Generated by Claude Code

Activity

  1. added theissue type on Aug 13, 2026
  2. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Concentrated findings round (maintainer-directed, 2026-08-13): HELD (finding + domain:devx + tooling unchanged). Type: Task.

    Why held: premise is not yet true (Blocked-by: #8330 — the duplication exists only once refreshBuiltClosure() merges), and the measured cost of leaving it is 9.5 s of turbo cache hits, while the cost of fixing it standalone is a lint.yml edit that drags the full workflow-gate family for a change worth seconds. The card's own lean is right: a comment in lint.yml pointing at refreshBuiltClosure() likely beats removal (keeps the build visible to a workflow reader, kills the two-owners drift risk).

    Named restart conditions: ① #8330 merged AND the next queued card that already touches .github/workflows/lint.yml — devx seat: name this card in that dispatch brief as a declared rider (comment-or-removal, dev decides against the card's trade-off analysis); ② the gate's filters and the workflow step's filters are ever observed to diverge — that is the drift this exists to prevent, promote immediately.


    Generated by Claude Code

  3. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    State-machine migration, maintainer-authorized (2026-08-13, live triage session: 「我授权你先迁标签」; semantics on #8449): this card is a graded hold — verdict and restart conditions are in the grading comment(s) above — so finding → pm:on-hold. Verdict, restart conditions, domain unchanged.


    Generated by Claude Code

  4. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    ContributorAuthor

    Daily executable-criteria batch (triage seat): Restart-when: stage 1 discharged — #8330 closed. The rider clause is now armed: any PR touching .github/workflows/lint.yml should carry the comment-or-removal as a declared rider; the alternative leg (observed divergence between the gate filters and the workflow step filters) also remains live. Hold stands (trigger-file form).


    Generated by Claude Code

  5. os-zhuang commented on Aug 19, 2026

    @os-zhuang
    ContributorAuthor

    Hold released → pm:queue (triage seat, session session_014qTKTqjme5Fp5BH9iRmy6t, rider-clause audit): the armed rider fired three times without being carried — PRs #9869, #9990 and #10005 all touched .github/workflows/lint.yml after the rider armed, and the "Build the ledgered packages' dependencies" step still runs the duplicate turbo build with no comment pointing at refreshBuiltClosure() (verified on origin/main; refreshBuiltClosure() confirmed at scripts/check-type-check-coverage.mjs:1679). A rider that is 0-for-3 is not a wake mechanism; the comment-or-removal gets its own dispatch. Release double-checks passed.


    Generated by Claude Code

  6. os-zhuang commented on Aug 21, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8331,
      "status": "done",
      "branch": "claude/issue-8331-lint-ledgered-deps-duplicate",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/10568",
      "premise_still_valid": true,
      "summary": "Re-derived on origin/main@ceb33a9f12: the duplication is REAL and byte-identical -- lint.yml:2487 (lane typecheck-debt) and refreshBuiltClosure() at scripts/check-type-check-coverage.mjs:1684 run the same turbo build with the same two filters. The card's decision was comment-vs-removal; the measured answer is COMMENT, and the reason is stronger than the card's 'it is documentation': `--re-measure` checks unbuiltClosure() BEFORE it calls refreshBuiltClosure() and refuses rather than building from nothing (deliberate -- a cold 7-task closure measured 3m2s on a gate people run before pushing), so the refresh owns the present-but-stale case only and cannot rescue a closure member no earlier step built. Of the 55 packages in the ledgered dependency closure the preceding `Build workspace packages` step reaches 54 through ./packages/*; the 55th, @objectstack/service-knowledge (pulled in by the ledgered knowledge-ragflow), arrives only through ./examples/*^... because examples/app-todo happens to depend on it -- the same accident lint.yml already records one step above for #3037. Removing the step would tie the lane's green to that one manifest, with the failure being a hard CI refusal whose remedy message names the deleted command. Two things also moved under the card: the drift argument FOR removal is now loud rather than silent (since #8330 the gate refreshes with its own filters, so divergence ends in that refusal or in extra cache hits, never in CI measuring a different world -- which was #8271), and the cost figure is environment-dependent (185ms of turbo cache hits on this container fully warm, against the card's 9.5s and lint.yml's own ~13s for the sibling step). PR is comment-only: 1 file changed, 30 insertions(+), no step, command, filter or ordering changed.",
      "tests": "Gate union re-derived with `node scripts/pm/dispatch-gates.mjs` (no path arguments) on the committed diff -- 10 families, identical before and after the commit; all run at HEAD 7b2ce42081, AFTER the final commit. Verdict lines as the gates printed them: `check-node-version: OK (32 setup-node step(s) across 26 workflow(s), all on Node 22).` / `check-required-contexts: 6 required context name(s) pinned across 2 workflow(s)` (+ its 124-assertion self-test) / `check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)` (+ 92 assertions) / `check-workflow-status-functions: OK (scanned 26 workflow file(s), 49 job(s), 24 job-level if: expression(s))` (+ 34 assertions) / `check-half-states self-test: 715 cases pass.` / `check-type-check-coverage: OK -- 64/77 workspace packages type-checked` (+ 109 self-test cases) / `check-type-check-coverage --re-measure: OK -- 33 ledger entr(ies) re-measured in 254.2s, 1924 raw tsc error(s) total, none above its recorded number.` / `check-nul-bytes: OK (scanned 6178 text file(s) ... no raw ASCII control bytes).` The two heavy legs went through scripts/pm/os-verify-lock.sh: closure build `VERDICT command-exit 0 - held the lock 194s (3m14s)`, check:type-check-debt `VERDICT command-exit 0 - held the lock 255s (4m15s)`. ABLATIONS, both legs on disk, both restored: (1) closure fully built + turbo present, `rm -rf packages/services/service-knowledge/dist` -- mutation confirmed on disk by inventory (6 files / 1 .d.ts before, directory absent after), NOT by an editor exit code -- gives `Error: --re-measure cannot run: 1 workspace dependenc(ies) ... have no built type entry point on disk -- @objectstack/service-knowledge.`, and grep over that run's output counts 0 turbo task lines, so refreshBuiltClosure() never ran although turbo would have rebuilt it in ~1s; restored by rebuilding that package, md5sum over the dist tree byte-identical to the state the green run measured. (2) Positive control the other way: closure fully built, node_modules/.bin/turbo moved aside (presence asserted both before and after) -- the run gets PAST the refusal and dies inside the refresh instead: `Error: --re-measure needs the workspace's own turbo at .../node_modules/.bin/turbo`. Together they pin the ordering in both directions. (3) Positive control for the set arithmetic behind `C \\ A = 0`: `C \\ (./packages/* alone)` is non-empty (exactly @objectstack/service-knowledge), so the zero is a measurement, not a comparison that cannot see. NOT VERIFIED, stated plainly: a workflow file cannot be executed end-to-end locally -- step ordering and run bodies were verified by PARSING the edited lint.yml with the `yaml` package (typecheck-debt steps 9/10 still `Build the ledgered packages' dependencies` then `Re-measure the type-check DEBT / TEST_DEBT ledger`, both run strings unchanged), and how GitHub schedules the lane is CI's to confirm. DECLARED NARROWING: `node scripts/pm/check-half-states.mjs` without --self-test (the live org-API scan run by half-state-patrol.yml) was not run -- it scans issues and is unaffected by a YAML comment; the --self-test half, which is what lint.yml invokes, is green above. Changeset: none -- publish surface at diff time is empty (one YAML comment), `skip-changeset` applied and READ BACK on PR #10568: `ci/cd | size/s | skip-changeset`.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #10572: lint.yml's aggregator comment declares `Build the ledgered packages' dependencies must stay immediately above the re-measure` as prose, nothing enforces it, and violating it is currently SILENT rather than loud -- because the preceding step's coverage of the closure is total on this tree, moving or deleting the step lets refreshBuiltClosure() quietly do the work and the lane stays green. Names the cheap remedy that needs no new gate and no new lint.yml step (check-type-check-coverage.mjs's observed() already reads lint.yml and already derives CI-wiring facts its evaluate() asserts), and the argument against doing it. Unassigned, `finding`, not queued."
      ]
    }

    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