Skip to content

docs(platform): design the native cotal connector renderer - #10

Open
mattwilkinsonn wants to merge 5 commits into
mainfrom
cotal-connector-renderer-design
Open

docs(platform): design the native cotal connector renderer#10
mattwilkinsonn wants to merge 5 commits into
mainfrom
cotal-connector-renderer-design

Conversation

@mattwilkinsonn

@mattwilkinsonn mattwilkinsonn commented Jul 10, 2026

Copy link
Copy Markdown

Design record for the native OMP renderer in the cotal connector — the two-birds fix for how the connector renders in an OMP session. Design only; no implementation code. Freezes on merge as the contract the implementation builds against.

What

  • Inbound peer messages render as one flat unreadable blob today (drive() sends the formatInjection string as content with an empty details: {}, no renderer). Fix: carry the InboxItem[] through sendMessage's typed details and register a MessageRenderer for cotal:incoming / cotal:nudge that lays out one card per item. content is retained as the LLM-visible / non-TUI fallback — the renderer is display-only.
  • Outbound cotal_* tool cards show OMP's animated-spinner glyph because they register without renderCall/renderResult. Fix: add those hooks once in registerSpec() (both the cotal_inbox and generic branches), which moves the tools off the spinner fallback.

Two independent workstreams (outbound is smaller and API-version-independent; inbound depends on the renderer API). Every API/code claim in the record carries a file+line — the OMP renderer APIs are source-verified present at 16.3.4 (< the connector's resolved 16.3.15), so no version bump.

Record: docs/designs/platform/cotal-connector-renderer.md (sibling to the durable-delivery-acl-provisioning design).

Open questions for review

The ## Open Questions section is the pre-merge staging area — please resolve #1 before merge (it's load-bearing; I'll fold your answer in as a Decision), #2 is a documented non-load-bearing deferral:

  1. [load-bearing] Landing target given fix(connector): deliver peer messages as nextTurn, not steer #8's fork-base hold. The implementation must build on the connector code, which lives only on fix(connector): deliver peer messages as nextTurn, not steer #8 (fork-base held on feat(oh-my-pi): Cotal connector — headless peer + interactive extension #5). Options: (a) stack the impl branch on fix(connector): deliver peer messages as nextTurn, not steer #8 and park it until the fork-base clears, or (b) wait for fix(connector): deliver peer messages as nextTurn, not steer #8 → main then branch off main. I recommend (a) — execute-ready the moment the fork-base clears, no idle wait.
  2. [non-load-bearing] One PR or split. Ship inbound + outbound together or split. I recommend split — the outbound spinner-fix is a tight, API-version-independent diff that can land first. Fine to merge deferred either way.

(A separate note: OMP owns the parallel commit-core root-cause fix — renderless cards never commit unsealed — which is independent of this connector-side rendering work.)


Summary by cubic

Adds a design record for a native OMP renderer in the cotal connector to render inbound peer messages as readable cards and render cotal_* tool calls, removing the spinner fallback. Clarifies the renderer contract, send path, and API floor.

  • New Features
    • Structured renderer for cotal:incoming and cotal:nudge via registerMessageRenderer; branches on message.customType (not items length); keeps content unchanged.
    • Nudge messages render a single line from message.content (override text), not formatInjection(items).
    • Tool renderers (renderCall, optional renderResult) for cotal_inbox and other cotal_* tools; default to two rows (merged single-card is not available via registerTool).
    • Types and deps: widen PeerHost.sendMessage details to CotalInjectionDetails { items: InboxItem[] } (seam is non-generic and bound to pi; type-safety comes from the underlying generic); verified with @oh-my-pi/pi-coding-agent 16.3.12; corrected source anchors in the record.

Written for commit 066bbfa. Summary will update on new commits.

Review in cubic

Co-Authored-By: seal noreply@sealedsecurity.com

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a design record for native OMP rendering in the cotal connector, covering structured inbound messages, custom outbound tool cards, implementation tasks, smoke tests, API constraints, and delivery decisions.

Changes

Cotal renderer design

Layer / File(s) Summary
Renderer contracts
docs/designs/platform/cotal-connector-renderer.md
Describes inbound cotal:incoming and cotal:nudge rendering with structured details while preserving content, and specifies renderCall/renderResult hooks for cotal_* tools.
Implementation plan and constraints
docs/designs/platform/cotal-connector-renderer.md
Defines renderer wiring, type widening, registrations, smoke-test assertions, OMP API requirements, connector boundaries, delivery gating, and open questions.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Poem

I’m a rabbit with a renderer bright,
Tidying messages into view,
Tool cards hop without spinner fright,
Structured details come hopping through,
While content stays true.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR documents a renderer design, but linked issue #1 requires a wave coordination structure with channels, persona roles, and auth-mode checks. Update the PR to address the coordination-design requirements in #1, or relink the renderer design to the correct issue.
Out of Scope Changes check ⚠️ Warning The renderer-focused design record is unrelated to the linked wave-coordination objectives and appears out of scope for this issue. Either remove the renderer work from this PR or split it into a separate PR tied to the correct renderer-related issue.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately summarizes the PR as a design doc for the native cotal connector renderer.
Description check ✅ Passed The description matches the changeset and explains the renderer design contract and scope.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cotal-connector-renderer-design

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

Copy link
Copy Markdown
Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sealedsecurity-bot
sealedsecurity-bot marked this pull request as ready for review July 10, 2026 13:59
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a design record for native OMP rendering in the cotal connector. The main changes are:

  • Structured inbound rendering for cotal:incoming and cotal:nudge.
  • Tool render hooks for cotal_* registrations.
  • API constraints, implementation tasks, and open delivery decisions.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
docs/designs/platform/cotal-connector-renderer.md Adds the connector renderer design and documents the nudge rendering contract.

Reviews (5): Last reviewed commit: "docs(platform): name one sendMessage rec..." | Re-trigger Greptile

Comment thread docs/designs/platform/cotal-connector-renderer.md Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/platform/cotal-connector-renderer.md`:
- Around line 85-93: Clarify the MessageRenderer/renderCotalMessage contract so
nudge messages are rendered from details.kind === "nudge" and content even when
details.items is empty; return undefined only when details are absent or an
incoming message has no items. Update the documented handling for incoming,
nudge, and fallback cases consistently, including the referenced call-site
section.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 05d1fbee-db6d-4968-8f25-f12494a661ae

📥 Commits

Reviewing files that changed from the base of the PR and between 4b0553c and 778f348.

📒 Files selected for processing (1)
  • docs/designs/platform/cotal-connector-renderer.md

Comment thread docs/designs/platform/cotal-connector-renderer.md Outdated
Comment thread docs/designs/platform/cotal-connector-renderer.md
@sealedsecurity-bot

Copy link
Copy Markdown

Design-critic pass (SEA-1188) — folded, 2 freeze-blocking forks for your call

Ran the mandated adversarial critic pass on this record before freeze (read-only, grounded against the installed OMP 16.3.12 tree + the connector source on the #8 branch). Pushed the fold in 637b754. 6 findings; 4 folded silently, 2 code-false core claims surfaced as load-bearing Open Questions that block freeze — need your ruling:

  • OQ#4 — the cards do NOT inherit OMP's frame. The record said the inbound renderer "mirrors CustomMessageComponent's frame." That's wrong: renderFramedMessage (message-frame.ts:50-56) returns a custom renderer's Component unframed — the outlined card is built only on the undefined-return fallback. So we either (a) return undefined and pre-format content so OMP frames it for free (lightest, no internal coupling — my recommendation), or (b) hand-build a frame and couple to unexported internal theme keys.
  • OQ#5 — ircToolRenderer is not a drop-in template. Its inline/mergeCallAndResult live on OMP's internal ToolRenderer; the extension ToolDefinition exposes only renderCall/renderResult. So irc's merged single-card look isn't reachable via registerTool(a) accept two rows (recommend; the spinner-fix is met either way) or (b) hand-build a merged look in renderResult.

Folded without needing you (all verified against source):

  • Discriminate on message.customType (authoritative by dispatch) — dropped the redundant kind field from the payload and call site.
  • Thread sendMessage<CotalInjectionDetails> so details is type-checked, not erased to unknown.
  • Re-anchored every OMP cite — the record cited stale 16.3.4/16.3.15 coords; the dep resolves 16.3.12 (one messages.ts line even pointed at image-stripping).
  • Sharpened the Problem: formatInjection already newline-joins bullets; the defect is default-Markdown reflow with no renderer, not a literal paragraph.

Everything else (details-passthrough, the discriminator choice itself, display-only invariant, registerSpec chokepoint, split-PR shape) survived the attack. Both forks are recommendation-(a) if you'd rather not decide each — (a) is the lighter, lower-coupling path in both.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/designs/platform/cotal-connector-renderer.md`:
- Around line 98-105: Clarify step 4 so the generic sendMessage call uses a
single, consistent receiver name. Distinguish the OMP-facing
pi.sendMessage<CotalInjectionDetails> call from the PeerHost.sendMessage
implementation only if both are separate links in the call chain, and explicitly
map how the payload flows between them.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f86734b1-fa12-4156-9181-a3fcb1d01c94

📥 Commits

Reviewing files that changed from the base of the PR and between 860781d and b018349.

📒 Files selected for processing (1)
  • docs/designs/platform/cotal-connector-renderer.md

Comment thread docs/designs/platform/cotal-connector-renderer.md Outdated
mattwilkinsonn and others added 5 commits July 26, 2026 12:35
Structured per-item rendering of inbound peer messages (via sendMessage
details + registerMessageRenderer) and renderCall/renderResult on the
cotal_* tools to leave OMP's animated-spinner fallback. Two independent
workstreams; implementation gated on #8's fork-base clearing, the design
record is not.

Co-Authored-By: seal <noreply@sealedsecurity.com>
Greptile P1: the renderer contract returned undefined on empty items, which would send every cotal:nudge (items always []) to the raw-content fallback. Branch the render decision on details.kind instead; undefined is now only the details-absent case.
Adversarial read-only critic pass (SEA-1188) before freeze. Five findings
folded into the body; two code-false core claims surfaced as load-bearing
Open Questions for Matt's call before freeze.

Forks (now OQ#4/#5, blocking freeze):
- The inbound cards do NOT inherit OMP's frame. renderFramedMessage mounts a
  returned MessageRenderer Component unframed; the outlined card is built only
  on the undefined-return fallback. Record's "mirror CustomMessageComponent
  frame" was wrong. OQ#4: return undefined + pre-format content (OMP frames it)
  vs hand-build a frame (couples to unexported internal theme keys).
- ircToolRenderer is not a drop-in template. Its inline/mergeCallAndResult live
  on OMP's internal ToolRenderer; the extension ToolDefinition exposes only
  renderCall/renderResult. OQ#5: accept two rows vs hand-build a merged card.

Folded improvements:
- Discriminate on message.customType (authoritative by dispatch), drop the
  redundant kind field from CotalInjectionDetails and the call site.
- Thread sendMessage<CotalInjectionDetails> so details is type-checked, not
  erased to unknown at the pi boundary.
- Re-anchor every OMP cite to the installed 16.3.12 tree (record cited stale
  16.3.4/16.3.15 coords; dep resolves 16.3.12).
- Sharpen the Problem: formatInjection already newline-joins bullets; the defect
  is default-Markdown reflow with no renderer, not a literal paragraph.

Co-Authored-By: seal <noreply@sealedsecurity.com>
…tion

The NUDGE branch renders from message.content, which on the drive(override)
path is the bare nudge string (interactive-loop.ts:70), not
formatInjection(items) — that runs only on the incoming branch (:75). Name the
source explicitly so the empty-items contract can't be misread as an empty
nudge line.

Co-Authored-By: seal <noreply@sealedsecurity.com>
CodeRabbit flagged step 4 vs task B1 naming the same call two ways
(`pi.sendMessage` vs `host.sendMessage`), both attaching a
`<CotalInjectionDetails>` type-arg to the non-generic `PeerHost` seam. Name
the single send path `host.sendMessage` (host binds to `pi` at
`extension.ts:99`) and state the real mechanism: widening the seam's `details`
field type-checks the payload; the seam stays non-generic (no type-arg at the
call), and OMP's generic `sendMessage<T>` keeps it sound end-to-end.

Also correct source line anchors that had drifted to a divergent local
checkout: on PR #8, `text = override` is `:74`, the mention-recall caller
`:127`, the `formatInjection` call `:79`, and `host` binds to `pi` at
`extension.ts:99`.

Co-Authored-By: seal <noreply@sealedsecurity.com>
@seal-agent
seal-agent force-pushed the cotal-connector-renderer-design branch from 066bbfa to 0c0c426 Compare July 26, 2026 16:37
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