docs(platform): design the native cotal connector renderer - #10
docs(platform): design the native cotal connector renderer#10mattwilkinsonn wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughAdds 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. ChangesCotal renderer design
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a design record for native OMP rendering in the cotal connector. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "docs(platform): name one sendMessage rec..." | Re-trigger Greptile |
There was a problem hiding this comment.
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
📒 Files selected for processing (1)
docs/designs/platform/cotal-connector-renderer.md
Design-critic pass (SEA-1188) — folded, 2 freeze-blocking forks for your callRan 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
Folded without needing you (all verified against source):
Everything else (details-passthrough, the discriminator choice itself, display-only invariant, |
There was a problem hiding this comment.
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
📒 Files selected for processing (1)
docs/designs/platform/cotal-connector-renderer.md
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>
066bbfa to
0c0c426
Compare

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
drive()sends theformatInjectionstring ascontentwith an emptydetails: {}, no renderer). Fix: carry theInboxItem[]throughsendMessage's typeddetailsand register aMessageRendererforcotal:incoming/cotal:nudgethat lays out one card per item.contentis retained as the LLM-visible / non-TUI fallback — the renderer is display-only.cotal_*tool cards show OMP's animated-spinner glyph because they register withoutrenderCall/renderResult. Fix: add those hooks once inregisterSpec()(both thecotal_inboxand 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 Questionssection 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:(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.cotal:incomingandcotal:nudgeviaregisterMessageRenderer; branches onmessage.customType(not items length); keepscontentunchanged.message.content(override text), notformatInjection(items).renderCall, optionalrenderResult) forcotal_inboxand othercotal_*tools; default to two rows (merged single-card is not available viaregisterTool).PeerHost.sendMessagedetails toCotalInjectionDetails{ items: InboxItem[] }(seam is non-generic and bound topi; type-safety comes from the underlying generic); verified with@oh-my-pi/pi-coding-agent16.3.12; corrected source anchors in the record.Written for commit 066bbfa. Summary will update on new commits.
Co-Authored-By: seal noreply@sealedsecurity.com