Merge pull request #112 from haverstack/claude/issue-110-plan-ot4zqw

POST /records honors unlistedAt, un-skip four stale conformance fixtures

Author: cuibonobo

docs/api.md

@@ -162,6 +162,8 @@ Answers `200` with the updated record — it bumps `version` like any other muta
 
 **`includeUnlisted` is owner-only.** `GET /records`, `POST /records/query`, and `GET /changes` all accept it (excluded by default, like `includeDeleted`), and all three refuse it with `403` for any requester but the owner acting alone — enumeration standing rests on nothing but ownership, so no grant or delegation carries it. On `GET /changes` the refusal happens before the SSE stream opens.
 
+A `POST /records` body may carry `unlistedAt` to create the record already unlisted, so there is no window where it exists and is enumerable before a later `PUT .../unlisted` catches up — presence of the field is what matters; its value is not honored (`unlistedAt` on the response is stamped to the real create time, like `version`). This is **not** owner-only, unlike `createdAt`/`updatedAt`: it's gated the same way the `permissions` field is at create time — refused with `403` for a delegated app acting for anyone but the owner, but available to any other creator holding a create grant for the type.
+
 ### The feed transitions
 
 `unlist` and `list` are new change-feed ops (`unlist` maps to `kind: "deleted"`, `list` to `kind: "changed"` — see [Change feed](#change-feed)). Only the `unlist` transition needs special-casing; every other row falls out of checking the record's current `unlistedAt` against the subscriber's `includeUnlisted`:

src/routes/records.ts

@@ -97,6 +97,13 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono<Ap
   // both on any create (a record body is a whole record), and forwarding
   // them unfiltered would turn an ordinary grantee create into a 403, since
   // ScopedStack.create() refuses backdating rather than ignoring it.
+  //
+  // unlistedAt is different: it's opt-in (only present when a client
+  // actually wants an unlisted create), so it's forwarded unconditionally
+  // as the `unlisted` boolean ScopedStack.create() expects — same as
+  // permissions/associations below. ScopedStack.create() gates it itself
+  // (mayGrantAccess(), the same rule as the permissions field), refusing
+  // with 403 rather than silently dropping it.
   app.post('/', requireAuth(), async (c) => {
     const auth = c.get('auth')!;
     const body = await readJson<Record<string, unknown>>(c);
@@ -121,6 +128,7 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono<Ap
         associations: Array.isArray(body.associations)
           ? (body.associations as Association[])
           : undefined,
+        unlisted: typeof body.unlistedAt === 'string' ? true : undefined,
         createdAt,
         updatedAt,
       });

tests/conformance.test.ts

@@ -225,9 +225,6 @@ describe('createRecord fixtures', () => {
     // same fileId via an attachment association or file-ref field — exercised
     // by hand in tests/routes/attachments.test.ts instead of reconstructed here.
     'create-attachment-record-non-owner-carve-out-succeeds',
-    // POST /records does not forward unlistedAt to create(), so an
-    // unlisted create silently produces a listed record.
-    'create-record-unlisted',
   ]);
   const handled = new Set<string>();
 
@@ -305,6 +302,22 @@ describe('createRecord fixtures', () => {
     expect((data as Record<string, unknown>).content).toEqual(body.content);
   });
 
+  test('create-record-unlisted — unlistedAt in the create body suppresses enumeration from create time', async () => {
+    const fixture = createRecordFixtures.find((f) => f.name === 'create-record-unlisted')!;
+    handled.add(fixture.name);
+    const body = withFreshId(fixture.requestBody as WireRecord);
+    const { status, data } = await req(t.app, 'POST', fixture.path, { token: TEST_TOKEN, body });
+    expect(status).toBe(fixture.responseStatus);
+    const d = data as Record<string, unknown>;
+    expect(d.id).toBe(body.id);
+    expect(d.content).toEqual(body.content);
+    expect(d.version).toBe(1);
+    // Core stamps unlistedAt to the real create time rather than honoring a
+    // backdated value from the body it's echoed alongside — presence is
+    // what this fixture pins, not the literal value.
+    expect(typeof d.unlistedAt).toBe('string');
+  });
+
   test('coverage', () => {
     assertCoverage(
       createRecordFixtures.map((f) => f.name),
@@ -865,9 +878,6 @@ describe('error response fixtures', () => {
     // Not implemented by this server: no query-time bound exists to trip.
     // A real gap, tracked separately from this harness.
     'error-timeout-search-exceeds-server-bound',
-    // Behaves correctly already — 403 on both query routes — but is not
-    // dispatched here yet.
-    'error-permission-denied-includeUnlisted-non-owner',
   ]);
   const handled = new Set<string>();
 
@@ -958,6 +968,17 @@ describe('error response fixtures', () => {
     expectError(status, data, fixture);
   });
 
+  test('error-permission-denied-includeUnlisted-non-owner', async () => {
+    const fixture = find('error-permission-denied-includeUnlisted-non-owner');
+    const { token } = await t.ctx.adapter.createToken(CONTRIBUTOR_ID);
+    const { status, data } = await dispatch(fixture, token);
+    expectError(status, data, fixture);
+    // Same refusal on GET /records — includeUnlisted is owner-only on every
+    // route that accepts it, not just the fixture's own POST /records/query.
+    const get = await req(t.app, 'GET', '/records?includeUnlisted=true', { token });
+    expect(get.status).toBe(403);
+  });
+
   test('error-not-found — write against a nonexistent id', async () => {
     const fixture = find('error-not-found');
     const { status, data } = await dispatch(fixture, TEST_TOKEN);
@@ -1120,11 +1141,6 @@ describe('changeFeed fixtures', () => {
     // unrecognized frame — there's nothing server-side for this fixture to
     // exercise until a later minor adds one.
     'change-feed-unknown-frame-names-are-ignored',
-    // The unlist/list transitions and the suppression of edits made while
-    // unlisted all behave correctly already, but are not dispatched here yet.
-    'change-feed-unlist-frame-is-a-deleted-kind',
-    'change-feed-list-frame-is-a-changed-kind',
-    'change-feed-unlisted-record-produces-no-frame-by-default',
   ]);
 
   test('change-feed-ready-leads-every-connection', async () => {
@@ -1372,6 +1388,82 @@ describe('changeFeed fixtures', () => {
     }
   });
 
+  test('change-feed-unlist-frame-is-a-deleted-kind', async () => {
+    const fixture = changeFeedFixtures.find(
+      (f) => f.name === 'change-feed-unlist-frame-is-a-deleted-kind',
+    )!;
+    handled.add(fixture.name);
+    const record = await t.ctx.stack.create(NOTE_TYPE, { title: 'Hello' });
+    const conn = await openChangeFeed(t.app, '/changes', { token: TEST_TOKEN });
+    try {
+      await conn.waitForFrames(1); // ready
+      await req(t.app, 'PUT', `/records/${record.id}/unlisted`, {
+        token: TEST_TOKEN,
+        body: { unlisted: true },
+      });
+      const [, frame] = await conn.waitForFrames(2);
+      const data = frameData(frame);
+      expect(data.kind).toBe('deleted');
+      expect(data.op).toBe('unlist');
+      expect(data.recordId).toBe(record.id);
+    } finally {
+      await conn.close();
+    }
+  });
+
+  test('change-feed-list-frame-is-a-changed-kind', async () => {
+    const fixture = changeFeedFixtures.find(
+      (f) => f.name === 'change-feed-list-frame-is-a-changed-kind',
+    )!;
+    handled.add(fixture.name);
+    const record = await t.ctx.stack.create(NOTE_TYPE, { title: 'Hello' });
+    await t.ctx.stack.setUnlisted(record.id, true);
+    const conn = await openChangeFeed(t.app, '/changes', { token: TEST_TOKEN });
+    try {
+      await conn.waitForFrames(1); // ready
+      await req(t.app, 'PUT', `/records/${record.id}/unlisted`, {
+        token: TEST_TOKEN,
+        body: { unlisted: false },
+      });
+      const [, frame] = await conn.waitForFrames(2);
+      const data = frameData(frame);
+      expect(data.kind).toBe('changed');
+      expect(data.op).toBe('list');
+      expect(data.recordId).toBe(record.id);
+    } finally {
+      await conn.close();
+    }
+  });
+
+  test('change-feed-unlisted-record-produces-no-frame-by-default', async () => {
+    const fixture = changeFeedFixtures.find(
+      (f) => f.name === 'change-feed-unlisted-record-produces-no-frame-by-default',
+    )!;
+    handled.add(fixture.name);
+    const unlistedRecord = await t.ctx.stack.create(NOTE_TYPE, { title: 'Unlisted' });
+    await t.ctx.stack.setUnlisted(unlistedRecord.id, true);
+    const visibleRecord = await t.ctx.stack.create(NOTE_TYPE, { title: 'Visible' });
+    const conn = await openChangeFeed(t.app, '/changes', { token: TEST_TOKEN });
+    try {
+      await conn.waitForFrames(1); // ready
+      await req(t.app, 'PATCH', `/records/${unlistedRecord.id}`, {
+        token: TEST_TOKEN,
+        body: { title: 'Edited while unlisted' },
+      });
+      await req(t.app, 'PATCH', `/records/${visibleRecord.id}`, {
+        token: TEST_TOKEN,
+        body: { title: 'Edited, visible' },
+      });
+      // Exactly one more frame — the visible record's — proves the
+      // unlisted edit produced none rather than merely arriving later.
+      const [, frame] = await conn.waitForFrames(2);
+      const data = frameData(frame);
+      expect(data.recordId).toBe(visibleRecord.id);
+    } finally {
+      await conn.close();
+    }
+  });
+
   test('coverage', () => {
     assertCoverage(
       changeFeedFixtures.map((f) => f.name),

tests/routes/changes.test.ts

@@ -480,6 +480,30 @@ describe('GET /changes', () => {
       }
     });
 
+    it('POST /records with unlistedAt in the body creates unlisted and emits nothing', async () => {
+      // Same transition as "created unlisted emits nothing" above, but
+      // through the actual HTTP route with unlistedAt in the body rather
+      // than stack.create({ unlisted: true }) directly — pins that the
+      // route really forwards it end to end.
+      const conn = await openChangeFeed(t.app, '/changes', { token: TEST_TOKEN });
+      try {
+        await conn.waitForFrames(1); // ready
+        await req(t.app, 'POST', '/records', {
+          token: TEST_TOKEN,
+          body: {
+            typeId: NOTE_TYPE,
+            content: { title: 'secret' },
+            unlistedAt: new Date().toISOString(),
+          },
+        });
+        const proof = await t.ctx.stack.create(NOTE_TYPE, { title: 'public' });
+        const [, frame] = await conn.waitForFrames(2);
+        expect((frame.data as { recordId: string }).recordId).toBe(proof.id);
+      } finally {
+        await conn.close();
+      }
+    });
+
     it('listed -> unlisted emits a deleted/unlist frame', async () => {
       const record = await t.ctx.stack.create(NOTE_TYPE, { title: 'x' });
       const conn = await openChangeFeed(t.app, '/changes', { token: TEST_TOKEN });

tests/routes/records.test.ts

@@ -1218,6 +1218,81 @@ describe('Records', () => {
     });
   });
 
+  describe('POST /records — unlistedAt', () => {
+    it('unlistedAt in the body creates the record already unlisted', async () => {
+      const { status, data } = await req(t.app, 'POST', '/records', {
+        token: TEST_TOKEN,
+        body: {
+          typeId: NOTE_TYPE_ID,
+          content: { body: 'link-shared draft' },
+          unlistedAt: new Date().toISOString(),
+        },
+      });
+      expect(status).toBe(200);
+      expect((data as { unlistedAt?: string }).unlistedAt).toBeDefined();
+    });
+
+    it('an unlisted create is absent from an unfiltered GET /records', async () => {
+      const { data: created } = await req(t.app, 'POST', '/records', {
+        token: TEST_TOKEN,
+        body: {
+          typeId: NOTE_TYPE_ID,
+          content: { body: 'link-shared draft' },
+          unlistedAt: new Date().toISOString(),
+        },
+      });
+      const id = (created as { id: string }).id;
+
+      const { data: listed } = await req(t.app, 'GET', '/records', { token: TEST_TOKEN });
+      const ids = (listed as { records: { id: string }[] }).records.map((r) => r.id);
+      expect(ids).not.toContain(id);
+
+      // Still reachable by id — unlisted withholds enumeration, not access.
+      const { status: getStatus } = await req(t.app, 'GET', `/records/${id}`, {
+        token: TEST_TOKEN,
+      });
+      expect(getStatus).toBe(200);
+    });
+
+    it('a non-owner grantee with a plain create grant may create unlisted — not owner-only', async () => {
+      await t.ctx.stack.grant(OTHER_ENTITY_ID, [{ actions: ['create'], typeId: NOTE_TYPE_ID }]);
+      const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID);
+      const { status, data } = await req(t.app, 'POST', '/records', {
+        token,
+        body: {
+          typeId: NOTE_TYPE_ID,
+          content: { body: 'not the owner' },
+          unlistedAt: new Date().toISOString(),
+        },
+      });
+      expect(status).toBe(200);
+      expect((data as { unlistedAt?: string }).unlistedAt).toBeDefined();
+    });
+
+    it('refuses unlistedAt from a delegated app acting for a non-owner subject', async () => {
+      // Neither identity is the stack owner, so mayGrantAccess() — the same
+      // gate the permissions field uses at create time — refuses: a
+      // delegated app carries none of the owner's extra trust, and this
+      // record is the subject's own to begin with.
+      const appId = 'com.example.thirdparty';
+      const subjectId = 'entity-third-party-subject';
+      await t.ctx.stack.grant(subjectId, [{ actions: ['create'], typeId: NOTE_TYPE_ID }]);
+      await t.ctx.stack.grant(appId, [{ actions: ['create'], typeId: NOTE_TYPE_ID }]);
+      const { token } = await t.ctx.adapter.createToken(appId, { onBehalfOf: subjectId });
+
+      const { status, data } = await req(t.app, 'POST', '/records', {
+        token,
+        body: {
+          typeId: NOTE_TYPE_ID,
+          content: { body: 'delegated' },
+          unlistedAt: new Date().toISOString(),
+        },
+      });
+      expect(status).toBe(403);
+      expect((data as { error: { code: string } }).error.code).toBe('permission');
+    });
+  });
+
   describe('PATCH /records/:id — If-Match / optimistic concurrency', () => {
     it('succeeds when If-Match names the current version', async () => {
       const record = await seedRecord(t.ctx);