Skip to content

J2: completion-result validator, replay redaction, must_refuse vectors - #32

Merged
leifj merged 10 commits into
mainfrom
j2-completion
Oct 29, 2026
Merged

leifj merged 10 commits into
mainfrom
j2-completion

Conversation

@leifj

@leifj leifj commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Parity chunk J2, stacked on #31 (base: j1-harness; retarget to main when #31 merges).

  • validateCompletionResult(result, secretMembers?), validateMatchResult, DEFAULT_SECRET_MEMBERS, REPLAY_SECRET_MEMBERS, redactedMembers, splitReplaySecrets (mirror of go-wmp SplitReplaySecrets; strict scanner: escaped names, duplicate names, case variants refused).
  • Peer refuses non-conforming completions (not delivered; synthesized -32602 flow.error, no member echoed). validateCompletions: false opts out. redacted_members surfaces as unrecoverable.
  • Harness executes all completion/match must_refuse vectors and expected_replay vectors; select_credentials must_refuse are an explicit pending list until J5. Verified also against main + wmp#22/build(deps): bump the js-deps group across 1 directory with 3 updates #25 vectors (13 match must_refuse).

Deviation: a result listing a member in redacted_members while also carrying it is refused (go-wmp does not check this).
Non-vacuity proven by reverting each rule.

🤖 Generated with Claude Code

https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

- completion.ts: validateCompletionResult (DEFAULT_SECRET_MEMBERS),
  validateMatchResult (closed set, openid4x 3.3), REPLAY_SECRET_MEMBERS,
  redactedMembers, splitReplaySecrets mirroring go-wmp SplitReplaySecrets
  (escaped names, duplicate names, case variants, existing redacted_members all
  refused; verbatim values kept). Errors never echo a member value.
- Peer refuses a non-conforming wmp.flow.complete: not delivered, the flow ends
  with a synthesized -32602 flow.error; validateCompletions: false opts out.
  redacted_members is exposed as unrecoverable via redactedMembers().
- Harness executes every flow-lifecycle completion/match vector including all
  must_refuse ones, and the expected_replay vectors. select_credentials
  must_refuse vectors are listed as pending until J5.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Match completions can bypass closed-set privacy validation, while generic validation can silently drop rejected completions without notifying the receiving handler of terminal failure.

2 open issues
What changed in this PR

This J2 parity update builds on J1’s vector harness with OpenID4x result validators, replay-secret redaction, and default completion checks in Peer.

Changes:

  • Adds completion and match validators plus helpers for withholding replay secrets.
  • Adds configurable completion refusal with sanitized error callbacks.
  • Exercises completion/match refusal and replay vectors, leaving select-credentials checks pending for J5.
File Description
test/​conformance.test.ts Executes flow-result refusal and expected-replay vectors.
test/​completion.test.ts Tests validators, replay redaction, and Peer refusal behavior.
src/​peer.ts Adds completion validation options and synthesized failure delivery.
src/​index.ts Exports validators, replay helpers, constants, and types.
src/​completion.ts Implements result validation and strict replay-secret splitting.

Review effort: Balanced

Comment thread src/peer.ts Resolved
Comment thread src/peer.ts Resolved
leifj and others added 2 commits October 28, 2026 23:23
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn
…eric validator

- Peer records the flow type (and for match the DCQL query ids) when a flow
  starts, outgoing or incoming, and checks a completion by it: a match flow's
  result is the closed set (validateMatchResult), including a completion with
  no result; never selected by members of the result.
- A non-conforming completion is refused before opts.validator runs, so a
  validator rejecting the message no longer drops it silently; authorization
  still comes first.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Critical flow-context tracking gaps let match completions bypass required validation, allowing forbidden claims or out-of-query IDs.

4 open issues
2 resolved since last review

Review effort: Balanced

Comment thread src/peer.ts Resolved
Comment thread src/peer.ts Resolved
Comment thread src/peer.ts Resolved
Comment thread src/peer.ts Resolved
…ty, envelope

- The flow type / query-id context is recorded in Peer.call for every outgoing
  flow.start (convenience method and profiles alike) and for incoming starts.
- A start under an active flow_id is refused (-32602) so it cannot replace the
  first flow's context; a start that fails (handler error, error response,
  timeout) leaves nothing recorded. flow.cancel frees the id.
- The context of an unfinished flow is never evicted: a new start beyond 4096
  active flows is refused (-31007).
- The early result-refusal path applies only to a completion with the basic
  envelope (flow_id and a wmp object with a version); anything else is left to
  the configured validator.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Completed incoming flows and successfully cancelled outgoing flows leave stale recordedFlows entries, causing new flows to be rejected after 4,096 finished flows despite none remaining active.

1 open issue
4 resolved since last review

Review effort: Balanced

Comment thread src/peer.ts Resolved
leifj and others added 2 commits October 28, 2026 23:55
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn
Peer.notify (shared by flowComplete, flowError and profiles) releases the
validation context of a flow after a successful complete/error send, and a
successful flow.cancel response in Peer.call releases it too, so finished flows
no longer accumulate toward the active-flow limit. Tests run 4100 sequential
flows through each path.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Critical completion-validation bypasses allow malformed or replayed claim-bearing match results to reach handlers, leaving protocol safeguards unenforced.

4 open issues
1 resolved since last review

Review effort: Balanced

Comment thread src/peer.ts Resolved
Comment thread src/peer.ts Resolved
Comment thread src/completion.ts Resolved
Comment thread src/peer.ts Resolved
…ns, sorted markers, session scope

- Finished, refused and cancelled flows are remembered (terminalSeen, never
  evicted, no active slot): a further terminal message for them is ignored
  instead of being judged by the general rules, so a refused match completion
  sent twice, or a claim-bearing repeat after a normal end, cannot reach the
  handler. A restarted id is judged afresh.
- A completion without the basic envelope is not delivered when no schema
  validator is configured to judge it.
- redacted_members must be in alphabetical order (profile 3.1.4).
- The flow contexts and markers are cleared when the session is closed or
  replaced (local and incoming create/close, resume of another session) and
  kept when the same session is resumed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Unbounded terminal history, including completions for never-started flow IDs, lets a remote peer exhaust memory without consuming active-flow slots.

1 open issue
4 resolved since last review

Review effort: Balanced

Comment thread src/peer.ts Resolved
Only flows this peer knew (recorded as active) are remembered when they end; a
terminal message for a flow it never saw start is delivered as before but not
remembered, so a remote peer cannot grow the history with fresh ids. The history
is capped at 65536 finished flows per session and is never evicted: at the cap
new flows are refused (-31007) until the session is renewed, while an id already
in the history can still be reused.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🔵 Needs a closer look

Critical flow-lifecycle races and session-tracking gaps require human assessment because they can disrupt replacement flows and allow late terminal messages through validation.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

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.

🟡 Changes recommended

Locally terminated flows lack terminal markers, allowing late completions to bypass recorded-flow validation and deliver claims, while responder session handling can retain stale flow state.

1 open issue

Review effort: Balanced

Comment thread src/peer.ts Resolved
An incoming create records the session id its handler returned and an incoming
resume installs the resumed session, so a session.close naming it clears the flow
contexts and markers, a resume of a different session drops the old state (also
when none was held) and a resume of the same session keeps it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🟡 Changes recommended

Incoming session-close handling leaves sessionId set, causing peer.session and subsequent convenience messages to reference a closed session.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread src/peer.ts Resolved
The peer no longer reports (peer.session) or addresses (wmpMeta) a session that
the remote side closed, matching closeSession().

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011tAXPr6RCod7mUPqXo1pRn

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.

🔵 Needs a closer look

Human review is needed because cancellation and terminal-send races can allow late completions to bypass validation or delete a newer session’s flow state.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@leifj
leifj changed the base branch from j1-harness to main October 29, 2026 10:55
@leifj leifj closed this Oct 29, 2026
@leifj leifj reopened this Oct 29, 2026
@sonarqubecloud

Copy link
Copy Markdown

@leifj
leifj added this pull request to stack #36 October 29, 2026 10:59
@leifj
leifj merged commit a37b199 into main Oct 29, 2026
13 checks passed
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