Skip to content

test(rfc64): add release-shaped M1 testnet adapter - #2028

Open
branarakic wants to merge 2 commits into
codex/rfc64-m1-cli-runtime-manifestfrom
codex/rfc64-m1-testnet-adapter
Open

test(rfc64): add release-shaped M1 testnet adapter#2028
branarakic wants to merge 2 commits into
codex/rfc64-m1-cli-runtime-manifestfrom
codex/rfc64-m1-testnet-adapter

Conversation

@branarakic

Copy link
Copy Markdown
Contributor

Impact

This stacked PR closes the deployment gap between the fail-closed M1 evidence contract and a real testnet run. It adds a closed-schema operator adapter that controls isolated local or SSH-hosted Publisher, Edge, and Core CLI processes while all product data movement still uses DKG and chain paths.

Operators can now create a deterministic five-CG corpus spanning public/private, open/curated, on-demand/always-on/unselected cells; publish two VM and SWM waves; run the exact M1 sequence; and obtain a canonical artifact bound to the clean source head, runtime closure, corpus digest, three stable peer identities, process/restart identities, exact payload inventories, and automatic sync journals.

The adapter:

  • cross-checks each live node reported commit against the full tested head;
  • rejects unexpected payload anywhere in a scoped VM or SWM plane;
  • distinguishes metadata-only state from actual data convergence;
  • fails immediately on terminal catch-up errors while retaining bounded retry for transient transport/query errors;
  • proves local or remote process and durable-directory identity;
  • leaves existing live services untouched by using operator-provided isolated commands, ports, stores, and homes;
  • never receives the corpus, trust anchor, or artifact paths from the launcher.

Before

sequenceDiagram
  participant O as Operator
  participant G as M1 gate
  participant X as Missing deployment adapter
  O->>G: Start live test
  G->>X: Framed runtime command
  X--xG: No implementation supplied
  G-->>O: No testnet evidence
Loading

After

sequenceDiagram
  participant O as Operator
  participant G as M1 gate
  participant A as Testnet operator adapter
  participant P as Isolated Publisher
  participant E as Isolated Edge
  participant C as Isolated Core
  O->>G: Start anchored live test
  G->>A: Sequenced runtime commands
  A->>P: Start and publish two VM and SWM waves
  A->>E: Select, sync, restart, and observe
  A->>C: Start cold and observe automatic public coverage
  P-->>A: Live identity and exact snapshots
  E-->>A: Job and reconciler journal evidence
  C-->>A: Bounded scheduler journal evidence
  A-->>G: Closed decoded observations
  G-->>O: Verified canonical PASS artifact
Loading

Validation

  • pnpm typecheck:m1:rfc64-selective-coverage
  • pnpm test:m1:rfc64-selective-coverage:unit (75 existing contract tests plus 3 operator boundary/observation tests)
  • live release-CLI adapter start/readiness/shutdown smoke
  • live scoped metadata and VM query probe against the isolated local testnet node
  • git diff --check

The full three-process testnet evidence run will be posted here after this exact head is staged on the isolated roles. No production or existing canary service is modified by this PR.


async function dataDirectoryIdentity(role: TestnetOperatorRoleV1): Promise<string> {
if (role.transport === 'local') {
const path = realpathSync(role.dataDir);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: Relative dataDir evidence is resolved against the adapter cwd, not the node cwd

What's wrong
The restart proof depends on dataDirectoryIdentity matching the durable directory used by the node. The adapter launches local commands with cwd=repoRoot and SSH commands after cd repoRoot, but computes dataDir identity from the adapter/remote shell's current directory. For relative dataDir values, the evidence can describe the wrong directory while still passing the same-before-and-after restart check.

Example
With repoRoot=/srv/dkg-edge and dataDir=data, the node process runs from /srv/dkg-edge and uses /srv/dkg-edge/data, but the adapter records realpathSync('data') from the adapter cwd, for example /var/lib/review-agent/.../data. The restart receipt can then claim durable directory reuse for a directory that was not the node's actual data directory.

Suggested direction
Resolve dataDir using the same base directory that the role command uses, or reject relative dataDir values during config decoding.

Confidence note
This depends on operators being allowed to provide relative dataDir values; the schema currently accepts them and the launcher runs each role from repoRoot, so that is a realistic configuration.

For Agents
In devnet/rfc64-m1-selective-coverage/testnet-operator-adapter.ts, resolve relative dataDir against role.repoRoot for local roles and run the remote identity probe from role.repoRoot or require dataDir to be absolute. Preserve absolute-path behavior. Add a focused test or harness check proving a relative dataDir is identified as repoRoot/data for both start and restart evidence.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: Relative dataDir is measured from the wrong directory

What's wrong
The adapter’s durable directory evidence can describe a different path than the node actually uses. The M1 gate relies on dataDirectoryIdentity to prove the Edge restart reused the same durable state, so a relative path resolved from the adapter cwd or SSH home breaks that proof and can also make otherwise valid configs fail before readiness.

Example
A valid-looking role config with repoRoot: "/opt/dkg-edge" and dataDir: ".dkg" launches the node against /opt/dkg-edge/.dkg, but the adapter records or stats .dkg relative to the adapter process cwd locally, or the SSH login directory remotely. That can fail startup even though the node would run, or bind the restart receipt to the wrong durable directory.

Suggested direction
Resolve or validate dataDir using the same base directory as the role launch command, and apply the same rule to SSH probes.

For Agents
In testnet-operator-adapter.ts, resolve dataDir relative to role.repoRoot for local roles and run the remote readlink/stat probe from role.repoRoot or require/enforce absolute dataDir. Preserve the restart invariant that the old and new Edge processes reuse the same actual node data directory. Add a test with a relative dataDir and a different repoRoot proving the reported identity points at the launched node directory.

`${roleName} process exit after shutdown`,
);
} finally {
this.processes.delete(roleName);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: Failed shutdown removes the process handle before the process has exited

What's wrong
The controller loses ownership of a launched node whenever the shutdown request fails before running.exited resolves. That turns a cleanup failure into an untracked live process, which is a lifecycle bug for an adapter that is supposed to run isolated Publisher/Edge/Core nodes and prove clean process boundaries.

Example
If /api/shutdown returns 500 or hangs until withTimeout rejects, stop('edge') throws but the child process is removed from this.processes. A later stopAll cannot see or terminate that still-running Edge process, leaving it bound to the same ports/data directory and able to affect a retry or the next local testnet run.

Suggested direction
Only delete the role from processes after confirmed exit, or pair deletion with a forced kill path that guarantees the launched process is no longer running.

For Agents
In TestnetOperatorController.stop, keep the process tracked until it has actually exited, and on graceful shutdown failure send SIGTERM/SIGKILL or otherwise wait for/verify exit before deleting it. Preserve the existing abnormal-exit error reporting. Add a test with a fake child and failing shutdown response proving stopAll still terminates or reports a tracked live process.

assert.equal(plane.dataTripleCount, 2);
assert.equal(plane.metadataTripleCount, 7);
}
assert.equal(requests.length, 4);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: Observation test can pass without scoped queries

What's wrong
The new test claims to cover scoped VM/SWM evidence, but it stubs responses based only on whether the SPARQL contains COUNT(*). It does not fail when the production code stops sending the scope that makes the evidence trustworthy.

Example
If observePlane accidentally dropped contextGraphId from the data query or queried the wrong metadata graph IRI, this fake server would still return the same planned bindings/count and the test would remain green, even though live observations would no longer prove scoped VM/SWM evidence.

Suggested direction
Turn the fake node into a stricter contract test for the query shapes, not just the returned digest values.

For Agents
Tighten testnet-operator-common.test.ts: make the fake /api/query handler reject unexpected bodies, and assert every request has contextGraphId: 'cg-1'; data queries use the exact VM/SWM view; metadata queries use includeContextGraphPartitions: true and the expected _meta/_shared_memory_meta graph IRIs. Preserve the current digest assertions.

type TestnetOperatorRoleV1,
} from './testnet-operator-common.ts';

const COMMAND_SCHEMA = 'dkg-rfc64-m1-selective-coverage-runtime-command-v1';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Move the runtime wire contract out of the adapter

What's wrong
This makes the first concrete adapter a parallel owner of the runtime protocol. Future versioning or envelope changes now require synchronized edits in the launcher and adapter, which is exactly the kind of boundary drift the existing codec modules appear designed to avoid.

Example
process-runtime.ts defines SELECTIVE_COVERAGE_RUNTIME_COMMAND_SCHEMA, SELECTIVE_COVERAGE_RUNTIME_RESULT_SCHEMA, and SELECTIVE_COVERAGE_RUNTIME_RESULT_PREFIX; this adapter repeats the literals at lines 33-35 and separately validates/emits the envelope later.

Suggested direction
Create a small shared wire module, likely beside runtime-wire.ts, for command/result schemas, prefixes, and envelope parsing/serialization instead of letting the concrete adapter become a second source of truth.

For Agents
Extract the runtime line-protocol constants and command/result envelope codec into a shared module used by both process-runtime.ts and testnet-operator-adapter.ts. Preserve the existing JSON line protocol and add a focused unit test proving client encode/decode and adapter decode/encode use the same contract.

roleName: SelectiveCoverageRuntimeRole,
roleConfig: TestnetOperatorRoleV1,
): { child: ChildProcessWithoutNullStreams; pid: Promise<number> } {
if (roleConfig.transport === 'local') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Collapse repeated local/ssh branches into a transport driver

What's wrong
The adapter currently mixes role orchestration with transport mechanics. That makes the controller harder to scan and creates several places where transport-specific behavior can drift.

Example
Changing SSH invocation semantics would require edits around lines 521, 564, 618, and 637, even though those call sites all represent the same underlying transport concern.

Suggested direction
Parse or wrap each role into a transport-specific driver once, then let the controller operate on capabilities rather than branching on role.transport across unrelated lifecycle and provenance helpers.

For Agents
Introduce a RoleTransport or RoleDriver abstraction from the parsed role config with methods such as launch, capture, runtimeProvenance, and dataDirectoryIdentity. Preserve local/SSH behavior, env handling, PID evidence, and identity evidence; cover command construction for both transports with focused tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Transport details leak through the orchestration layer

What's wrong
The PR models transport as a flag and then scatters that flag through process launch, command capture, runtime provenance, and data-directory identity. That makes the adapter harder to extend and harder to reason about because runtime orchestration is coupled to remote shell quoting and process mechanics.

Example
Starting a role currently walks through runtimeProvenance -> roleCapture, dataDirectoryIdentity -> roleCapture, and then launchRole; each helper separately knows how local and ssh roles work. A new transport or a change to ssh command construction would require edits across all of those helpers.

Suggested direction
Introduce a RoleTransport / RoleRunner boundary when reading the config, with local and ssh implementations behind it. The controller should call capabilities instead of branching on transport in unrelated helpers.

For Agents
In testnet-operator-common.ts / testnet-operator-adapter.ts, preserve local and ssh behavior but decode each role into a transport adapter such as capture, launch, and dataDirectoryIdentity. Then keep TestnetOperatorController.start focused on lifecycle orchestration and add a small unit test around local/ssh command construction.

} catch (error) {
throw new Error('M1 testnet operator config is not valid JSON', { cause: error });
}
const root = record(input, 'operator config');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Reuse the existing boundary-codec for operator config parsing

What's wrong
This adds a parallel JSON-boundary implementation inside the same package. Even if it validates today’s shape, maintainers now have to reason about two subtly different closed-schema helper sets, and the new key lists are not tied to the TypeScript interfaces.

Example
readTestnetOperatorConfig uses raw string arrays in assertExactKeys; the existing codec style uses defineRecordKeys<T>(), which type-checks key lists when model fields change.

Suggested direction
Move the operator config decoder into the same pattern as evidence-codec.ts and runtime-wire.ts, using defineRecordKeys, closedRecord, closedArray, and shared bounded validators rather than a second validation dialect.

For Agents
Refactor readTestnetOperatorConfig around boundary-codec primitives and typed key lists. Preserve the closed schema, exact five-graph plan, and one-selected/one-final asset rules; keep the current unknown-key tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Operator config adds a second closed-schema decoder stack

What's wrong
This new trust boundary duplicates the RFC-64 closed-schema decoding infrastructure instead of extending it. The result is more code to maintain, less type-linked schema coverage, and a higher chance that operator config validation drifts from the package’s canonical boundary behavior.

Example
If TestnetOperatorGraphV1 gains a field, the plain assertExactKeys([...]) list here will not fail type-checking. The existing defineRecordKeys<TestnetOperatorGraphV1>()(...) pattern would force the decoder to be updated with the model.

Suggested direction
Reuse the canonical boundary-codec primitives and typed record-key declarations instead of carrying a separate validation mini-framework in this new module.

For Agents
Move operator config parsing toward the existing boundary-codec.ts / evidence-codec.ts style. Keep readTestnetOperatorConfig as the file-read wrapper, add typed key lists for config/role/graph/asset shapes, and preserve the current closed-schema behavior covered by testnet-operator-common.test.ts.

);
}

private async waitForGraphSnapshot(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Deduplicate complete snapshot projection

What's wrong
The snapshot shape is a core evidence model invariant, but this PR now has multiple hand-written projections. If the manifest shape or completeness rule changes, the adapter and common helper can drift independently.

Example
Both paths copy headDigest, inventoryDigest, assetCount, and dataTripleCount from vm/swm only after checking reportedComplete and non-null digests.

Suggested direction
Keep the adapter’s polling behavior, but move the observation-to-expectation projection and completeness invariant into one common helper.

Confidence note
The existing helper cannot be used as-is inside the polling loop because the adapter also waits for an expected asset count, but the completeness check and projection logic are still shared model logic and should be factored out.

For Agents
Extract a pure helper such as toGraphSnapshotExpectation(observation, label) from testnet-operator-common.ts, use it from both observeGraphSnapshot and adapter polling after the expected asset-count predicate, and cover complete/incomplete observations in tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Graph snapshot projection is duplicated instead of centralized

What's wrong
The adapter reimplements a model transformation that already lives in the common module. That makes the snapshot shape less canonical and turns a simple polling wrapper into another place that owns snapshot semantics.

Example
Both helpers check reportedComplete, headDigest, and inventoryDigest, then copy headDigest, inventoryDigest, assetCount, and dataTripleCount for VM and SWM. A future snapshot field or completeness rule would have to be updated in both places.

Suggested direction
Keep polling in the adapter, but move the complete-observation-to-snapshot projection into one shared helper and reuse it from both observeGraphSnapshot and waitForGraphSnapshot.

For Agents
Extract a pure helper in testnet-operator-common.ts, for example completeGraphSnapshot(observation, label), and have both observeGraphSnapshot and adapter polling call it. Preserve the adapter’s expected asset-count polling behavior.


constructor(private readonly cfg: TestnetOperatorConfigV1) {}

async handle(command: string, payload: Record<string, unknown>): Promise<unknown> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Real testnet adapter commands are untested

What's wrong
This PR changes the live gate from requiring an external adapter to shipping a concrete process controller, but the tests only cover the generic runtime boundary and shared helper functions. The actual adapter dispatch, polling, failure handling, and state tracking have no regression coverage.

Example
A regression in the adapter where synchronize-edge emits a success envelope before waitForCatchup, or where core-automatic-round ignores lastCoreSequence, would not be caught by the current suite because only the generic process-runtime fixture and common query helpers are tested.

Suggested direction
Add focused integration-style tests for the shipped adapter command loop and high-risk command handlers, using fake node endpoints rather than a live testnet.

For Agents
Add adapter-level tests around testnet-operator-adapter.ts, preferably by launching it as a subprocess with a temp operator config and fake HTTP nodes. Cover at least command envelope round-trip, publish-wave, terminal catch-up failure, reconciler/core journal handling, and shutdown cleanup.

"scripts": {
"typecheck": "tsc --noEmit -p tsconfig.json",
"test": "node --experimental-strip-types --test verifier.test.ts runtime.test.ts process-runtime.test.ts adapter-environment.test.ts operator-input.test.ts boundary-codec.test.ts",
"test": "node --experimental-strip-types --test verifier.test.ts runtime.test.ts process-runtime.test.ts adapter-environment.test.ts operator-input.test.ts boundary-codec.test.ts && node --import tsx --test testnet-operator-common.test.ts",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: Unit coverage does not exercise the shipped testnet adapter

What's wrong
This PR adds the production adapter that now drives the live M1 gate, but the added tests only cover the shared helper/decoder functions. That leaves the riskiest changed behavior unverified: process start/stop evidence, command dispatch, Edge catch-up terminal failures, reconciler journal handling, and Core automatic round evidence can regress while the new unit suite stays green.

Example
A regression that treats /api/sync/catchup-status { "status": "denied" } as transient, or drops includeSharedMemory: true from the synchronize-edge subscribe request, would still pass the current unit suite. A focused test could run the adapter against a fake node API, send a framed synchronize-edge command, and assert the terminal status produces a closed failure result before the retry timeout.

Suggested direction
Add focused adapter-level tests, using a temporary operator config and fake HTTP node endpoints, so the new adapter's command dispatch and fail-closed polling behavior are verified without requiring a real testnet.

For Agents
Look at devnet/rfc64-m1-selective-coverage/testnet-operator-adapter.ts and add a test target that exercises the shipped adapter, not only testnet-operator-common.ts. Preserve the framed process protocol and prove at least one high-risk path such as synchronize-edge terminal catch-up failure, successful job ID propagation, or journal-driven Core/Edge operation evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants