From 13cec5157d2088f0c10610f1d7e497c9a00996c5 Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Sun, 26 Jul 2026 07:50:16 +0800 Subject: [PATCH 1/2] fix(review): delegate merge-train low-signal classification to canonical path-matchers src/review/merge-train.ts hand-rolled its own low-signal file/dir detection that had drifted from the canonical set its sibling files already use: - LOW_SIGNAL_FILENAME_RE matched only package-lock.json/yarn.lock/pnpm-lock.yaml/Cargo.lock (4 names), so two PRs sharing only a poetry.lock, go.sum, npm-shrinkwrap.json, etc. were treated as a real overlap and forced into a spurious merge-train wait. - LOW_SIGNAL_DIR_RE matched only dist/build/coverage/node_modules, missing 'out' and the vendored-code family (vendor/vendored/third_party/bower_components/jspm_packages). Replace the filename regex with the canonical isLockfile() helper from path-matchers.ts (the same dependency-free leaf utility review-diff.ts and review-grounding.ts already delegate to, covering all 24+ lockfile formats), and extend the directory regex to the canonical generated/vendored set. Two new tests prove PRs sharing only poetry.lock/go.sum or vendor/third_party paths now decide {wait:false} where they previously decided {wait:true}. --- src/review/merge-train.ts | 22 +++++++++++++--------- test/unit/merge-train.test.ts | 12 ++++++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/review/merge-train.ts b/src/review/merge-train.ts index d96cb13819..7d24551063 100644 --- a/src/review/merge-train.ts +++ b/src/review/merge-train.ts @@ -19,6 +19,8 @@ // acceptable, bounded tradeoff given the 24h staleness cap already prevents an abandoned PR from blocking // forever. +import { isLockfile } from "../signals/path-matchers"; + /** The subset of a sibling PR's fields this gate actually needs -- kept minimal and independent of * `PullRequestRecord`'s full shape so this module has zero import surface beyond plain data. */ export type MergeTrainSibling = { @@ -43,17 +45,19 @@ export const MERGE_TRAIN_MAX_WAIT_MS = 24 * 60 * 60 * 1000; export type MergeTrainDecision = { wait: true; blockingPr: number } | { wait: false }; -/** Low-priority path buckets (lockfiles, generated/build output, dist/ artifacts) that overlapping alone - * never counts as real conflict risk -- ported from `review-grounding.ts`'s `diffFilePriority` classification - * (bucket 4, "least useful to review") so this module stays dependency-free rather than importing a whole - * review-pipeline module for one number. Kept as a small, explicit suffix/name list rather than a generic - * "some overlap" check: a shared `package-lock.json` or `dist/bundle.js` touch is routine noise, not the - * same-area conflict risk this gate exists to catch. */ -const LOW_SIGNAL_FILENAME_RE = /(?:^|\/)(?:package-lock\.json|yarn\.lock|pnpm-lock\.yaml|Cargo\.lock)$/i; -const LOW_SIGNAL_DIR_RE = /(?:^|\/)(?:dist|build|coverage|node_modules)\//i; +/** Low-priority path buckets (lockfiles, generated/build output, vendored third-party trees) that overlapping + * alone never counts as real conflict risk. Lockfile-NAME matching delegates to the canonical `isLockfile` + * helper -- a dependency-free leaf utility in path-matchers.ts, the same delegation `review-diff.ts` and + * `review-grounding.ts` already use -- so this classification never drifts behind the 24+ canonical lockfile + * formats the way the hand-rolled 4-name regex did (a shared `poetry.lock`, `go.sum`, or `npm-shrinkwrap.json` + * was wrongly treated as real overlap; #8647). The directory bucket mirrors the same canonical generated/ + * vendored set: build output (`dist`/`build`/`out`/`coverage`) and installed/vendored dependency trees + * (`node_modules` and the `vendor`/`third_party` family). A shared `package-lock.json` or `third_party/x` + * touch is routine noise, not the same-area conflict risk this gate exists to catch. */ +const LOW_SIGNAL_DIR_RE = /(?:^|\/)(?:dist|build|out|coverage|node_modules|vendor|vendored|third_party|third-party|bower_components|jspm_packages)\//i; function isMeaningfulPath(path: string): boolean { - return !LOW_SIGNAL_FILENAME_RE.test(path) && !LOW_SIGNAL_DIR_RE.test(path); + return !isLockfile(path) && !LOW_SIGNAL_DIR_RE.test(path); } /** True when `thisPr` and `sibling` overlap enough to carry real conflict/duplicate-effort risk: a shared diff --git a/test/unit/merge-train.test.ts b/test/unit/merge-train.test.ts index d7db0719af..57ba0c6b02 100644 --- a/test/unit/merge-train.test.ts +++ b/test/unit/merge-train.test.ts @@ -132,6 +132,18 @@ describe("shouldWaitForOlderSiblings (#selfhost-merge-train)", () => { expect(decide(110, "2026-07-07T11:00:00.000Z", siblings, NOW, { thisPrLinkedIssues: [1], thisPrChangedFiles: ["package-lock.json", "dist/bundle.js"] })).toEqual({ wait: false }); }); + it("does NOT treat a shared non-npm canonical lockfile (poetry.lock/go.sum) as meaningful overlap (#8647)", () => { + // The hand-rolled 4-name regex only knew package-lock/yarn/pnpm/Cargo, so these forced a spurious wait; + // delegating to the canonical isLockfile covers all 24+ formats. + const siblings: MergeTrainSibling[] = [{ number: 105, createdAt: "2026-07-07T10:00:00.000Z", linkedIssues: [99], changedFiles: ["poetry.lock", "backend/go.sum"] }]; + expect(decide(110, "2026-07-07T11:00:00.000Z", siblings, NOW, { thisPrLinkedIssues: [1], thisPrChangedFiles: ["poetry.lock", "backend/go.sum"] })).toEqual({ wait: false }); + }); + + it("does NOT treat a shared vendored-directory path (vendor/third_party) as meaningful overlap (#8647)", () => { + const siblings: MergeTrainSibling[] = [{ number: 105, createdAt: "2026-07-07T10:00:00.000Z", linkedIssues: [99], changedFiles: ["vendor/lib/x.go", "third_party/pkg/y.js"] }]; + expect(decide(110, "2026-07-07T11:00:00.000Z", siblings, NOW, { thisPrLinkedIssues: [1], thisPrChangedFiles: ["vendor/lib/x.go", "third_party/pkg/y.js"] })).toEqual({ wait: false }); + }); + it("a sibling with no linkedIssues field at all (undefined) can still match via a shared changed file", () => { const siblings: MergeTrainSibling[] = [{ number: 105, createdAt: "2026-07-07T10:00:00.000Z", changedFiles: ["src/a.ts"] }]; expect(decide(110, "2026-07-07T11:00:00.000Z", siblings, NOW, { thisPrLinkedIssues: [1], thisPrChangedFiles: ["src/a.ts"] })).toEqual({ wait: true, blockingPr: 105 }); From 0a621fe60c60f45670da0f16a7606617af87a00c Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Sun, 26 Jul 2026 07:57:33 +0800 Subject: [PATCH 2/2] fix(api): allow authenticated sessions to reach registration-readiness and gittensor-config-recommendation src/api/routes.ts's registration-readiness and gittensor-config-recommendation route handlers carry no per-repo ownership guard by design -- they are advisory, any-authenticated-user lookups (the owner panel takes a free-text repo name, and buildRegistrationReadinessResponse already strips owner-private context via stripOwnerPolicyContext before returning). But neither path had a predicate in canSessionAccessPath, so a real non-operator browser session got 403 on the owner panel's only two data calls, while operators/server tokens (which bypass the allowlist) worked. Add the two missing path predicates to the session allowlist so any authenticated session may reach them, matching the routes' existing intended open access. This unblocks the existing behavior only -- it adds no new authorization restriction (none was found to be intended: the handlers have no per-repo check, and the readiness payload is deliberately owner-context-stripped for public reads). Test: a session-cookie-authenticated user who maintains no repo here now gets 200 (not 403) on both routes for an arbitrary repo, in test/unit/access-boundary.test.ts. --- src/api/routes.ts | 15 +++++++++++++++ test/unit/access-boundary.test.ts | 11 +++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/api/routes.ts b/src/api/routes.ts index 7908188a13..2368be675b 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -6110,6 +6110,13 @@ function canSessionAccessPath(env: Env, identity: Extract { expect((await app.request(SETTINGS_A, { headers: { cookie } }, env)).status).toBe(200); expect((await app.request(SETTINGS_B, { headers: { cookie } }, env)).status).toBe(403); }); + + it("any authenticated session reaches registration-readiness / gittensor-config-recommendation for any repo (#8654)", async () => { + // These two advisory routes are intentionally open to any logged-in user (no per-repo ownership scope), but + // were omitted from the session allowlist, so every real non-operator browser session got 403 on the owner + // panel's only two data calls. charlie maintains nothing here, yet must reach both for an arbitrary repo. + const { app, env } = await setup(); + const { token } = await createSessionForGitHubUser(env, { login: "charlie", id: 999 }); + const cookie = `loopover_session=${token}`; + expect((await app.request("/v1/repos/alice/repo-a/registration-readiness", { headers: { cookie } }, env)).status).toBe(200); + expect((await app.request("/v1/repos/alice/repo-a/gittensor-config-recommendation", { headers: { cookie } }, env)).status).toBe(200); + }); }); describe("access boundary: contributor (miner) data is self-scoped", () => {