Skip to content

miner(purge): right-to-be-forgotten never deletes a policy-verdict-cache row, and the regression test hides it #10001

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

loopover-miner purge --repo <owner/repo> is the operator-invoked right-to-be-forgotten sweep. It calls each
store's purgeByRepo with the plain owner/repo string, at
packages/loopover-miner/lib/purge-cli.ts:268-280:

function purgeOneStore(target: PurgeTarget, options: PurgeCliOptions, repoFullName: string): PurgeStoreResult {
  const ownsStore = options[target.optionKey] === undefined;
  let store: PurgeableStore | undefined;
  try {
    store = (options[target.optionKey] ?? target.opener)();
    const purged = store.purgeByRepo(repoFullName);

repoFullName here is the value produced by parseRepoArg
(packages/loopover-miner/lib/purge-cli.ts:143-154), i.e. exactly `${owner}/${repo}` — never a forge-scoped
key.

The policy-verdict cache, however, is not keyed on owner/repo. Its purge spec is
packages/loopover-miner/lib/store-maintenance.ts:66:

export const POLICY_VERDICT_CACHE_PURGE_SPEC: LedgerPurgeSpec = { table: "policy_verdict_cache", repoColumn: "repo_scope" };

and purgeStoreByRepo (packages/loopover-miner/lib/store-maintenance.ts:207-213) emits an exact-equality delete:

  const info = db.prepare(`DELETE FROM ${spec.table} WHERE ${spec.repoColumn} = ?`).run(repoFullName);

But every row this store actually holds is keyed by a repo scope, built in
packages/loopover-miner/lib/opportunity-fanout.ts:309-311:

function policyVerdictCacheKey(apiBaseUrl: string, repoFullName: string): string {
  return `${apiBaseUrl}::${repoFullName}`;
}

resolveRepoAiPolicy uses that as the only key it ever writes with
(packages/loopover-miner/lib/opportunity-fanout.ts:406, :420, :432-438), so a real row's repo_scope is
https://api.github.com::acme/widgets, not acme/widgets.

DELETE FROM policy_verdict_cache WHERE repo_scope = 'acme/widgets' therefore matches zero rows, always. The
CLI reports policy-verdict-cache=0 (packages/loopover-miner/lib/purge-cli.ts:290-303) as if there had been
nothing to remove, and --dry-run reports the same 0 because countStoreByRepo
(packages/loopover-miner/lib/store-maintenance.ts:219-228) uses the identical = ? predicate. A resolved
AI-usage-policy verdict for the purged repo survives the sweep indefinitely and is reused by the next discover
run (resolveOrCacheVerdict, packages/loopover-miner/lib/opportunity-fanout.ts:383-397).

The existing regression test does not catch this because it seeds the store with a key shape production never
writes — test/unit/miner-purge-cli.test.ts:845-851:

  it("REGRESSION (#6987): really deletes policy-verdict-cache rows for the repo, leaving other repos intact", () => {
    ...
    seeded.put("acme/widgets", "AI-USAGE.md", '"v1"', POLICY_VERDICT);

put accepts it because normalizeRepoScope
(packages/loopover-miner/lib/policy-verdict-cache.ts:69-74) only trims and requires non-empty. The same
fabricated key is used by the dry-run fixture at test/unit/miner-purge-cli.test.ts:162-164, which is why
wouldPurge: 1 is asserted at :267.

Note that repo_scope cannot be matched with a bare LIKE '%::' || ?: REPO_SEGMENT_PATTERN
(packages/loopover-miner/lib/repo-clone.ts:72) permits _, which is a SQL LIKE single-character wildcard, so
purging acme/my_repo would over-match acme/myXrepo.

Requirements

  • loopover-miner purge --repo <owner/repo> must delete every policy_verdict_cache row whose repo_scope
    identifies that repo, for every forge host prefix — matching the cross-host posture
    GOVERNOR_REPUTATION_HISTORY_PURGE_SPEC and DENY_HOOK_SYNTHESIS_PURGE_SPEC already document at
    packages/loopover-miner/lib/store-maintenance.ts:53-60 and :68-78.
  • purge --repo <owner/repo> --dry-run must count exactly the rows the real purge would delete, using the same
    predicate — no divergence between countStoreByRepo and the real delete for this store.
  • The match must be exact on the owner/repo suffix after the :: separator. A repo name containing _ or %
    must not cause an over-match: purging acme/my_repo must leave a row scoped to acme/myXrepo intact.
  • A row scoped to a different repo on the same host, and a row scoped to the same-named repo path embedded in
    a longer name (e.g. https://api.github.com::acme/my_repo-extra), must both survive.
  • PolicyVerdictCacheStore.purgeByRepo's parameter must be documented for whichever contract it now takes
    (owner/repo or a full scope) and every caller must agree with it. Today the JSDoc at
    packages/loopover-miner/lib/policy-verdict-cache.ts:43 says "one repo scope" while its only production caller
    passes owner/repo; that disagreement must be gone after this change.
  • Do NOT change policyVerdictCacheKey or the shape of the keys opportunity-fanout.ts writes — the host must
    stay in the key (packages/loopover-miner/lib/opportunity-fanout.ts:305-311 explains why).
  • Do NOT change the purge behaviour of any other store in REAL_PURGE_TARGETS.

⚠️ Required pattern: keep the fix inside packages/loopover-miner/lib/policy-verdict-cache.ts's purgeByRepo
plus the POLICY_VERDICT_CACHE_PURGE_SPEC / countStoreByRepo pairing in
packages/loopover-miner/lib/store-maintenance.ts:66 and :219-228, mirroring how
WORKTREE_ALLOCATOR_PURGE_SPEC (store-maintenance.ts:86) already keeps a store-specific real purge and its
dry-run count in lockstep via extraWhereSql. What does NOT satisfy this issue: (a) a bare
LIKE '%::' || ? predicate with no wildcard escaping — _ is a LIKE wildcard and repo names may contain it;
(b) rewriting purgeStoreByRepo generically for every store, which changes the blast radius of eleven other
purge specs; (c) a test-only PR that changes the #6987 fixture at test/unit/miner-purge-cli.test.ts:850 to a
real apiBaseUrl::owner/repo key without changing the purge itself.

Deliverables

  • Given a policy-verdict-cache.sqlite3 seeded via
    put("https://api.github.com::acme/widgets", "AI-USAGE.md", '"v1"', verdict) and
    put("https://forge.example.com::acme/widgets", "AI-USAGE.md", '"v2"', verdict),
    runPurge(["--repo", "acme/widgets", "--json"], …) reports { store: "policy-verdict-cache", purged: 2 }
    and both rows are gone — asserted in test/unit/miner-purge-cli.test.ts.
  • With the same seed plus put("https://api.github.com::acme/other", …), the acme/other row survives the
    purge — asserted in test/unit/miner-purge-cli.test.ts.
  • runPurgeDryRun({ repoFullName: "acme/widgets", json: true }, …) against that same on-disk file reports
    { store: "policy-verdict-cache", wouldPurge: 2 } — the identical number the real purge removes — asserted
    in test/unit/miner-purge-cli.test.ts.
  • Wildcard safety: with rows scoped https://api.github.com::acme/my_repo and
    https://api.github.com::acme/myXrepo, purging acme/my_repo removes exactly one row and leaves
    acme/myXrepo intact — asserted in test/unit/miner-purge-cli.test.ts.
  • The #6987 regression test at test/unit/miner-purge-cli.test.ts:845 and the dry-run fixture at
    test/unit/miner-purge-cli.test.ts:162-164 are updated to seed the store with the real
    policyVerdictCacheKey(apiBaseUrl, repoFullName) shape, so no test in this file still asserts against a
    bare owner/repo repo_scope.
  • A regression test at test/unit/miner-purge-cli.test.ts named for this bug (e.g. REGRESSION: purge deletes policy-verdict-cache rows keyed by the real apiBaseUrl::owner/repo scope) that fails against the current
    code.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one that
fixes the real delete but leaves countStoreByRepo on the old = ? predicate, so --dry-run still previews 0 —
does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include lists
packages/loopover-miner/lib/**/*.ts, so both purge-cli.ts, policy-verdict-cache.ts and store-maintenance.ts
are measured and gated. Every branch the change introduces needs both arms tested: the suffix-match vs exact-match
predicate selection (whichever form is chosen), the spec.extraWhereSql present/absent arm in countStoreByRepo
(store-maintenance.ts:225) if that path is reused, and the owns/injected-store arm of purgeOneStore
(purge-cli.ts:269). Escape-character handling must be covered by a repo name that contains _ and by one that
does not.

Expected Outcome

loopover-miner purge --repo <owner/repo> actually removes the target repo's cached AI-usage-policy verdicts
across every forge host, --dry-run previews the same count the real purge deletes, and the store's regression
test exercises the key shape production writes instead of a fabricated one.

Links & Resources

  • packages/loopover-miner/lib/store-maintenance.ts:66POLICY_VERDICT_CACHE_PURGE_SPEC
  • packages/loopover-miner/lib/store-maintenance.ts:207-228purgeStoreByRepo / countStoreByRepo
  • packages/loopover-miner/lib/policy-verdict-cache.ts:136-140purgeByRepo
  • packages/loopover-miner/lib/opportunity-fanout.ts:305-311policyVerdictCacheKey, the real key shape
  • packages/loopover-miner/lib/purge-cli.ts:126, :268-280 — the purge target entry and the owner/repo call
  • packages/loopover-miner/lib/repo-clone.ts:72REPO_SEGMENT_PATTERN, which permits _
  • test/unit/miner-purge-cli.test.ts:162-164, :845-878 — the fixtures that hide the bug

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions