fix: Codex PR #595 round-4 — sharding-table eligibility, fail-closed VerifyCollector, reject deprecated fields

Three legitimate findings from round-4 review addressed in this commit.

(D) packages/cli/src/daemon/routes/context-graph.ts

  The previous round-2 fix tried to preserve backward compatibility by
  stripping `participantIdentityIds` / `requiredSignatures` from request
  bodies and warning. Round-4 (and the project owner) made clear that
  silent stripping lets callers believe they created a roster-constrained
  CG when those constraints were actually discarded. We now reject
  outright any request body that carries either deprecated field — code
  `DEPRECATED_CONTEXT_GRAPH_FIELDS`, structured `deprecatedFields` array,
  HTTP 400. No backward-compat strip path.

(C) packages/publisher/src/verify-collector.ts

  `VerifyCollector.collect` used to silently default `requiredSignatures`
  to 1 when the caller omitted it and the `getMinimumRequiredSignatures`
  probe failed (missing probe / RPC outage / garbage value). Since
  `chain.verify()` does NOT submit collected signatures on-chain, the
  local quorum check is the only enforcement; defaulting to 1 lets the
  proposer self-approve and pass. Now all three failure modes throw
  with actionable errors pointing at the explicit `requiredSignatures`
  override knob. 4 new unit tests pin the behaviour.

