Skip to content

Reconcile decidePoolEligibility with writeCacheRow: the pre-check refuses 6 rows the authority pools #170

Description

@myselfsiddharth

Context

Filed out of #169 (which closed #166). Deliberately not fixed there — see "Why it was not fixed in #169" below.

pool_eligible has two fail-closed implementations of one rule:

Where Role
decidePoolEligibility src/compiler/pool.ts compiler pre-check, writes the flag into the bundle file
writeCacheRow / buildPoolRow src/cache/write.ts the authority, decides what reaches disk

Until #166 the authority had no caller outside tests/, so the two could not be compared on real data — the flag that reached disk came from the pre-check, and the authority was defended by the canary suite and called by nothing. paragent compile --to-cache now routes every row through the authority, which makes the comparison observable for the first time.

What the divergence is, measured

On the committed 12-step live gate bundle (artifacts/compiled/traj-gate-live-create-stat-dashboard-from-testdata-9.5.21.bundle.json):

$ npm run compile -- --in experiments/gate-v1/trajectories/grafana-create-stat-dashboard-from-testdata-9.5.21.json --to-cache .cache/paragent
rows=12 pool_eligible=1
cached 12 steps to .cache/paragent (7 pool-eligible, 5 tenant-only)
  note: authority pooled 6 step(s) the compiler pre-check did not: 2, 3, 4, 5, 8, 9

Compiler: 1 of 12 poolable. Authority: 7 of 12.

One rule accounts for the entire gap — the topology_only fallback, and the two implementations point it in opposite directions:

  • buildPoolRow: no pool-safe locator survives taint filtering, but the row carries flow_topology → emit a pool row whose chain is [{ strategy: "topology_only", tenant_scoped: false }]. It carries no locator at all — only "a click happened here, in main, between a click and a fill".
  • decidePoolEligibility: topologyOnlypool_eligible: false, reason topology_only_degraded. Refused outright.

The six affected steps (2, 3, 4, 5, 8, 9) are all rows whose whole locator chain is tainted, e.g. step 8:

{"strategy": "role_name", "role": "button", "name": "Save dashboard", "tenant_scoped": true}

Why this is worth an issue rather than a quiet fix

It is the safe direction, and that is exactly why it will rot. A pre-check may be stricter than the authority; it may never be looser. The dangerous direction is already pinned by tests/integration/live-bundle-pool.test.ts, and nothing tenant-derived escapes under either implementation — a topology_only row has no locator to leak. So there is no live privacy hole here and no test failing today.

What there is: two fail-closed implementations of one rule, disagreeing about 6 of 12 rows, with only one of them now load-bearing. That is the drift src/shared/program-id.ts and #74 exist to prevent, and the reason src/compiler/pool.ts's own comment says "the compiler's job is to agree with it."

It also understates a §9 input. pool_eligible is the cross-tenant reuse population (ADR-0014). If the authority would pool 7 rows and the artifact says 1, any reasoning from the bundle file about how much of a task is shareable is off by 6 rows on the one real task in the tree.

Why it was not fixed in #169

Changing decidePoolEligibility changes what every committed bundle artifact claims about pool eligibility, and pool_eligible is a privacy-boundary field. That is a decision with an ADR-shaped blast radius, not a rider on a wiring fix — the same reasoning docs/gate/compiler.md already applies to the open question about whether B5 should refuse a URL path at all.

What to decide

Pick one and say why — this is a design decision, not a defect with an obvious repair:

  1. Teach the pre-check the topology_only fallback, so it agrees with the authority. The bundle's pool_eligible becomes predictive again. Requires regenerating committed bundles and re-reading live-bundle-pool.test.ts's expectations.
  2. Make the authority stricter — drop the topology_only pool fallback, so a row with no pool-safe locator is never pooled. Narrows cross-tenant reuse; needs an ADR-0014 amendment and a look at whether a locator-less row is useful to a different tenant at all.
  3. Delete the pre-check. Now that the authority runs on a shipped path, the bundle's pool_eligible is no longer what reaches disk. If it is not predictive and not authoritative, it may be a field that should not exist — which is a cache-row.schema.json question.

Whichever way it goes, the direction invariant (pre-check never looser than the authority) has to survive, and live-bundle-pool.test.ts is what enforces it.

How to test

npm run compile -- --in experiments/gate-v1/trajectories/grafana-create-stat-dashboard-from-testdata-9.5.21.json --to-cache /tmp/pc
# the "authority pooled N step(s) the compiler pre-check did not" line is the number to move
npm run ci
npm run test:canary   # merge-blocking; must stay green and unmodified

Before you open the PR

  • Option chosen and justified, with an ADR if the authority or the schema changes
  • The direction invariant still enforced by tests/integration/live-bundle-pool.test.ts
  • Committed bundle artifacts regenerated if the pre-check's output changed
  • docs/gate/compiler.md and docs/architecture.md updated — both currently record the divergence as an open finding
  • Canary suite green and unmodified

Metadata

Metadata

Labels

area: cacheTouches cachearea: compilerTouches compilerprivacy-boundaryTouches the privacy boundary — canary is merge-blockingproposalDesign / governance proposaltrack-1Technical gate / churn harness

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions