Skip to content

feat(l2ps): SR-4 WI-C — finalizeRfq transcript anchor on terminal#97

Closed
Shitikyan wants to merge 1 commit into
feat/sr4-wi-b-negotiate-rfqfrom
feat/sr4-wi-c-finalize-anchor
Closed

feat(l2ps): SR-4 WI-C — finalizeRfq transcript anchor on terminal#97
Shitikyan wants to merge 1 commit into
feat/sr4-wi-b-negotiate-rfqfrom
feat/sr4-wi-c-finalize-anchor

Conversation

@Shitikyan

Copy link
Copy Markdown
Contributor

Stacked on #96 (WI-B). Ties the terminal negotiate-rfq state to the WI-3 transcript anchor.

finalizeRfq orchestrates exportTranscriptanchorEncryptedTranscript with DACS-3 §8.7 disclosure-policy semantics:

  • none — transcript local, nothing anchored
  • encrypted-anchored-recommended — anchor on explicit consent
  • encrypted-anchored-required — MUST anchor; null result fails the phase

Guards: refuses non-accepted negotiations; requires an L2PS instance when anchoring. The anchor call is injectable so the orchestration is unit-tested without a node (the real anchor deploys an SR-2 storage program). 6/6 tests across all four policy paths + both guards, using a real signed offer→counter→accept transcript.

Remaining: WI-D (AgreementDocument co-sign) needs DACS-3 §8.5.1; WI-E (devnet E2E) needs a node running the messaging server. Rebase onto main once #95/#96 land.

Ties WI-B's terminal state to WI-3's transcript anchor: on an agreed
(accepted) negotiate-rfq, export the signed channel transcript and —
per the disclosure policy (DACS-3 §8.7) — anchor an encrypted copy,
returning the channelTranscriptRef the rfq output carries.

finalizeRfq orchestrates exportTranscript (sign the ordered messages) →
anchorEncryptedTranscript (encrypt-to-members + SR-2 anchor). Policy:
- none: transcript stays local, nothing anchored
- encrypted-anchored-recommended: anchor only on explicit consent
- encrypted-anchored-required: MUST anchor; a null result throws (the
  phase fails, §8.7)

Refuses to finalize a non-accepted negotiation; requires an L2PS
instance when the policy actually anchors. The anchor call is injectable
so the orchestration is unit-tested without a live node (the real anchor
deploys an SR-2 storage program). 6 tests cover all four policy paths +
the two guards, using a real signed offer→counter→accept so the exported
transcript is genuinely CCI-signed.

WI-D (AgreementDocument co-sign) needs DACS-3 §8.5.1; WI-E (devnet E2E)
needs a node running the messaging server.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Shitikyan, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 10 minutes and 25 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 97bf4644-96b1-42e5-89ff-58df7bda3432

📥 Commits

Reviewing files that changed from the base of the PR and between bd7860b and a3666f9.

📒 Files selected for processing (3)
  • src/l2ps/channel/finalize.ts
  • src/l2ps/channel/index.ts
  • src/tests/l2ps/finalize.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sr4-wi-c-finalize-anchor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds finalizeRfq, an orchestration function that ties an accepted RfqSession to the WI-3 transcript anchor by chaining exportTranscriptanchorEncryptedTranscript with DACS-3 §8.7 disclosure-policy semantics, and re-exports it through the channel barrel.

  • finalize.ts — new module with structural RfqLike / ChannelSessionView interfaces, injectable anchor for testability, and guards for non-accepted state and missing l2ps.
  • index.ts — barrel export of finalizeRfq and its four types; no other changes.
  • finalize.test.ts — six tests covering all four policy paths and both guards using a real signed offer→counter→accept session; the recommended+no-consent+no-l2ps combination is the one untested path.

Confidence Score: 3/5

The orchestration is mostly correct, but a guard is ordered before a consent check, causing the recommended+no-consent path to throw when l2ps is absent instead of returning the spec-compliant null ref.

The !opts.l2ps check runs before any consent evaluation, so a caller following §8.7 who withholds consent and reasonably omits l2ps hits an error instead of the expected { transcript, channelTranscriptRef: null }. This path is also not exercised by the test suite, so the failure is invisible until a real integration hits it.

src/l2ps/channel/finalize.ts — the guard ordering around lines 80–90 needs a second look before this ships.

Important Files Changed

Filename Overview
src/l2ps/channel/finalize.ts New orchestration layer tying RFQ terminal state to WI-3 transcript anchoring; the l2ps guard fires before the recommended+no-consent short-circuit, causing a valid §8.7 path to throw instead of returning a null ref.
src/l2ps/channel/index.ts Barrel re-exports finalizeRfq and its types; straightforward additive change with no issues.
src/tests/l2ps/finalize.test.ts Six tests cover the four policy paths and both guards using a real signed offer→counter→accept transcript; the recommended+no-consent+no-l2ps scenario is missing, leaving the guard bug untested.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([finalizeRfq called]) --> B{rfq.state\n=== 'accepted'?}
    B -- No --> E1([throw: not accepted])
    B -- Yes --> C[exportTranscript\nsign with opts.signer]
    C --> D{policy}
    D -- none --> R1([return transcript\nref = null])
    D -- recommended\nor required --> F{opts.l2ps\npresent?}
    F -- No --> E2([throw: requires L2PS\n⚠ fires even for\nrecommended+no consent])
    F -- Yes --> G[anchorFn called]
    G --> H{policy ===\nrequired AND\nref === null?}
    H -- Yes --> E3([throw: phase fails])
    H -- No --> R2([return transcript\n+ channelTranscriptRef])
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([finalizeRfq called]) --> B{rfq.state\n=== 'accepted'?}
    B -- No --> E1([throw: not accepted])
    B -- Yes --> C[exportTranscript\nsign with opts.signer]
    C --> D{policy}
    D -- none --> R1([return transcript\nref = null])
    D -- recommended\nor required --> F{opts.l2ps\npresent?}
    F -- No --> E2([throw: requires L2PS\n⚠ fires even for\nrecommended+no consent])
    F -- Yes --> G[anchorFn called]
    G --> H{policy ===\nrequired AND\nref === null?}
    H -- Yes --> E3([throw: phase fails])
    H -- No --> R2([return transcript\n+ channelTranscriptRef])
Loading

Reviews (1): Last reviewed commit: "feat(l2ps): SR-4 WI-C — finalizeRfq tran..." | Re-trigger Greptile

Comment on lines +80 to +90
if (opts.rfq.state !== "accepted") {
throw new Error(
`finalizeRfq: negotiation is "${opts.rfq.state}", not "accepted" — ` +
"only an agreed RFQ produces a transcript anchor",
)
}

const transcript = await exportTranscript({
channelId: opts.session.channelId,
members: [...opts.session.members],
messages: opts.session.messages(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 recommended + no consent still requires l2ps

The !opts.l2ps guard fires unconditionally for every non-none policy, but DACS-3 §8.7 says recommended without consent produces no anchor — the transcript stays local, same as none. A caller who correctly omits l2ps (because they know consent is withheld and the real anchor won't run) receives a misleading "requires an L2PS instance to encrypt the transcript" error instead of { transcript, channelTranscriptRef: null }. The test suite covers recommended+no-consent with a dummy l2ps: {} as never; the case of recommended+no-consent+no-l2ps is not exercised and throws today.

Comment on lines +35 to +38
readonly state: RfqState
outcome(): RfqOutcome
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 RfqLike.outcome() is declared but never used

finalizeRfq only reads rfq.state; it never calls rfq.outcome(). Keeping outcome() in the structural interface forces every test stub and future implementor to provide a method that this function ignores. Dropping it from RfqLike (while keeping outcome() on RfqSession itself) would be the leaner contract.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@sonarqubecloud

Copy link
Copy Markdown

@tcsenpai

Copy link
Copy Markdown
Contributor

Auto-closed by the merge of #99 (combined WI-A/B/C, merged as 2a6baa3). All Greptile findings from this slice were addressed there (5/5).

@tcsenpai tcsenpai deleted the feat/sr4-wi-c-finalize-anchor branch June 24, 2026 19:16
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