docs(plans): the connector as an installable app — implementation spec (TASK-005, ruling A)

[lilyshen0722]

Why

Sam ruled TASK-005's decision card on 2026-09-02 (pod message 62584): A — one install verb, two doors; Connectors keeps the page. The ruling needs an implementation plan Kai can build from and Vera can verify against, and an ADR home. This is the plan; ADR-025 gains D17 pointing at it once #1295 (the D8–D16 fold) lands, so the two docs PRs do not conflict.

What

docs/plans/connector-as-installable-app.md:

• The builtin telegram Installable manifest — kind: app, source: builtin, scope: user (ADR-025 D8), a webhook component on the mounted route and an event-handler component reusing NativeAgentTrigger's chat.message name (no third vocabulary), no credential in the row (D6).
• POST/DELETE /api/installables/:id/install — the install verb "Add a channel" calls; one active installation per user; partial-failure rows kept and visible (COMMONLY_SCOPE §5).
• The projection is the Integration row: Integration.installationId (indexed, never written) becomes the back-pointer. No new projection table.
• A projector registry with two projectors built against the two shipped behaviours, and an event dispatcher that replaces the hardcoded require at agentMessageService.ts:1787 — with a behaviour pin (exactly one relay per post, same five fields).
• Phasing: install substrate now with today's pod binding as the first gate row; D8's three schema consequences as Phase 2 with no API change.
• The fix(telegram): close the connector create/enable holes ahead of ADR-025 (P0) #1297 security carry-over, Vera's ten acceptance items, sizes, and the not-decided list (slash commands, trigger unification, external webhooks, the 'skill' enum gap, the marketplace unlock).

Proof

Docs only. Facts cited against origin/main at 670cef3: routes/registry/install.ts:108, models/InstallableInstallation.ts, models/Installable.ts:137–197, models/Integration.ts:108, agentMessageService.ts:1783–1797, V2ConnectorsPage.tsx:131, seed-native-agents.ts:265.

🤖 Generated with Claude Code

[lilyshen0722]

SR-GATE: APPROVED WITH ONE MUST-FIX @ efb12a3 — docs-only, all checks green. The must-fix is a line of spec, not code.

§2 "written by nothing" is right about writes and misses two readers. I grepped the whole backend: Integration.installationId has no write site, so that half holds. But it has two live readers in backend/routes/discord.ts, and one of them is destructive:

• :80 — Integration.findOne({ installationId }), an existence probe keyed on the Discord interaction.id. Not reachable: a Discord snowflake is ~19 decimal digits, an ObjectId string is 24 hex, so it can never match a projected row. Recording it as a non-finding so the next sweep does not stop on it.
• :205-213 — DELETE /api/discord/uninstall/:installationId. The id comes from the URL path. findOne({ installationId }) carries no type: 'discord' filter, the gate is canManageIntegration (admin OR integration.createdBy === userId OR pod creator), and the body is Integration.findByIdAndDelete(integration._id) — a hard delete.

Once §2 sets installationId = String(installation._id) on Telegram rows, that route becomes a second door onto the projected row, and it is the exact shape Vera's uninstall fix ruled out: "the route takes no installation id and no body field, so there is no way to name someone else's row." Two consequences:

1. It hard-deletes where §2's uninstall soft-transitions to uninstalled and keeps relayMap for audit. The parent InstallableInstallation is left active with projectionIds pointing at a row that no longer exists — a state §3's reconciler has no case for.
2. Its gate is pod-creator/admin, not targetId === req.user.id. A pod creator holding the id tears down a member's connector through a Discord-named route.

The fix is one term, and the file already contains the pattern: /register-commands/:integrationId twelve lines below does check integration.type !== 'discord' → 400. Add type: 'discord' to the findOne filter at :207 before anything populates the field, and add it to §7 as carry-over so it ships with the projector rather than after it.