(B) packages/chain/src/{chain-adapter,evm-adapter,mock-adapter}.ts
    + packages/agent/src/dkg-agent.ts

  SPEC_CG_MEMORY_MODEL §4.3 promises that only sharding-table members
  can ACK a VM publish, but the post-LU2 verify path counted every
  resolved signer regardless of membership. Added an optional
  `isShardingTableMember(identityId)` to the ChainAdapter interface,
  wired EVMChainAdapter to `ShardingTableStorage.nodeExists(uint72)`,
  and made the mock return true for any non-zero identityId (mocks
  don't model the sharding table). The agent verify path now probes
  membership for every resolved signer (including the proposer) and
  drops approvals from non-members with a fail-closed per-signer log.
  Results are cached per batch to avoid hammering the RPC.

Tests:
  - cli/test/daemon-http-behavior-extra.test.ts: replace round-2 strip-
    tolerance tests with reject tests covering every deprecated-field
    combination (with and without id/name).
  - publisher/test/verify-collector.test.ts: four new tests covering
    missing-probe / probe-throws / probe-returns-garbage / probe-honoured
    paths.
  - chain/test/mock-adapter-parity.test.ts: extend the parity manifest
    + add a direct positive/negative assertion for the new method.

All targeted suites pass locally:
  ✓ publisher/verify-collector            (10/10)
  ✓ chain/mock-adapter-parity             (13/13)
  ✓ chain/abi-pinning                     (13/13)
  ✓ cli/daemon-http-behavior-extra        (5/5 in-scope subset)
  ✓ agent/{e2e-publish-protocol,
           e2e-context-graph,
           v10-ack-provider,
           swm-ack-quorum,
           swm-ack-quorum-integration}    (73/73)

Holds the line on round-4 Bug A (`KnowledgeAssetsLib.sol:71` packed-
struct layout): ContextGraphStorage is not behind an upgradeable proxy;
the deploy pattern is "deploy fresh + register in Hub by name". File
itself states `// Storage layout — fresh design (no prior deployments
to preserve)`. V10 is brand-new on-chain (only testnet artifact:
base_sepolia_v10_contracts.json). No storage-slot reservation needed.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: 2 people

packages/agent/src/dkg-agent.ts

@@ -11806,9 +11806,47 @@ export class DKGAgent {
     const isEligibleParticipant = (identityId: bigint): boolean =>
       participantIdentityIds === null || participantIdentityIds.has(identityId);
 
+    // Sharding-table eligibility (Codex PR #595 round-4):
+    // SPEC_CG_MEMORY_MODEL §4.3 + §6 promise that only sharding-table
+    // members can ACK a VM publish. Pre-LU2 the per-CG hosting-committee
+    // allowlist did this gating implicitly; post-LU2 that allowlist is
+    // gone and we have to check membership at signer-resolution time.
+    // Failure modes:
+    //   - adapter doesn't implement `isShardingTableMember` → log once
+    //     and fall back to legacy behavior (test mocks / no-chain envs).
+    //   - membership probe throws for a specific signer → drop that
+    //     approval (fail-closed per signer, not per batch).
+    // Cache decisions per batch so we don't hammer the RPC for repeated
+    // approvers across re-tries.
+    const shardingMembershipCache = new Map<string, boolean>();
+    const probeShardingTableMembership = async (identityId: bigint): Promise<boolean> => {
+      if (identityId <= 0n) return false;
+      if (typeof this.chain.isShardingTableMember !== 'function') return true;
+      const key = identityId.toString();
+      const cached = shardingMembershipCache.get(key);
+      if (cached !== undefined) return cached;
+      try {
+        const ok = await this.chain.isShardingTableMember(identityId);
+        shardingMembershipCache.set(key, ok);
+        return ok;
+      } catch (err: any) {
+        this.log.warn(
+          ctx,
+          `[verify] isShardingTableMember(${identityId}) probe failed (${err?.message ?? err}); ` +
+          `dropping that signer's approval as fail-closed`,
+        );
+        shardingMembershipCache.set(key, false);
+        return false;
+      }
+    };
+
     const resolvedSignatures: Array<{ identityId: bigint; r: Uint8Array; vs: Uint8Array }> = [];
     const resolvedSignerAddresses: string[] = [];
-    if (this.identityId > 0n && isEligibleParticipant(this.identityId)) {
+    if (
+      this.identityId > 0n
+      && isEligibleParticipant(this.identityId)
+      && await probeShardingTableMembership(this.identityId)
+    ) {
       resolvedSignatures.push({
         identityId: this.identityId,
         r: ethers.getBytes(proposerSig.r),
@@ -11824,6 +11862,7 @@ export class DKGAgent {
       );
       if (!id || id === 0n) continue;
       if (!isEligibleParticipant(id)) continue;
+      if (!(await probeShardingTableMembership(id))) continue;
       resolvedSignatures.push({ identityId: id, r: a.signatureR, vs: a.signatureVS });
       resolvedSignerAddresses.push(a.approverAddress);
       if (participantIdentityIds !== null && participantIdentityIds.has(id)) {

packages/chain/src/chain-adapter.ts

@@ -725,6 +725,20 @@ export interface ChainAdapter {
   /** Read minimumRequiredSignatures from ParametersStorage. Used by ACKCollector. */
   getMinimumRequiredSignatures?(): Promise<number>;
 
+  /**
+   * Sharding-table membership check (Codex PR #595 round-4 follow-up).
+   *
+   * SPEC_CG_MEMORY_MODEL §4.3 promises that only sharding-table members
+   * can ACK a VM publish. The agent's verify path calls this on every
+   * resolved signer identityId and DROPS any approval whose signer is
+   * not in the sharding table. Adapters that can't probe membership
+   * (test mocks, legacy in-tree fakes) return `true` for any non-zero
+   * identityId — those environments aren't security-sensitive.
+   *
+   * Implementation maps to `ShardingTableStorage.nodeExists(uint72)`.
+   */
+  isShardingTableMember?(identityId: bigint): Promise<boolean>;
+
   /** Verify that a recovered signer address is a registered operational key for the given identity. */
   verifyACKIdentity?(recoveredAddress: string, claimedIdentityId: bigint): Promise<boolean>;
 

packages/chain/src/evm-adapter.ts

@@ -2315,6 +2315,19 @@ export class EVMChainAdapter implements ChainAdapter {
     return Number(await this.contracts.parametersStorage.minimumRequiredSignatures());
   }
 
+  async isShardingTableMember(identityId: bigint): Promise<boolean> {
+    if (identityId <= 0n) return false;
+    await this.init();
+    const storage = await this.resolveContract('ShardingTableStorage');
+    if (!storage) {
+      throw new Error(
+        'isShardingTableMember: ShardingTableStorage contract is not resolvable. ' +
+        'Verify path cannot enforce sharding-table eligibility without it.',
+      );
+    }
+    return Boolean(await storage.nodeExists(identityId));
+  }
+
   async verifyACKIdentity(recoveredAddress: string, claimedIdentityId: bigint): Promise<boolean> {
     await this.init();
     const identityStorage = await this.resolveContract('IdentityStorage');

packages/chain/src/mock-adapter.ts

@@ -760,6 +760,14 @@ export class MockChainAdapter implements ChainAdapter {
     return this.minimumRequiredSignatures;
   }
 
+  // Codex PR #595 round-4: mock environments don't model the sharding
+  // table, so any registered (non-zero) identity counts as a member.
+  // Tests that need to exercise non-membership rejection should
+  // override this with a vi.spyOn / monkey-patch.
+  async isShardingTableMember(identityId: bigint): Promise<boolean> {
+    return identityId > 0n;
+  }
+
   async verifyACKIdentity(recoveredAddress: string, claimedIdentityId: bigint): Promise<boolean> {
     // Strict binding: recovered address must match the identity's registered address
     const normalizedAddress = recoveredAddress.toLowerCase();

packages/chain/test/mock-adapter-parity.test.ts

@@ -151,6 +151,7 @@ const NO_CHAIN_EXEMPT_FROM_EVM = new Set<string>([
   'signACKDigest',
   'getACKSignerKey',
   'getMinimumRequiredSignatures',
+  'isShardingTableMember',
   'verifyACKIdentity',
   'verifySyncIdentity',
   'ensureOperationalWalletsRegistered',
@@ -218,6 +219,17 @@ describe('MockChainAdapter API parity with EVMChainAdapter [CH-8]', () => {
     expect(mock.isV10Ready()).toBe(true);
   });
 
+  // Codex PR #595 round-4: isShardingTableMember gates VM ACK eligibility.
+  // The mock can't model a real sharding table, so it treats every
+  // registered (non-zero) identity as a member; tests needing
+  // non-membership behaviour spy/monkey-patch the method.
+  it('isShardingTableMember returns true for any non-zero identityId and false for 0n', async () => {
+    const mock = new MockChainAdapter();
+    expect(await mock.isShardingTableMember(0n)).toBe(false);
+    expect(await mock.isShardingTableMember(1n)).toBe(true);
+    expect(await mock.isShardingTableMember(99999n)).toBe(true);
+  });
+
   it('getEvmChainId returns a bigint (not a namespaced string like "mock:31337")', async () => {
     const mock = new MockChainAdapter();
     const id = await mock.getEvmChainId();

packages/cli/src/daemon/routes/context-graph.ts

@@ -424,39 +424,29 @@ export async function handleContextGraphRoutes(ctx: RequestContext): Promise<voi
 
 
   // POST /api/context-graph/create — context graph definition create.
-  // LU-2: per SPEC_CG_MEMORY_MODEL the legacy multisig-creation branch
-  // (driven by a `participantIdentityIds`-only body, no id/name) is gone.
-  // CGs no longer carry per-CG hosting committees or quorum overrides;
-  // hosts are picked from the network sharding table at publish time
-  // and the ACK quorum is `parametersStorage.minimumRequiredSignatures()`.
-  //
-  // Two client groups exist for these deprecated fields:
-  //   (a) modern clients that send the full `{ id, name, participantIdentityIds, requiredSignatures }`
-  //       body — we strip the dead fields and continue.
-  //   (b) legacy multisig-only clients that send `{ participantIdentityIds, requiredSignatures }`
-  //       with no `id`/`name` — there is no automatic translation that
-  //       preserves caller intent (we can't synthesise a meaningful slug),
-  //       so we fail with an explicit 410-style deprecation error pointing
-  //       at the new combined flow instead of letting the request fall
-  //       through to a generic "Missing id or name" 400.
+  // SPEC_CG_MEMORY_MODEL / Codex PR #595 round-4: per-CG hosting
+  // committees and per-CG quorum overrides were removed end-to-end.
+  // The on-chain contract no longer accepts those args, so silently
+  // stripping `participantIdentityIds` / `requiredSignatures` from the
+  // request body would let callers believe they created an M-of-N /
+  // roster-constrained CG when those constraints were actually
+  // discarded. We reject any body that still carries either field with
+  // a structured 400 + machine-readable `code`, forcing callers to
+  // migrate. The on-chain semantics those fields requested no longer
+  // exist; there is no faithful translation.
   if (req.method === "POST" && path === "/api/context-graph/create") {
     const body = await readBody(req, SMALL_BODY_BYTES);
     const parsed = JSON.parse(body);
     if (parsed.participantIdentityIds !== undefined || parsed.requiredSignatures !== undefined) {
-      const isLegacyMultisigOnly =
-        typeof parsed.id !== 'string' && typeof parsed.name !== 'string';
-      if (isLegacyMultisigOnly) {
-        return jsonResponse(res, 400, {
-          error:
-            'The multisig-only POST /api/context-graph/create flow (participantIdentityIds without id/name) was removed in SPEC_CG_MEMORY_MODEL. Per-CG hosting committees and per-CG quorum overrides no longer exist. Send a regular `{ id, name, accessPolicy?, publishPolicy? }` body and (if you want chain registration) follow up with POST /api/context-graph/register.',
-          code: 'DEPRECATED_MULTISIG_CREATE_FLOW',
-        });
-      }
-      console.warn(
-        '[DKG-Daemon] WARN: POST /api/context-graph/create — `participantIdentityIds` and `requiredSignatures` are deprecated and ignored; per-CG hosting committees and quorums were removed in SPEC_CG_MEMORY_MODEL.',
-      );
-      delete parsed.participantIdentityIds;
-      delete parsed.requiredSignatures;
+      return jsonResponse(res, 400, {
+        error:
+          '`participantIdentityIds` and `requiredSignatures` were removed in SPEC_CG_MEMORY_MODEL. Per-CG hosting committees and per-CG quorum overrides no longer exist on-chain — every CG uses the system-wide ACK quorum (parametersStorage.minimumRequiredSignatures()) and the network sharding table for hosting. Remove these fields from the request body and use `{ id, name, accessPolicy?, publishPolicy?, allowedAgents? }` instead.',
+        code: 'DEPRECATED_CONTEXT_GRAPH_FIELDS',
+        deprecatedFields: [
+          ...(parsed.participantIdentityIds !== undefined ? ['participantIdentityIds'] : []),
+          ...(parsed.requiredSignatures !== undefined ? ['requiredSignatures'] : []),
+        ],
+      });
     }
     // Body has `id` + `name` → context-graph-style context graph definition create (handled below)
     const { id, name, description, allowedAgents, allowedPeers, participantAgents, publishPolicy, accessPolicy, register } = parsed;

packages/cli/test/daemon-http-behavior-extra.test.ts

@@ -725,37 +725,41 @@ describe('CLI-7 — SPARQL endpoint 4xx matrix', () => {
     expect(body.error).not.toMatch(/^0x[0-9a-fA-F]+$/);
   });
 
-  // SPEC_CG_MEMORY_MODEL / Codex PR #595 round-2: per-CG hosting
-  // committees and per-CG quorum overrides were removed. Two paths
-  // exist for legacy clients:
-  //   (a) modern `{ id, name, ...deprecated }` body — the daemon
-  //       strips the dead fields and the request succeeds.
-  //   (b) legacy multisig-only `{ participantIdentityIds, requiredSignatures }`
-  //       body with no `id`/`name` — pre-LU2 there was a dedicated
-  //       branch that bypassed id/name; post-LU2 we MUST fail with an
-  //       explicit deprecation error pointing at the new combined flow
-  //       rather than silently falling through to the generic
-  //       "Missing id or name" 400 (which doesn't tell the caller
-  //       their request shape was retired).
-  it('strips deprecated participantIdentityIds/requiredSignatures from a modern body and still creates the CG', async () => {
-    const d = daemon!;
-    const cgId = 'depr-fields-modern-' + Math.random().toString(36).slice(2, 8);
-    const res = await fetch(urlFor(d, '/api/context-graph/create'), {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json', ...authHeaders(d) },
-      body: JSON.stringify({
-        id: cgId,
-        name: cgId,
-        participantIdentityIds: ['1', '2', '3'],
-        requiredSignatures: 2,
-      }),
+  // SPEC_CG_MEMORY_MODEL / Codex PR #595 round-4: per-CG hosting
+  // committees and per-CG quorum overrides were removed end-to-end.
+  // The on-chain contract no longer accepts those args, so silently
+  // stripping them from the request body would let callers believe
+  // they created a roster-constrained / M-of-N CG when those
+  // constraints were actually discarded. We reject any body that
+  // carries either field, regardless of whether `id`/`name` are also
+  // present — there is no faithful translation.
+  for (const fields of [
+    { participantIdentityIds: ['1', '2'], requiredSignatures: 1 },
+    { participantIdentityIds: ['1', '2'] },
+    { requiredSignatures: 1 },
+  ] as const) {
+    const presentKeys = Object.keys(fields).sort().join('+');
+    it(`rejects POST /api/context-graph/create with deprecated fields (${presentKeys}) — even alongside valid id/name`, async () => {
+      const d = daemon!;
+      const cgId = 'depr-reject-' + Math.random().toString(36).slice(2, 8);
+      const res = await fetch(urlFor(d, '/api/context-graph/create'), {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', ...authHeaders(d) },
+        body: JSON.stringify({ id: cgId, name: cgId, ...fields }),
+      });
+      expect(res.status).toBe(400);
+      const body = (await res.json()) as {
+        error?: string;
+        code?: string;
+        deprecatedFields?: string[];
+      };
+      expect(body.code).toBe('DEPRECATED_CONTEXT_GRAPH_FIELDS');
+      expect(body.error ?? '').toMatch(/SPEC_CG_MEMORY_MODEL/);
+      expect(body.deprecatedFields).toEqual(Object.keys(fields).sort());
     });
-    expect([200, 201]).toContain(res.status);
-    const body = (await res.json()) as { created?: string };
-    expect(body.created).toBe(cgId);
-  });
+  }
 
-  it('rejects legacy multisig-only POST /api/context-graph/create (no id/name) with an explicit deprecation 400', async () => {
+  it('rejects POST /api/context-graph/create with deprecated fields (no id/name) — naming the missing fields explicitly', async () => {
     const d = daemon!;
     const res = await fetch(urlFor(d, '/api/context-graph/create'), {
       method: 'POST',
@@ -767,9 +771,7 @@ describe('CLI-7 — SPARQL endpoint 4xx matrix', () => {
     });
     expect(res.status).toBe(400);
     const body = (await res.json()) as { error?: string; code?: string };
-    expect(body.code).toBe('DEPRECATED_MULTISIG_CREATE_FLOW');
-    expect(body.error ?? '').toMatch(/SPEC_CG_MEMORY_MODEL/);
-    expect(body.error ?? '').toMatch(/id.*name/);
+    expect(body.code).toBe('DEPRECATED_CONTEXT_GRAPH_FIELDS');
   });
 });
 

packages/publisher/src/verify-collector.ts

@@ -99,19 +99,37 @@ export class VerifyCollector {
       batchId, merkleRoot, entities, proposerSignature,
       timeoutMs, allowPartial = false,
     } = params;
+    // FAIL-CLOSED (Codex PR #595 round-4): a caller that omits an
+    // explicit `requiredSignatures` MUST get the system-parameter
+    // quorum. Defaulting to 1 on lookup failure would let the
+    // proposer self-approve and pass quorum, and `chain.verify()`
+    // doesn't re-check signatures on-chain, so this local count is
+    // the only enforcement gate. If we can't determine the system
+    // minimum (no probe wired, RPC fails, garbage value), refuse to
+    // proceed.
     let requiredSignatures = params.requiredSignatures ?? 0;
-    if (requiredSignatures <= 0 && this.deps.getMinimumRequiredSignatures) {
+    if (requiredSignatures <= 0) {
+      if (!this.deps.getMinimumRequiredSignatures) {
+        throw new Error(
+          'VerifyCollector: requiredSignatures was omitted and no `getMinimumRequiredSignatures` probe was wired. ' +
+          'Pass `params.requiredSignatures` explicitly or supply a probe at construction.',
+        );
+      }
+      let sysMin: number;
       try {
-        const sysMin = await this.deps.getMinimumRequiredSignatures();
-        if (Number.isInteger(sysMin) && sysMin >= 1) {
-          requiredSignatures = sysMin;
-        }
-      } catch {
-        // Fall through to the 1-of-1 default below.
+        sysMin = await this.deps.getMinimumRequiredSignatures();
+      } catch (err: any) {
+        throw new Error(
+          `VerifyCollector: getMinimumRequiredSignatures() failed (${err?.message ?? err}). ` +
+          `Pass params.requiredSignatures explicitly or fix the probe.`,
+        );
       }
-    }
-    if (requiredSignatures <= 0) {
-      requiredSignatures = 1;
+      if (!Number.isInteger(sysMin) || sysMin < 1) {
+        throw new Error(
+          `VerifyCollector: getMinimumRequiredSignatures() returned invalid value ${sysMin} (must be a positive integer).`,
+        );
+      }
+      requiredSignatures = sysMin;
     }
     const boundedTimeoutMs = Math.min(
       assertVerifyCollectionTimeoutMs(timeoutMs),

packages/publisher/test/verify-collector.test.ts

@@ -190,4 +190,68 @@ describe('VerifyCollector', () => {
 
     expect(vi.getTimerCount()).toBe(0);
   });
+
+  // Codex PR #595 round-4: a caller that omits `requiredSignatures` MUST
+  // get the system minimum, never a silent default of 1. `chain.verify()`
+  // does not revalidate signatures on-chain, so this local count is the
+  // only enforcement gate — defaulting to 1 would let the proposer
+  // self-approve and pass quorum on a single signature.
+  describe('fail-closed when requiredSignatures is omitted', () => {
+    const baseCollectArgs = {
+      contextGraphId: 'test-cg',
+      contextGraphIdOnChain: 42n,
+      verifiedMemoryId: 1n,
+      batchId: 100n,
+      merkleRoot: new Uint8Array(32),
+      entities: [],
+      proposerSignature: { r: new Uint8Array(32), vs: new Uint8Array(32) },
+      timeoutMs: 1000,
+    } as const;
+
+    it('throws when no `getMinimumRequiredSignatures` probe is wired', async () => {
+      const collector = new VerifyCollector({
+        sendP2P: async () => new Uint8Array(0),
+        getParticipantPeers: () => ['peer-a'],
+        // no getMinimumRequiredSignatures
+      });
+      await expect(collector.collect(baseCollectArgs)).rejects.toThrow(
+        /requiredSignatures was omitted and no `getMinimumRequiredSignatures` probe/,
+      );
+    });
+
+    it('throws when the probe rejects (RPC outage)', async () => {
+      const collector = new VerifyCollector({
+        sendP2P: async () => new Uint8Array(0),
+        getParticipantPeers: () => ['peer-a'],
+        getMinimumRequiredSignatures: async () => { throw new Error('RPC down'); },
+      });
+      await expect(collector.collect(baseCollectArgs)).rejects.toThrow(
+        /getMinimumRequiredSignatures\(\) failed.*RPC down/,
+      );
+    });
+
+    it('throws when the probe returns garbage (non-positive integer)', async () => {
+      const collector = new VerifyCollector({
+        sendP2P: async () => new Uint8Array(0),
+        getParticipantPeers: () => ['peer-a'],
+        getMinimumRequiredSignatures: async () => 0,
+      });
+      await expect(collector.collect(baseCollectArgs)).rejects.toThrow(
+        /returned invalid value 0/,
+      );
+    });
+
+    it('honours the system minimum when the probe is wired correctly', async () => {
+      const collector = new VerifyCollector({
+        sendP2P: async () => new Uint8Array(0),
+        getParticipantPeers: () => [],
+        getMinimumRequiredSignatures: async () => 3,
+      });
+      // remoteRequired = 3 - 1 = 2 > 0 peers → verify_no_peers (proves
+      // the probe value was used, not the silent fallback of 1).
+      await expect(collector.collect(baseCollectArgs)).rejects.toThrow(
+        /verify_no_peers/,
+      );
+    });
+  });
 });