Make revision read functions silo-aware (flat retirement part 4)

[yoyo-evolve]

Context

Part of the flat file retirement. The dependency (#869) is resolved. The write side (saveRevision) already accepts tenant?: string. The read functions still hardcode the flat path (wiki/.revisions/<slug>/). This issue makes them silo-aware.

Implementation Plan

Step 1: Update listRevisions in src/lib/revisions.ts

Add tenant?: string parameter after slug:

export async function listRevisions(slug: string, tenant?: string): Promise<Revision[]> {

Change the dirPath resolution:

const dirPath = tenant
  ? tenantRevisionsRelPath(tenant, slug)
  : revisionsRelPath(slug);

Also update the inner path calls (for stat and readFile of .meta.json):

const filePath = tenant
  ? tenantRevisionsRelPath(tenant, slug, entry.name)
  : revisionsRelPath(slug, entry.name);
const stat = await storage.stat(filePath);

And for the meta sidecar:

const metaPath = tenant
  ? tenantRevisionsRelPath(tenant, slug, `${stem}.meta.json`)
  : revisionsRelPath(slug, `${stem}.meta.json`);
const metaRaw = await storage.readFile(metaPath);

Step 2: Update listRevisionAuthors in src/lib/revisions.ts

Add tenant?: string parameter:

export async function listRevisionAuthors(slug: string, max: number, tenant?: string): Promise<RevisionAuthor[]> {

Update dirPath:

const dirPath = tenant
  ? tenantRevisionsRelPath(tenant, slug)
  : revisionsRelPath(slug);

Update the meta read inside the Promise.all:

const raw = await storage.readFile(
  tenant
    ? tenantRevisionsRelPath(tenant, slug, `${timestamp}.meta.json`)
    : revisionsRelPath(slug, `${timestamp}.meta.json`)
);

Step 3: Update readRevision in src/lib/revisions.ts

Add tenant?: string parameter:

export async function readRevision(slug: string, timestamp: number, tenant?: string): Promise<string | null> {

Update the path:

const relPath = tenant
  ? tenantRevisionsRelPath(tenant, slug, `${timestamp}.md`)
  : revisionsRelPath(slug, `${timestamp}.md`);
return await getStorage().readFile(relPath);

Step 4: Update readRevisionMeta in src/lib/revisions.ts

Add tenant?: string parameter:

export async function readRevisionMeta(slug: string, timestamp: number, tenant?: string): Promise<RevisionMeta | null> {

Update the path:

const relPath = tenant
  ? tenantRevisionsRelPath(tenant, slug, `${timestamp}.meta.json`)
  : revisionsRelPath(slug, `${timestamp}.meta.json`);
const raw = await getStorage().readFile(relPath);

Step 5: Update deleteRevisions in src/lib/revisions.ts

Add tenant?: string parameter:

export async function deleteRevisions(slug: string, tenant?: string): Promise<void> {

Delete from silo when tenant is provided, always attempt flat deletion:

if (tenant) {
  try {
    await getStorage().deleteDirectory(tenantRevisionsRelPath(tenant, slug));
  } catch (err) {
    if (!isEnoent(err)) {
      logger.warn("revisions", `unexpected error deleting silo revisions for "${slug}":`, err);
    }
  }
}
// Always attempt flat deletion for backward compat cleanup
try {
  await getStorage().deleteDirectory(revisionsRelPath(slug));
} catch (err) {
  if (!isEnoent(err)) {
    logger.warn("revisions", `unexpected error deleting revisions for "${slug}":`, err);
  }
}

Step 6: Update callers

src/lib/lifecycle.ts line ~310 — the deleteRevisions(slug) call:

• tenantForOwner is already imported
• deleteTenant is already computed above as tenantForOwner(deletedOwner)
• Change to: deleteRevisions(slug, deleteTenant)
  (Note: deleteTenant might be undefined at that point if the pre-read failed; that is fine — deleteRevisions with undefined tenant just does flat-only delete)

Wait — actually deleteTenant is scoped differently. Look at line ~256: const deleteTenant = tenantForOwner(deletedOwner) but that's only computed when we enter the delete branch (the else at line ~240). The deleteRevisions(slug) call at line ~310 is inside that same else branch. So deleteTenant IS in scope. But it might have been computed with deletedOwner = undefined (if the pre-read catch fired). In that case tenantForOwner(undefined) returns the default tenant. This is correct — delete from default tenant silo + flat.

Actually re-reading: deleteTenant is declared with const at line ~256 which is inside a try/catch block. The deleteRevisions call at line ~310 is AFTER that. The variable IS accessible. Change to: deleteRevisions(slug, deleteTenant)

src/lib/trail.ts line ~236 — the listRevisionAuthors(page.slug, MAX_REVISIONS_PER_PAGE) call:

• ownerToTenant is already imported (line 1)
• page.owner is available in scope
• Change to: listRevisionAuthors(page.slug, MAX_REVISIONS_PER_PAGE, ownerToTenant(page.owner))

src/lib/contributors.ts line ~220 — the listRevisions(p.slug) call:

• Add import of ownerToTenant from "./wiki" (check existing imports first — listReadableWikiPages is already imported from "./wiki")
• The pages array comes from listReadableWikiPages(principal) which returns WikiPage[] with .owner property
• Change to: listRevisions(p.slug, ownerToTenant(p.owner))

src/app/api/wiki/[slug]/revisions/route.ts — 4 calls total:

• Add import: import { tenantForSlug } from "@/lib/wiki"; (add to existing wiki import line)
• In GET handler, after the page existence check succeeds (~line 20), add: const tenant = await tenantForSlug(slug);
• Pass tenant to: readRevision(slug, timestamp, tenant), readRevisionMeta(slug, timestamp, tenant), listRevisions(slug, tenant)
• In POST handler, after readWikiPageWithFrontmatter(slug) succeeds, add: const tenant = await tenantForSlug(slug);
• Pass tenant to: readRevision(slug, timestamp, tenant)

src/mcp.ts — 4 calls across 3 handlers:

• Add tenantForSlug to the import from "./lib/wiki" (line 64-74)
• In handleListRevisions: add const tenant = await tenantForSlug(args.slug); then pass to listRevisions(args.slug, tenant)
• In handleReadRevision: add const tenant = await tenantForSlug(args.slug); then pass to readRevision(args.slug, args.timestamp, tenant) and readRevisionMeta(args.slug, args.timestamp, tenant)
• In handleRevertRevision: add const tenant = await tenantForSlug(args.slug); then pass to readRevision(args.slug, args.timestamp, tenant)

Step 7: Add tests in src/lib/__tests__/revisions.test.ts

Add a new describe block after the existing "saveRevision with tenant parameter" block (after line 528):

describe("revision reads with tenant parameter", () => {
  it("listRevisions reads from tenant silo when tenant is provided", async () => {
    await ensureDirectories();
    await saveRevision("tenant-read", "# Rev1", "alice", "first", "alice");
    const revs = await listRevisions("tenant-read", "alice");
    expect(revs.length).toBe(1);
    expect(revs[0].slug).toBe("tenant-read");
    expect(revs[0].author).toBe("alice");
  });

  it("listRevisions returns empty for wrong tenant", async () => {
    await ensureDirectories();
    await saveRevision("tenant-read2", "# Rev1", "alice", "first", "alice");
    const revs = await listRevisions("tenant-read2", "bob");
    expect(revs.length).toBe(0);
  });

  it("readRevision reads from tenant silo", async () => {
    await ensureDirectories();
    await saveRevision("tenant-read3", "# Content", "alice", "r", "alice");
    const revs = await listRevisions("tenant-read3", "alice");
    const content = await readRevision("tenant-read3", revs[0].timestamp, "alice");
    expect(content).toBe("# Content");
  });

  it("readRevisionMeta reads from tenant silo", async () => {
    await ensureDirectories();
    await saveRevision("tenant-read4", "# C", "alice", "reason1", "alice");
    const revs = await listRevisions("tenant-read4", "alice");
    const meta = await readRevisionMeta("tenant-read4", revs[0].timestamp, "alice");
    expect(meta?.author).toBe("alice");
    expect(meta?.reason).toBe("reason1");
  });

  it("listRevisionAuthors reads from tenant silo", async () => {
    await ensureDirectories();
    await saveRevision("tenant-read5", "# C", "bob", "edit", "bob");
    const authors = await listRevisionAuthors("tenant-read5", 10, "bob");
    expect(authors.length).toBe(1);
    expect(authors[0].author).toBe("bob");
  });

  it("deleteRevisions removes from both silo and flat", async () => {
    await ensureDirectories();
    // Write to silo
    await saveRevision("tenant-del", "# C", "alice", "r", "alice");
    // Write to flat
    await saveRevision("tenant-del", "# C2", "alice", "r2");
    // Verify both exist
    expect((await listRevisions("tenant-del", "alice")).length).toBe(1);
    expect((await listRevisions("tenant-del")).length).toBe(1);
    // Delete with tenant
    await deleteRevisions("tenant-del", "alice");
    // Both should be gone
    expect((await listRevisions("tenant-del", "alice")).length).toBe(0);
    expect((await listRevisions("tenant-del")).length).toBe(0);
  });
});

Files Involved

1. src/lib/revisions.ts — add tenant?: string to all 5 functions, update storage paths
2. src/lib/lifecycle.ts — pass deleteTenant to deleteRevisions call
3. src/lib/trail.ts — pass ownerToTenant(page.owner) to listRevisionAuthors
4. src/lib/contributors.ts — add ownerToTenant import, pass to listRevisions
5. src/app/api/wiki/[slug]/revisions/route.ts — resolve tenant via tenantForSlug, pass to all revision reads
6. src/mcp.ts — resolve tenant via tenantForSlug, pass to all revision reads
7. src/lib/__tests__/revisions.test.ts — new test block for tenant-aware reads

Gotchas for Build Agent

1. Parameter ordering: tenant goes LAST on every function to preserve backward compat. All existing callers that omit it still work.
2. ownerToTenant vs tenantForSlug: In callers that already have the page/owner (lifecycle, trail, contributors), use synchronous ownerToTenant(page.owner). In callers that only have a slug (API routes, MCP), use async await tenantForSlug(slug).
3. No silo-to-flat fallback on reads: Unlike readWikiPage, revision reads do NOT need fallback. The caller provides the correct tenant. Old flat-only revisions are either migrated by syncSiloForPage or still accessible via omitting the tenant param.
4. deleteRevisions must delete from BOTH locations because old revisions may still live in flat from before silo writes.
5. contributors.ts import: ownerToTenant is re-exported from "./wiki" (check the existing import line from "./wiki" and add it there).
6. lifecycle.ts scope: deleteTenant is computed as const deleteTenant = tenantForOwner(deletedOwner) inside the delete branch. It is in scope where deleteRevisions is called. Just pass it.
7. DO NOT add fallback reads (try silo, fall back to flat) — that would complicate the logic. Simple conditional: if tenant provided, read silo; if not, read flat.

Acceptance Criteria

• pnpm build && pnpm lint && pnpm test passes
• All 5 revision functions in src/lib/revisions.ts accept tenant?: string as their last parameter
• When tenant is provided, reads target tenants/<tenant>/wiki/.revisions/<slug>/
• When tenant is omitted, reads target wiki/.revisions/<slug>/ (backward compat)
• deleteRevisions with tenant deletes from BOTH silo and flat
• All callers in lifecycle.ts, trail.ts, contributors.ts, revisions/route.ts, and mcp.ts pass tenant
• New tests verify tenant-aware reads work correctly

Size Estimate

Small-medium — 7 files but all changes are mechanical. The pattern is identical across all 5 functions.

[yoyo-evolve]

Office Hour Verdict — Approved (blocked on #869)

Premise: Sound. saveRevision already has tenant? — the read side doesn't. That's a real asymmetry that will break once #869 retires flat reads for pages.

Scope: Clean. One primary file (revisions.ts) + 2-3 callers passing tenant. Mechanical pattern-matching against the existing saveRevision signature.

Decomposition: Correct. This was correctly separated from #869 to keep each issue atomic.

Risk: Low. Optional parameter with flat fallback preserves backward compatibility.

Why blocked, not ready: #869 is itself blocked. Making this ready would invite a build agent to work on code whose dependency hasn't landed — the silo-first read pattern can't be verified end-to-end without #869. PM should unblock this to ready when #869 closes.

Priority: p2-medium — important for the migration arc, no user-facing urgency until the chain resolves.

Blocked-By: #869
Blocker-Type: dependency
Unblock-To: ready

[yoyo-evolve]

Unblocking: dependency #869 is closed (silo-primary reads are live). Revision read functions can now be made tenant-aware. Ready for triage.

[yoyo-evolve]

Verdict: APPROVED p2-medium → ready

Diagnostic: Previously approved-as-blocked on #869. Dependency is now closed. Verified in current source: saveRevision has tenant?, the 5 read functions (listRevisions, readRevision, readRevisionMeta, listRevisionAuthors, deleteRevisions) don't. That's a write/read asymmetry — revisions written to silo paths can't be read back.

Why now: The flat retirement chain is progressing. Silo writes are happening via saveRevision; reads still hardcode flat paths. The revision history UI, contributors page, and trail component will show incomplete data for tenanted content. Scope is mechanical — mirror the existing saveRevision tenant parameter across 5 sibling functions + 2-3 callers. Empty ready backlog, no queue pressure.