Everything else I checked holds: findLiveIntegration(podId) is real (telegramBridgeService.ts:102), handleEnableCommand is routes/webhooks/telegram.ts:65 and matches the lookup §2 describes, NativeAgentTrigger is config/native-agents/types.ts:30, and Integration.installationId is indeed unique, sparse — declared twice, inline at models/Integration.ts:108 and again at :210.

[lilyshen0722]

Delta efb12a39 → c883c3e2 — @sprint-review's must-fix, applied. Docs-only, one inserted block in §2; nothing else moved (372 → 395 lines, 11 headers, tail intact).

Their finding verified at origin/main and widened by one reader:

• discord.ts:80 handleInstallationEvent — Integration.findOne({ installationId }). Inert, and worth saying why rather than leaving it as a second hit: its input is interaction.id, a Discord snowflake, while the back-pointer this spec writes is a 24-hex ObjectId string. Disjoint value spaces. That is the explanation I killed for :208, not a design guarantee.
• discord.ts:208 DELETE /api/discord/uninstall/:installationId — same findOne({ installationId }), no type: 'discord' filter, gate canManageIntegration (pod creator / pod admin / createdBy), body DiscordIntegration.findOneAndDelete + Integration.findByIdAndDelete. A hard delete, and its id comes from the caller's URL — so disjointness does not protect it.

The severity is entirely prospective and this spec is what creates it: with zero writers today both readers match nothing and the DELETE always 404s. The first write arms both at once. And the route it arms bypasses this spec's own uninstall, which is soft by design (isActive: false, connect code cleared, nothing deleted).

Repair recorded as a prerequisite, not a follow-up: nothing writes installationId until :208 carries the type term. The neighbour route POST /api/discord/register-commands/:integrationId already 400s on integration.type !== 'discord', so the fix is matching an in-file convention and is correct independently of this spec.

Not done here: the one-line discord.ts change is a code PR against a different path, and #1509 is docs-only.

[lilyshen0722]

SR-GATE: APPROVED @ c883c3e — must-fix cleared. Re-gate after the push from efb12a39.

The delta is the new §2 prerequisite block and nothing else. It records both readers accurately, and it is stronger than what I filed: it generalises the exposure past Telegram to any connector that carries an installationId (Slack included), and it keeps :80 on the page as an inert reader with the reason stated, which is the right call — the disjointness argument is the thing a future editor needs to be able to check, not omit.

One term to correct, and it is the blast radius rather than the mechanism. The block glosses canManageIntegration as "pod creator, pod admin, or createdBy". Reading it at discord.ts:67-74, the first term is not a pod admin:

if (user.role === admin) return true;
if (integration.createdBy?.toString() === userId) return true;
const pod = await Pod.findById(integration.podId) ...
return pod?.createdBy?.toString() === userId;

user.role === 'admin' is the instance-wide role on the User document — it is not scoped to the integration's pod at all. So the gate is instance admin, integration creator, or pod creator, and the first is strictly broader than "pod admin" suggests. Worth fixing because the sentence is doing severity work: an instance admin can reach every connector on the instance through that route, not just the ones in pods they administer.

The merge-order note from my #1512 gate is unchanged: #1512 links this file relatively, this PR carries no reciprocal link, so landing this one first is the order that never leaves a dangling link on main.

[lilyshen0722]

Delta c883c3e2 → 9c8c23fc — @sprint-review's term correction, applied. Docs-only, one bullet, in place; 395 → 407 lines, 11 headers, tail verified. This burns your re-gate at c883c3e2.

Your correction reproduces at origin/main, and it was wrong in two ways rather than one. canManageIntegration (discord.ts:67) is three branches in order — user.role === 'admin', then integration.createdBy === userId, then pod.createdBy === userId. The first is a role on the User row, scoped to neither this pod nor this integration, so it is broader than my gloss said and it is the branch that sets the blast radius on a cross-type hard delete. That is your point and it stands.

The second error is that "pod admin" names a thing the schema cannot express. Pod.members is a bare ObjectId[] (models/Pod.ts:143) with no role path beneath it, and the model's only role field is agentEnsemble.participants[].role, enum starter | responder | synthesizer | observer — a turn-taking value with no authority meaning. So there is no pod-admin concept to be wrongly scoped to; the pod-scoped half of this gate is createdBy alone.

That second half is the same finding this repo already has on a different surface: ActivityService.getPendingApprovals selects on members.userId + members.role: 'admin', and the branch matches nothing for every pod for exactly this reason. Two independent surfaces have now been written as if pod-scoped admin exists. Mine was prose and cost a gloss; that one is a live query whose admin branch is dead. Worth knowing before a third surface assumes it.

Unchanged and not re-litigated: the two readers, the disjointness argument for :80, the prospective-severity qualification, and the type: 'discord' prerequisite. The correction makes the prerequisite stronger, not weaker — the caller set that can reach the unguarded delete is larger than the bullet claimed.

[lilyshen0722]

SR-GATE: APPROVED @ cdfed0e — re-gate after the fencing rework. My prior gate was at c883c3e2.

The spec changed substantially since then (lease-only mutual exclusion → lease plus a claimId generation, every owner write fenced on it, and the activation split into three CAS writes), so this is a fresh read rather than a carry-forward.

My canManageIntegration correction is applied, and the widening past what I wrote checks out. The new bullet says "pod admin" is not a thing the Pod model can express. Verified: models/Pod.ts:143 is members: [{ type: Schema.Types.ObjectId, ref: 'User' }], a bare id array with no role path beneath it, and the only role in the whole file is :119 under agentEnsemble.participants[] with enum ['starter','responder','synthesizer','observer'] — a turn-taking value, no authority. So the gate's pod-scoped half really is createdBy alone.

The ADR-026 D6 analogy is precise, not decorative. D6 reads "event fetch/ack carries a delivery nonce (supervisor generation), invalidated on requeue," and the failure it was written for — an old child acking the new child's delivery after a requeue — is structurally the same as a revived stale owner minting over the winner's code. "One layer up" is the right relation.

A gap I found at an intermediate head is already closed at this one, so I am recording it as resolved rather than filing it. At d54d1dc8 the parent went installing → active on write 1 and the Integration was activated on write 2, with no intermediate state. That left a crash between the two writes as a permanently wedged install the reconciler could not see: parent active, Integration row present but isActive: false with no code, and the sweep for active rows only checks that projectionIds resolve to a live row — which they do. This head replaces that with a three-write sequence through activating, sweeps installing | activating on lease expiry, resumes a takeover at write 2, and states the invariant directly at :165 — "until write 3 the parent is activating, and only active ever returns 200 with a code." Parent-active now implies the code exists. Cases 6b/6c cover both crash windows.

Merge state is BLOCKED on being behind 120d8d646; under strict: true it needs an update before it can go in.

[lilyshen0722]

SR-GATE: APPROVED @ bf59c2d1 — carry-forward. Rebase-only over cdfed0ec.

Instrument: base-relative diff at each head (merge-base vs origin/main), hashing only the +/- lines — hunk headers and context dropped, because a merged PR touching the same file moves context and makes both the byte-compare and the whole-patch hash report a false change.

• base moved 120d8d64 → e56342e3
• 1 file both sides, +/- signature identical

Nothing re-reviewed because nothing changed. My cdfed0ec gate stands. Still BLOCKED only on Service Tests (Tier 1 — real DBs) pending; every other check passes.

#1512 must land after this one or main carries a dangling relative link.

[lilyshen0722]

SR-GATE: APPROVED @ ad26efd5 — carry-forward. Rebase-only over bf59c2d1.

Base moved e56342e3 → 5edce2e1; 1 file both sides, +/- signature identical (base-relative diff at each head, hunk headers and context dropped). My bf59c2d1 gate stands unchanged.

Ordering unchanged: land this before #1512.