Skip to content

Commit 01ac8ea

Browse files
authored
feat(libtv-video): direct Feishu delivery via detached background waiter (#973)
* feat: harden libtv delivery notifications * docs: add libtv direct-key routing design * refactor(libtv-video): deliver directly to Feishu via detached waiter Replace the sessions_spawn + subagent delivery attempt (and the earlier HTTP-notify callback path) with a simple, stable design: create-session forks a detached wait-and-deliver process, and that process calls the Feishu file/message APIs directly on terminal success. Skill changes (apps/desktop/static/bundled-skills/libtv-video/): - scripts/libtv_video.py: - create-session takes --channel and --chat-id CLI args. The model extracts sender_id from the inbound Feishu metadata block and passes them explicitly, the same way deploy-skill's submit command does. - Persist only {channel, chat_id} in libtv-sessions.json. No more account_id / session_key / thread_id — those were the root cause of the earlier silent-delivery bug where stale account_id values made the controller gateway unable to route. - _spawn_background_waiter uses subprocess.Popen(start_new_session= True, stdin=DEVNULL, close_fds=True) to fork a detached worker that survives the parent's exit. Parent closes its copy of the log file descriptor in a try/finally to avoid fd leaks. - cmd_wait_and_deliver polls libtv until terminal, then shells out to feishu_send_video.py for each result URL. Persists a delivered_at idempotency marker so re-invocations on a delivered session are a safe no-op. - Transient network errors (RemoteDisconnected, ConnectionError, URLError, TimeoutError) in the poll loop are caught and retried on the next tick instead of crashing the waiter. - scripts/feishu_send_video.py (new file): - Copied from medeo-video's proven helper as the starting point. - Extended with _extract_permission_grant_url + _handle_upload_permission_error: if upload fails with Feishu's im:resource:upload scope error (code 99991672), fall back to a plain text message containing the video URL AND the grant URL so the user can enable native-media delivery for future runs. Uses the already-granted im:message scope. - On permission error with no chat_id available, emit an explicit diagnostic instead of silently falling through. - SKILL.md: - New delivery contract section explaining the background waiter pattern. - CRITICAL instruction that the model must extract sender_id from the inbound Feishu metadata block and pass it as --chat-id on every create-session invocation. Includes a concrete example. - Updated guard checklist to match the new contract. Controller cleanup (apps/controller/): - Delete POST /api/internal/libtv-notify, its request/response schemas, and the getBotIdFromSessionKey helper from src/routes/desktop-routes.ts. - Delete the whole apps/controller/tests/desktop-routes.test.ts file — it only contained tests for the removed route. - Delete tests/controller/desktop-routes.test.ts (re-export of above). - Regenerate openapi.json, sdk.gen.ts, types.gen.ts via pnpm generate-types. Tests (tests/desktop/libtv-video-skill.test.ts): - Full rewrite against the new contract. 6 tests cover: Seedance URL constant, submit-confirmation JSON shape, persisted delivery block, empty-delivery fallback, explicit video ratio override, sk-libtv- direct API routing, malformed submit rejection. All green. - Use LIBTV_SKIP_BACKGROUND_WAITER=1 to prevent tests from leaking detached processes into CI. E2E validation: - CLI smoke-tested against real libtv direct API with sk-libtv- key — real video generated and locally downloaded. - Live Feishu DM test with hot-swapped state-dir skill: the model read the updated SKILL.md, ran create-session with --channel feishu --chat-id ou_333..., the detached waiter polled libtv, invoked feishu_send_video.py, and delivered a native media video message to the user's Feishu chat. End-to-end round trip: ~3 seconds after create-session. Follow-ups (not in this commit): - Support additional channels by dropping a <channel>_send_video.py helper alongside feishu_send_video.py and adding a branch to _deliver_results. - Add a scripts/probe/feishu-send-probe.mjs as a reusable smoke probe for bisecting "skill vs channel" delivery failures. * chore(libtv-video): rename display name and surface image generation Rename the bundled skill's display name from "LibTV Video" to "LibTV - Image&Video(Seedance 2.0)" and update the description to make it clear the skill supports BOTH image and video generation, powered by Seedance 2.0. The skill directory (libtv-video) and ledger slug stay the same so persisted state, the compiled openclaw config, and the e2e delivery path from the previous commit remain valid. Updated surfaces: - SKILL.md frontmatter name + description - SKILL.md H1 heading - libtv_video.py module docstring - libtv_video.py ArgumentParser description * chore(libtv-video): update description and drop Seedance 2.0 prompt pin - Rewrite SKILL.md frontmatter description to match the approved copy: "Seedance 2.0 video & image generation via LibTV Gateway - AI text-to-video, image-to-video, video continuation, style transfer, and text-to-image using Seedance 2.0 model. Also supports Kling 3.0, Wan 2.6, Midjourney, Seedream 5.0. Trigger phrases: ..." This expands trigger coverage to image-generation verbs (draw, generate image, make an image) and enumerates the supported alternative models (Kling 3.0, Wan 2.6, Midjourney, Seedream 5.0). - Drop the hardcoded MODEL_HINT constant and _append_model_hint helper from libtv_video.py. The skill used to silently append ", please use Seedance 2.0" to every prompt that did not mention a model, which forced Seedance 2.0 even when the user was happy with the LibTV backend default or wanted another supported model. _build_session_message now only appends the video ratio hint. * feat(skillhub): always install latest libtv-video from bundle on boot Add replaceLibtvVideoFromBundle and call it from skillhub-service.initialize() after copyStaticSkills. Unconditionally wipes and re-copies the bundled libtv-video into the state dir on every controller startup, then upserts the managed ledger record to status: installed. Ships bundled libtv-video updates (including the detached-waiter + direct-Feishu-delivery refactor) to existing users on their next app boot. Bypasses copyStaticSkills on purpose — its knownSlugs check skips on any ledger source, which would silently fail if the user has a workspace or user-level libtv-video entry. The dedicated function only touches the state-dir copy under <openclawSkillsDir>/libtv-video/ and only the managed ledger record. User-scoped copies in ~/.agents/skills/ and per-agent copies under agents/<id>/skills/ are left untouched. Does not respect the managed record's uninstalled status — libtv-video is treated as a core bundled capability that always tracks the shipped version. New tests in apps/controller/tests/replace-libtv-video-from-bundle. test.ts (5 cases, real SkillDb on a temp ledger + real filesystem): - fresh-install - replaces stale content while keeping managed record installed - resurrects over an uninstalled managed record instead of honoring it - leaves workspace records for the same slug byte-identical - returns bundle-missing when bundled source dir is absent Updated the existing skillhub-service test mock to expose the new export so the 18 mocked-SkillDb cases still pass. * chore: gitignore docs/superpowers and remove stale design doc docs/superpowers/ is a working directory for agent-generated plans and design notes that should not be tracked in the repo. Add it to .gitignore. Remove the stale direct-key routing design doc committed in adf2ece — it describes an interim approach that has since been superseded by the detached background waiter + direct Feishu delivery design landed in 5fca9ad. * fix(libtv-video): exit non-zero when feishu send_video_message fails main() was ignoring the False return from send_video_message and exiting 0, which caused _deliver_feishu_video in libtv_video.py to treat the failed send as successful and set delivered_at — the user would never receive the video but the skill would record delivery as complete. Now sys.exit(1) on False so the caller correctly detects the failure and skips setting delivered_at.
1 parent a655585 commit 01ac8ea

10 files changed

Lines changed: 1951 additions & 135 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,4 @@ apps/desktop/build-config.json
3939
.claude/worktrees/
4040
.task/
4141
.opencode/
42+
docs/superpowers/

apps/controller/src/services/skillhub-service.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { existsSync } from "node:fs";
22
import type { ControllerEnv } from "../app/env.js";
33
import { CatalogManager } from "./skillhub/catalog-manager.js";
4-
import { copyStaticSkills } from "./skillhub/curated-skills.js";
4+
import {
5+
copyStaticSkills,
6+
replaceLibtvVideoFromBundle,
7+
} from "./skillhub/curated-skills.js";
58
import { InstallQueue } from "./skillhub/install-queue.js";
69
import { SkillDb } from "./skillhub/skill-db.js";
710
import { SkillDirWatcher } from "./skillhub/skill-dir-watcher.js";
@@ -163,6 +166,17 @@ export class SkillhubService {
163166
if (copied.length > 0) {
164167
this.db.recordBulkInstall(copied, "managed");
165168
}
169+
170+
// Step 1b: Force-refresh libtv-video on every boot so bundled
171+
// libtv-video updates (detached background waiter + direct
172+
// Feishu delivery) reach existing users on their next app boot.
173+
// copyStaticSkills' first-install-only semantics would otherwise
174+
// never refresh it. See replaceLibtvVideoFromBundle for rationale.
175+
replaceLibtvVideoFromBundle({
176+
staticDir: this.env.staticSkillsDir,
177+
targetDir: this.env.openclawSkillsDir,
178+
skillDb: this.db,
179+
});
166180
}
167181

168182
// Step 2: Enqueue curated skills from ClawHub that aren't on disk yet

apps/controller/src/services/skillhub/curated-skills.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
import { cpSync, existsSync, mkdirSync } from "node:fs";
1+
import { cpSync, existsSync, mkdirSync, rmSync } from "node:fs";
22
import { resolve } from "node:path";
33
import type { SkillDb } from "./skill-db.js";
44

5+
const LIBTV_VIDEO_SLUG = "libtv-video";
6+
57
/**
68
* Skills to install from ClawHub on first launch.
79
*/
@@ -99,6 +101,67 @@ export function copyStaticSkills(params: {
99101
return { copied, skipped };
100102
}
101103

104+
/**
105+
* Unconditionally install the latest bundled libtv-video into the state
106+
* dir on every controller startup. If a previous copy exists, it is
107+
* wiped and replaced; if not, a fresh copy is installed. The managed
108+
* ledger record is upserted via `recordInstall`, which also flips any
109+
* prior `uninstalled` status back to `installed` (intentional — see
110+
* below).
111+
*
112+
* Why this exists: `copyStaticSkills` only copies a static skill on
113+
* first install (both `destDir/SKILL.md` and `knownSlugs.has(slug)`
114+
* guards skip thereafter), so bundled libtv-video updates never reach
115+
* existing users on an app update. This function is how the libtv-video
116+
* refactor (detached background waiter + direct Feishu delivery via
117+
* `feishu_send_video.py`) ships to existing users on their next boot.
118+
*
119+
* Scope constraints:
120+
* - Only libtv-video. Other bundled static skills keep the existing
121+
* first-install-only semantics from `copyStaticSkills`.
122+
* - Only touches `<targetDir>/libtv-video/`. User-scoped copies under
123+
* `~/.agents/skills/libtv-video/` and per-agent workspace copies
124+
* under `<openclawStateDir>/agents/<agentId>/skills/libtv-video/`
125+
* are left alone — they represent explicit user choices under
126+
* different ledger sources.
127+
* - Only modifies the `source: "managed"` ledger record. Workspace /
128+
* user / custom records for the same slug are left untouched.
129+
* - Does NOT respect the managed record's uninstalled status —
130+
* libtv-video is treated as a core bundled capability that always
131+
* tracks the shipped version. `recordInstall` upserts any prior
132+
* `uninstalled` record back to `installed`.
133+
*
134+
* Why a dedicated function instead of reusing `copyStaticSkills`: its
135+
* `knownSlugs.has(slug)` guard skips on any ledger source, so even if
136+
* we removed the `managed` record beforehand, any stray workspace or
137+
* user record for libtv-video would still cause a silent skip. This
138+
* function bypasses that check deterministically.
139+
*/
140+
export function replaceLibtvVideoFromBundle(params: {
141+
staticDir: string;
142+
targetDir: string;
143+
skillDb: SkillDb;
144+
}): {
145+
installed: boolean;
146+
reason: "bundle-missing" | "fresh-install" | "replaced";
147+
} {
148+
const srcDir = resolve(params.staticDir, LIBTV_VIDEO_SLUG);
149+
if (!existsSync(srcDir)) {
150+
return { installed: false, reason: "bundle-missing" };
151+
}
152+
153+
const destDir = resolve(params.targetDir, LIBTV_VIDEO_SLUG);
154+
const existed = existsSync(destDir);
155+
if (existed) {
156+
rmSync(destDir, { recursive: true, force: true });
157+
}
158+
mkdirSync(destDir, { recursive: true });
159+
cpSync(srcDir, destDir, { recursive: true });
160+
params.skillDb.recordInstall(LIBTV_VIDEO_SLUG, "managed");
161+
162+
return { installed: true, reason: existed ? "replaced" : "fresh-install" };
163+
}
164+
102165
export type CuratedInstallResult = {
103166
installed: string[];
104167
skipped: string[];
Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
2+
import { mkdtemp, rm } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { resolve } from "node:path";
5+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
6+
import { replaceLibtvVideoFromBundle } from "../src/services/skillhub/curated-skills.js";
7+
import { SkillDb } from "../src/services/skillhub/skill-db.js";
8+
9+
/**
10+
* Seed a fake bundled libtv-video source directory at <root>/bundle/libtv-video/
11+
* containing a minimal SKILL.md and one script file. Content is deterministic
12+
* so tests can assert exact equality after the copy.
13+
*/
14+
function seedBundle(bundleRoot: string, scriptContent: string): string {
15+
const srcDir = resolve(bundleRoot, "libtv-video");
16+
mkdirSync(resolve(srcDir, "scripts"), { recursive: true });
17+
writeFileSync(
18+
resolve(srcDir, "SKILL.md"),
19+
[
20+
"---",
21+
"name: libtv-video",
22+
"description: bundled test fixture",
23+
"---",
24+
"",
25+
"# LibTV Video (test fixture)",
26+
"",
27+
].join("\n"),
28+
"utf8",
29+
);
30+
writeFileSync(
31+
resolve(srcDir, "scripts", "libtv_video.py"),
32+
scriptContent,
33+
"utf8",
34+
);
35+
return srcDir;
36+
}
37+
38+
describe("replaceLibtvVideoFromBundle", () => {
39+
let workspaceRoot: string;
40+
let bundleRoot: string;
41+
let targetRoot: string;
42+
let ledgerPath: string;
43+
44+
beforeEach(async () => {
45+
workspaceRoot = await mkdtemp(resolve(tmpdir(), "nexu-libtv-refresh-"));
46+
bundleRoot = resolve(workspaceRoot, "bundle");
47+
targetRoot = resolve(workspaceRoot, "state-skills");
48+
ledgerPath = resolve(workspaceRoot, "skill-ledger.json");
49+
mkdirSync(bundleRoot, { recursive: true });
50+
mkdirSync(targetRoot, { recursive: true });
51+
});
52+
53+
afterEach(async () => {
54+
await rm(workspaceRoot, { recursive: true, force: true });
55+
});
56+
57+
it("installs fresh when the state dir is empty and the ledger has no record", async () => {
58+
seedBundle(bundleRoot, "# fresh bundle v1\n");
59+
const db = await SkillDb.create(ledgerPath);
60+
61+
const result = replaceLibtvVideoFromBundle({
62+
staticDir: bundleRoot,
63+
targetDir: targetRoot,
64+
skillDb: db,
65+
});
66+
67+
expect(result).toEqual({ installed: true, reason: "fresh-install" });
68+
69+
const destSkillMd = resolve(targetRoot, "libtv-video", "SKILL.md");
70+
expect(existsSync(destSkillMd)).toBe(true);
71+
expect(readFileSync(destSkillMd, "utf8")).toContain("name: libtv-video");
72+
73+
const destScript = resolve(
74+
targetRoot,
75+
"libtv-video",
76+
"scripts",
77+
"libtv_video.py",
78+
);
79+
expect(readFileSync(destScript, "utf8")).toBe("# fresh bundle v1\n");
80+
81+
const installed = db.getAllInstalled();
82+
const libtvRecord = installed.find((r) => r.slug === "libtv-video");
83+
expect(libtvRecord).toBeDefined();
84+
expect(libtvRecord?.source).toBe("managed");
85+
expect(libtvRecord?.status).toBe("installed");
86+
});
87+
88+
it("replaces stale state-dir content and keeps the managed record installed", async () => {
89+
// First install with bundle v1
90+
seedBundle(bundleRoot, "# bundle v1\n");
91+
const db = await SkillDb.create(ledgerPath);
92+
replaceLibtvVideoFromBundle({
93+
staticDir: bundleRoot,
94+
targetDir: targetRoot,
95+
skillDb: db,
96+
});
97+
98+
// Simulate a bundled update: v2 content in the source dir.
99+
seedBundle(bundleRoot, "# bundle v2 — refactored\n");
100+
101+
const result = replaceLibtvVideoFromBundle({
102+
staticDir: bundleRoot,
103+
targetDir: targetRoot,
104+
skillDb: db,
105+
});
106+
107+
expect(result).toEqual({ installed: true, reason: "replaced" });
108+
109+
const destScript = resolve(
110+
targetRoot,
111+
"libtv-video",
112+
"scripts",
113+
"libtv_video.py",
114+
);
115+
expect(readFileSync(destScript, "utf8")).toBe("# bundle v2 — refactored\n");
116+
117+
const libtvRecord = db
118+
.getAllInstalled()
119+
.find((r) => r.slug === "libtv-video");
120+
expect(libtvRecord?.status).toBe("installed");
121+
expect(libtvRecord?.source).toBe("managed");
122+
});
123+
124+
it("resurrects over an uninstalled managed record instead of honoring it", async () => {
125+
seedBundle(bundleRoot, "# bundle resurrect\n");
126+
127+
// Pre-seed the ledger file with an uninstalled managed record so the
128+
// newly-created SkillDb parses it through the schema at load time.
129+
const seededLedger = {
130+
skills: [
131+
{
132+
slug: "libtv-video",
133+
source: "managed",
134+
status: "uninstalled",
135+
version: null,
136+
installedAt: "2024-01-01T00:00:00.000Z",
137+
uninstalledAt: "2024-06-01T00:00:00.000Z",
138+
agentId: null,
139+
},
140+
],
141+
};
142+
writeFileSync(ledgerPath, JSON.stringify(seededLedger, null, 2), "utf8");
143+
144+
const db = await SkillDb.create(ledgerPath);
145+
146+
const result = replaceLibtvVideoFromBundle({
147+
staticDir: bundleRoot,
148+
targetDir: targetRoot,
149+
skillDb: db,
150+
});
151+
152+
expect(result).toEqual({ installed: true, reason: "fresh-install" });
153+
154+
// State dir has the skill and the ledger record is flipped back to
155+
// installed — libtv-video is treated as a core bundled capability
156+
// that always tracks the shipped version, so uninstall is NOT
157+
// honored.
158+
const destSkillMd = resolve(targetRoot, "libtv-video", "SKILL.md");
159+
expect(existsSync(destSkillMd)).toBe(true);
160+
161+
const libtvRecord = db
162+
.getAllInstalled()
163+
.find((r) => r.slug === "libtv-video");
164+
expect(libtvRecord?.status).toBe("installed");
165+
expect(libtvRecord?.source).toBe("managed");
166+
});
167+
168+
it("leaves workspace records for the same slug untouched", async () => {
169+
seedBundle(bundleRoot, "# bundle coexistence\n");
170+
171+
// Seed ledger with BOTH a managed libtv-video and a workspace-scoped
172+
// libtv-video under a specific agent. The refresh must only modify
173+
// the managed record.
174+
const seededLedger = {
175+
skills: [
176+
{
177+
slug: "libtv-video",
178+
source: "managed",
179+
status: "installed",
180+
version: null,
181+
installedAt: "2024-01-01T00:00:00.000Z",
182+
uninstalledAt: null,
183+
agentId: null,
184+
},
185+
{
186+
slug: "libtv-video",
187+
source: "workspace",
188+
status: "installed",
189+
version: "user-local-v0.1",
190+
installedAt: "2024-02-01T00:00:00.000Z",
191+
uninstalledAt: null,
192+
agentId: "agent-xyz",
193+
},
194+
],
195+
};
196+
writeFileSync(ledgerPath, JSON.stringify(seededLedger, null, 2), "utf8");
197+
198+
const db = await SkillDb.create(ledgerPath);
199+
200+
replaceLibtvVideoFromBundle({
201+
staticDir: bundleRoot,
202+
targetDir: targetRoot,
203+
skillDb: db,
204+
});
205+
206+
const installed = db.getAllInstalled();
207+
const managedRecord = installed.find(
208+
(r) => r.slug === "libtv-video" && r.source === "managed",
209+
);
210+
const workspaceRecord = installed.find(
211+
(r) => r.slug === "libtv-video" && r.source === "workspace",
212+
);
213+
214+
// Managed record is still present and installed.
215+
expect(managedRecord?.status).toBe("installed");
216+
217+
// Workspace record is byte-identical to the seeded one — not
218+
// clobbered by the refresh.
219+
expect(workspaceRecord).toBeDefined();
220+
expect(workspaceRecord?.agentId).toBe("agent-xyz");
221+
expect(workspaceRecord?.status).toBe("installed");
222+
expect(workspaceRecord?.version).toBe("user-local-v0.1");
223+
expect(workspaceRecord?.installedAt).toBe("2024-02-01T00:00:00.000Z");
224+
});
225+
226+
it("returns bundle-missing when the bundled source directory is absent", async () => {
227+
// Intentionally do NOT seed the bundle.
228+
const db = await SkillDb.create(ledgerPath);
229+
230+
const result = replaceLibtvVideoFromBundle({
231+
staticDir: bundleRoot,
232+
targetDir: targetRoot,
233+
skillDb: db,
234+
});
235+
236+
expect(result).toEqual({ installed: false, reason: "bundle-missing" });
237+
expect(existsSync(resolve(targetRoot, "libtv-video"))).toBe(false);
238+
expect(db.getAllInstalled()).toHaveLength(0);
239+
});
240+
});

apps/controller/tests/skillhub-service.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ const mocks = vi.hoisted(() => {
131131
skipped: [] as string[],
132132
}));
133133

134+
const mockReplaceLibtvVideoFromBundle = vi.fn(() => ({
135+
installed: false as boolean,
136+
reason: "bundle-missing" as "bundle-missing" | "fresh-install" | "replaced",
137+
}));
138+
134139
return {
135140
mockSkillDbCreate,
136141
catalogManagerInstances,
@@ -140,6 +145,7 @@ const mocks = vi.hoisted(() => {
140145
dirWatcherInstances,
141146
MockSkillDirWatcher,
142147
mockCopyStaticSkills,
148+
mockReplaceLibtvVideoFromBundle,
143149
};
144150
});
145151

@@ -163,6 +169,7 @@ vi.mock("../src/services/skillhub/skill-dir-watcher.js", () => ({
163169

164170
vi.mock("../src/services/skillhub/curated-skills.js", () => ({
165171
copyStaticSkills: mocks.mockCopyStaticSkills,
172+
replaceLibtvVideoFromBundle: mocks.mockReplaceLibtvVideoFromBundle,
166173
}));
167174

168175
import { SkillhubService } from "../src/services/skillhub-service.js";

0 commit comments

Comments
 (0)