proof: implement local governed execution proof path - #70
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3daed70ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const nowIso = new Date().toISOString(); | ||
| const node={nodeId:'local-node-1',trustStatus:'trusted_local',attestationStatus:'not_required',telemetryState:'unavailable',profileSource:'local-default',registeredAt:nowIso}; |
There was a problem hiding this comment.
Remove runtime timestamp from deterministic proof artifacts
The demo claims a deterministic proof path, but registeredAt is set from new Date().toISOString(), which changes on every run and therefore changes manifestHash and proofpack.json output each time. In practice, running npm run demo:local-proof twice yields different hashes, so replay/evidence comparisons are not stable even when inputs are identical.
Useful? React with 👍 / 👎.
| function loadTopic(topic: OperatorTopic, rootDir: string, source?: string): OperatorRecord[] { | ||
| const sourceDir = source ? path.resolve(source, "operator") : path.join(rootDir, "fixtures", "demo"); | ||
| const file = path.join(sourceDir, `${topic}.json`); | ||
| return JSON.parse(fs.readFileSync(file, "utf8")) as OperatorRecord[]; |
There was a problem hiding this comment.
WARNING: No error handling for file read operations. If the file doesn't exist or JSON is malformed, the CLI will crash with an unhandled exception. For a tool meant for operator inspection, this should provide a helpful error message instead of crashing.
Consider adding try-catch with validation:
function loadTopic(topic: OperatorTopic, rootDir: string, source?: string): OperatorRecord[] {
const sourceDir = source ? path.resolve(source, "operator") : path.join(rootDir, "fixtures", "demo");
const file = path.join(sourceDir, `${topic}.json`);
try {
const content = fs.readFileSync(file, "utf8");
return JSON.parse(content) as OperatorRecord[];
} catch (err) {
throw new Error(`Failed to load operator topic '${topic}': ${err.message}`);
}
}
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Note: The non-deterministic timestamp issue at line 12 of Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)No additional observations outside the diff. Files Reviewed (3 files)
Reviewed by laguna-m.1-20260312:free · 534,932 tokens |
Motivation
Description
scripts/demo/run-local-governed-proof.mjsthat generates deterministic run artifacts under.artifacts/local-governed-proof/includingmanifest.json,proofpack.json, andoperator/*.jsontopic files and emits the lifecycle events expected by the proof path.--sourceflag insrc/lib/commands/operator.tsand updatedloadTopicso operator topics can be loaded from<source>/operator/<topic>.jsonwhen inspecting a real run directory.demo:local-proofnpm script topackage.jsonto run the demo, and createddocs/demo/local-governed-proof.mddocumenting the demo, artifacts, inspection commands, and explicit limitations.nvidia-smiis not present, fails closed on unavailable pieces, and redacts sensitive-looking tokens in operator outputs.Testing
npm run demo:local-proofand observed the expected lifecycle output sequence and creation of the deterministic artifacts under.artifacts/local-governed-proof/(success).git diff --checkto validate no whitespace/format issues (success).npm installto prepare the wider verification suite but full dependency installation/verify suite was not completed in this environment (partial; install attempted but environment limits prevented finishing full verification).Codex Task