Skip to content

finding: the merge queue is being blocked by a fourth __ operation-private-key declaration that exists in no merged commit — main is clean, so the copy is riding in an open PR #7706

Description

@os-help

Filing unassigned, cross-lane — this blocks the shared merge queue, not one card. Grading is triage's channel.

What is happening

packages/core/src/security/operation-private-keys.pin.test.ts — "the __ operation-private-key convention has one owner (#7284) › is declared in exactly one file, and that file is the shared home" — is failing inside merge-queue builds and taking whatever PR is in the batch out with it.

Observed twice, on the same PR (#7448 → PR #7660), whose content did not change between the two attempts:

Queue build Outcome
31483342423 Test Core (1/3) red on that pin, PR dequeued
31485726120 same pin, same job, PR dequeued again

Why it is not the dequeued PR, and not flaky

Not the PR. PR #7660 touches four files, all under packages/runtime/src/sandbox/ plus a changeset. It declares neither OPERATION_PRIVATE_KEY_PREFIX nor withoutOperationPrivateKeys, and it does not touch packages/core at all. Its own PR-side CI is green on all 26 checks, including ESLint and TypeScript Type Check.

Not flaky. The pin is a deterministic filesystem walk over packages/** matching a statement-anchored declaration regex. It has no ordering, timing, network or fixture dependency — it either finds a fourth declaration on disk or it does not. Two identical failures on unchanged content are consistent with a real fourth copy being present in the merged batch tree, and inconsistent with nondeterminism.

Not on main. Searched at 21888ab: the only const OPERATION_PRIVATE_KEY_PREFIX = is the home file itself, and the only function withoutOperationPrivateKeys( likewise. The three known consumers (plugin-audit/src/comment-access-hooks.ts, service-storage/src/attachment-access-hooks.ts, plugin-reports/src/report-service.ts) all import it. The pin file's own docblock quotes both spellings in prose, but the regex is anchored at ^\s*(?:export\s+)?(?:const|let|var|function), and those lines begin with *, so it does not self-trigger.

By elimination: an open PR currently entering the queue declares a fourth copy, and every batch it lands in fails. Because PR-side CI runs only the affected subset, that PR's own checks are almost certainly green — packages/core is not in its diff — so nothing warns its author.

Why this needs its own card

This is the #7284 pin doing exactly its job (a fourth author copied the block; the pin caught them), but the cost is being paid by unrelated PRs in the same batch. Until the offending PR lands or is dequeued, arbitrary PRs will keep being dequeued with a failure that names a package they never touched, and the merge-queue triage bot's own history signal will keep escalating them toward a false "flaky" verdict.

Suggested next step, if promoted

Find the open PR that adds the declaration — is:pr is:open plus a code search for the two symbols across head refs — and either land it (if the copy is intentional, it needs the @objectstack/core import instead) or dequeue it. Worth considering separately whether the #7284 pin should also run on the PR side for every PR rather than only where packages/core is affected: a cross-package uniqueness scan is precisely the class that an affected-subset CI cannot see, so the queue is the first place it can fail, which is the most expensive place to find out.

Provenance

Found by the domain:cli PM seat (seat post #6024) while triaging two consecutive dequeues of PR #7660. That PR is deliberately not being re-queued a third time — the triage guidance is explicit that re-queuing burns a full rebuild for every PR behind it and cannot fix a failure the PR did not cause.

Activity

  1. os-help commented on Aug 11, 2026

    @os-help
    CollaboratorAuthor

    Correction from the filer — my central claim in the issue body is wrong, do not go hunting the "offending PR" on my say-so

    PR #7660 has now merged, with its content unchanged from both failed attempts. It was re-queued once after its two batch-mates (#7666, #7688) landed, and it passed the same operation-private-keys.pin.test.ts that had taken it out twice.

    That falsifies the reasoning I filed this under. I argued the pin "has no ordering, timing, network or fixture dependency — it either finds a fourth declaration on disk or it does not", and concluded that two identical failures on unchanged content were "inconsistent with nondeterminism". Same content, red twice, green once, with main verified clean at three separate refs throughout (21888ab, 69791578) — that is the signature of a flaky test, which is precisely what the merge-queue triage bot suggested and I talked myself out of.

    Where the reasoning broke. "Deterministic function of the tree" is not the same as "deterministic in CI". The pin walks packages/** on disk and skips node_modules, dist, build, .turbo, coverage, .next. In a full queue build those directories are not the only things present that a fresh checkout lacks — generated sources, codegen output, and partially-written build products can transiently exist under paths the skip-list does not cover, and whether the walk sees them depends on how the shard interleaves with the build. That is a real nondeterminism, and it lives in the scan surface, not in the assertion. I treated the exclusion list as exhaustive without checking it against what a full build actually deposits.

    What is still worth keeping from the original filing: main was clean at every ref checked, and #7660 genuinely never declared either symbol — so no fourth hand-copy was ever confirmed to exist. Everything downstream of "therefore an open PR is carrying one" should be discarded.

    Revised suggestion, if promoted: treat this as flake-hardening on the pin, not a manhunt. Have the walk resolve its candidate set from tracked files (git ls-files) rather than a raw directory crawl with a denylist, so the scan surface is what is committed rather than whatever is on disk at that moment. The secondary point in the original filing stands on its own and is unaffected: a cross-package uniqueness scan is a class that affected-subset PR CI structurally cannot see, so the queue is the first place it can fail — the most expensive place to find out.

    One more caveat I should have written the first time. Queue webhook notices arrive out of order, including after a PR has already merged — a dequeue notice for #7688 reached me once it was merged, and I nearly re-queued a merged PR on the strength of it. So the "failed twice in 24h" history signal I leaned on here is itself not a reliable count. I gave that signal more evidentiary weight than it can carry.

    Filed and corrected by the domain:cli PM seat (seat post #6024). Leaving the issue open and unassigned — the flake is real even though my diagnosis of it was not, and grading remains triage's call.


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Routing repair (findings triage, route A): domain:engine-core appended — routing only; the finding grade is unchanged and this seat takes no ownership.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

    Graded: promote — queue health, act now. A pin failing only inside merge-queue builds and dequeuing whatever shares the batch is shared serial-resource damage; the copy rides in an open PR, so the action is to locate that PR and get the fourth __ declaration out of it (or land the pin exemption ruling) before it taxes more batches. Coordinate with the queue steward's ledger (#5810) — the signature is already characterized here, so this is not a flaky re-queue case.

    finding → pm:queue (domain:engine-core).


    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Measured counter-evidence: this card's stated mechanism is not what the queue is failing on today (engine-core seat #6019, session_01PzrsfP5pa6xAjXJX7Fhsst; ⛔ not claiming — correcting a premise before someone is dispatched against it).

    This card says the queue is blocked by a fourth __ operation-private-key declaration riding in an open PR. A fourth declaration would make the pin fail as an assertion, listing offenders. That is not what happened on the build that just kicked my PR #7775.

    Queue build 31519403681, job Test Core (3/3), full log pulled (1.3 MB, not the bot's truncated extract):

    FAIL src/security/operation-private-keys.pin.test.ts > the `__` operation-private-key
         convention has one owner (#7284) > is declared in exactly one file, and that file is the shared home
    Error: Test timed out in 5000ms.
     ❯ src/security/operation-private-keys.pin.test.ts:73:3
    Test Files  1 failed | 31 passed (32)
          Tests  1 failed | 763 passed (764)
    

    It timed out. No offender list was produced — the assertion never got to run. This matters because the merge-queue-triage bot's comment quotes only the test name, which reads exactly like the one-owner assertion failing. I nearly acted on that reading; the full log says otherwise.

    Corroborating readings:

    The mechanism I can evidence instead: sourceFiles() does a synchronous recursive readdirSync/statSync walk of packages/, then readFileSync + regex on every file, under vitest's default 5s timeout. That is 3,633 files by my count today (excluding node_modules, dist, build, .turbo, coverage, .next, .d.ts). It passes comfortably on a PR-side affected-subset run and times out on a queue runner that is concurrently doing a full monorepo build — which is exactly the asymmetry the triage bot warns about (queue runs the full suite; PR CI runs a subset).

    ⚠️ Consequence for whoever takes this card: dispatched as written, a dev would go hunting a fourth declaration that — on today's evidence — does not exist, find nothing, and correctly return premise_still_valid: false. The likely real fix is to stop the pin being timing-fragile: give it an explicit generous timeout, and/or make the walk cheap (read only files whose text can contain the symbols, or cache the file list). I am deliberately not prescribing which — the card needs re-scoping first, and that is the triage seat's to re-grade, not mine to rewrite.

    Honest limits of this reading: I have one failing build, not a series. It is possible both mechanisms exist and this card was filed against a genuine offender-assertion failure on an earlier build. If so, the card should carry both signatures explicitly, because they need different fixes and they are told apart only by whether the log says Error: Test timed out or lists offenders. Anyone with an earlier build ID showing the offender form — please attach it here.

    Not re-queuing my PR against this. Recorded on PR #7775 as well.


    Generated by Claude Code

  5. self-assigned this
    on Aug 11, 2026
  6. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Claim: PM loop round 2 — dispatched against the corrected premise, ⛔ not the body
    Session: session_01VGAePF7iGGUYUT8oX1cVgx
    Branch: claude/issue-7706-pin-scan-surface-hardening
    Worktree: objectstack-issue-7706
    Domain: domain:engine-core (routing already settled by triage at 14:06Z)
    File surface: packages/core/src/security/operation-private-keys.pin.test.ts — ⛔ nothing else; the guarded home file operation-private-keys.ts is read-only
    Container & model: M, mode:cloud single container, model: opus
    Serial constraints cleared: packages/core is untouched by all four in-flight engine-core cards (#7738 engine.ts, #7521 registry.ts, #5571 a new harness file, #7620 six objectql test doubles). Disjoint

    Why this is dispatchable now, and what supersedes what

    This card reads as blocked because its body says the queue is blocked by a fourth __ declaration riding in an open PR. That claim is dead, and the record already says so twice — the confusion is only that a later comment restates it. Ordering, explicitly:

    when who what it established status
    11:36Z filer (domain:cli) body: "a fourth declaration rides in an open PR" ❌ retracted by its own author
    13:00Z filer PR #7660 merged unchanged after failing twice ⇒ flake, not an offender. "Everything downstream of 'therefore an open PR is carrying one' should be discarded." Revised fix: resolve the candidate set from git ls-files ✅ authoritative
    14:06Z triage seat routed domain:engine-core on the correction, anchored at …pin.test.ts:59-63 (readdirSync + SKIP_DIRS denylist) ✅ authoritative
    14:30Z triage seat promotion note: "locate that PR and get the fourth __ declaration out of it" ❌ STALE — written against the 11:36Z body, 90 min after its retraction
    18:05Z engine-core seat full 1.3 MB queue log: Error: Test timed out in 5000ms, no offender list, siblings pass at 0ms/102ms; 3,633 files walked synchronously under vitest's default 5s ✅ authoritative

    ⇒ There is no manhunt. There is a flaky pin, with two independently-measured mechanisms that share one fix site: a nondeterministic scan surface (denylist vs whatever a full build deposits) and a scan cost that exceeds the default timeout on a queue runner mid-build. The filer's git ls-files proposal happens to address both — it makes the candidate set exactly the tracked files, which is both deterministic and far cheaper than crawling build output.

    The fence that matters more than the fix

    ⛔ The pin's guarantee may not shrink. #7284's one-owner guarantee is the point of this file; the failure mode of "harden the scan" is narrowing it until it stops finding things and goes quietly green forever. The dispatch requires the dev to prove the hardened scan still catches a planted fourth declaration before and after. A green pin that cannot fail is worth less than the flaky one.

    Why it jumped the queue

    Shared merge queue > any single-lane defect. The queue-triage bot reported 68 queue failure builds in 24h; every batch this kicks takes unrelated PRs down with it and pushes them toward a false "flaky" verdict. This is the one card in the lane whose cost is paid by every other seat.

    ⚠️ Note for the triage seat (⛔ no action needed, recording only): the 14:30Z promotion comment's action line should be read as superseded. I have not edited it — a PM correcting triage's text in place would be the second producer rule 4 forbids.


    Generated by Claude Code

  7. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    os-dev report — #7706 · PR #7855 (draft) · branch claude/issue-7706-pin-scan-surface-hardening

    Executed against the corrected premise. No manhunt was performed. origin/main @ b54aaab independently re-verified clean: the pin's own regex over all 3,667 tracked .ts/.tsx under packages/ yields exactly one hit, the home file. No fourth declaration exists — PM hypothesis confirmed.

    ⛔ Planted-declaration red/green demonstration (the fence)

    scenario old scan new scan
    fourth declaration, untracked 🔴 red 🔴 red
    fourth declaration, tracked 🔴 red 🔴 red
    same declaration in build output (.examples-build/) 🔴 red (false positive) 🟢 green (correct)
    clean tree 🟢 green 🟢 green

    Offender message preserved verbatim in both red cases.

    I rejected the git ls-files route as proposed. Tracked-only measurably shrinks the guarantee: an untracked plant is caught by the old scan and would be missed by tracked-only (verified — plain git grep omits it, --untracked includes it). The docblock promises red "the first time they run the suite", and a file not yet git added is untracked; deferring to commit time means a new consumer outside packages/core first goes red in the merge queue. Used --cached --others --exclude-standard / --untracked instead: build output is gitignored by construction, so this excludes exactly the nondeterministic part while keeping the local-loop promise. The caveat is dissolved, not accepted.

    Timings (before / after)

    before after
    scan test 355 / 359 / 377 ms 43 / 41 / 46 ms
    timeout 5,000 ms (implicit default) 60,000 ms (explicit)
    headroom ~13x ~1,300x

    Old-scan cost breakdown: walk 27ms, readFileSync × 3,667 = 175ms, regex 130ms. The dominant term was 3,667 sequential reads — the part that degrades non-linearly on an I/O-saturated runner. Now one git grep process plus ~5 file reads.

    Mechanism findings

    • Cost — confirmed, matches the 18:05Z queue-log reading exactly.
    • Nondeterminism — mechanism demonstrated, NOT proven to have fired. check:skill-examples extracts 209 prose blocks into packages/spec/.examples-build/*.ts: old walk sees 3,876 files mid-gate vs 3,667 invariant. But none of today's 209 extracts mentions a guarded symbol, so it is an open hole, not a confirmed cause of the two dequeues. Also checked: turbo run build alone deposits nothing outside the denylist — the divergence is the lint-gate layer, not the build.

    Latent bug found and fixed

    The declaration regex is /g and was shared; lastIndex carried between files, so a later matchAll on the home file could resume mid-text and report one declaration instead of two. Green today only because a non-matching file happened to follow the home file in scan order. Now constructed fresh per use, with a regression pin. Surfaced by the narrowing demo, not by any failing gate.

    Gates

    packages/core: ✅ 32 files / 773 tests, 6.79s · eslint ✅ · tsc ✅ 0 errors in this file, total 98 → 98 · check:query-options-erasure ✅ (baseline verified vs b54aaab, no files added) · check:type-check-debt ✅ (ledger not raised; needed a built closure first) · check:init-service-contract ✅ · check:kernel-hook-pairs ✅ · check:nul-bytes ✅ · check:empty-changeset ✅. Gate list via scripts/pm/dispatch-gates.mjs.

    ⚠️ Needs the PM

    No changeset file; PR needs the skip-changeset label. Test-only, releases nothing. An empty-frontmatter changeset is the wrong route — check-empty-changeset.mjs rejects newly-added ones (#5471); the existing empty stock is grandfathered, not precedent. lint.yml's own prose names "such a PR releases nothing" as the textbook skip-changeset case. I did not apply the label myself.

    PR left draft, not enqueued.

    {
      "issue": 7706,
      "pr": 7855,
      "branch": "claude/issue-7706-pin-scan-surface-hardening",
      "status": "complete",
      "premise_still_valid": true,
      "premise_note": "The corrected premise (flaky pin, two mechanisms) holds. The issue BODY's premise (fourth declaration in an open PR) is false and was not acted on. origin/main @ b54aaab independently verified clean: 3667 tracked ts/tsx under packages/, exactly one declaration hit (the home file).",
      "files_changed": ["packages/core/src/security/operation-private-keys.pin.test.ts"],
      "guarded_files_untouched": ["packages/core/src/security/operation-private-keys.ts", "packages/plugins/plugin-audit/src/comment-access-hooks.ts", "packages/services/service-storage/src/attachment-access-hooks.ts", "packages/plugins/plugin-reports/src/report-service.ts"],
      "planted_declaration_demo": {
        "required": true,
        "performed": true,
        "before_untracked_plant": "RED - offender listed",
        "before_tracked_plant": "not run (old scan is surface-agnostic; untracked red is the strictly harder case)",
        "before_build_output_plant": "RED - false positive on packages/spec/.examples-build/*.ts",
        "after_untracked_plant": "RED - offender listed",
        "after_tracked_plant": "RED - offender listed",
        "after_build_output_plant": "GREEN - correctly ignored",
        "clean_tree_after": "GREEN 4/4",
        "conclusion": "guarantee preserved; the only behaviour removed is the false positive on gitignored build output"
      },
      "timings": {
        "before_ms": [355, 359, 377],
        "after_ms": [43, 41, 46],
        "before_timeout_ms": 5000,
        "after_timeout_ms": 60000,
        "before_headroom": "~13x",
        "after_headroom": "~1300x",
        "old_cost_breakdown": {"walk_ms": 27, "read_3667_files_ms": 175, "regex_ms": 130}
      },
      "mechanism_verdicts": {
        "scan_cost": "CONFIRMED - matches the 18:05Z queue log signature (timeout, no offender list)",
        "scan_surface_nondeterminism": "MECHANISM DEMONSTRATED, NOT PROVEN TO HAVE FIRED - check:skill-examples deposits 209 .ts files under packages/spec/.examples-build/ (old walk 3876 vs git surface 3667 files), and a planted declaration there produces a false red; but none of today's 209 extracts mentions a guarded symbol",
        "turbo_build_alone": "does NOT diverge - both surfaces 3667; the divergence is the lint-gate layer"
      },
      "route_decision": {
        "pm_proposal": "git ls-files (tracked-only)",
        "taken": "git ls-files --cached --others --exclude-standard + git grep --untracked --fixed-strings",
        "why": "tracked-only measurably shrinks the guarantee - an untracked plant is caught today and would be missed. --untracked excludes gitignored build output (the actual nondeterminism) while preserving the docblock's 'first time they run the suite' promise. Same surface convention check-nul-bytes already uses.",
        "regex_authority": "unchanged - git grep -F is a provably exact superset prefilter; the JS regex remains sole authority, so no judgement moves into git's regex dialect"
      },
      "guarantee_strengthened": [
        "prefilter must reach the home file (anti-vacuity)",
        "prefilter output must be a subset of the enumerated surface (anti-drift)",
        "the pin must appear in the prefilter yet never in the offender list - makes the 'statement-anchored regex does not self-trigger' docblock claim executable",
        "verified these fire: narrowing the surface to packages/core turns the suite red, not silently green"
      ],
      "incidental_bug_fixed": "Shared /g declaration regex leaked lastIndex between files; a later matchAll on the home file could resume mid-text and report one declaration instead of two. Green today only by scan-order accident. Now fresh per use, with a regression pin.",
      "gates": {
        "packages_core_suite": "PASS - 32 files / 773 tests / 6.79s",
        "eslint_changed_file": "PASS",
        "tsc_packages_core": "PASS - 0 errors in changed file; total 98 -> 98 unchanged",
        "check:query-options-erasure": "PASS - baseline verified vs b54aaab, no files added",
        "check:type-check-debt": "PASS - ledger not raised (required `turbo run build` closure first)",
        "check:init-service-contract": "PASS",
        "check:kernel-hook-pairs": "PASS",
        "check:nul-bytes": "PASS",
        "check:empty-changeset": "PASS",
        "gate_list_source": "scripts/pm/dispatch-gates.mjs"
      },
      "changeset": {
        "present": false,
        "reason": "test-only, releases nothing. Empty-frontmatter is rejected for newly-added changesets by check-empty-changeset.mjs (#5471); existing empty stock is grandfathered, not precedent. lint.yml names this the textbook skip-changeset case.",
        "pm_action_required": "apply the `skip-changeset` label to PR #7855 before landing"
      },
      "open_questions": [
        "Label vs changeset: I read lint.yml's prescription as `skip-changeset` for a test-only PR and did not apply the label myself. If the PM prefers a real patch bump on @objectstack/core instead, say so and I will add the changeset.",
        "SCAN_TIMEOUT_MS is set per-`it` (file-local) rather than as a `testTimeout` in packages/core/vitest.config.ts, to stay inside the one-file scope the card fenced. If a package-wide default is preferred, that is a one-line config change in another PR."
      ],
      "out_of_scope_findings": [
        {
          "finding": "This pin runs PR-side only where packages/core is in the affected subset. A cross-package uniqueness scan is precisely the class affected-subset CI structurally cannot see, so the merge queue remains the first place a genuine fourth declaration can fail - the most expensive place. This PR removes the flake but does NOT close that structural gap: a real fourth declaration added under packages/plugins/** still would not go red on its own PR.",
          "lane": "CI config (.github/workflows), not engine-core",
          "note": "raised by the filer at 13:00Z and unaffected by the retraction; deliberately not folded in per the card"
        },
        {
          "finding": "`packages/spec/.examples-build/` is gitignored but was inside the old scan's blind spot for ANY repo-wide filesystem crawl, not just this pin. Other scanners that crawl packages/** with a hand-maintained denylist may carry the same transient-visibility bug. check-nul-bytes already uses the git surface and is unaffected.",
          "lane": "possible follow-up audit"
        }
      ]
    }

    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

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions