Skip to content

feat: define scheduler handoff v4 artifact - #11

Merged
amittell merged 29 commits into
mainfrom
codex/handoff-v4
Jul 19, 2026
Merged

feat: define scheduler handoff v4 artifact#11
amittell merged 29 commits into
mainfrom
codex/handoff-v4

Conversation

@amittell

@amittell amittell commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • define the canonical, immutable scheduler handoff v4 artifact and a public strict JSON Schema
  • compile full execution, approval, identity, proof, authorization, credential, delegation, output, verification, and evidence bindings without raw-secret persistence
  • bind scheduler routing and reliability controls, including payload scope, resource pool, and job class
  • negotiate v4 only against the scheduler exact schema, canonicalization, binding, feature, and minimum database contract
  • fail closed for runtime downgrade, proof revocation, verified key identity, mandatory certificate possession, unsupported proof methods, and malformed artifacts
  • preserve artifact integrity across explicit compile environments and adoption/update origin rebinding
  • retain handoff v1-v3 compatibility and harden npm release CI with exact scheduler commit pinning

Validation

  • 806/806 tests pass against openclaw-scheduler bb9dae60270d2945bec62403bc6f08adc7c50ec8
  • scheduler integration suite passes 12/12
  • ESLint passes with zero warnings
  • extracted npm package contract passes
  • npm audit reports zero vulnerabilities
  • shared conformance fixture is byte-identical with the scheduler fixture at the paired release commits

Paired release heads:

  • AgentCLI: 656abee
  • openclaw-scheduler: bb9dae60270d2945bec62403bc6f08adc7c50ec8

Companion to amittell/openclaw-scheduler#19.

Copilot AI review requested due to automatic review settings July 19, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds scheduler handoff v4 support to agentcli by introducing an immutable, content-addressed “handoff artifact” (plus shared conformance fixtures), extending compilation/apply to negotiate v4 only when the runtime advertises the required capabilities, and adding proof/evidence verification hooks and stronger release CI/package contract checks.

Changes:

  • Introduce handoff v4 artifact generation + validation, shared conformance fixture, and extensive tests covering determinism/tamper-evidence, credential presentation mapping, apply semantics, and proof verification.
  • Extend scheduler field projection/version negotiation to include v4 fields and update feature/capability handling, targets baseline, and inspect surface for new immutable runtime entities.
  • Harden release/CI with fixture parity checks and a packaged-artifact verification script; update docs/changelog for v4 and version bump to 0.5.0.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/integration-scheduler.test.js Updates expected negotiated handoff field version to allow v4.
test/handoff-v4.test.js Adds end-to-end tests for v4 artifact determinism, apply behavior, credential presentation, and proof verification.
test/agentcli.test.js Updates feature baseline assertions and adjusts proof value_from shape expectations.
src/targets.js Expands openclaw-scheduler feature baseline to include v4-related capabilities.
src/scheduler-fields.js Adds v4 scheduler fields and registers them under field version 4.
src/inspect.js Adds new inspect entities (evidence, artifacts, events, provider sessions, credential presentations).
src/handoff/v4.js Implements v4 artifact builder + validator (canonicalization, bindings, and safety checks).
src/handoff/index.js Exposes the v4 handoff module entrypoint via a stable index.
src/evidence/payload.js Extends evidence bindings and binding validation to include artifact/run lineage fields.
src/evidence/index.js Adds optional verification-time binding check against an audit record.
src/compiler/openclaw-scheduler.js Adds schedulerHandoffVersion support and emits v4 artifact fields when enabled.
src/capabilities.js Adds v4 required-feature list and helper to determine v4 support; adds v4 capability checks for v4 jobs.
src/authorization-proof/jwt.js Tightens v4 JWT requirements (artifact binding, exp/iat/jti, replay + revocation hooks).
src/authorization-proof/detached-signature.js Adds v4 detached-signature envelope/signing content + replay/revocation guards and artifact binding.
src/authorization-proof/certificate.js Adds v4 certificate envelope/signing content + replay/revocation guards and artifact binding.
src/apply.js Adds v4 negotiation path, v4 field projection support, and includes artifact payload as a JSON blob field.
scripts/verify-package.mjs Adds a package-contract verification script (pack/extract checks + required/forbidden paths).
README.md Documents expanded inspect entities and updates openclaw-scheduler target description for v4.
package.json Bumps version to 0.5.0, adds exports for new modules, includes fixtures/scripts in package, adds verify:package.
package-lock.json Updates lockfile version fields to match 0.5.0.
fixtures/handoff-v4/conformance.json Adds shared positive/negative conformance fixture for v4 artifact parity.
docs/versioning.md Updates documented package version to 0.5.0.
docs/spec.md Adds scheduler handoff v4 specification section (artifact contract, runtime gates, compatibility).
docs/runtime-integration-backlog.md Marks v4 handoff work as delivered and updates status wording.
docs/roadmap.md Updates roadmap to reference v4 handoff and its capabilities.
docs/protocol.md Updates example outputs/handshake docs to include v0.5.0 and handoff field version 4.
docs/adoption.md Updates adoption doc to reference capability negotiation through handoff v4.
CHANGELOG.md Adds release notes for 0.5.0 summarizing v4 handoff, proofs, evidence, inspect, and fixtures.
.github/workflows/publish.yml Hardens publish pipeline (exact-main gate, main CI success gate, fixture parity, package verification, pinned actions).
.github/workflows/ci.yml Adds fixture parity + package verification to CI and pins actions versions/refs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/apply.js Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1405312070

ℹ️ 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".

Comment thread src/targets.js Outdated
Comment thread src/authorization-proof/jwt.js
Comment thread src/handoff/v4.js Outdated
Comment thread src/apply.js Outdated
Comment thread src/handoff/v4.js
Comment thread src/capabilities.js
Comment thread src/authorization-proof/detached-signature.js Outdated
Comment thread src/authorization-proof/jwt.js
Comment thread src/apply.js Outdated
@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

@amittell

Copy link
Copy Markdown
Owner Author

@codex review

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

Review summary for the current HEAD (cd1298a)

The PR is in good shape. Here are my findings:

Previously flagged issue — resolved ✅
The v4 gate in apply.js:276 now correctly calls supportsSchedulerHandoffV4(runtimeCaps) against the raw runtime response. effectiveResult (with its static fallback defaults) is only used downstream for capability validation and field-version negotiation, which is the right split. The regression test at test/handoff-v4.test.js:452 covers both feature === false and delete feature[key] (omitted key), confirming fail-closed behavior in both cases.

No new issues found

  • spawnSync('ssh-keygen', [...]) in detached-signature.js uses an array argument list — no shell-injection surface.
  • validateHandoffV4Structure in v4.js explicitly rejects bindings carrying raw credential material (value, credential, secret, token keys) before the artifact is emitted.
  • publicKeyId in key-identity.js canonicalizes to SPKI-DER before hashing — no ambiguity in key representation.
  • All 791 tests pass.

The implementation is correct and the security-sensitive paths (proof verification, credential presentation, v4 gating) all fail closed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

schedulerRunner.updateJob(job.id, schedulerUpdateSpec(job, { fieldVersion: handoffVersion }));

P1 Badge Preserve the existing origin when updating v4 jobs

When a stable job has a non-system origin—most notably after the new adopt-by-name path preserves a legacy origin—a subsequent apply compiles the replacement artifact against origin: "system", while schedulerUpdateSpec deliberately removes origin from the update. The stored job therefore retains its legacy origin but receives a scheduler_job_binding.digest calculated for a different origin, causing v4 binding validation to reject the update or prevent dispatch. Rebind the v4 artifact using the existing row's origin before calling updateJob.

ℹ️ 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".

Comment thread src/handoff/schema-v4.js Outdated
Comment thread src/apply.js
Comment thread src/authorization-proof/certificate.js Outdated
Comment thread src/handoff/schema-v4.js Outdated
@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

@codex review

Please review exact head cce0e40. This head addresses every prior inline thread and the top-level origin-preservation finding: valid on-failure cleanup, fail-closed proof method validation, mandatory v4 certificate proof of possession, safe rejection of v4 runtime downgrades, and exact existing-origin rebinding on updates. Local gates: 806/806 tests, lint, package verification, and npm audit all pass.

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

@codex review...

Review of cce0e40

All five claimed fixes are correct and well-tested:

  • Origin preservation (apply.js): existingJob is now correctly populated in both the exactMatch branch and the existingById lookup (previously has() was used instead of get(), so existingJob was never set for non-adopt updates). The existingJob?.origin rebind is applied consistently for both update and adopt paths.

  • V4 downgrade protection (apply.js): The new guard throws unsupported_capability before any write if the stored job is v4 but the recompiled artifact is not. No mutation reaches the scheduler in the degraded-runtime case.

  • Mandatory certificate proof of possession for v4 (certificate.js): envelope.v4 || context.requireProofOfPossession makes possession verification unconditional for v4 artifacts regardless of what the call site sets in context. The new test at line 952 confirms that passing requireProofOfPossession: false still fails a forged signature when envelope.v4 is truthy.

  • on-failure cleanup policy (schema-v4.js): Added to the JSON Schema enum. validateHandoffV4Structure drives the runtime validator via validateNode, so the new policy is accepted and an unrecognised policy is rejected at artifact validation time.

  • Proof method enum restriction (schema-v4.js): authorization_proof.method is now a closed enum ['none','jwt','detached-signature','certificate',null] in the JSON Schema. The typo test ('jtw') at line 867 confirms rejection; the CRYPTOGRAPHIC_PROOF_METHODS set downstream remains consistent with the schema.

794/794 tests pass. No issues found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cce0e4066b

ℹ️ 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".

Comment thread src/apply.js Outdated
Comment thread src/authorization-proof/jwt.js Outdated
Comment thread src/authorization-proof/jwt.js Outdated
Comment thread src/handoff/v4.js
Comment thread src/handoff/v4.js Outdated
Comment thread src/capabilities.js
Comment thread src/authorization-proof/certificate.js Outdated
@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

@codex review

Please review final exact head 656abee. The only change since the clean cce0e40 exact-head code review is immutable CI/publish pinning to fully reviewed and green openclaw-scheduler bb9dae60270d2945bec62403bc6f08adc7c50ec8. Local cross-repo gate: 806/806 tests, 12/12 scheduler integration, lint, package verification, fixture parity, and npm audit all pass.

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

@codex review...

Review of 656abee (final head)

The only change from the previously clean cce0e40 is a two-file CI update: OPENCLAW_SCHEDULER_REF, the ref: checkout value, and the git rev-parse HEAD parity check are all updated to bb9dae60270d2945bec62403bc6f08adc7c50ec8 in both ci.yml and publish.yml. All three values are in lock-step within each file and across both files — no ref mismatch. The actions/checkout pin is unchanged.

794/794 tests pass. No issues found.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 656abee8d6

ℹ️ 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".

@amittell

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact head 3d710f3. Focus on exact v4 capability contracts, proof digest canonicalization, precompile delegated negotiation and one-off binding, canonical persisted JSON, evidence declaration parity, and v4 evidence lineage. Hosted CI and all local release gates pass; all current review threads are resolved.

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

Please review exact head 3d710f3. This head closes all six delayed findings plus the independent canonical manifest-digest and null-options edge cases...

Review of 3d710f31 (six delayed findings + canonical manifest digest + null-options edge cases)

All six changes are correct and well-tested.

1. Canonical JSON blob serialization for v4 (apply.js)

projectSchedulerSpec gains a canonicalJsonBlobs option. When fieldVersion === '4', both schedulerCreateSpec and schedulerUpdateSpec pass canonicalJsonBlobs: true, serializing JSON blob fields with canonicalStringify (sort-keys-sha256) instead of JSON.stringify. This ensures the persisted handoff_artifact_payload string is byte-for-byte identical to the canonicalized form used for digest computation, removing a source of digest mismatch when key order varies. The test at line 9972 asserts job_spec.delete_after_run === 1 and 'handoff_version' in result.job_spec === false for the dry-run path (v3 compile, so no v4 artifact is present on the spec).

2. Exact v4 runtime contract (capabilities.js)

supportsExactV4Contract now additionally checks Array.isArray(contract) (arrays are objects), sorts and compares key lengths, and confirms all keys match positionally. This closes the gap where an extended runtime contract (extra undeclared keys) would previously have passed the every check. The new test case 'extended contract' with future_semantics: true added to HANDOFF_V4_RUNTIME_CONTRACT confirms rejection. Verified: exact match passes, extra key fails, missing key fails, array fails, null fails.

3. Precompile delegated v4 negotiation and one-off binding (runtime/openclaw-scheduler.js, compiler/openclaw-scheduler.js)

Capability negotiation is moved before compileManifestForDispatch. schedulerHandoffVersion is set to '4' only if supportsSchedulerHandoffV4(runtimeCaps) — passing the raw capability response, not the static fallback. The oneOffSource parameter propagates the target workflow_id/task_id into the compiler, which sets delete_after_run: 1 on the matching job atomically — before the v4 artifact is sealed. The previous approach spread { ...job, delete_after_run: 1 } after compilation, meaning the v4 artifact's lifecycle.delete_after_run would have been false while the persisted column was 1, causing a projection mismatch on the first rebind. The integration test at line 10051 spins up a fake scheduler binary, executes via executeTask, captures the written spec, and asserts captured.delete_after_run === 1, artifact.lifecycle.delete_after_run === true, captured.handoff_version === 4, and that assertValidSchedulerHandoffV4Job(captured) passes — covering all four corners of the fix.

4. Canonical persisted JSON in assertValidSchedulerHandoffV4Job (v4.js)

validatePersistedEvidenceBinding reads job.evidence via normalizeJsonValue (handles both raw object and stringified JSON) and compares each evidence field with the artifact binding via canonicalStringify, tolerating undefinednull normalization. The storedV4 sub-test confirms that schedulerCreateSpec followed immediately by assertValidSchedulerHandoffV4Job passes — i.e., the canonical serialization round-trips. The test also confirms JSON.parse(storedV4.evidence).payload.context.policy_version === null (the undefined value is normalized to null by sortKeysDeep), while the v3 spec confirms the key is absent entirely.

5. V4 evidence declaration lineage (v4.js)

validatePersistedEvidenceBinding enforces: (a) payload_hash and provider_config_hash are present and valid, (b) provider_config is null, (c) payload_hash matches hashObject(declaration.payload), (d) evidence_ref and declaration.ref are consistent, and (e) all nine evidence binding fields match the artifact exactly. Two new tamper tests confirm that mutating payload_hash or provider_config_hash in the artifact triggers HANDOFF_ARTIFACT_INVALID.

Additionally, validateSchedulerHandoffV4Artifact now rejects non-null payload_hash and provider_config_hash when the evidence block carries no evidence declaration, preventing a hash smuggle into a structurally empty binding. The test at line 348 drives both hash fields with a non-null value against a no-evidence payload and asserts rejection with must be null when evidence is not declared.

6. Canonical artifact digest format for JWT bindings (jwt.js)

normalizeDigest is removed. For v4, both the trusted context digest and the JWT claim are validated against CANONICAL_SHA256_PATTERN before comparison; bare hex strings (no sha256: prefix) and uppercase digests are rejected explicitly. The non-v4 path retains the old prefix-stripping comparison for backward compatibility. Eight new test cases (four for artifact digest, four for manifest digest) confirm rejection of bare and uppercase forms on both the trusted and claimed sides.

7. listJobs(null) null-options safety (apply.js)

listJobs({ includeHandoffArtifacts = false } = {}) is replaced with listJobs(options = {}) + options?.includeHandoffArtifacts === true, so listJobs(null) no longer throws a destructuring TypeError. The test at line 988 drives listJobs(null) and asserts an empty array with no --include-handoff-artifacts argument.

No issues found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d710f31df

ℹ️ 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".

Comment thread src/evidence/payload.js Outdated
Comment thread src/authorization-proof/detached-signature.js
Comment thread src/handoff/v4.js
Comment thread src/handoff/v4.js Outdated
Comment thread src/authorization-proof/jwt.js Outdated
@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

@codex review

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

@codex review

Review of 3c85978 (final verification hardening)

810/810 tests pass. CI is green. All six changes are correct and well-tested.

1. Centralized replayClaimAccepted helper (replay.js)

The new helper tightens the prior replay === true || replay?.claimed === true || replay?.ok === true check on all three verifiers. The key semantic change is that { ok: true } without a claimed property now returns false — requiring explicit opt-in from the replay store via claimed: true. Object.hasOwn blocks prototype-inherited properties (the inherited-claim test case). The conflicting-values matrix (claimed: true, ok: false and claimed: false, ok: true) is also fully covered. The { claimed: true } (no ok key) case is intentionally accepted for backwards compatibility with replay stores that don't populate ok.

2. Canonical SHA-256 enforcement in certificate and detached-signature verifiers

Both verifiers now introduce a trustedArtifactDigest field (populated only in artifact mode) and validate it — plus the proof-side artifact_digest — against CANONICAL_SHA256_PATTERN before strict equality comparison. This closes the case where a non-canonical input (bare hex, uppercase) could pass the prior normalizeDigest-based comparison. The test matrix covers bare and uppercase variants of both the proof digest and the trusted digest. The artifact_bound audit field is updated to use trustedArtifactDigest for parity. JWT was already strict and required no changes beyond the replay helper.

3. argv_sha256 always computed (v4.js)

The prior command ? canonicalDigest(...) : null guard is removed. For prompt/non-shell tasks, program is null and argsHashes is [], so argv_sha256 = canonicalDigest([null]) — a deterministic, valid SHA-256. The validator now also: (a) type-checks args_sha256 as an array before calling validateHashCollection, (b) checks args_count === args_sha256.length, and (c) recomputes and compares argv_sha256. The prompt-task test confirms the null-program path validates cleanly.

4. Evidence payload_hash always a SHA-256 when evidence is declared (v4.js, compiler/openclaw-scheduler.js)

canonicalDigest(evidence.payload ?? {}) replaces the null-for-absent-payload path, producing the digest of {} for payloadless profiles — matching the validator's SHA256_PATTERN.test(artifactEvidence.payload_hash) requirement. hasEvidenceDeclaration is extended with signed_or_provider_verified_required === true so the payload_hash requirement fires even when no ref/provider/method fields are present.

5. sourceRunRequired split (evidence/payload.js)

The refactor correctly separates sourceRunMarker (structural flags: child_run, parent_id, etc.) from sourceRunFieldsPresent (actual field values). Markers now only gate on v4; the legacy test confirms a non-v4 record with parent_id no longer spuriously requires source_run_id. Field presence still unconditionally triggers source_run validation.

6. result.status and result.structured_hash bindings (evidence/payload.js)

v4 evidence records now require both fields to be present as non-empty strings (for status) or null/SHA-256 (for structured_hash) and to match between the signed payload and the audit record. The legacy path (fields absent in both payload and record with no v4 marker) correctly passes.

No issues found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c859782da

ℹ️ 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".

Comment thread src/handoff/v4.js Outdated
Comment thread src/authorization-proof/jwt.js Outdated

@amittell amittell left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Submitting the accumulated inline resolution evidence for the v4 review fixes; each addressed thread cites its exact implementation commit and regression gates.

Comment thread src/apply.js Outdated
Comment thread src/handoff/v4.js
Comment thread src/capabilities.js Outdated
Comment thread src/compiler/openclaw-scheduler.js
Comment thread src/evidence/payload.js
Comment thread src/authorization-proof/jwt.js Outdated
Comment thread src/handoff/v4.js Outdated
Comment thread src/evidence/payload.js Outdated
Comment thread src/handoff/v4.js
Comment thread src/authorization-proof/detached-signature.js

@amittell amittell left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Submitting the remaining accumulated inline resolution evidence for the v4 discovery fixes.

Comment thread src/inspect.js
@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

@codex review

Copilot AI commented Jul 19, 2026

Copy link
Copy Markdown

@copilot review

@codex review

Review of 6fe2790 (final v4 validation compatibility)

810/810 tests pass. CI is green. Both changes are correct and well-tested.

1. v4-scoped trustedKeyId identity check + split key_id fields (jwt.js)

The context.trustedKeyId !== verifiedKeyId mismatch check is now guarded by v4Required. This is the correct scope: in legacy (non-v4) flows the caller's trustedKeyId is typically a logical provider key identifier (e.g. a JWKS kid), not a derived SPKI fingerprint, so strict equality against verifiedKeyId would false-reject valid tokens. v4 requires the caller's trustedKeyId to be the derived SPKI fingerprint, so the check belongs exclusively there.

The result now exposes three distinct fields: key_id (backward-compatible: verifiedKeyId for v4, trustedKeyId || verifiedKeyId for legacy), trusted_key_id (the raw context.trustedKeyId), and verified_key_id (the SPKI-derived key fingerprint when signature was verified, null otherwise). describeVerification forwards all three. The test matrix covers:

  • v4 with no trustedKeyId: key_id === verified_key_id, trusted_key_id === null
  • v4 with matching trustedKeyId: all three agree
  • legacy with trustedKeyId: key_id === trusted_key_id, verified_key_id is SPKI fingerprint, they can diverge without failure
  • legacy without trustedKeyId: key_id === verified_key_id, trusted_key_id === null

2. Non-array identity.presentation.bindings type guard (v4.js)

if (Array.isArray(presentationBindings)) replaces the prior unconditional iteration. A non-array bindings value (null, object, string) is already caught by validateHandoffV4Structure via the JSON Schema type: 'array' check in schema-v4.js (line 140), which emits artifact.identity.presentation.bindings must be array, received .... The manual validateSchedulerHandoffV4Artifact guard correctly defers to the schema check rather than duplicating it. The four malformed-bindings test cases confirm each non-array type is rejected by the schema-level path, and assertValidSchedulerHandoffV4Job correctly propagates the error as HANDOFF_ARTIFACT_INVALID.

No issues found.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 6fe2790340

ℹ️ 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".

@amittell

Copy link
Copy Markdown
Owner Author

@copilot review exact head 24e25dd. This pin-only release follow-up changes the three AgentCLI CI/publish acceptance sites to final stable Scheduler dbd353e9eef4342fe4b5cdeb63e2ffa2006a5119. Local lint, 822/822 tests including exact Scheduler integration 12/12, fixture byte parity, package verification, npm audit 0, diff check, and independent review are green.

@amittell

Copy link
Copy Markdown
Owner Author

@codex review exact head 24e25dd. This pin-only release follow-up changes the three AgentCLI CI/publish acceptance sites to final stable Scheduler dbd353e9eef4342fe4b5cdeb63e2ffa2006a5119. Local lint, 822/822 tests including exact Scheduler integration 12/12, fixture byte parity, package verification, npm audit 0, diff check, and independent review are green.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 24e25ddb97

ℹ️ 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".

@amittell

Copy link
Copy Markdown
Owner Author

@copilot review

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.

4 participants