From 9222f374bf749eee51798211fb7e60c26908f999 Mon Sep 17 00:00:00 2001 From: Bruno Bornsztein Date: Thu, 13 Aug 2026 15:55:42 -0500 Subject: [PATCH] Spec review: peel server integrations into ty plugins (Slack first) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviews the "peel taskyou-os server integrations into ty plugins" spec against the shipped ty 0.3.21 binary and this repo's code. Verdict: ship v1 as scoped. Three corrections, four additions. Corrections: - linear-poll cannot become a ty routine — routines are unattended *agent* runs (prompt.md), and linear-poll is 347 lines of deterministic GraphQL. Converting it is a rewrite into 720 metered LLM runs/day/GM. - `ty plugins remove` already exists; the v2 gate is pinning + disable only. - The warn-and-continue failure mode is a two-line fix, not a reason to migrate. Additions: - v1 must disable and delete ty-slack.service first, or existing GMs run two bridges and post every Slack notification twice. - `command: node ./slack-bridge.mjs` won't find node under the daemon's minimal PATH; symlink the resolved node into ~/bin. - Moving from systemd sibling to daemon child is a real coupling trade, acceptable only because the bridge is verifiably restart-safe. - .env and .slack-state.json inside the code dir block v2's git-pull model. Also answers the five open decisions and reorders the three ty supervisor gates to status -> logs -> restart. Co-Authored-By: Claude Opus 5 (1M context) --- ...-peel-integrations-to-ty-plugins-design.md | 313 ++++++++++++++++++ 1 file changed, 313 insertions(+) create mode 100644 docs/plans/2026-08-13-peel-integrations-to-ty-plugins-design.md diff --git a/docs/plans/2026-08-13-peel-integrations-to-ty-plugins-design.md b/docs/plans/2026-08-13-peel-integrations-to-ty-plugins-design.md new file mode 100644 index 0000000..4b339f5 --- /dev/null +++ b/docs/plans/2026-08-13-peel-integrations-to-ty-plugins-design.md @@ -0,0 +1,313 @@ +# Peel taskyou-os server integrations into ty plugins — spec review & revised plan + +**Date:** 2026-08-13 +**Reviews:** the "peel taskyou-os server integrations into ty plugins (start with Slack)" spec +**Verdict:** **Ship v1 as scoped.** The core thesis is right and the gating is honest. Three +corrections and four additions below; one of them (§3.1) is a duplicate-message bug in the v1 +plan as written. + +Everything here is checked against the code in this repo and against the **installed `ty` +binary (0.3.21)**, not against the spec's description of ty. + +--- + +## 1. What the review confirmed + +The spec's factual claims about ty hold up. From `ty 0.3.21`: + +| Spec claim | Status | +|---|---| +| `services:` capability exists | ✅ Binary contains `Started plugin services`, `plugin service started`, `plugin service stopping`, `plugin service failed to start` | +| `TY_DB_PATH` / `TY_API_URL` injected | ✅ Both present | +| Supervisor has **no log capture** | ✅ No `service.log` / `service exited` strings | +| Supervisor has **no restart/backoff** | ✅ No `backoff` / `restarting` strings | +| `ty plugins add` is unpinned clone + `git pull` | ✅ `add --help`: "Re-running add on the same plugin updates it in place with git pull". No `--ref`. | +| `setup.sh` has no min-ty-version gate | ✅ `curl -fsSL https://taskyou.dev/install.sh \| bash` (L746, L971) | +| `ty routines schedule --every 2m` exists | ✅ `--every duration` / `--cron` → launchd plist or tagged crontab line | + +And in this repo: + +- `setup_slack_remote` (L387–441) is 55 lines ending in **warn-and-continue** on + `systemctl --user enable` failure (L432–433). A silently dead bridge is a supported outcome — + as claimed. +- `setup_slack_local` (L446–474) ends at "start it with `node …` (or a launchd agent)" — local + mode is genuinely unsupervised. +- The bridge **is restart-safe**, which the spec doesn't state but which the whole plan depends + on: `.slack-state.json` persists `notifyOffset` and the thread↔task maps, the offset advances + *before* posting (L655: "so a failed post never re-storms"), and a lost state file resets the + offset to EOF rather than replaying history (L634). Moving to a supervisor that restarts more + often is therefore safe. Good. + +**So: the three v1 gates are real and correctly identified.** Nothing below undermines the plan; +it sharpens it. + +--- + +## 2. Corrections + +### 2.1 `linear-poll` is not a routine. Drop the "Periodic jobs = routines" section. + +This is the one materially wrong claim in the spec. + +`ty routines --help`, verbatim: + +> Routines are named, unattended **agent runs**: a `prompt.md` (plus optional `env.sh` for +> secrets/fail-fast checks) in `~/.config/task/routines//`. + +A routine is an LLM agent run. `linear-poll.mjs` is 347 lines of deterministic GraphQL: a +cursor (`lastChecked`), a dedup set (`processedCommentIds`), a `pendingTasks` queue, and +`execSync` calls to `ty` and the `linear` CLI. Those are not the same primitive. + +The spec says converting it would be "not a rewrite." It is exactly a rewrite, and into a worse +shape: + +- **Cost.** `--every 2m` = **720 agent runs per day, per GM**. Bruno runs several GMs. That is a + real recurring bill to replace a free `node` process. +- **Determinism.** A cursor + a seen-set becomes "an agent decides which comments it already + handled." The failure mode is duplicate Linear replies and duplicate ty tasks — silent, and + charged for. +- **Latency/timeout.** Default 30m timeout on a job that runs every 2m. + +The spec's own "What does NOT move" section already reaches the right answer ("`linear-poll` — it's +a cron job… Defer"). The routines section then contradicts it by supplying a migration path. +Delete the section. If you later want ty to own linear-poll, the honest ask is the +**`timers:`/scheduled-command capability the spec ruled out** — a scheduled *command*, not a +scheduled *agent*. That is a separate decision, and not one v1 needs. + +Note the secondary irony: `ty routines schedule` explicitly **"captures your current PATH so the +agent can find ty, claude"**. Routines solved the PATH problem. Plugin services have not — see +§3.2. + +### 2.2 `ty plugins remove` already exists — the v2 gate is smaller than stated + +The spec gates v2 on the CLI growing "**ref pinning** and **remove/disable**." `remove` shipped: +`ty plugins remove` — "Uninstall a plugin by deleting its directory." The outstanding v2 gates are +**`--ref` pinning** and **per-plugin disable**. Doesn't change the recommendation to defer v2, but +it's one less thing to build. + +### 2.3 "Deletes the worst code in setup.sh" oversells the migration + +The worst behavior — warn-and-continue on a failed bridge install — is a **two-line fix you can +ship today**, independent of any of this. Don't let a multi-release migration be the vehicle for +it. Fix the failure mode now (§5, step 0); migrate for the reasons that actually require +migrating: one supervision model, local == remote, no lingering. + +Ranking the stated justifications by how much weight they can actually bear: + +- **Strong:** local mode gets supervision for free; one supervisor instead of systemd-user + + lingering; no `loginctl enable-linger` dependency. +- **Medium:** best available dogfood for `services:` — you will find the supervisor's gaps, which + is worth something on its own. +- **Weak (defer-dependent):** "makes the integration reusable & reviewable" — that's v2, which + the spec correctly defers. It can't justify v1. + +--- + +## 3. Additions — things v1 needs that the spec doesn't have + +### 3.1 The migration must remove the old systemd unit, or existing GMs post everything twice + +**This is a bug in the v1 plan as written.** Every GM already running has +`~/.config/systemd/user/ty-slack.service` enabled and a bridge at `~/scripts/slack/` with its own +`.slack-state.json`. v1 installs a *second* bridge under `~/.config/task/plugins/slack/` with a +*separate* state file. Result: + +- Both tail `~/notifications.jsonl` from independent offsets → **every task notification posts to + Slack twice.** +- Both open Socket Mode connections for the same app → Slack load-balances inbound events across + them, so `@taskyou` commands land in whichever bridge, and the thread↔task map splits across + two state files. Thread replies start going to the wrong place. + +v1 must, before installing the plugin: + +```sh +systemctl --user disable --now ty-slack 2>/dev/null || true +rm -f ~/.config/systemd/user/ty-slack.service +systemctl --user daemon-reload +``` + +…and should carry `~/scripts/slack/.slack-state.json` over to the plugin dir (or accept one +offset reset — which is safe, it resets to EOF, but loses the thread map). Local mode needs the +equivalent: the operator may have a hand-rolled launchd agent or a bare `node` in a tmux pane; +setup.sh can't kill that, so **print an explicit "stop your existing bridge" instruction** in +local mode. + +### 3.2 `command: node ./slack-bridge.mjs` will fail to find node on the server + +`setup_slack_remote` resolves node deliberately: + +```sh +node_bin=$(ssh "$ssh_target" "bash -lc 'command -v node'") # L421 +``` + +…and bakes the absolute path into `ExecStart` (`{{NODE_BIN}}`), with the comment "so systemd's +minimal PATH still finds it." That workaround exists because it was needed. + +`ty-daemon.service` sets: + +``` +Environment="PATH=%h/bin:%h/.local/bin:%h/.npm-global/bin:/usr/local/bin:/usr/bin:/bin" +``` + +That does **not** include the asdf node path that setup.sh's own `remote_with_path` relies on +(`/home/deploy/.asdf/installs/nodejs/24.13.0/bin`, L167). A plugin service spawned by the daemon +inherits the daemon's environment. So on an asdf box, `command: node ./slack-bridge.mjs` → +`plugin service failed to start` → and with today's supervisor, **no log line saying why**. +That's the exact silent-death failure v1 is supposed to eliminate, reintroduced through a +different door. + +Same applies to `CLAUDE_BIN` (defaults to bare `claude`, used by the classifier) and `TY_PATH` +(the .env sets it absolute — fine). + +**Cheapest fix, no ty change and `plugin.yaml` stays a static file:** `%h/bin` *is* on the +daemon's PATH, so have setup.sh symlink the resolved node into it: + +```sh +ssh "$ssh_target" "mkdir -p $remote_home/bin && ln -sf '$node_bin' $remote_home/bin/node" +``` + +Alternatives, both worse for v1: make `plugin.yaml` a `.tmpl` rendered with `{{NODE_BIN}}` (kills +the "scp the checkout verbatim" property, and means the plugin dir can never become a real repo), +or ask ty to expand `.env` vars in `command:` (a new ty feature on the critical path). + +**Add this as gate #0.** It's a setup.sh-side fix, so it doesn't block on a ty release — but v1 is +broken without it. + +### 3.3 Supervision coupling is a real trade, not a pure win + +Today `ty-slack.service` is a **sibling** of `ty-daemon.service` — `After=` + `Wants=`, not +`PartOf=`. The bridge survives daemon restarts and vice versa. Under `services:` the bridge becomes +a **child** of the daemon: every daemon restart drops the Socket Mode WebSocket and the +notification tail. And restart frequency goes *up*, because setup.sh re-runs restart the daemon and +`curl | bash` pulls latest ty. + +You are also stacking two supervisors (systemd supervises the daemon; the daemon supervises the +bridge) where you had one battle-tested one. + +This is **acceptable** — the bridge is restart-safe (§1), so the cost is seconds of downtime, not +duplicates — but it should be written down as the price rather than framed as free. One residual +gap worth a line in the code: `seenEvents` (Slack retry dedup, L455) is **in-memory only**. More +restarts ⇒ slightly higher odds that a Slack event retry straddling a restart gets handled twice +(e.g. a duplicate task created from one `@taskyou` message). Low frequency, but persisting the +last N event IDs into `.slack-state.json` is ~5 lines and removes the class. + +### 3.4 Config and state live *inside* the code directory — fine for v1, disqualifying for v2 + +Both `.env` and `.slack-state.json` sit next to `slack-bridge.mjs` (`join(__dirname, …)`). Under +v1's scp drop-in that's fine — scp doesn't delete, so both survive re-runs. + +Under **v2** (`ty plugins add` = git clone, update = `git pull`) the plugin dir is a git working +tree, and putting untracked secrets + mutable state inside it is fragile: `git pull` can conflict +if the upstream repo ever adds those paths, `git clean` wipes both, and the secret is one +`git status` away from a screenshot. + +This answers Open Decision #2 cleanly: **bless `.env`-in-dir as the v1 convention (zero work, +both modules already do it), and make "config + state outside the checkout" a hard prerequisite of +v2, not a nice-to-have.** Deferring costs nothing because v1 never `git pull`s. + +--- + +## 4. Answers to the five open decisions + +**1. Supervisor hardening before v-next, or dual-path?** +**Block on the ty release. Do not build dual-path.** Dual-path means keeping +`ty-slack.service.tmpl`, the lingering dance, *and* the plugin path, plus a version probe to +choose — i.e. you carry the code you're migrating to delete, and you double the QA matrix in +`qa/run-qa.sh` forever. The gates are small and they're yours to land. Ship step 0 (§5) so the +current path stops failing silently in the meantime; that removes the urgency that would otherwise +argue for dual-path. + +**2. Secrets contract: `.env` in the plugin dir, or `ty plugins config`?** +**`.env` in the dir for v1. `ty plugins config` (or any out-of-checkout store) as a v2 gate** — +see §3.4. Add `.env` and `.slack-state.json` to a `.gitignore` inside `modules/slack/` now, so the +directory is already safe to become a repo later. + +**3. Is "restart the daemon" acceptable for a token change?** +**Yes for v1.** Token rotation is rare (measured in months), and the daemon restart is the same +blast radius you already accept on every `setup.sh` re-run. Executors are tracked in the DB, not in +daemon memory. Don't put `ty plugins restart ` on the critical path — but do put it in v1.1, +because the moment the bridge dies you'll want to bounce *it* rather than the box's brain. + +**4. Min-ty-version gate in setup.sh.** +**Do both, in this order: a numeric `ty --version` fail-fast, then an outcome check.** The version +compare is the guard (fail loudly with "Slack needs ty ≥ X.Y.Z; run `curl … install.sh | bash`"), +and the outcome check is what actually proves it worked — mirroring what setup.sh does today with +`systemctl --user is-active`: + +```sh +ty plugins list | grep -q 'slack.*running' || die "slack service did not start; check " +``` + +That outcome check is precisely why **gate #3 (status in `ty plugins list`) is the most valuable of +the three gates, not the least** — it's the only one setup.sh can act on. Reorder the gates: +**status → log capture → restart/backoff.** Status unblocks setup.sh, log capture makes failures +diagnosable, restart is the smallest real-world win because a bridge that crashes at init (rotated +token) will just crash-loop anyway — what you actually need there is the log. + +**5. Collection semantics: per-plugin opt-in for `services:` in v2?** +**Yes.** A hook runs on an event you already triggered; a service is a process the daemon starts on +its own, holding `TY_DB_PATH` and API access, from an unpinned default-branch HEAD. That's a +different trust bar, and "installing a collection silently started five processes" is the kind of +thing you only get to discover once. Make it explicit opt-in at install time. But note this is a +**v2 question and v1 must not wait on it** — v1 is a local directory copy, not an install from +anywhere. + +--- + +## 5. Revised sequence + +**Step 0 — ship this week, independent of everything (≈1 hour).** +Make `setup_slack_remote` fail loudly instead of warn-and-continue (L432–439). This is the actual +worst failure mode in the repo and it does not need a migration to fix. Also add +`modules/slack/.gitignore` for `.env` + `.slack-state.json` (§3.4) and persist `seenEvents` into +the state file (§3.3). + +**Step 1 — ty supervisor gates, in this order (§4.4).** +1. Status + pid in `ty plugins list` — unblocks setup.sh's outcome check. +2. stdout/stderr → `~/.config/task/plugins//service.log`. +3. Restart-on-exit with backoff. +Cut a ty release. Note its version — that's the number `setup.sh` gates on. + +**Step 2 — taskyou-os v1: Slack as an in-repo drop-in plugin.** +As specced, plus the three additions: +- `modules/slack/plugin.yaml` (static, not a template). +- setup.sh: `ln -sf $node_bin ~/bin/node` **before** the daemon restart (§3.2). +- setup.sh: disable + delete `ty-slack.service`, migrate `.slack-state.json`, and print a + stop-your-old-bridge notice in local mode (§3.1). +- setup.sh: `ty --version` fail-fast + `ty plugins list` outcome check (§4.4). +- Delete `templates/ty-slack.service.tmpl`, the lingering dance, and the local-mode + "start it yourself" warning. +- Extend `qa/run-qa.sh` to cover: fresh install, **upgrade from a GM that already has + `ty-slack.service`** (the §3.1 case — this is the one that can regress live GMs), and local mode. + +**Step 3 — v1.1.** `ty plugins restart `. + +**Step 4 — v2 gates.** `ty plugins add --ref`, per-plugin `disable`, out-of-checkout config/state, +per-plugin opt-in for `services:` in collection installs. *Then* extract to `taskyou/plugins`. + +**Step 5 — `notifications.jsonl` → plugin `hooks:` / SSE.** Separately. The spec's +"one variable at a time" instinct is right; hold it. + +**Dropped from the sequence:** the Linear/routines migration (§2.1). `linear-poll` stays a cron +job. Revisit only if ty grows a scheduled-*command* primitive, and treat that as its own decision. + +--- + +## 6. One-paragraph summary + +The spec is sound and the gating is honest — ship it. Three corrections: `linear-poll` cannot +become a routine (routines are *agent* runs; converting a 347-line deterministic poller into a +prompt firing 720×/day per GM is a rewrite into a worse, metered shape — drop that section), +`ty plugins remove` already exists so the v2 gate is just pinning + disable, and the "deletes the +worst code" argument oversells a two-line fix you should ship this week regardless. Three +additions v1 needs: **it must disable and delete the old `ty-slack.service`, or every existing GM +runs two bridges and posts every notification to Slack twice**; `command: node ./slack-bridge.mjs` +will not find node under the daemon's minimal PATH (symlink the resolved node into `~/bin`, which +is already on that PATH); and moving the bridge from systemd sibling to daemon child is a real +coupling trade — acceptable only because the bridge is verifiably restart-safe. On the open +decisions: block on the ty release rather than building dual-path, keep `.env`-in-dir for v1 but +make out-of-checkout config a hard v2 gate, accept daemon-restart for token changes, gate on +`ty --version` *and* a `ty plugins list` outcome check, and yes — require per-plugin opt-in for +`services:` in v2 collection installs. Reorder the three ty gates to **status → logs → restart**: +status is what `setup.sh` can actually act on, and a crash-at-init bridge needs a log more than it +needs a retry.