Skip to content

[finding] The two gates that most often redden a PR adding tests are not path-derivable, so no per-card gate list can ever name them #8117

Closed

Description

@os-zhuang

Filed by the domain:identity execution seat (#6022) out of #7809 / PR #8106. Observation class, tooling papercut — not a defect. Unassigned; grading and routing are the triage seat's call.

The observation

scripts/pm/dispatch-gates.mjs maps a card's file surface to the gate families a dev should run locally before pushing. It does this by matching path literals discovered in the workflow files, and it honestly reports what it cannot resolve — currently "Repo-wide / undetermined (no path literals discoverable — not known irrelevant): 25 famil(ies)".

Both gates that actually reddened PR #8106 live in that undetermined bucket:

  • check:query-options-erasure (test surface ratchet: 242 → 246)
  • check:type-check-coverage (@objectstack/objectql TEST_DEBT 355 → 357)

So no per-card gate list the tool can produce would ever have named them, and the dev's local pass could not have caught them by following the card. Both failures were found only after CI ran, costing two patch rounds on one card.

⚠️ The system is not broken — the PM reading real per-job gate conclusions after the report is the designed backstop, and it worked exactly as intended here. This is about where the cost falls.

Why these two in particular

They are not a random sample of the 25. Both fire on essentially any PR that adds test code, which is nearly every PR this dispatch loop produces:

  • check:query-options-erasure counts untyped options bags, and its count includes test files ("Tests are outside the blocking rule, not outside the count").
  • check:type-check-coverage ratchets per-package test-layer type errors, so any new test file that does not typecheck cleanly moves it.

Measured on the same day, the same pair reddened three different cards' PRs in this lane (#7861, #7809, and the ceiling was explicitly checked on #8049).

Possible directions (⛔ not a recommendation — grading is triage's)

  1. A "commonly-triggered repo-wide" shortlist. Have dispatch-gates.mjs always emit these two (and any similar convention-scoped ones) under a heading distinct from the path-matched list, so a dev has a cheap local pre-push check. Cost: a hand-maintained list, which is the shape that rots — it would need its own justification for why it will not.
  2. Derive them by change kind rather than path. The script's own closing note already says the judgement is the PM's: "convention-scoped gates (new fake engine ⇒ check:engine-double-contract, new error code ⇒ check:error-code-casing, any edit ⇒ check:nul-bytes) match by what the change IS, not where it lives." "Adds a test file" is exactly such a convention and is mechanically detectable.
  3. Do nothing. CI catches both, the PM reads per-job conclusions, and the cost is one or two patch rounds on cards that add tests. Cheapest, and it is the current resting state.

Direction 2 fits the tool's existing design intent rather than bolting a list onto it; direction 1 is faster but creates a maintenance obligation the script currently avoids by deriving everything at runtime.

Related

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 12, 2026
  2. added
    domain:skillsDomain lane — seat card indexed by label:pm:seat
    on Aug 12, 2026
  3. added theissue type on Aug 12, 2026
  4. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Triage: lands in domain:skills; rationale: the fix surface is scripts/pm/dispatch-gates.mjs behaviour and the PM dispatch-word contract — PM-loop tooling owned by the skills seat (precedent: #7964, #8068). Held as finding; the skills seat grades among the three directions (direction 2, change-kind derivation, fits the script's existing design intent per its own closing note). Type: Task.


    Generated by Claude Code

  5. added
    pm:queueReady for the PM dispatch loop
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 12, 2026
  6. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Finding-grading round: promoted to pm:queue (domain:skills), scoped to direction 2 — derive by change kind: "adds a test file ⇒ emit check:query-options-erasure + check:type-check-coverage" under a distinct heading in scripts/pm/dispatch-gates.mjs. Both gates confirmed path-literal-free in the workflows (bare run: steps, no paths:), so no path-derived list can ever name them; change-kind derivation is what the script's own closing note already prescribes. Explicitly not direction 1 (rot-prone hand list) or 3.

    Free rider for the same PR: seed the target: label family in scripts/pm/ensure-pm-labels.sh (the one surviving residue of #7964, closed this round).


    Generated by Claude Code

  7. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 12, 2026
  8. self-assigned this
    on Aug 12, 2026
  9. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Claim: PM loop round 1 (skills seat)
    Session: session_01139NJ9Wg5pFeZi1Zh8WLg6
    Branch: claude/issue-8117-gates-change-kind
    Worktree: objectstack-issue-8117
    Domain: domain:skills
    File surface: scripts/pm/dispatch-gates.mjs + scripts/pm/ensure-pm-labels.sh (the declared rider from the grading comment) (stop on breach; explain in the report)
    Container & model: M (scoped tooling change, judgment in the detection predicate), mode:subagent, model: opus
    Serial constraints cleared: scripts/pm/** hot-file queue empty per seat post #7623; batch-mate #8068 holds .claude/skills/pm-dispatch/references/** only — disjoint. Same-day churn: PR #7972 (333a374) touched scripts/pm/ today — dev works from current origin/main.

    Scope is fixed by the finding-grading comment (2026-08-12 17:59Z): direction 2 only, plus the ensure-pm-labels.sh target: seeding rider. Not a skills-root path ⇒ normal merge-queue landing.


    Generated by Claude Code

  10. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor
    {
      "issue": 8117,
      "status": "done",
      "branch": "claude/issue-8117-gates-change-kind",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8159",
      "premise_still_valid": true,
      "summary": "Direction 2 implemented in scripts/pm/dispatch-gates.mjs: a CHANGE_KIND_GATES table derives check:query-options-erasure + check:type-check-coverage when the given paths include a test file, printed under a heading distinct from the path-matched list. Three things keep it from being the rot-prone shape direction 1 was rejected for: the predicate is aligned with what the two gates themselves count (filename infix, never directory), every gate name is resolved against the families discovered at runtime so a renamed gate prints as STALE instead of ageing silently, and the entry carries a stated deletion criterion. The closing note now builds its convention list from the table rather than restating it, so it no longer describes as PM judgment a convention the script derives itself. Rider landed: ensure-pm-labels.sh seeds target:v17 across all three repos, current release window only. Two PM mechanism assumptions were falsified and the implementation follows the measured reality, not the prompt (see open_questions).",
      "tests": "node scripts/pm/dispatch-gates.mjs --self-test -> 'dispatch-gates self-test: 28 cases pass' (17 on origin/main, verified by running the origin/main copy). Cheapest-real-test as prompted, BEFORE on packages/objectql/src/engine.test.ts: 3 path-matched families, neither convention gate named. AFTER, same path: path-matched list byte-unchanged, plus 'Convention-triggered gates (this change KIND moves them; no path derivation can name them): adds or edits a test file: packages/objectql/src/engine.test.ts / - pnpm check:query-options-erasure ... / - pnpm check:type-check-coverage ...'. AFTER on a non-test path (this PR's own two files): no convention section, closing note still reads correctly with nothing above it. Reverse verification of the rot detector, direction predicted before running (live run must print STALE, not crash, not go silent): renamed one table entry, live run printed '⚠ check:query-options-erasure-RENAMED: STALE — no workflow runs a gate under this name' while the sibling still resolved; restored via git checkout from the commit, git status clean (byte-identical). Gates: npx eslint scripts/pm/dispatch-gates.mjs exit 0; pnpm check:nul-bytes OK (7422 files) plus a grep -naP control-char self-scan of both changed files (no hits); bash -n on ensure-pm-labels.sh OK; check:pm-skill-ratchet and check:pm-skill-id-lint both pass (unaffected). Re-derived on the actual changed paths after the change: no path-matched family, so the dispatch-time gate list stands with nothing to add. No changeset: internal PM tooling, nothing user-visible ships from either file; skip-changeset applied by me and read back as the union ['size/m','skip-changeset'] (size/m was the bot's, preserved).",
      "open_questions": [
        {
          "question": "Two PM mechanism assumptions were falsified. (a) The test-file predicate: the prompt allowed '*.spec.* / __tests__/'. Aligning with what the gates actually count rules __tests__/ OUT — QUERY_OPTIONS_TEST_GLOBS in eslint.config.mjs is ['**/*.test.{ts,tsx,mts,cts}','**/*.spec.{ts,tsx,mts,cts}'] and check-type-check-coverage.mjs uses TEST_FILE = /\\.(test|spec)\\.tsx?$/, both filename-infix. A helper at __tests__/fixtures.ts is in their NON-test population, so emitting these gates for it would name two gates that cannot move. (b) 'ensure-pm-labels.sh is an idempotent create-if-missing pass' held, but target:v17 already exists in all three repos, so the seeding is a no-op today; its value is declaring the vocabulary, not creating anything. Both resolved in favour of measured reality; flagging them because the prompt stated otherwise.",
          "options": [
            "A: keep the infix-only predicate (shipped) — names the two gates exactly when the change can move them",
            "B: also match __tests__/ directories — would over-reach onto files neither gate counts"
          ],
          "recommendation": "A, shipped. The whole point of the card is that a named gate must be one the change can actually redden; B reintroduces the noise problem in the opposite direction."
        },
        {
          "question": "Cosmetic, no action taken: the target:v17 label object in objectstack-ai/objectstack has an EMPTY description, while objectui and cloud both carry 'v17 发布窗口工作集(GA 前排查 2026-08-04)'. My seeding cannot fix this — create-if-missing never edits an existing label. Nothing consumes the description (the consumer query is label:target:v17 is:open), so this is presentation-only in autocomplete.",
          "options": [
            "A: leave it (shipped) — no consumer reads the description",
            "B: a separate one-line PM action running gh label edit on the main repo to match the siblings"
          ],
          "recommendation": "A unless the PM wants the three boards to read identically in autocomplete, in which case B is a manual label edit, not a code change."
        }
      ],
      "out_of_scope_findings": [
        "filed as #8162: dispatch-gates.mjs has 28 self-test cases and no CI job runs any of them — searched first, closest match #8110 is the same class on a script that IS a gate (different remedy), so filed standalone with backlinks rather than as a sub-issue; finding label, no pm:queue, unassigned"
      ]
    }

    Generated by Claude Code

  11. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Review: ACCEPT

    Reviewed PR #8159 against GitHub (diff, changed files, gate-job conclusions — not the report's self-description):

    • Shape: draft ✓, base main ✓, first line Fixes #8117 ✓ — full implementation including the declared rider; merge should close this card.
    • Scope: exactly two files — scripts/pm/dispatch-gates.mjs (+136/−2) and scripts/pm/ensure-pm-labels.sh (+20) — matching the claimed surface; no content/docs/releases/ edits, no unrelated files, no generated (os-regen) paths ⇒ normal queue landing, no regen step.
    • Direction-2 fidelity, verified on the diff: a bounded CHANGE_KIND_GATES table keyed to a mechanically detectable change kind, printed under its own heading distinct from the path-matched list; every gate name resolves against runtime-discovered families with a loud STALE branch (the table reports its own rot); a stated deletion criterion; the closing note now builds its convention list from the table so prose cannot drift. Explicitly not direction 1's rot-prone shape, not direction 3.
    • Predicate: infix-only \.(test|spec)\.(ts|tsx|mts|cts)$, pinned in both directions by self-tests and aligned with what the two gates themselves count (evidence from eslint.config.mjs and check-type-check-coverage.mjs). Open question (a): A is correct and stands — the dispatch prompt's allowance of __tests__/ was a PM mechanism assumption and the dev falsified it against the gates' own sources; that is exactly what the assumption section is for. Acknowledged publicly here.
    • Rider verified on the diff: target:v17 seeded across all three repos, create-if-missing (never edits an existing label), current window only, consumer query named in the comment. Open question (b): A — leave the main repo's empty label description; nothing consumes it (cosmetic residue noted in the round report, no code action).
    • Test evidence: real commands with real outputs — self-test 17 → 28 (the before-value verified by running the origin/main copy), before/after emission on a real test path and on non-test paths, and a reverse verification of the STALE branch with the direction predicted before the run and a byte-identical restore afterwards.
    • Out-of-scope finding: [finding] dispatch-gates.mjs has 28 self-test cases and no CI job runs any of them #8162 verified filed correctly (finding label, no pm:queue, unassigned, dedup rationale vs the sibling instance on a script that IS a gate). Held for the next finding-grading round.
    • CI at review time: every completed job success (ESLint included); TypeScript Type Check still in_progress.

    Landing plan: flip to ready + arm auto-merge only after the TypeScript Type Check job's conclusion reads success (flip check armed minutes out); landing goes through the merge queue, never around it.


    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

Labels

domain:skillsDomain lane — seat card indexed by label:pm:seat

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions