From dca379156847a322a01ebc072269c3897a3ed49d Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Wed, 8 Jul 2026 19:11:57 -0400 Subject: [PATCH 01/11] fix(cli): cotal mint reuses the existing identity unless --force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mint.ts unconditionally called newIdentity() on every mint, so re-minting an agent (e.g. to refresh its channels from the persona file) rotated the mesh id. The durable ACL row and the dm/dlv delivery durables are all keyed by the nkey public key, so rotation orphaned them → the agent went @mention-wake-blind with 'consumer not found' until re-provisioned (hit live 6x during a fleet relaunch). Option B (maintainer ruling on design PR #3, cubic P1): re-mint reuses the same identity. New core helper identityFromCreds(creds): Identity — the id-preserving sibling of idFromCreds (which it reuses for the id + JWT-subject cross-check), returning { id, seed } from the creds' seed block. mint computes the out path before minting; if a creds file exists there and --force is not passed, it re-signs that SAME id+seed with fresh ACLs; otherwise it mints a new identity. --force keeps the rotation escape hatch (compromised key / deliberate new id). Orthogonal to the still-open mint-strategy fork (#3): changes WHICH id mint uses, not WHERE the ACL write triggers. Pairs with the PR #4 DLV-durable fix. Refs #3. Co-Authored-By: seal --- implementations/cli/src/commands/mint.ts | 19 ++++++++++++++----- packages/core/src/identity.ts | 12 ++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/implementations/cli/src/commands/mint.ts b/implementations/cli/src/commands/mint.ts index 474e51d4c7..ca56809830 100644 --- a/implementations/cli/src/commands/mint.ts +++ b/implementations/cli/src/commands/mint.ts @@ -1,8 +1,9 @@ -import { existsSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import { resolve, join, dirname } from "node:path"; import { parseArgs } from "node:util"; import { agentFilePath, + identityFromCreds, loadAgentFile, mintCreds, mkSecretDir, @@ -28,7 +29,7 @@ export async function mint(argv: string[]): Promise { profile: { type: "string" }, out: { type: "string" }, signer: { type: "boolean" }, // emit a stripped signer file instead of agent/observer creds - force: { type: "boolean" }, // (--signer) overwrite an existing signer file + force: { type: "boolean" }, // overwrite an existing --signer file; or (agent) rotate to a fresh id instead of reusing the existing creds' "allow-subscribe": { type: "string" }, // read ACL override (comma-separated) "allow-publish": { type: "string" }, // post ACL override (comma-separated) }, @@ -85,12 +86,20 @@ export async function mint(argv: string[]): Promise { allowPublish = splitList(values["allow-publish"]) ?? def?.allowPublish; role = def?.role; } - const identity = newIdentity(); - const creds = await mintCreds(auth, identity, profile, { allowSubscribe, allowPublish, role }); + // Re-mint reuses the SAME identity by default. mint's read/post ACLs come from the persona file, + // so re-minting is how an agent's channels get refreshed — but the mesh id, its durable ACL row, + // and its dm/dlv durables are all keyed by the nkey public key. Rotating the id on every mint + // (the old behavior) orphaned that row + those durables, leaving the agent @mention-wake-blind + // until re-provisioned. So: if a creds file already exists here, re-sign its SAME id+seed with the + // fresh ACLs; only mint a brand-new identity when there's no creds yet, or --force rotates + // deliberately (a compromised key / intentional new identity). const out = resolve(values.out ?? join(dir, "creds", `${name}.creds`)); + const reuse = !values.force && existsSync(out); + const identity = reuse ? identityFromCreds(readFileSync(out, "utf8")) : newIdentity(); + const creds = await mintCreds(auth, identity, profile, { allowSubscribe, allowPublish, role }); mkSecretDir(dirname(out)); writeSecretFile(out, creds); console.log(c.green(`✓ minted ${profile} creds for "${name}"`)); - console.log(c.dim(` id: ${identity.id}`)); + console.log(c.dim(` id: ${identity.id}${reuse ? " (reused — re-mint kept the identity)" : " (new)"}`)); console.log(c.dim(` creds: ${out}`)); } diff --git a/packages/core/src/identity.ts b/packages/core/src/identity.ts index 86e16ffda8..14b2b895d9 100644 --- a/packages/core/src/identity.ts +++ b/packages/core/src/identity.ts @@ -42,3 +42,15 @@ export function idFromCreds(creds: string): string { if (sub && sub !== id) throw new Error(`creds: seed identity ${id} != JWT subject ${sub}`); return id; } + +/** The full identity (id + seed) carried by a creds file — the id-preserving sibling of + * {@link idFromCreds}. Re-mint reads this to RE-SIGN the SAME identity (stable id) with refreshed + * ACLs instead of rotating to a fresh nkey, so the agent's durable ACL row and dm/dlv durables (all + * keyed by id) stay valid across a re-mint. `idFromCreds` supplies the id AND its JWT-subject + * cross-check (a spliced seed+JWT throws there); the seed is the same block it validates, returned + * raw for {@link mintCreds} to re-embed. */ +export function identityFromCreds(creds: string): Identity { + const seedM = creds.match(/BEGIN USER NKEY SEED-----\s*([\s\S]*?)\s*------END USER NKEY SEED/); + if (!seedM) throw new Error("creds: no user nkey seed block found"); + return { id: idFromCreds(creds), seed: seedM[1].trim() }; +} From 4161ddd7cf3477b0e19e0930cba6c88090d3b47b Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Wed, 8 Jul 2026 19:31:23 -0400 Subject: [PATCH 02/11] fix(cli): gate mint id-reuse to the agent profile + fail loud on unparseable creds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the reuse logic (cubic P1 + greptile P2 on #7): - Reuse now requires profile === "agent". The durable-ACL-orphan rationale is agent-specific (persona-refresh workflow + dm/dlv durables keyed to the id); observer/admin creds have neither, and silently preserving a privileged admin key across re-mints would extend its lifetime unexpectedly. Observer/admin now always rotate, as before. - A present-but-unparseable creds file (empty, truncated, not a user creds file) now fails loud with an actionable error naming --force, instead of letting identityFromCreds throw a raw parse exception. Silently rotating there would orphan the durable row the existing id may still own, so fresh-mint is not a safe fallback — the operator must opt in via --force or remove the stale file. Verified: agent re-mint keeps id; admin/observer re-mint rotate; corrupt creds at the out path errors naming --force (no raw crash, no silent rotate). Refs #3. Co-Authored-By: seal --- implementations/cli/src/commands/mint.ts | 37 ++++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/implementations/cli/src/commands/mint.ts b/implementations/cli/src/commands/mint.ts index ca56809830..6c6292bbd8 100644 --- a/implementations/cli/src/commands/mint.ts +++ b/implementations/cli/src/commands/mint.ts @@ -10,6 +10,7 @@ import { newIdentity, stripSpaceAuth, writeSecretFile, + type Identity, type Profile, } from "@cotal-ai/core"; import { authDir, loadSpaceAuth } from "@cotal-ai/workspace"; @@ -86,16 +87,34 @@ export async function mint(argv: string[]): Promise { allowPublish = splitList(values["allow-publish"]) ?? def?.allowPublish; role = def?.role; } - // Re-mint reuses the SAME identity by default. mint's read/post ACLs come from the persona file, - // so re-minting is how an agent's channels get refreshed — but the mesh id, its durable ACL row, - // and its dm/dlv durables are all keyed by the nkey public key. Rotating the id on every mint - // (the old behavior) orphaned that row + those durables, leaving the agent @mention-wake-blind - // until re-provisioned. So: if a creds file already exists here, re-sign its SAME id+seed with the - // fresh ACLs; only mint a brand-new identity when there's no creds yet, or --force rotates - // deliberately (a compromised key / intentional new identity). + // Re-mint reuses the SAME identity by default — but only for the `agent` profile. mint's read/post + // ACLs come from the persona file, so re-minting is how an agent's channels get refreshed; and the + // mesh id, its durable ACL row, and its dm/dlv durables are all keyed by the nkey public key, so + // rotating the id on every mint (the old behavior) orphaned that row + those durables, leaving the + // agent @mention-wake-blind until re-provisioned. Observer/admin creds carry no persona-refresh + // workflow and no durable footprint to orphan, and silently extending a privileged admin key's + // lifetime across re-mints would be surprising — so they always rotate. Reuse only when: agent + // profile, a creds file already exists here, and --force did not ask for deliberate rotation + // (a compromised key / intentional new identity). const out = resolve(values.out ?? join(dir, "creds", `${name}.creds`)); - const reuse = !values.force && existsSync(out); - const identity = reuse ? identityFromCreds(readFileSync(out, "utf8")) : newIdentity(); + const reuse = profile === "agent" && !values.force && existsSync(out); + let identity: Identity; + if (reuse) { + try { + identity = identityFromCreds(readFileSync(out, "utf8")); + } catch (e) { + // A present-but-unreadable creds file (empty, truncated, or not a user creds file) must fail + // loud, not silently rotate — silently minting a fresh id here would orphan the durable row + // the existing id may still own. Point the operator at the deliberate-rotation escape hatch. + throw new Error( + `cotal mint: creds already exist at ${out} but could not be parsed to reuse the identity ` + + `(${e instanceof Error ? e.message : String(e)}). Pass --force to mint a fresh identity ` + + `(rotates the id), or remove the file if it is stale.`, + ); + } + } else { + identity = newIdentity(); + } const creds = await mintCreds(auth, identity, profile, { allowSubscribe, allowPublish, role }); mkSecretDir(dirname(out)); writeSecretFile(out, creds); From bd3edb6d420cac961be55dd60f9abc9b3b45c6aa Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Wed, 8 Jul 2026 19:49:55 -0400 Subject: [PATCH 03/11] test(cli): red-green regression for mint identity-reuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 1 — packages/core/smoke/identity.smoke.ts (offline, 4 asserts): identityFromCreds round-trips {id, seed} unchanged; agrees with idFromCreds (one-id-everywhere); rejects spliced creds (seed vs foreign JWT subject) and a missing seed block. Layer 2 — implementations/cli/smoke/mint-reuse.smoke.ts (hermetic, exercises the real mint() against a tmp .cotal root, 8 checks): re-minting an agent reuses the SAME id; --force rotates; a persona ACL change refreshes the baked sub.allow WITHOUT rotating the id; first mint of a new name mints fresh (absent-creds path, no crash); observer + admin re-mint rotate (reuse is agent-only); an unparseable existing creds file fails loud naming --force (no silent rotate, no raw crash). Red-green verified: reverting the reuse block to unconditional newIdentity() fails checks 1, 3, and 6 (two mints → two ids; the exact #3 cubic-P1 durable-orphan bug); the fix turns them green. Registered both as smoke:identity + smoke:mint-reuse and wired into smoke:ci. Refs #3. Co-Authored-By: seal --- implementations/cli/smoke/mint-reuse.smoke.ts | 134 ++++++++++++++++++ package.json | 4 +- packages/core/smoke/identity.smoke.ts | 51 +++++++ 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 implementations/cli/smoke/mint-reuse.smoke.ts create mode 100644 packages/core/smoke/identity.smoke.ts diff --git a/implementations/cli/smoke/mint-reuse.smoke.ts b/implementations/cli/smoke/mint-reuse.smoke.ts new file mode 100644 index 0000000000..d6beb985eb --- /dev/null +++ b/implementations/cli/smoke/mint-reuse.smoke.ts @@ -0,0 +1,134 @@ +/** + * `cotal mint` identity-reuse smoke (hermetic — no broker). Exercises the REAL mint() command against + * a tmp `.cotal/` root laid out exactly as `cotal up` writes it (auth.json via saveSpaceAuth + a + * persona file), and asserts the reuse-unless-force contract end to end. Run with: + * pnpm --filter @cotal-ai/cli exec tsx smoke/mint-reuse.smoke.ts + * + * The load-bearing regression: re-minting an AGENT keeps the SAME nkey id (so its durable ACL row + + * dm/dlv durables, all id-keyed, stay valid) — the old unconditional newIdentity() rotated the id on + * every mint and orphaned them, leaving the agent @mention-wake-blind. --force rotates deliberately; + * a persona ACL change refreshes the baked channels WITHOUT rotating; observer/admin always rotate + * (reuse is agent-only — no durable footprint to orphan, and re-signing a privileged key would + * silently extend its lifetime); and a present-but-unparseable creds file fails loud (never a silent + * fresh-mint that would orphan the id its predecessor may still own). + * + * mint() resolves its root via findCotalRoot() walking up from process.cwd(), so the harness chdir's + * into the tmp root and restores cwd in the finally. + */ +import { strict as assert } from "node:assert"; +import { randomUUID } from "node:crypto"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createSpaceAuth, idFromCreds } from "@cotal-ai/core"; +import { authDir, saveSpaceAuth } from "@cotal-ai/workspace"; +import { mint } from "../src/commands/mint.js"; + +let failures = 0; +function check(label: string, cond: boolean, extra?: unknown): void { + console.log(`${cond ? "✓" : "✗"} ${label}${cond ? "" : ` — ${JSON.stringify(extra)}`}`); + if (!cond) failures++; +} + +// The chat-read channels a minted creds file grants (decode the JWT's nats.sub.allow, keep chat.*. +// entries). Same JWT-decode shape as manager/smoke/persona-identity-acl.smoke.ts. +function credSubChat(path: string): string[] { + const jwt = readFileSync(path, "utf8").split("\n").find((l) => l && !l.startsWith("-") && l.split(".").length === 3)!; + const claims = JSON.parse(Buffer.from(jwt.split(".")[1], "base64url").toString("utf8")); + const allow: string[] = claims.nats?.sub?.allow ?? []; + return allow.filter((s) => s.includes(".chat.")); +} + +// mint() logs its result to stdout; silence it (restored even on throw) so the check output stays +// legible — the ids we assert on are read straight from the written creds file, not from the log. +async function mintQuiet(argv: string[]): Promise { + const orig = console.log; + console.log = () => {}; + try { + await mint(argv); + } finally { + console.log = orig; + } +} + +// A tmp `.cotal/` root with real space trust material (auth.json, exactly as `cotal up` persists it) +// and a `scout` agent persona reading/posting the `general` channel. +const space = `mint-reuse-${randomUUID().slice(0, 8)}`; +const auth = await createSpaceAuth(space); +const root = mkdtempSync(join(tmpdir(), "cotal-mint-reuse-")); +const agentsDir = join(root, ".cotal", "agents"); +mkdirSync(agentsDir, { recursive: true }); +saveSpaceAuth(authDir(root), auth); +const scoutPersona = join(agentsDir, "scout.md"); +writeFileSync(scoutPersona, "---\nname: scout\nsubscribe: [general]\nallowSubscribe: [general]\nallowPublish: [general]\n---\nbody\n"); +const scoutCreds = join(authDir(root), "creds", "scout.creds"); +const credsDir = join(authDir(root), "creds"); + +const prevCwd = process.cwd(); +process.chdir(root); // findCotalRoot() walks up from cwd — anchor it at our tmp root +try { + // 1) THE regression: re-minting an agent twice reuses the SAME id (before the fix, two mints gave + // two different ids and orphaned the id-keyed durables). + await mintQuiet(["scout"]); + const id1 = idFromCreds(readFileSync(scoutCreds, "utf8")); + await mintQuiet(["scout"]); + const id2 = idFromCreds(readFileSync(scoutCreds, "utf8")); + check("re-mint of an agent reuses the same id", id1 === id2, { id1, id2 }); + + // 2) --force is the deliberate-rotation escape hatch: a fresh id despite an existing creds file. + await mintQuiet(["scout", "--force"]); + const id3 = idFromCreds(readFileSync(scoutCreds, "utf8")); + check("mint --force rotates to a different id", id3 !== id2, { id2, id3 }); + + // 3) A persona ACL change is applied on re-mint (refreshed channels) WITHOUT rotating the id — + // the whole point: an agent's read scope is refreshed while its mesh id stays stable. + const beforeChans = credSubChat(scoutCreds); // channels baked before the persona change + writeFileSync(scoutPersona, "---\nname: scout\nsubscribe: [general]\nallowSubscribe: [general, review]\nallowPublish: [general]\n---\nbody\n"); + await mintQuiet(["scout"]); + const id4 = idFromCreds(readFileSync(scoutCreds, "utf8")); + const afterChans = credSubChat(scoutCreds); + check("re-mint after an ACL change keeps the identity", id4 === id3, { id3, id4 }); + check( + "re-mint bakes the NEW channel into sub.allow (review added, was absent before)", + afterChans.some((s) => s.endsWith(".chat.*.review")) && !beforeChans.some((s) => s.endsWith(".chat.*.review")), + { beforeChans, afterChans }, + ); + + // 4) First-ever mint of a brand-new name has no creds file — the reuse=false path must mint a fresh + // identity and write valid creds, not crash on the absent-file read. + writeFileSync(join(agentsDir, "newbie.md"), "---\nname: newbie\nsubscribe: [general]\nallowSubscribe: [general]\n---\nbody\n"); + const newbieCreds = join(credsDir, "newbie.creds"); + await mintQuiet(["newbie"]); + const idNew = idFromCreds(readFileSync(newbieCreds, "utf8")); + check("first mint of a brand-new name mints a fresh identity (absent-creds path, no crash)", idNew !== id4 && idNew.startsWith("U"), { idNew, id4 }); + + // 5) Reuse is AGENT-ONLY: re-minting an observer or admin creds file ROTATES the id (a privileged + // key must not silently extend its lifetime across re-mints). + for (const profile of ["observer", "admin"] as const) { + const pOut = join(credsDir, `${profile}-dash.creds`); + await mintQuiet([`${profile}-dash`, "--profile", profile]); + const a = idFromCreds(readFileSync(pOut, "utf8")); + await mintQuiet([`${profile}-dash`, "--profile", profile]); + const b = idFromCreds(readFileSync(pOut, "utf8")); + check(`re-mint of an ${profile} creds rotates the id (reuse is agent-only)`, a !== b, { profile, a, b }); + } + + // 6) A present-but-unparseable creds file at the out path fails LOUD naming --force — never a raw + // parse crash, and never a silent fresh-mint (which would orphan the predecessor id's durables). + mkdirSync(credsDir, { recursive: true }); + writeFileSync(join(credsDir, "corrupt.creds"), ""); // present but no seed block + let msg = ""; + try { + await mintQuiet(["corrupt"]); + } catch (e) { + msg = e instanceof Error ? e.message : String(e); + } + check("unparseable existing creds → actionable error naming --force (no silent rotate, no raw crash)", /could not be parsed/.test(msg) && /--force/.test(msg), { msg }); +} finally { + process.chdir(prevCwd); + rmSync(root, { recursive: true, force: true }); +} + +console.log(`\nmint-reuse smoke: ${failures === 0 ? "OK ✅" : "FAILED ❌"} (${failures} failing)`); +assert.equal(failures, 0, `${failures} check(s) failed`); +process.exit(0); diff --git a/package.json b/package.json index 30071edc70..7d81818fad 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,9 @@ "gen:schema": "node scripts/generate-cotal-schema.mjs", "test": "pnpm -r --if-present test", "check": "pnpm typecheck && pnpm test && pnpm smoke:view && pnpm smoke:spawn-from-anywhere && pnpm smoke:spawn-from-anywhere:live && pnpm smoke:connect && pnpm smoke:core-boundary && pnpm smoke:preflight && pnpm smoke:ci", - "smoke:ci": "pnpm smoke:read-acl:auth && pnpm smoke:sub-acl:auth && pnpm smoke:self-serve-join:auth && pnpm smoke:control-auth && pnpm smoke:manager-split && pnpm smoke:deprovision && pnpm smoke:control-reply-bound && pnpm smoke:channels:auth && pnpm smoke:e2e:acl && pnpm smoke:plane3:auth && pnpm smoke:delivery-lease:auth && pnpm smoke:delivery-leave-tombstone:auth && pnpm smoke:delivery-reconnect:auth && pnpm smoke:delivery-cred:auth && pnpm smoke:delivery-reply-injection:auth && pnpm smoke:membership:auth && pnpm smoke:membership-feed-confinement:auth && pnpm smoke:wildcard-backfill && pnpm smoke:opencode && pnpm smoke:opencode-coop && pnpm smoke:opencode-transcript && pnpm smoke:transcript-grant && pnpm smoke:persona-acl", + "smoke:ci": "pnpm smoke:read-acl:auth && pnpm smoke:sub-acl:auth && pnpm smoke:self-serve-join:auth && pnpm smoke:control-auth && pnpm smoke:manager-split && pnpm smoke:deprovision && pnpm smoke:control-reply-bound && pnpm smoke:channels:auth && pnpm smoke:e2e:acl && pnpm smoke:plane3:auth && pnpm smoke:delivery-lease:auth && pnpm smoke:delivery-leave-tombstone:auth && pnpm smoke:delivery-reconnect:auth && pnpm smoke:delivery-cred:auth && pnpm smoke:delivery-reply-injection:auth && pnpm smoke:membership:auth && pnpm smoke:membership-feed-confinement:auth && pnpm smoke:wildcard-backfill && pnpm smoke:opencode && pnpm smoke:opencode-coop && pnpm smoke:opencode-transcript && pnpm smoke:transcript-grant && pnpm smoke:persona-acl && pnpm smoke:identity && pnpm smoke:mint-reuse", + "smoke:identity": "tsx packages/core/smoke/identity.smoke.ts", + "smoke:mint-reuse": "tsx implementations/cli/smoke/mint-reuse.smoke.ts", "clean:dry": "git clean -ndX -- node_modules .pnpm-store packages extensions implementations examples remotion", "clean": "git clean -fdX -- node_modules .pnpm-store packages extensions implementations examples remotion", "cotal": "tsx bin/cotal.ts", diff --git a/packages/core/smoke/identity.smoke.ts b/packages/core/smoke/identity.smoke.ts new file mode 100644 index 0000000000..7004e6acbd --- /dev/null +++ b/packages/core/smoke/identity.smoke.ts @@ -0,0 +1,51 @@ +/** + * identityFromCreds unit smoke (pure, no broker) — run with: + * pnpm --filter @cotal-ai/core exec tsx smoke/identity.smoke.ts + * + * Defends the id-preserving creds reader that `cotal mint` uses to REUSE an agent's identity across a + * re-mint (the reuse-unless-force fix). The contract: the {id, seed} carried by a creds file round-trips + * unchanged; the id agrees with idFromCreds; and a creds file that is corrupt (no seed block) or spliced + * (a seed paired with a foreign JWT subject) is REJECTED — never silently handing back a wrong or + * mismatched id, which is exactly what would let a re-mint re-sign the wrong identity. + */ +import assert from "node:assert/strict"; +import { createSpaceAuth, mintCreds } from "../src/provision.js"; +import { newIdentity, idFromCreds, identityFromCreds } from "../src/identity.js"; + +// Offline key material — createSpaceAuth mints an operator→account chain locally, no broker needed. +const auth = await createSpaceAuth("test"); + +// (a) Round-trip: a freshly-minted agent creds file yields back the SAME id AND seed (both fields). +// This is the load-bearing property — reuse re-signs THIS identity, so a wrong id or a mangled +// seed here would silently rotate the agent despite the "reuse" intent. +{ + const id = newIdentity(); + const creds = await mintCreds(auth, id, "agent", { allowSubscribe: ["general"] }); + const got = identityFromCreds(creds); + assert.equal(got.id, id.id, "identityFromCreds must recover the minted id"); + assert.equal(got.seed, id.seed, "identityFromCreds must recover the minted seed verbatim"); +} + +// (b) Single-id cross-consistency at the API boundary: the id it returns agrees with idFromCreds on +// the same creds (the "one id everywhere" invariant — the two readers must never diverge). +{ + const creds = await mintCreds(auth, newIdentity(), "agent", { allowSubscribe: ["general"] }); + assert.equal(identityFromCreds(creds).id, idFromCreds(creds)); +} + +// (c) Spliced creds (A's seed + B's JWT ⇒ JWT subject ≠ seed identity) is REJECTED. identityFromCreds +// inherits idFromCreds's JWT-subject cross-check, so it can't return a seed whose JWT claims a +// different identity — the guard against re-signing a seed that was paired with someone else's JWT. +{ + const a = await mintCreds(auth, newIdentity(), "agent", { allowSubscribe: ["general"] }); + const b = await mintCreds(auth, newIdentity(), "agent", { allowSubscribe: ["general"] }); + const jwtBlock = /-----BEGIN NATS USER JWT-----[\s\S]*?------END NATS USER JWT------/; + const bJwt = b.match(jwtBlock)![0]; + const spliced = a.replace(jwtBlock, bJwt); // A's seed block, B's JWT subject + assert.throws(() => identityFromCreds(spliced), /!= JWT subject/); +} + +// (d) A creds string with no seed block throws the documented error rather than returning a bogus id. +assert.throws(() => identityFromCreds("this is not a creds file"), /no user nkey seed block found/); + +console.log("identity.smoke: all assertions passed"); From 994e85ce92a1b3a0ab26a53c2f85021b5b1fee5f Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Wed, 8 Jul 2026 19:55:50 -0400 Subject: [PATCH 04/11] test(cli): assert corrupt-creds mint leaves the file untouched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cubic P2 on #7: the corrupt-creds check only asserted the error message, so a future mint() refactor that wrote fresh creds *before* surfacing the parse error would still pass while silently rotating the id — the exact regression the test guards. Add a filesystem-state assertion: after the failed mint, the corrupt file must be byte-unchanged (still empty), proving no silent fresh-mint. Refs #3. Co-Authored-By: seal --- implementations/cli/smoke/mint-reuse.smoke.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/implementations/cli/smoke/mint-reuse.smoke.ts b/implementations/cli/smoke/mint-reuse.smoke.ts index d6beb985eb..b7052fb0d2 100644 --- a/implementations/cli/smoke/mint-reuse.smoke.ts +++ b/implementations/cli/smoke/mint-reuse.smoke.ts @@ -116,14 +116,27 @@ try { // 6) A present-but-unparseable creds file at the out path fails LOUD naming --force — never a raw // parse crash, and never a silent fresh-mint (which would orphan the predecessor id's durables). mkdirSync(credsDir, { recursive: true }); - writeFileSync(join(credsDir, "corrupt.creds"), ""); // present but no seed block + const corruptCreds = join(credsDir, "corrupt.creds"); + writeFileSync(corruptCreds, ""); // present but no seed block let msg = ""; try { await mintQuiet(["corrupt"]); } catch (e) { msg = e instanceof Error ? e.message : String(e); } - check("unparseable existing creds → actionable error naming --force (no silent rotate, no raw crash)", /could not be parsed/.test(msg) && /--force/.test(msg), { msg }); + check( + "unparseable existing creds → actionable error naming --force (no silent rotate, no raw crash)", + /could not be parsed/.test(msg) && /--force/.test(msg), + { msg }, + ); + // Assert the FILESYSTEM state, not just the message: the failed mint must not have written fresh + // creds over the corrupt file. A future refactor that mints-then-throws would still surface a + // parse error yet silently rotate the id — this catches that by proving the file is untouched. + check( + "failed corrupt-creds mint left the file untouched (no silent fresh-mint)", + readFileSync(corruptCreds, "utf8") === "", + { content: readFileSync(corruptCreds, "utf8").slice(0, 40) }, + ); } finally { process.chdir(prevCwd); rmSync(root, { recursive: true, force: true }); From b3d86f1d33d21315488417ec680970ae5c769bad Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Thu, 9 Jul 2026 22:31:24 -0400 Subject: [PATCH 05/11] fix(cli): mint reuses identity only from the canonical creds path Closes the greptile P1 on #9: `cotal mint --out ` reused that file's nkey id and re-signed it with 's ACLs, crossing the identity boundary (and clobbering the target). Creds identify an agent by nkey id, not by name, so the only name<->creds binding is the canonical `creds/.creds` path. Reuse is now canonical-path-only: a custom `--out` onto an existing creds file fails loud unless `--force` asks for the overwrite deliberately (which rotates to a fresh identity). Regression: mint-reuse smoke gains the cross-identity guard (red->green): refuse + leave the target byte-identical; --force overwrites fresh. Co-Authored-By: seal --- implementations/cli/smoke/mint-reuse.smoke.ts | 35 +++++++++++++++++++ implementations/cli/src/commands/mint.ts | 19 ++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/implementations/cli/smoke/mint-reuse.smoke.ts b/implementations/cli/smoke/mint-reuse.smoke.ts index b7052fb0d2..e5893e8645 100644 --- a/implementations/cli/smoke/mint-reuse.smoke.ts +++ b/implementations/cli/smoke/mint-reuse.smoke.ts @@ -137,6 +137,41 @@ try { readFileSync(corruptCreds, "utf8") === "", { content: readFileSync(corruptCreds, "utf8").slice(0, 40) }, ); + + // 7) Cross-identity --out guard (greptile P1). Creds carry the nkey id, NOT the agent name, so the + // only name↔identity binding is the canonical creds/.creds path. Reuse is therefore + // canonical-path-only: `cotal mint --out ` must never reuse (re-sign + // that file's id with 's ACLs) nor silently clobber it (overwrite its id, orphaning its + // durables). Without --force it fails loud; --force is the deliberate-overwrite escape hatch. + writeFileSync(join(agentsDir, "victim.md"), "---\nname: victim\nsubscribe: [general]\nallowSubscribe: [general]\n---\nbody\n"); + writeFileSync(join(agentsDir, "raider.md"), "---\nname: raider\nsubscribe: [general]\nallowSubscribe: [general, review, ops]\n---\nbody\n"); + const victimCreds = join(credsDir, "victim.creds"); + await mintQuiet(["victim"]); + const victimBefore = readFileSync(victimCreds, "utf8"); + const victimIdBefore = idFromCreds(victimBefore); + let raiderMsg = ""; + try { + await mintQuiet(["raider", "--out", victimCreds]); + } catch (e) { + raiderMsg = e instanceof Error ? e.message : String(e); + } + check( + "mint --out fails loud naming --force (no cross-identity reuse)", + /--force/.test(raiderMsg) && /belong/.test(raiderMsg), + { raiderMsg }, + ); + check( + "refused cross-identity mint left the target creds byte-identical (no re-sign of its id, no clobber)", + readFileSync(victimCreds, "utf8") === victimBefore && idFromCreds(readFileSync(victimCreds, "utf8")) === victimIdBefore, + { changed: readFileSync(victimCreds, "utf8") !== victimBefore }, + ); + // --force is the explicit escape hatch: it rotates to a FRESH identity and overwrites the target. + await mintQuiet(["raider", "--out", victimCreds, "--force"]); + check( + "mint --out --force overwrites with a fresh identity (escape hatch intact)", + idFromCreds(readFileSync(victimCreds, "utf8")) !== victimIdBefore, + { victimIdBefore, after: idFromCreds(readFileSync(victimCreds, "utf8")) }, + ); } finally { process.chdir(prevCwd); rmSync(root, { recursive: true, force: true }); diff --git a/implementations/cli/src/commands/mint.ts b/implementations/cli/src/commands/mint.ts index 6c6292bbd8..9eafb03b1d 100644 --- a/implementations/cli/src/commands/mint.ts +++ b/implementations/cli/src/commands/mint.ts @@ -96,8 +96,23 @@ export async function mint(argv: string[]): Promise { // lifetime across re-mints would be surprising — so they always rotate. Reuse only when: agent // profile, a creds file already exists here, and --force did not ask for deliberate rotation // (a compromised key / intentional new identity). - const out = resolve(values.out ?? join(dir, "creds", `${name}.creds`)); - const reuse = profile === "agent" && !values.force && existsSync(out); + const canonicalOut = resolve(join(dir, "creds", `${name}.creds`)); + const out = resolve(values.out ?? canonicalOut); + // The canonical `creds/.creds` path is the ONLY binding between an agent name and a creds + // file: the file bakes an nkey id, not the name (`identity.ts`), so a creds file at a custom + // `--out` cannot be attributed to . A custom `--out` onto an EXISTING creds file must + // therefore never be silently reused (re-signing another agent's id with this name's ACLs) nor + // overwritten (rotating that id, orphaning its id-keyed ACL row + dm/dlv durables) — fail loud + // unless --force asks for the overwrite deliberately. Identity reuse is thus canonical-path-only. + if (!values.force && out !== canonicalOut && existsSync(out)) { + throw new Error( + `cotal mint: --out ${out} already holds a creds file that may not belong to "${name}" — creds ` + + `identify an agent by nkey id, not by name, so this file cannot be safely reused or ` + + `overwritten for "${name}". Pass --force to overwrite it with a fresh identity, or point ` + + `--out at a path that does not exist yet.`, + ); + } + const reuse = profile === "agent" && !values.force && out === canonicalOut && existsSync(out); let identity: Identity; if (reuse) { try { From b96e3eab64629b0b926e55ff10dbcbd9d59efbda Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Thu, 9 Jul 2026 23:06:44 -0400 Subject: [PATCH 06/11] fix(cli): reject a symlinked creds path in mint Closes the second greptile P1 on #9: `out === canonicalOut` is a string compare (resolve() normalizes the path but does not follow symlinks), so it never proved the canonical file belongs to . If creds/.creds was a symlink to another agent's creds, existsSync/readFileSync/writeSecretFile all followed it, so `cotal mint ` read the target's id and wrote 's ACLs back through the link, clobbering the pointed-to agent (--force rotated a fresh id straight through it). mint now lstat's the resolved out path (lstat does not follow the link) and refuses a symlink outright, canonical or custom, with or without --force. Regression: mint-reuse smoke gains the symlinked-canonical-path case (read-through refused, link target byte-identical, --force still refused). Co-Authored-By: seal --- implementations/cli/smoke/mint-reuse.smoke.ts | 45 ++++++++++++++++++- implementations/cli/src/commands/mint.ts | 21 ++++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/implementations/cli/smoke/mint-reuse.smoke.ts b/implementations/cli/smoke/mint-reuse.smoke.ts index e5893e8645..7d25539cf9 100644 --- a/implementations/cli/smoke/mint-reuse.smoke.ts +++ b/implementations/cli/smoke/mint-reuse.smoke.ts @@ -17,7 +17,7 @@ */ import { strict as assert } from "node:assert"; import { randomUUID } from "node:crypto"; -import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, symlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createSpaceAuth, idFromCreds } from "@cotal-ai/core"; @@ -172,6 +172,49 @@ try { idFromCreds(readFileSync(victimCreds, "utf8")) !== victimIdBefore, { victimIdBefore, after: idFromCreds(readFileSync(victimCreds, "utf8")) }, ); + + // 8) Symlinked canonical creds path (greptile P1). `out === canonicalOut` is a STRING compare — it + // does not prove the file at that path belongs to . If creds/.creds is a symlink to + // another agent's creds, existsSync/readFileSync/writeSecretFile all FOLLOW it, so a plain + // `cotal mint ` would read the target's id and write 's ACLs back THROUGH the link, + // clobbering the pointed-to agent. A creds file must be a real file at its own path: mint refuses + // a symlinked out path (canonical or custom, with or without --force), leaving the target intact. + writeFileSync(join(agentsDir, "keeper.md"), "---\nname: keeper\nsubscribe: [general]\nallowSubscribe: [general]\n---\nbody\n"); + writeFileSync(join(agentsDir, "decoy.md"), "---\nname: decoy\nsubscribe: [general]\nallowSubscribe: [general, ops]\n---\nbody\n"); + const keeperCreds = join(credsDir, "keeper.creds"); + await mintQuiet(["keeper"]); + const keeperBefore = readFileSync(keeperCreds, "utf8"); + const keeperIdBefore = idFromCreds(keeperBefore); + symlinkSync(keeperCreds, join(credsDir, "decoy.creds")); // canonical decoy path → keeper's real creds + let symlinkMsg = ""; + try { + await mintQuiet(["decoy"]); // canonical path, no --out, no --force + } catch (e) { + symlinkMsg = e instanceof Error ? e.message : String(e); + } + check( + "mint whose canonical creds is a symlink fails loud (no read/write through the link)", + /symlink/.test(symlinkMsg), + { symlinkMsg }, + ); + check( + "refused symlink mint left the link target byte-identical (no clobber of the pointed-to agent)", + readFileSync(keeperCreds, "utf8") === keeperBefore && idFromCreds(readFileSync(keeperCreds, "utf8")) === keeperIdBefore, + { changed: readFileSync(keeperCreds, "utf8") !== keeperBefore }, + ); + // --force must NOT bypass the guard: rotating a fresh id straight through the link still clobbers the + // target. The escape hatch is to remove the link, never to write through it. + let symlinkForceMsg = ""; + try { + await mintQuiet(["decoy", "--force"]); + } catch (e) { + symlinkForceMsg = e instanceof Error ? e.message : String(e); + } + check( + "mint --force does NOT write through a symlinked creds path (still refuses, target intact)", + /symlink/.test(symlinkForceMsg) && readFileSync(keeperCreds, "utf8") === keeperBefore, + { symlinkForceMsg, changed: readFileSync(keeperCreds, "utf8") !== keeperBefore }, + ); } finally { process.chdir(prevCwd); rmSync(root, { recursive: true, force: true }); diff --git a/implementations/cli/src/commands/mint.ts b/implementations/cli/src/commands/mint.ts index 9eafb03b1d..7002707d17 100644 --- a/implementations/cli/src/commands/mint.ts +++ b/implementations/cli/src/commands/mint.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, lstatSync, readFileSync } from "node:fs"; import { resolve, join, dirname } from "node:path"; import { parseArgs } from "node:util"; import { @@ -98,6 +98,25 @@ export async function mint(argv: string[]): Promise { // (a compromised key / intentional new identity). const canonicalOut = resolve(join(dir, "creds", `${name}.creds`)); const out = resolve(values.out ?? canonicalOut); + // A creds file must be a REAL file at its own path. `out === canonicalOut` below is a string compare + // (resolve() normalizes the path but does NOT follow symlinks), so it cannot prove the file at the + // canonical path belongs to . If creds/.creds is a symlink, existsSync/readFileSync/ + // writeSecretFile all FOLLOW it — mint would read the link target's id and write 's ACLs back + // THROUGH the link, clobbering the pointed-to agent (and --force would rotate a fresh id straight + // through it). Refuse a symlinked out path outright — canonical or custom, with or without --force. + let outIsSymlink = false; + try { + outIsSymlink = lstatSync(out).isSymbolicLink(); // lstat does NOT follow the link (unlike existsSync) + } catch { + // ENOENT: nothing at `out`, not even a dangling link — not a symlink, leave false. + } + if (outIsSymlink) { + throw new Error( + `cotal mint: ${out} is a symlink — a creds file must be a real file at its own path, not a link ` + + `to another agent's creds (following it would read or overwrite the wrong identity). Remove the ` + + `symlink and mint to a real path.`, + ); + } // The canonical `creds/.creds` path is the ONLY binding between an agent name and a creds // file: the file bakes an nkey id, not the name (`identity.ts`), so a creds file at a custom // `--out` cannot be attributed to . A custom `--out` onto an EXISTING creds file must From bb1610dc021ce24d26c39cdfea953be857c4bcdb Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Fri, 10 Jul 2026 09:57:56 -0400 Subject: [PATCH 07/11] docs(platform): design the native cotal connector renderer 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 --- .../platform/cotal-connector-renderer.md | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 docs/designs/platform/cotal-connector-renderer.md diff --git a/docs/designs/platform/cotal-connector-renderer.md b/docs/designs/platform/cotal-connector-renderer.md new file mode 100644 index 0000000000..d602f6483d --- /dev/null +++ b/docs/designs/platform/cotal-connector-renderer.md @@ -0,0 +1,166 @@ +# Design: native OMP renderer for the cotal connector + +## Problem / Intent + +The cotal connector delivers mesh traffic into an oh-my-pi (OMP) session as a +`CustomMessage`, and registers the `cotal_*` tools natively. Both surfaces render +poorly today: + +- **Inbound peer messages render as one flat blob.** `drive()` builds the injection + text with `formatInjection(items)` and sends it as `content`, passing an empty + `details: {}` — with no registered renderer, OMP falls back to printing the raw + string, so a batch of peer messages collapses into a single unreadable paragraph + (Matt: "dumped into a single paragraph, hard to read"). +- **Outbound `cotal_*` tool cards show a double-render spinner glyph.** The tools + register via `pi.registerTool` with no `renderCall`/`renderResult`, so OMP uses its + generic animated-spinner fallback — producing the visible spinner artifact Matt + flagged. + +Both are fixed natively in the connector by using OMP's extension rendering API: +a `registerMessageRenderer` for the inbound custom-message types, and +`renderCall`/`renderResult` on the outbound tool registrations. + +This record is the design contract; the implementation ships separately (see +Global Constraints — it is gated on PR #8's fork-base clearing). + +## Approach + +**Inbound — structured message renderer.** Carry the already-in-hand `InboxItem[]` +through the `sendMessage` `details` field (today discarded as `{}`), and register a +`MessageRenderer` for the two custom types the loop emits (`cotal:incoming`, +`cotal:nudge`). OMP stores `details` on the `CustomMessage` and hands the whole +message to the renderer, which reads `message.details.items` and lays out one card +per item (sender · role · kind/channel badge · mention/historical markers · text), +mirroring OMP's own `CustomMessageComponent` frame. The `content` string (the +`formatInjection` blob) is retained unchanged as the no-renderer / non-TUI fallback +and as the LLM-visible text — the renderer is display-only and never alters what the +model reads. + +This requires widening the connector's internal `PeerHost.sendMessage` `details` +type from `unknown` to a structured payload, and threading the real `items` (plus a +`kind` discriminator) at the call site instead of `{}`. + +**Outbound — tool renderers.** Add `renderCall` (and optionally `renderResult`) to +the `pi.registerTool` options in `registerSpec()`. A tool carrying either hook takes +OMP's custom-renderer branch instead of the generic animated-spinner fallback, which +removes the glyph. There are two registration branches to cover: the `cotal_inbox` +branch and the generic branch (all other `cotal_*` tools). Copy the shape of OMP's +built-in `ircToolRenderer` (`src/tools/irc.ts`, wired in `src/tools/renderers.ts`), +which is the closest analogue (a messaging tool). + +**Why this approach (vs alternatives).** The `details`-passthrough is the sanctioned +OMP extension path — `sendMessage`'s typed `details` field exists precisely for +extension-specific structured data (`session-entries.ts:197`, "Extension-specific +data (not sent to LLM)"), and `registerMessageRenderer` is the matching read side. +The alternative — parsing the flat `formatInjection` string back into rows inside a +renderer — is fragile (re-parsing text we already have structured) and was rejected. +Hooks are attached once in `registerSpec` (not per-tool inline) because that helper +is the single registration chokepoint for every cotal tool. + +## Plan + +Two independent workstreams. The outbound tool-renderer work is smaller and fully +API-version-independent; the inbound work depends on the `registerMessageRenderer` / +`details` path (both source-verified present — see Global Constraints). They share no +code beyond living in the same two files, and can land as separate PRs (see Open +Questions #2). + +**Workstream A — outbound tool renderers (`extension.ts`)** + +1. Add `renderCall` to the `cotal_inbox` `registerTool` options and to the generic + `registerTool` options in `registerSpec()`. Minimal form: a titled single-line + `Text` (tool label + a one-line summary of args) — enough to leave the + spinner-fallback branch. Optionally add `renderResult` for a compact result line. +2. Enrich `renderCall` per surface: for send-class tools (`cotal_send`, `cotal_dm`, + `cotal_anycast`) show recipient/channel + a message preview from `args`; for + `cotal_status` show the new status/activity; for `cotal_inbox` a static + "peek inbox" label. +3. Extend the connector smoke to assert the registered tools carry a `renderCall` + (so the spinner-fallback regression is caught). + +**Workstream B — inbound message renderer (`interactive-loop.ts` + `extension.ts`)** + +4. Widen `PeerHost.sendMessage`'s `details` type from `unknown` to a structured + `CotalInjectionDetails` payload, and export that type. Thread the real payload at + the `drive()` call site: `details: { items, kind: override ? "nudge" : "incoming" }` + (in the nudge branch `items` is `[]` — the renderer handles the bare-string nudge + case distinctly). +5. Implement a `MessageRenderer` (`renderCotalMessage`) that reads + `message.details.items` and returns a per-item card `Component` built from + `@oh-my-pi/pi-tui` `Box`/`Container` + OMP `theme`, mirroring + `CustomMessageComponent`. Handle: incoming (N item cards), nudge (single + compact line), and the empty/absent-details fallback (render nothing extra — + `content` already carries the text). +6. Register it in `cotalMesh(pi)` for both custom types: + `pi.registerMessageRenderer("cotal:incoming", renderCotalMessage)` and + `pi.registerMessageRenderer("cotal:nudge", renderCotalMessage)`, using the exported + `INCOMING` / `NUDGE` constants. +7. Extend the interactive-loop smoke to assert `details` carries the structured + `items` (not `{}`) on an incoming batch, and that a registered renderer is invoked. + +## Tasks + +- [ ] **A1** — `renderCall` on both `registerTool` branches in `registerSpec()` + (spinner-fallback fix, minimal). + - `Interfaces:` consumes OMP `ToolDefinition.renderCall?: (args: Static, options: ToolRenderResultOptions, theme: Theme) => Component` (`types.ts:464`); produces a `Component` from `@oh-my-pi/pi-tui`. Attaches inside `registerSpec` (`extension.ts:141`), both the `cotal_inbox` `pi.registerTool` (`extension.ts:158-169`) and the generic `pi.registerTool` (`extension.ts:176-185`). +- [ ] **A2** — per-surface `renderCall` enrichment (recipient/channel/preview from `args`) + optional `renderResult`. + - `Interfaces:` `renderResult?: (result: AgentToolResult, options: ToolRenderResultOptions, theme: Theme, args?: Static) => Component` (`types.ts:467-472`). Copy template: `ircToolRenderer` (`src/tools/irc.ts`, registered in `src/tools/renderers.ts:106`). +- [ ] **A3** — smoke assertion: registered `cotal_*` tools expose a `renderCall`. + - `Interfaces:` extends `extensions/connector-oh-my-pi/*.smoke.ts`; asserts against the fake `pi` capturing `registerTool` options. +- [ ] **B1** — widen + export `CotalInjectionDetails`; thread real payload at the `drive()` call site. + - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[]; kind: "incoming" | "nudge" }` (`InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44-64`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:34-40`); call site `interactive-loop.ts:96-98` `details: {}` → `details: { items, kind: override ? "nudge" : "incoming" }`. +- [ ] **B2** — implement `renderCotalMessage: MessageRenderer`. + - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`types.ts:905-909`); reads `message.details?.items` (`CustomMessage.details?: T`, `session/messages.ts:467-472`). Builds `Component` via `Box`/`Container` from `@oh-my-pi/pi-tui` + `theme`, mirroring `CustomMessageComponent` (`src/modes/components/custom-message.ts`). Renders per-`InboxItem` card; returns `undefined` when `details?.items` is empty/absent (fallback to `content`). +- [ ] **B3** — register the renderer for both custom types in `cotalMesh(pi)`. + - `Interfaces:` `pi.registerMessageRenderer(customType, renderCotalMessage): void` (`types.ts:1077`); called in `cotalMesh(pi: ExtensionAPI)` (`extension.ts:41`) for `INCOMING` (`"cotal:incoming"`) and `NUDGE` (`"cotal:nudge"`) (`interactive-loop.ts:42-43`). +- [ ] **B4** — smoke: `details` carries structured `items` on an incoming batch; renderer invoked. + - `Interfaces:` extends `extensions/connector-oh-my-pi/interactive-loop.smoke.ts`; asserts the fake host records `details.items.length > 0` (not `{}`) and `registerMessageRenderer` was called for both types. + +## Global Constraints + +- **OMP API floor.** Connector depends on `@oh-my-pi/pi-coding-agent: "^16.3.12"` + (`extensions/connector-oh-my-pi/package.json:34`), which resolves 16.3.15. The + renderer APIs are **source-verified present** in OMP at **16.3.4** (older than + 16.3.15 → present a fortiori; the CHANGELOG shows `registerMessageRenderer` landed + many versions earlier): `registerMessageRenderer` (`types.ts:1077`), + `MessageRenderer` (`types.ts:905`), `CustomMessage.details?: T` + (`session/messages.ts:472`), `ToolDefinition.renderCall/renderResult` + (`types.ts:464/467`). No version bump is required. +- **Import path.** The connector deep-imports OMP types from + `@oh-my-pi/pi-coding-agent/extensibility/extensions/types` (the published root + barrel is currently unconsumable under `nodenext`; see `extension.ts:23-28`). + Renderer/tool-renderer types come from that same subpath; TUI primitives + (`Component`, `Box`, `Container`) from `@oh-my-pi/pi-tui`. +- **Lane boundary.** Changes are confined to `extensions/connector-oh-my-pi` (and the + `InboxItem` type it already imports from `connector-core`). No core/protocol change. +- **Display-only.** The renderer never alters `content` (the LLM-visible text / + non-TUI fallback); it only adds a TUI `Component`. `formatInjection` + + `ORIENTATION_BOOTSTRAP` behavior is unchanged. +- **Implementation is gated (design is not).** Connector code exists only on branch + `cotal-connector-esc-composer` (PR #8), which is fork-base held (stacked on #5), so + the implementation cannot merge until the fork-base clears. This design record is a + markdown file off `main` and is not gated — it ships and freezes independently. +- **Conventions.** Conventional Commits; `Co-Authored-By: seal ` + trailer; maintainer voice. +- **OMP-side fix is separate.** OMP owns a parallel commit-core root-cause fix + (renderless cards never commit unsealed); it is independent of this connector-side + rendering work and not in this lane. + +## Open Questions + +1. **[RESOLVED — not for review] Do the renderer APIs exist in the build version?** + Yes — `registerMessageRenderer`, `MessageRenderer`, `CustomMessage.details`, and + `ToolDefinition.renderCall/renderResult` are all source-verified in OMP 16.3.4 + (< the resolved 16.3.15). Resolved from OMP source rather than asked; folded into + Global Constraints as the API floor. Recorded here only as a resolved note. +2. **[LOAD-BEARING] Landing target given #8's fork-base hold.** The implementation must + build on the connector code, which lives only on #8 (fork-base held). Options: + (a) stack the implementation branch on #8 and park it until the fork-base clears; + (b) wait for #8 → `main`, then branch off `main`. **Recommend (a)** — the work is + execute-ready the moment the fork-base clears, with no idle wait. Needs Matt's call + before implementation starts. +3. **[non-load-bearing] One PR or two?** Ship inbound (Workstream B) and outbound + (Workstream A) as one PR, or split. **Recommend split** — Workstream A (outbound + spinner-fix) is smaller and fully API-version-independent, so it can land first + with a tight diff; Workstream B follows. Deferred: the design is correct either + way; this is a delivery-shape choice ratified at merge. From 05cd7efe7e7fc888c13bfbe7c06bbcaa734d98d5 Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Fri, 10 Jul 2026 10:03:59 -0400 Subject: [PATCH 08/11] docs(platform): render nudge messages by kind, not items-emptiness 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. --- .../platform/cotal-connector-renderer.md | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/docs/designs/platform/cotal-connector-renderer.md b/docs/designs/platform/cotal-connector-renderer.md index d602f6483d..7d2d8e8980 100644 --- a/docs/designs/platform/cotal-connector-renderer.md +++ b/docs/designs/platform/cotal-connector-renderer.md @@ -29,9 +29,10 @@ Global Constraints — it is gated on PR #8's fork-base clearing). through the `sendMessage` `details` field (today discarded as `{}`), and register a `MessageRenderer` for the two custom types the loop emits (`cotal:incoming`, `cotal:nudge`). OMP stores `details` on the `CustomMessage` and hands the whole -message to the renderer, which reads `message.details.items` and lays out one card -per item (sender · role · kind/channel badge · mention/historical markers · text), -mirroring OMP's own `CustomMessageComponent` frame. The `content` string (the +message to the renderer, which **branches on `details.kind`**: an `incoming` message +lays out one card per `details.items` entry (sender · role · kind/channel badge · +mention/historical markers · text), and a `nudge` renders a single compact line from +`content` (it carries no items). Both mirror OMP's own `CustomMessageComponent` frame. The `content` string (the `formatInjection` blob) is retained unchanged as the no-renderer / non-TUI fallback and as the LLM-visible text — the renderer is display-only and never alters what the model reads. @@ -83,14 +84,18 @@ Questions #2). 4. Widen `PeerHost.sendMessage`'s `details` type from `unknown` to a structured `CotalInjectionDetails` payload, and export that type. Thread the real payload at the `drive()` call site: `details: { items, kind: override ? "nudge" : "incoming" }` - (in the nudge branch `items` is `[]` — the renderer handles the bare-string nudge - case distinctly). -5. Implement a `MessageRenderer` (`renderCotalMessage`) that reads - `message.details.items` and returns a per-item card `Component` built from - `@oh-my-pi/pi-tui` `Box`/`Container` + OMP `theme`, mirroring - `CustomMessageComponent`. Handle: incoming (N item cards), nudge (single - compact line), and the empty/absent-details fallback (render nothing extra — - `content` already carries the text). + (in the nudge branch `items` is `[]` by design — the renderer keys on `kind`, not + `items` emptiness, so nudges still render; see step 5). +5. Implement a `MessageRenderer` (`renderCotalMessage`) that **branches on + `message.details?.kind`**, not on `items` emptiness (nudge legitimately carries + `items: []`, so an emptiness check would wrongly send nudges to the fallback): + - `kind === "incoming"` → one card `Component` per `details.items` entry, built + from `@oh-my-pi/pi-tui` `Box`/`Container` + OMP `theme`, mirroring + `CustomMessageComponent`. + - `kind === "nudge"` → a single compact line rendered from `message.content` (the + nudge string; `items` is `[]` here by design). + - `details` absent/undefined (a non-cotal custom message) → return `undefined` so + OMP falls back to `content`. This is the only `undefined` case. 6. Register it in `cotalMesh(pi)` for both custom types: `pi.registerMessageRenderer("cotal:incoming", renderCotalMessage)` and `pi.registerMessageRenderer("cotal:nudge", renderCotalMessage)`, using the exported @@ -110,7 +115,7 @@ Questions #2). - [ ] **B1** — widen + export `CotalInjectionDetails`; thread real payload at the `drive()` call site. - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[]; kind: "incoming" | "nudge" }` (`InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44-64`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:34-40`); call site `interactive-loop.ts:96-98` `details: {}` → `details: { items, kind: override ? "nudge" : "incoming" }`. - [ ] **B2** — implement `renderCotalMessage: MessageRenderer`. - - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`types.ts:905-909`); reads `message.details?.items` (`CustomMessage.details?: T`, `session/messages.ts:467-472`). Builds `Component` via `Box`/`Container` from `@oh-my-pi/pi-tui` + `theme`, mirroring `CustomMessageComponent` (`src/modes/components/custom-message.ts`). Renders per-`InboxItem` card; returns `undefined` when `details?.items` is empty/absent (fallback to `content`). + - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`types.ts:905-909`); reads `message.details` (`CustomMessage.details?: T`, `session/messages.ts:467-472`). **Branches on `details?.kind`**: `"incoming"` → per-`InboxItem` card built via `Box`/`Container` from `@oh-my-pi/pi-tui` + `theme`, mirroring `CustomMessageComponent` (`src/modes/components/custom-message.ts`); `"nudge"` → compact single line from `message.content` (`items` is `[]` by design, so it does NOT gate on `items.length`); `details` absent → `undefined` (fall back to `content`) — the sole `undefined` case. - [ ] **B3** — register the renderer for both custom types in `cotalMesh(pi)`. - `Interfaces:` `pi.registerMessageRenderer(customType, renderCotalMessage): void` (`types.ts:1077`); called in `cotalMesh(pi: ExtensionAPI)` (`extension.ts:41`) for `INCOMING` (`"cotal:incoming"`) and `NUDGE` (`"cotal:nudge"`) (`interactive-loop.ts:42-43`). - [ ] **B4** — smoke: `details` carries structured `items` on an incoming batch; renderer invoked. From 5e05af11123164808501feffa389296b8ca73ff2 Mon Sep 17 00:00:00 2001 From: Matt Wilkinson Date: Sat, 11 Jul 2026 15:29:32 -0400 Subject: [PATCH 09/11] docs(platform): fold design-critic pass into connector-renderer record 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 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 --- .../platform/cotal-connector-renderer.md | 151 +++++++++++++----- 1 file changed, 112 insertions(+), 39 deletions(-) diff --git a/docs/designs/platform/cotal-connector-renderer.md b/docs/designs/platform/cotal-connector-renderer.md index 7d2d8e8980..4393461c54 100644 --- a/docs/designs/platform/cotal-connector-renderer.md +++ b/docs/designs/platform/cotal-connector-renderer.md @@ -8,9 +8,14 @@ poorly today: - **Inbound peer messages render as one flat blob.** `drive()` builds the injection text with `formatInjection(items)` and sends it as `content`, passing an empty - `details: {}` — with no registered renderer, OMP falls back to printing the raw - string, so a batch of peer messages collapses into a single unreadable paragraph - (Matt: "dumped into a single paragraph, hard to read"). + `details: {}`. `formatInjection` (`connector-core/src/control.ts:76-81`) already + emits one `•`-bullet per item joined by `\n`, so the string itself is structured — + but with no registered renderer OMP falls back to rendering `content` as its default + Markdown body (`message-frame.ts:58-89`), which reflows/soft-wraps those bullet lines + so the per-message structure is lost on screen (Matt: "dumped into a single + paragraph, hard to read"). The fix is the renderer (or, per OQ#4 option (a), + pre-formatting `content` so OMP's own frame renders it cleanly) — not a change to + `formatInjection`, which is already correct. - **Outbound `cotal_*` tool cards show a double-render spinner glyph.** The tools register via `pi.registerTool` with no `renderCall`/`renderResult`, so OMP uses its generic animated-spinner fallback — producing the visible spinner artifact Matt @@ -29,29 +34,38 @@ Global Constraints — it is gated on PR #8's fork-base clearing). through the `sendMessage` `details` field (today discarded as `{}`), and register a `MessageRenderer` for the two custom types the loop emits (`cotal:incoming`, `cotal:nudge`). OMP stores `details` on the `CustomMessage` and hands the whole -message to the renderer, which **branches on `details.kind`**: an `incoming` message -lays out one card per `details.items` entry (sender · role · kind/channel badge · -mention/historical markers · text), and a `nudge` renders a single compact line from -`content` (it carries no items). Both mirror OMP's own `CustomMessageComponent` frame. The `content` string (the -`formatInjection` blob) is retained unchanged as the no-renderer / non-TUI fallback +message to the renderer, which **branches on `message.customType`** — the value OMP +already dispatched the renderer on, so it is authoritative by construction (no +separate discriminator to keep in sync): an `incoming` message lays out one card per +`details.items` entry (sender · role · kind/channel badge · mention/historical +markers · text), and a `nudge` renders a single compact line from `content` (it +carries no items). The framing of these cards is a load-bearing Open Question — a +returned `MessageRenderer` Component does **not** inherit OMP's card frame (see OQ#4); +the record no longer assumes it "mirrors" one. The `content` string (the +`formatInjection` block) is retained unchanged as the no-renderer / non-TUI fallback and as the LLM-visible text — the renderer is display-only and never alters what the model reads. This requires widening the connector's internal `PeerHost.sendMessage` `details` -type from `unknown` to a structured payload, and threading the real `items` (plus a -`kind` discriminator) at the call site instead of `{}`. +type from `unknown` to a structured payload (`{ items: InboxItem[] }` — no `kind` +field; the renderer keys on `customType`), and threading the real `items` at the call +site instead of `{}`. **Outbound — tool renderers.** Add `renderCall` (and optionally `renderResult`) to the `pi.registerTool` options in `registerSpec()`. A tool carrying either hook takes OMP's custom-renderer branch instead of the generic animated-spinner fallback, which removes the glyph. There are two registration branches to cover: the `cotal_inbox` -branch and the generic branch (all other `cotal_*` tools). Copy the shape of OMP's -built-in `ircToolRenderer` (`src/tools/irc.ts`, wired in `src/tools/renderers.ts`), -which is the closest analogue (a messaging tool). +branch and the generic branch (all other `cotal_*` tools). **Port the `renderCall` / +`renderResult` bodies** from OMP's built-in `ircToolRenderer` (`src/tools/irc.ts`) — +but only those two functions: irc's `inline` / `mergeCallAndResult` flags live on +OMP's *internal* `ToolRenderer` record, not on the extension `ToolDefinition` surface +`pi.registerTool` uses, so the merged single-card look is not reachable this way (see +OQ#5). Two rows (call + result) is the natural `ToolDefinition` shape and fully +removes the spinner glyph regardless. **Why this approach (vs alternatives).** The `details`-passthrough is the sanctioned OMP extension path — `sendMessage`'s typed `details` field exists precisely for -extension-specific structured data (`session-entries.ts:197`, "Extension-specific +extension-specific structured data (`session/session-entries.ts:187`, "Extension-specific data (not sent to LLM)"), and `registerMessageRenderer` is the matching read side. The alternative — parsing the flat `formatInjection` string back into rows inside a renderer — is fragile (re-parsing text we already have structured) and was rejected. @@ -83,19 +97,24 @@ Questions #2). 4. Widen `PeerHost.sendMessage`'s `details` type from `unknown` to a structured `CotalInjectionDetails` payload, and export that type. Thread the real payload at - the `drive()` call site: `details: { items, kind: override ? "nudge" : "incoming" }` - (in the nudge branch `items` is `[]` by design — the renderer keys on `kind`, not - `items` emptiness, so nudges still render; see step 5). + the `drive()` call site: `details: { items }` (in the nudge/override branch `items` + is `[]` — the renderer keys on `message.customType`, not `items` emptiness, so + nudges still render; see step 5). Also type the OMP-facing call as + `pi.sendMessage({...})` (the host method is generic — + `types.ts:1105`) so the payload is checked against `CustomMessagePayload` + rather than erased to `unknown` at the `pi` boundary. 5. Implement a `MessageRenderer` (`renderCotalMessage`) that **branches on - `message.details?.kind`**, not on `items` emptiness (nudge legitimately carries - `items: []`, so an emptiness check would wrongly send nudges to the fallback): - - `kind === "incoming"` → one card `Component` per `details.items` entry, built - from `@oh-my-pi/pi-tui` `Box`/`Container` + OMP `theme`, mirroring - `CustomMessageComponent`. - - `kind === "nudge"` → a single compact line rendered from `message.content` (the - nudge string; `items` is `[]` here by design). + `message.customType`** (`INCOMING` vs `NUDGE`) — the value OMP already dispatched + the renderer on, so it is authoritative and needs no separate `kind` field; it + does NOT gate on `items` emptiness (nudge legitimately carries `items: []`, so an + emptiness check would wrongly send nudges to the fallback): + - `customType === INCOMING` → one card `Component` per `details.items` entry. The + card frame is OQ#4 (a returned Component is unframed — option (a) returns + `undefined` after pre-formatting `content`; option (b) hand-builds a frame). + - `customType === NUDGE` → a single compact line rendered from `message.content` + (the nudge string; `items` is `[]` here by design). - `details` absent/undefined (a non-cotal custom message) → return `undefined` so - OMP falls back to `content`. This is the only `undefined` case. + OMP falls back to `content`. This is the only unconditional `undefined` case. 6. Register it in `cotalMesh(pi)` for both custom types: `pi.registerMessageRenderer("cotal:incoming", renderCotalMessage)` and `pi.registerMessageRenderer("cotal:nudge", renderCotalMessage)`, using the exported @@ -107,30 +126,30 @@ Questions #2). - [ ] **A1** — `renderCall` on both `registerTool` branches in `registerSpec()` (spinner-fallback fix, minimal). - - `Interfaces:` consumes OMP `ToolDefinition.renderCall?: (args: Static, options: ToolRenderResultOptions, theme: Theme) => Component` (`types.ts:464`); produces a `Component` from `@oh-my-pi/pi-tui`. Attaches inside `registerSpec` (`extension.ts:141`), both the `cotal_inbox` `pi.registerTool` (`extension.ts:158-169`) and the generic `pi.registerTool` (`extension.ts:176-185`). + - `Interfaces:` consumes OMP `ToolDefinition.renderCall?: (args: Static, options: ToolRenderResultOptions, theme: Theme) => Component` (`extensibility/extensions/types.ts:476`); produces a `Component` from `@oh-my-pi/pi-tui`. Attaches inside `registerSpec` (`extension.ts:141`), both the `cotal_inbox` `pi.registerTool` (`extension.ts:158`) and the generic `pi.registerTool` (`extension.ts:176`). - [ ] **A2** — per-surface `renderCall` enrichment (recipient/channel/preview from `args`) + optional `renderResult`. - - `Interfaces:` `renderResult?: (result: AgentToolResult, options: ToolRenderResultOptions, theme: Theme, args?: Static) => Component` (`types.ts:467-472`). Copy template: `ircToolRenderer` (`src/tools/irc.ts`, registered in `src/tools/renderers.ts:106`). + - `Interfaces:` `renderResult?: (result: AgentToolResult, options: ToolRenderResultOptions, theme: Theme, args?: Static) => Component` (`extensibility/extensions/types.ts:479-485`). **Port only the `renderCall`/`renderResult` function bodies** from `ircToolRenderer` (`src/tools/irc.ts:814`, registered into the *internal* renderer table at `src/tools/renderers.ts:86`); its `inline`/`mergeCallAndResult` flags are on OMP's internal `ToolRenderer` type (`src/tools/renderers.ts:34-72`), NOT on the extension `ToolDefinition` — do not copy them (see OQ#5). - [ ] **A3** — smoke assertion: registered `cotal_*` tools expose a `renderCall`. - `Interfaces:` extends `extensions/connector-oh-my-pi/*.smoke.ts`; asserts against the fake `pi` capturing `registerTool` options. - [ ] **B1** — widen + export `CotalInjectionDetails`; thread real payload at the `drive()` call site. - - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[]; kind: "incoming" | "nudge" }` (`InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44-64`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:34-40`); call site `interactive-loop.ts:96-98` `details: {}` → `details: { items, kind: override ? "nudge" : "incoming" }`. + - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[] }` (no `kind` field — the renderer keys on `customType`; `InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:35-40`); call site `interactive-loop.ts:96-97` `details: {}` → `details: { items }`, and types the OMP-facing call `host.sendMessage(...)` (host method generic at `extensibility/extensions/types.ts:1105`) so the payload is checked, not erased to `unknown`. - [ ] **B2** — implement `renderCotalMessage: MessageRenderer`. - - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`types.ts:905-909`); reads `message.details` (`CustomMessage.details?: T`, `session/messages.ts:467-472`). **Branches on `details?.kind`**: `"incoming"` → per-`InboxItem` card built via `Box`/`Container` from `@oh-my-pi/pi-tui` + `theme`, mirroring `CustomMessageComponent` (`src/modes/components/custom-message.ts`); `"nudge"` → compact single line from `message.content` (`items` is `[]` by design, so it does NOT gate on `items.length`); `details` absent → `undefined` (fall back to `content`) — the sole `undefined` case. + - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`extensibility/extensions/types.ts:917-921`); reads `message.details` (`CustomMessage.details?: T`, `session/messages.ts:556`) and `message.customType` (`session/messages.ts:554`). **Branches on `message.customType`**: `INCOMING` → per-`InboxItem` card `Component` (frame per OQ#4 — a returned Component is *not* wrapped in OMP's card by `renderFramedMessage` (`modes/components/message-frame.ts:50-56`); option (a) returns `undefined` after pre-formatting `content`, option (b) hand-builds the frame); `NUDGE` → compact single line from `message.content`; `details` absent → `undefined` (fall back to `content`) — the sole unconditional `undefined` case. - [ ] **B3** — register the renderer for both custom types in `cotalMesh(pi)`. - - `Interfaces:` `pi.registerMessageRenderer(customType, renderCotalMessage): void` (`types.ts:1077`); called in `cotalMesh(pi: ExtensionAPI)` (`extension.ts:41`) for `INCOMING` (`"cotal:incoming"`) and `NUDGE` (`"cotal:nudge"`) (`interactive-loop.ts:42-43`). + - `Interfaces:` `pi.registerMessageRenderer(customType, renderCotalMessage): void` (`extensibility/extensions/types.ts:1089`); called in `cotalMesh(pi: ExtensionAPI)` (`extension.ts:41`) for `INCOMING` (`"cotal:incoming"`) and `NUDGE` (`"cotal:nudge"`) (`interactive-loop.ts:42-43`). - [ ] **B4** — smoke: `details` carries structured `items` on an incoming batch; renderer invoked. - `Interfaces:` extends `extensions/connector-oh-my-pi/interactive-loop.smoke.ts`; asserts the fake host records `details.items.length > 0` (not `{}`) and `registerMessageRenderer` was called for both types. ## Global Constraints - **OMP API floor.** Connector depends on `@oh-my-pi/pi-coding-agent: "^16.3.12"` - (`extensions/connector-oh-my-pi/package.json:34`), which resolves 16.3.15. The - renderer APIs are **source-verified present** in OMP at **16.3.4** (older than - 16.3.15 → present a fortiori; the CHANGELOG shows `registerMessageRenderer` landed - many versions earlier): `registerMessageRenderer` (`types.ts:1077`), - `MessageRenderer` (`types.ts:905`), `CustomMessage.details?: T` - (`session/messages.ts:472`), `ToolDefinition.renderCall/renderResult` - (`types.ts:464/467`). No version bump is required. + (`extensions/connector-oh-my-pi/package.json:34`), which resolves 16.3.12 (the + only version in the pnpm store; there is no 16.3.15). The renderer APIs are + **source-verified present in the installed 16.3.12 tree**: + `registerMessageRenderer` (`extensibility/extensions/types.ts:1089`), + `MessageRenderer` (`types.ts:917-921`), `CustomMessage.details?: T` + (`session/messages.ts:556`), `ToolDefinition.renderCall`/`renderResult` + (`types.ts:476`/`479`). No version bump is required. - **Import path.** The connector deep-imports OMP types from `@oh-my-pi/pi-coding-agent/extensibility/extensions/types` (the published root barrel is currently unconsumable under `nodenext`; see `extension.ts:23-28`). @@ -150,13 +169,20 @@ Questions #2). - **OMP-side fix is separate.** OMP owns a parallel commit-core root-cause fix (renderless cards never commit unsealed); it is independent of this connector-side rendering work and not in this lane. +- **Design-critic pass (SEA-1188).** This record went through one adversarial + read-only critic pass before freeze (2026-07-11). It folded five findings: the + Problem mechanism (F6), the `customType` discriminator replacing a redundant `kind` + field (F5), the generic-threaded `sendMessage` (F4), the corrected OMP anchors to + the installed 16.3.12 tree (F3), and the `ircToolRenderer` port scope (F2). Two + code-false core claims survived as load-bearing Open Questions #4 and #5 (the frame + and the merged-card target) — both need Matt's call before freeze. ## Open Questions 1. **[RESOLVED — not for review] Do the renderer APIs exist in the build version?** Yes — `registerMessageRenderer`, `MessageRenderer`, `CustomMessage.details`, and - `ToolDefinition.renderCall/renderResult` are all source-verified in OMP 16.3.4 - (< the resolved 16.3.15). Resolved from OMP source rather than asked; folded into + `ToolDefinition.renderCall/renderResult` are all source-verified in the installed + OMP 16.3.12 tree. Resolved from OMP source rather than asked; folded into Global Constraints as the API floor. Recorded here only as a resolved note. 2. **[LOAD-BEARING] Landing target given #8's fork-base hold.** The implementation must build on the connector code, which lives only on #8 (fork-base held). Options: @@ -169,3 +195,50 @@ Questions #2). spinner-fix) is smaller and fully API-version-independent, so it can land first with a tight diff; Workstream B follows. Deferred: the design is correct either way; this is a delivery-shape choice ratified at merge. +4. **[LOAD-BEARING — from design-critic pass] Does a returned `MessageRenderer` + Component inherit OMP's frame, or must the renderer rebuild it?** The Approach + (and Task B2) says the inbound cards "mirror OMP's own `CustomMessageComponent` + frame." **This is code-false as written.** `renderFramedMessage` + (`modes/components/message-frame.ts:50-56`) hands a custom renderer's returned + Component straight back to the caller **unframed** — `if (component) return + component;` — and builds the rounded-outline `Box`/`Text`/`Markdown` card + (`message-frame.ts:58-89`) **only** on the fallback path where the renderer + returns `undefined`/throws. So a `MessageRenderer` that returns a Component owns + the *entire* visual; it inherits no frame. Two materially different shapes: + - **(a) Return `undefined` for the incoming case and fix the `content` string.** + `formatInjection` (`connector-core/src/control.ts:76-81`) already emits one + `•`-bullet per item joined by `\n`; pre-format it as a clean multi-line block + and let OMP's own `renderFramedMessage` build the canonical card — the real OMP + frame for free, zero internal-theme coupling. The lightest fix; but the cards + stay text, not per-item Components. + - **(b) Build a bespoke per-item frame in the renderer.** Gets rich per-item + Components, but the card must be hand-built from `@oh-my-pi/pi-tui` primitives, + and to match every *other* injected-message card it must reuse + `renderFramedMessage`'s box styling (`theme.boxRound`, `theme.fg('borderMuted')`, + `customMessageBg` — `message-frame.ts:60`, `custom-message.ts:23`), which are + **internal theme keys not exported to extensions** — a new coupling the Lane-boundary + constraint does not currently acknowledge. + **Recommend (a)** for the first cut (lightest, no internal coupling; directly fixes + the flat-blob Problem), with (b) as a follow-up if per-item Components are wanted. + Needs Matt's call — the record's current "mirror the frame" answer is wrong and the + choice changes both the Lane-boundary coupling and Task B2's shape. +5. **[LOAD-BEARING — from design-critic pass] Merged single card vs two rows for the + `cotal_*` tool renderers?** The Approach (and Task A2) says to "copy the shape of + OMP's built-in `ircToolRenderer`." **`ircToolRenderer` is not a drop-in template + for an extension tool.** It is `{ inline: true, mergeCallAndResult: true, + renderCall, renderResult }` (`tools/irc.ts:814-816`) plugged into OMP's *internal* + `ToolRenderer` record (`tools/renderers.ts:34-72`), which carries those flags. The + surface the connector actually registers through — `pi.registerTool` → + `ToolDefinition` (`extensibility/extensions/types.ts:440-484`) — exposes **only** + `renderCall?` (`:476`) and `renderResult?` (`:479`); it has **no `inline` / + `mergeCallAndResult` / animated-pending fields** (grep of the type: none). So only + the `renderCall`/`renderResult` *function bodies* port; irc's merged single-card + behavior is unreachable via `registerTool`. Fork on the visual target: + - **(a) Accept two rows** (a call row + a result row), the natural `ToolDefinition` + shape. Simplest; the spinner-fix (the stated Problem) is fully met either way. + - **(b) Hand-build a merged look inside `renderResult`** if a single irc-style card + is wanted — more work, and it only approximates irc's merge. + **Recommend (a)**; the spinner artifact (the actual Problem) is removed the moment + either hook is present. Needs Matt's call on whether the merged look is a + requirement. Task A2's "copy the shape" wording is corrected below to "port the + render bodies; the wrapper flags are internal-only." From 43e5cf33702566a1a4bf023f1c2f1eb1d1cc7879 Mon Sep 17 00:00:00 2001 From: mattwilkinsonn Date: Sat, 11 Jul 2026 23:57:24 -0400 Subject: [PATCH 10/11] docs(platform): clarify nudge renderer reads content, not formatInjection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/designs/platform/cotal-connector-renderer.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/designs/platform/cotal-connector-renderer.md b/docs/designs/platform/cotal-connector-renderer.md index 4393461c54..3130a27ed6 100644 --- a/docs/designs/platform/cotal-connector-renderer.md +++ b/docs/designs/platform/cotal-connector-renderer.md @@ -111,8 +111,13 @@ Questions #2). - `customType === INCOMING` → one card `Component` per `details.items` entry. The card frame is OQ#4 (a returned Component is unframed — option (a) returns `undefined` after pre-formatting `content`; option (b) hand-builds a frame). - - `customType === NUDGE` → a single compact line rendered from `message.content` - (the nudge string; `items` is `[]` here by design). + - `customType === NUDGE` → a single compact line rendered from `message.content`. + For a nudge, `content` is the bare nudge **string** set on the `drive(override)` + path (`interactive-loop.ts:70` `text = override`; the sole caller is the focus + mention-recall at `:118`, a non-empty literal), NOT `formatInjection(items)` — + `formatInjection` runs only on the incoming branch (`:75`, inside the `else`). So + `content` is always present for a nudge and `items` is `[]` here by design; the + renderer never reads `items` on this branch. - `details` absent/undefined (a non-cotal custom message) → return `undefined` so OMP falls back to `content`. This is the only unconditional `undefined` case. 6. Register it in `cotalMesh(pi)` for both custom types: From 0c0c426fd69265b44779937ea243f93d4b011e54 Mon Sep 17 00:00:00 2001 From: mattwilkinsonn Date: Sun, 12 Jul 2026 00:25:53 -0400 Subject: [PATCH 11/11] docs(platform): name one sendMessage receiver; correct source anchors CodeRabbit flagged step 4 vs task B1 naming the same call two ways (`pi.sendMessage` vs `host.sendMessage`), both attaching a `` 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` 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 --- .../platform/cotal-connector-renderer.md | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/designs/platform/cotal-connector-renderer.md b/docs/designs/platform/cotal-connector-renderer.md index 3130a27ed6..82889c9226 100644 --- a/docs/designs/platform/cotal-connector-renderer.md +++ b/docs/designs/platform/cotal-connector-renderer.md @@ -99,10 +99,16 @@ Questions #2). `CotalInjectionDetails` payload, and export that type. Thread the real payload at the `drive()` call site: `details: { items }` (in the nudge/override branch `items` is `[]` — the renderer keys on `message.customType`, not `items` emptiness, so - nudges still render; see step 5). Also type the OMP-facing call as - `pi.sendMessage({...})` (the host method is generic — - `types.ts:1105`) so the payload is checked against `CustomMessagePayload` - rather than erased to `unknown` at the `pi` boundary. + nudges still render; see step 5). The loop's only send path is `host.sendMessage(...)` + where `host: PeerHost` — the connector's internal seam, satisfied by OMP's + `ExtensionAPI` and bound to `pi` at the factory (`extension.ts:99`, + `runPeerLoop({ mesh: agent, host: pi })`). Type safety comes from widening that + seam's `details` field, which checks the payload at the `host.sendMessage` call + site; `PeerHost` stays non-generic, so no `` type-arg is + written there. (OMP's underlying method is itself generic — `sendMessage`, + `types.ts:1105` — which makes the typed `details` sound end-to-end; the type-arg + would appear only if `pi.sendMessage` were called directly, which the loop does + not do.) 5. Implement a `MessageRenderer` (`renderCotalMessage`) that **branches on `message.customType`** (`INCOMING` vs `NUDGE`) — the value OMP already dispatched the renderer on, so it is authoritative and needs no separate `kind` field; it @@ -113,9 +119,9 @@ Questions #2). `undefined` after pre-formatting `content`; option (b) hand-builds a frame). - `customType === NUDGE` → a single compact line rendered from `message.content`. For a nudge, `content` is the bare nudge **string** set on the `drive(override)` - path (`interactive-loop.ts:70` `text = override`; the sole caller is the focus - mention-recall at `:118`, a non-empty literal), NOT `formatInjection(items)` — - `formatInjection` runs only on the incoming branch (`:75`, inside the `else`). So + path (`interactive-loop.ts:74` `text = override`; the sole caller is the focus + mention-recall at `:127`, a non-empty literal), NOT `formatInjection(items)` — + `formatInjection` runs only on the incoming branch (the `else` at `:75`, call at `:79`). So `content` is always present for a nudge and `items` is `[]` here by design; the renderer never reads `items` on this branch. - `details` absent/undefined (a non-cotal custom message) → return `undefined` so @@ -137,7 +143,7 @@ Questions #2). - [ ] **A3** — smoke assertion: registered `cotal_*` tools expose a `renderCall`. - `Interfaces:` extends `extensions/connector-oh-my-pi/*.smoke.ts`; asserts against the fake `pi` capturing `registerTool` options. - [ ] **B1** — widen + export `CotalInjectionDetails`; thread real payload at the `drive()` call site. - - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[] }` (no `kind` field — the renderer keys on `customType`; `InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:35-40`); call site `interactive-loop.ts:96-97` `details: {}` → `details: { items }`, and types the OMP-facing call `host.sendMessage(...)` (host method generic at `extensibility/extensions/types.ts:1105`) so the payload is checked, not erased to `unknown`. + - `Interfaces:` produces `export interface CotalInjectionDetails { items: InboxItem[] }` (no `kind` field — the renderer keys on `customType`; `InboxItem` from `@cotal-ai/connector-core`, shape at `connector-core/src/agent.ts:44`). Changes `PeerHost.sendMessage` `message.details` from `unknown` → `CotalInjectionDetails` (`interactive-loop.ts:35-40`); call site `interactive-loop.ts:96-97` `details: {}` → `details: { items }`. The seam stays non-generic — no `` type-arg at the `host.sendMessage` call; widening the `details` field is what checks the payload there (rather than erasing it to `unknown`). OMP's `ExtensionAPI.sendMessage` is itself generic (`extensibility/extensions/types.ts:1105`), which keeps the typed `details` sound where `host` binds to `pi` (`extension.ts:99`). - [ ] **B2** — implement `renderCotalMessage: MessageRenderer`. - `Interfaces:` `MessageRenderer = (message: CustomMessage, options: MessageRenderOptions, theme: Theme) => Component | undefined` (`extensibility/extensions/types.ts:917-921`); reads `message.details` (`CustomMessage.details?: T`, `session/messages.ts:556`) and `message.customType` (`session/messages.ts:554`). **Branches on `message.customType`**: `INCOMING` → per-`InboxItem` card `Component` (frame per OQ#4 — a returned Component is *not* wrapped in OMP's card by `renderFramedMessage` (`modes/components/message-frame.ts:50-56`); option (a) returns `undefined` after pre-formatting `content`, option (b) hand-builds the frame); `NUDGE` → compact single line from `message.content`; `details` absent → `undefined` (fall back to `content`) — the sole unconditional `undefined` case. - [ ] **B3** — register the renderer for both custom types in `cotalMesh(pi)`.