diff --git a/Cargo.lock b/Cargo.lock index 28e94c47..aa44a6a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -154,6 +154,7 @@ dependencies = [ "nix", "rand 0.8.6", "reqwest", + "schemars", "semver", "serde", "serde_json", diff --git a/apps/extension/src/content/__tests__/record-capture.test.ts b/apps/extension/src/content/__tests__/record-capture.test.ts index 30108d8c..77aee7dd 100644 --- a/apps/extension/src/content/__tests__/record-capture.test.ts +++ b/apps/extension/src/content/__tests__/record-capture.test.ts @@ -1517,4 +1517,59 @@ describe("record-capture semantic", () => { expect(steps.map((s) => s.op)).toEqual(["click"]); }); + it("records a click on a control whose page happens to contain a search box", () => { + // MediaWiki Vector ships ``, which + // `closest('[class*="search"]')` used to match for every click on the page. + document.body.className = "skin-vector skin-vector-search-vue"; + document.body.innerHTML = ` +
+ + `; + const capture = startRecordCapture("rec-search-body", (step) => steps.push(step)); + click(document.querySelector("span")!); + capture.dispose(); + document.body.className = ""; + + expect(steps).toEqual([ + expect.objectContaining({ + op: "click", + target: expect.objectContaining({ name: "\u9690\u85cf\u76ee\u5f55" }), + }), + ]); + }); + + it("records a link click inside a form that also holds a search box", () => { + document.body.innerHTML = ` +
+ + Help +
+ `; + const capture = startRecordCapture("rec-search-form", (step) => steps.push(step)); + click(document.querySelector("a")!); + capture.dispose(); + + expect(steps).toEqual([ + expect.objectContaining({ op: "click", target: expect.objectContaining({ name: "Help" }) }), + ]); + }); + + it("still redirects a click on bare search chrome into a fill session", () => { + document.body.innerHTML = ` + + `; + const capture = startRecordCapture("rec-search-chrome", (step) => steps.push(step)); + const icon = document.querySelector(".icon")!; + click(icon); + const input = document.querySelector("input")!; + input.value = "hello"; + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + capture.dispose(); + + expect(steps.map((s) => s.op)).toEqual(["fill"]); + }); }); diff --git a/apps/extension/src/content/__tests__/record-frame-agent.test.ts b/apps/extension/src/content/__tests__/record-frame-agent.test.ts index 7c55de3d..27c2bd7a 100644 --- a/apps/extension/src/content/__tests__/record-frame-agent.test.ts +++ b/apps/extension/src/content/__tests__/record-frame-agent.test.ts @@ -2,7 +2,14 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { RecordFramePortMessage } from "@/lib/recording/frame-bridge"; import { RECORD_FRAME_START } from "@/lib/recording/frame-bridge"; import { RECORD_DOCUMENT_ATTRIBUTE } from "@/shared/recording-document-identity"; -import { RecordFrameAgent } from "../recording/frame-agent"; +import { attachRecordFrameAgent, RecordFrameAgent } from "../recording/frame-agent"; + +/** happy-dom's PageTransitionEvent drops `persisted`, so set it directly. */ +function pageShowEvent(persisted: boolean): Event { + const event = new Event("pageshow"); + Object.defineProperty(event, "persisted", { value: persisted }); + return event; +} class PortListeners unknown> { readonly values = new Set(); @@ -37,6 +44,7 @@ function portHarness() { return { port, outbound, + disconnectListeners: onDisconnect.values, receive(message: RecordFramePortMessage) { for (const listener of onMessage.values) listener(message); }, @@ -121,4 +129,54 @@ describe("RecordFrameAgent", () => { expect(sendMessage).toHaveBeenCalledTimes(3); expect(document.documentElement.hasAttribute(RECORD_DOCUMENT_ATTRIBUTE)).toBe(false); }); + it("re-arms the restored Document after a back/forward-cache pageshow", async () => { + const harness = portHarness(); + const connect = vi.fn(() => harness.port); + const sendMessage = vi.fn(async (message: { type?: string }) => + message.type === "bsk-record-frame-query" + ? { active: true, requestId: "rec-bfcache", startedAtMs: 10 } + : undefined, + ); + vi.stubGlobal("chrome", { + runtime: { connect, sendMessage, onMessage: new PortListeners() }, + }); + + const dispose = attachRecordFrameAgent(); + await vi.waitFor(() => + expect(document.documentElement.hasAttribute(RECORD_DOCUMENT_ATTRIBUTE)).toBe(true), + ); + expect(connect).toHaveBeenCalledTimes(1); + + // Entering the cache drops the port, which tears capture down here. + for (const listener of harness.disconnectListeners) listener(); + expect(document.documentElement.hasAttribute(RECORD_DOCUMENT_ATTRIBUTE)).toBe(false); + + window.dispatchEvent(pageShowEvent(true)); + await vi.waitFor(() => expect(connect).toHaveBeenCalledTimes(2)); + expect(document.documentElement.hasAttribute(RECORD_DOCUMENT_ATTRIBUTE)).toBe(true); + + dispose(); + }); + + it("ignores a pageshow that is not a cache restore", async () => { + const harness = portHarness(); + const connect = vi.fn(() => harness.port); + const sendMessage = vi.fn(async () => ({ + active: true, + requestId: "rec-plain", + startedAtMs: 10, + })); + vi.stubGlobal("chrome", { + runtime: { connect, sendMessage, onMessage: new PortListeners() }, + }); + + const dispose = attachRecordFrameAgent(); + await vi.waitFor(() => expect(connect).toHaveBeenCalledTimes(1)); + + window.dispatchEvent(pageShowEvent(false)); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(connect).toHaveBeenCalledTimes(1); + + dispose(); + }); }); diff --git a/apps/extension/src/content/record-capture.ts b/apps/extension/src/content/record-capture.ts index 22f7001b..7b77f861 100644 --- a/apps/extension/src/content/record-capture.ts +++ b/apps/extension/src/content/record-capture.ts @@ -102,12 +102,56 @@ function fillableFromTarget(target: EventTarget | null): FillableElement | null return null; } +const SEARCH_CHROME_SELECTOR = + '[id*="chat-input"], [id*="search"], [class*="search"], form, [role="search"]'; + +/** + * A control the user can name is its own step, so it must never be rewritten + * into "focus the search box next to it". + */ +const SEARCH_CHROME_CONTROL_SELECTOR = [ + "a[href]", + "button", + "select", + "textarea", + "input", + "summary", + '[role="button"]', + '[role="link"]', + '[role="tab"]', + '[role="menuitem"]', + '[role="menuitemcheckbox"]', + '[role="menuitemradio"]', + '[role="checkbox"]', + '[role="radio"]', + '[role="switch"]', + '[role="option"]', +].join(", "); + +/** + * Search chrome is a small wrapper drawn around the input, so only a handful of + * ancestors may claim a click. Without the bound, `closest` happily returns a + * page-level node — MediaWiki Vector renders ``, and + * a site-wide `
` is just as common — and every click on the page would be + * swallowed into a fill session on whatever search box the page happens to have. + */ +const SEARCH_CHROME_MAX_DEPTH = 4; + +function isSearchChromeWrapper(container: Element, target: Element): boolean { + if (container === document.body || container === document.documentElement) return false; + let node: Element | null = target; + for (let depth = 0; node && depth <= SEARCH_CHROME_MAX_DEPTH; depth += 1) { + if (node === container) return true; + node = node.parentElement; + } + return false; +} + /** Clicks on search chrome that only focus the nearby input should not become steps. */ function nearbyFillableFromSearchChrome(target: Element): FillableElement | null { - const container = target.closest( - '[id*="chat-input"], [id*="search"], [class*="search"], form, [role="search"]', - ); - if (!container) return null; + if (target.closest(SEARCH_CHROME_CONTROL_SELECTOR)) return null; + const container = target.closest(SEARCH_CHROME_SELECTOR); + if (!container || !isSearchChromeWrapper(container, target)) return null; const fillable = container.querySelector( 'textarea, input[type="search"], input[name="q"], #chat-textarea', ); diff --git a/apps/extension/src/content/recording/frame-agent.ts b/apps/extension/src/content/recording/frame-agent.ts index 5d1b64fe..504f0e3a 100644 --- a/apps/extension/src/content/recording/frame-agent.ts +++ b/apps/extension/src/content/recording/frame-agent.ts @@ -192,19 +192,30 @@ export function attachRecordFrameAgent(): () => void { return true; }; chrome.runtime.onMessage.addListener(onMessage); - void chrome.runtime - .sendMessage({ type: RECORD_FRAME_QUERY }) - .then((response: RecordFrameQueryResponse | undefined) => { - if (!response?.active || !response.requestId || response.startedAtMs === undefined) return; - return agent.start({ - type: RECORD_FRAME_START, - requestId: response.requestId, - startedAtMs: response.startedAtMs, - }); - }) - .catch(() => {}); + const armFromBackground = () => + chrome.runtime + .sendMessage({ type: RECORD_FRAME_QUERY }) + .then((response: RecordFrameQueryResponse | undefined) => { + if (!response?.active || !response.requestId || response.startedAtMs === undefined) return; + return agent.start({ + type: RECORD_FRAME_START, + requestId: response.requestId, + startedAtMs: response.startedAtMs, + }); + }) + .catch(() => {}); + void armFromBackground(); + // Entering the back/forward cache disconnects the recording port, which tears + // capture down in this Document. The content script does not run again on + // restore, so the restored Document has to re-arm itself. `start` is a no-op + // while the same recording is already active. + const onPageShow = (event: PageTransitionEvent) => { + if (event.persisted) void armFromBackground(); + }; + window.addEventListener("pageshow", onPageShow); return () => { chrome.runtime.onMessage.removeListener(onMessage); + window.removeEventListener("pageshow", onPageShow); agent.dispose(); }; } diff --git a/apps/extension/src/tools/__tests__/navigation.test.ts b/apps/extension/src/tools/__tests__/navigation.test.ts index 921771e2..2a748de3 100644 --- a/apps/extension/src/tools/__tests__/navigation.test.ts +++ b/apps/extension/src/tools/__tests__/navigation.test.ts @@ -173,6 +173,11 @@ function makeFakeCdp(opts?: { const payload = { name, frameId, loaderId }; for (const listener of [...events]) listener({ tabId: 4 }, "Page.lifecycleEvent", payload); }, + fireNavigatedWithinDocument(frameId = opts?.navigateFrameId ?? "frame-1", url = "#anchor") { + for (const listener of [...events]) { + listener({ tabId: 4 }, "Page.navigatedWithinDocument", { frameId, url }); + } + }, fireFrameNavigated( frameId = opts?.navigateFrameId ?? "frame-1", loaderId = opts?.navigateLoaderId ?? "loader-after", @@ -522,6 +527,54 @@ describe("handleNavigateBack / Forward", () => { expect(res.previous_url).toBe("https://b.example/"); }); + it("finishes a fragment-only back hop on the same-document event", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const fake = makeFakeCdp({ + historyIndex: 1, + historyEntries: [ + { id: 11, url: "https://a.example/page" }, + { id: 12, url: "https://a.example/page#section" }, + ], + }); + const navP = handleNavigateBack( + sm, + { session_id: "aa11", wait_until: "load", timeout_ms: 1_000 }, + { cdp: fake.cdp, tabsApi: fake.tabsApi }, + ); + await new Promise((r) => setTimeout(r, 5)); + // No new Document, so no lifecycle event will ever arrive for this hop. + fake.fireNavigatedWithinDocument(); + const res = await navP; + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(res.reached).toBe("same_document"); + expect(res.error_text).toBeUndefined(); + }); + + it("keeps waiting for load when the history hop changes more than the fragment", async () => { + const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); + await sm.start("aa11"); + const fake = makeFakeCdp({ + historyIndex: 1, + historyEntries: [ + { id: 11, url: "https://a.example/" }, + { id: 12, url: "https://b.example/#x" }, + ], + }); + const navP = handleNavigateBack( + sm, + { session_id: "aa11", wait_until: "load", timeout_ms: 1_000 }, + { cdp: fake.cdp, tabsApi: fake.tabsApi }, + ); + await new Promise((r) => setTimeout(r, 5)); + fake.fireNavigatedWithinDocument(); + await new Promise((r) => setTimeout(r, 5)); + fake.fireLifecycle("load"); + const res = await navP; + if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`); + expect(res.reached).toBe("load"); + }); + it("returns invalid_params when there is no previous history", async () => { const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) }); await sm.start("aa11"); diff --git a/apps/extension/src/tools/__tests__/record-cross-document.test.ts b/apps/extension/src/tools/__tests__/record-cross-document.test.ts new file mode 100644 index 00000000..bb039c98 --- /dev/null +++ b/apps/extension/src/tools/__tests__/record-cross-document.test.ts @@ -0,0 +1,425 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { RECORD_START, RECORD_STEP } from "@/lib/record-bridge"; +import { + RECORD_FRAME_PORT, + RECORD_FRAME_QUERY, + RECORD_FRAME_START, + type RecordFrameQueryResponse, +} from "@/lib/recording/frame-bridge"; +import { RecordFrameCoordinator } from "@/lib/recording/frame-coordinator"; +import type { SessionManager } from "@/session-manager/manager"; +import type { CdpRunner } from "@/tools/shared"; +import type { RecordStopResult, TraceV3 } from "@/transport/types"; +import { + attachRecordStepListener, + handleRecordStart, + handleRecordStop, + resetBrowserObservationForTests, +} from "../record"; + +const AGENT_WINDOW_ID = 100; +const TAB_ID = 4; +const START_URL = "https://start.example/"; +const NEXT_URL = "https://next.example/article"; + +class ListenerSet unknown> { + readonly listeners = new Set(); + addListener = (listener: T) => this.listeners.add(listener); + removeListener = (listener: T) => this.listeners.delete(listener); + emit = (...args: Parameters) => { + for (const listener of [...this.listeners]) listener(...args); + }; +} + +type RuntimeListener = ( + message: unknown, + sender: chrome.runtime.MessageSender, + sendResponse: (response: unknown) => void, +) => unknown; + +function installChrome() { + const runtimeOnMessage = new ListenerSet(); + const runtimeOnConnect = new ListenerSet<(port: chrome.runtime.Port) => void>(); + const webNavigationOnCompleted = new ListenerSet< + (details: chrome.webNavigation.WebNavigationFramedCallbackDetails) => unknown + >(); + const webNavigationOnCommitted = new ListenerSet< + (details: chrome.webNavigation.WebNavigationTransitionCallbackDetails) => unknown + >(); + vi.stubGlobal("chrome", { + runtime: { onMessage: runtimeOnMessage, onConnect: runtimeOnConnect }, + tabs: { + onActivated: new ListenerSet(), + onCreated: new ListenerSet(), + onUpdated: new ListenerSet(), + }, + webNavigation: { + onCompleted: webNavigationOnCompleted, + onCommitted: webNavigationOnCommitted, + }, + }); + return { + runtimeOnMessage, + runtimeOnConnect, + webNavigationOnCompleted, + webNavigationOnCommitted, + }; +} + +function fakeManager() { + return { + get: (id: string) => + id === "abcd" + ? { + sessionId: "abcd", + agentWindowId: AGENT_WINDOW_ID, + refStore: { resolve: () => null, replace: () => {} }, + borrowedTabs: new Map(), + } + : null, + findByWindowId: (windowId: number) => + windowId === AGENT_WINDOW_ID ? { sessionId: "abcd" } : null, + } as unknown as SessionManager; +} + +function makeFakeCdp(): CdpRunner { + type EventListener = (source: chrome.debugger.Debuggee, method: string, params: unknown) => void; + const events: EventListener[] = []; + const handlers: Record unknown> = { + "Page.enable": () => ({}), + "Page.setLifecycleEventsEnabled": () => ({}), + "Page.getFrameTree": () => ({ + frameTree: { frame: { id: "frame-1", loaderId: "loader-before" } }, + }), + "Page.navigate": () => { + for (const listener of [...events]) { + listener({ tabId: TAB_ID }, "Page.lifecycleEvent", { + name: "load", + frameId: "frame-1", + loaderId: "loader-after", + }); + } + return { frameId: "frame-1", loaderId: "loader-after" }; + }, + "Page.getLayoutMetrics": () => ({ + cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 }, + }), + "Runtime.enable": () => ({}), + "Runtime.evaluate": (params: unknown) => { + const expression = String((params as { expression?: string })?.expression ?? ""); + if (!expression.includes("__bskRecordQuiet")) return { result: { value: "complete" } }; + return { result: { value: { idleMs: 10_000, readyState: "complete" } } }; + }, + "Accessibility.enable": () => ({}), + "Accessibility.getFullAXTree": () => ({ + nodes: [ + { + nodeId: "1", + backendDOMNodeId: 1, + role: { type: "role", value: "RootWebArea" }, + name: { type: "computed", value: "Page" }, + childIds: ["2"], + }, + { + nodeId: "2", + parentId: "1", + backendDOMNodeId: 2, + role: { type: "role", value: "button" }, + name: { type: "computed", value: "Menu" }, + }, + ], + }), + }; + return { + send: (async (tabId: number, method: string, params: unknown) => { + const handler = handlers[method]; + if (!handler) throw new Error(`unsupported CDP call ${method}`); + return handler(params, tabId); + }) as CdpRunner["send"], + trackSessionTab: () => {}, + onEvent: (handler: EventListener) => { + events.push(handler); + return { + dispose: () => { + const index = events.indexOf(handler); + if (index >= 0) events.splice(index, 1); + }, + }; + }, + } as unknown as CdpRunner; +} + +function makeTabsApi() { + const tab = { + id: TAB_ID, + windowId: AGENT_WINDOW_ID, + active: true, + status: "complete", + url: START_URL, + title: "Start", + } as chrome.tabs.Tab; + return { + get: async () => tab, + query: async () => [tab], + goTo(url: string, title: string) { + Object.assign(tab, { url, title }); + }, + }; +} + +/** + * One simulated content-script Document: the `record-frame` entrypoint's + * behaviour reduced to what the background can observe. + */ +interface FakeDocument { + documentId: string; + frameId: number; + producerId: string; + started: boolean; + requestId?: string; + nextSequence: number; + disconnect(): void; +} + +function makeBrowser(chromeApi: ReturnType) { + const documents = new Map(); + let live: FakeDocument | null = null; + + function sender(doc: FakeDocument): chrome.runtime.MessageSender { + return { + tab: { id: TAB_ID, active: true }, + frameId: doc.frameId, + documentId: doc.documentId, + } as chrome.runtime.MessageSender; + } + + /** The content script's `chrome.runtime.connect` + `ready` handshake. */ + function connect(doc: FakeDocument, requestId: string): void { + const onMessage = new ListenerSet<(message: unknown) => void>(); + const onDisconnect = new ListenerSet<() => void>(); + const port = { + name: RECORD_FRAME_PORT, + sender: sender(doc), + onMessage, + onDisconnect, + // The frame agent answers `stop` once its queued steps are delivered. + postMessage: vi.fn((message: { type?: string; commandId?: string }) => { + if (message.type !== "stop") return; + queueMicrotask(() => + onMessage.emit({ + type: "stopped", + requestId, + commandId: message.commandId, + ok: true, + }), + ); + }), + disconnect: vi.fn(() => onDisconnect.emit()), + } as unknown as chrome.runtime.Port; + chromeApi.runtimeOnConnect.emit(port); + onMessage.emit({ type: "ready", requestId, producerId: doc.producerId }); + doc.started = true; + doc.requestId = requestId; + doc.disconnect = () => onDisconnect.emit(); + } + + return { + get live() { + return live; + }, + /** Create a new main-frame Document, as a cross-document navigation does. */ + navigate(documentId: string): FakeDocument { + live?.disconnect(); + const doc: FakeDocument = { + documentId, + frameId: 0, + producerId: `producer-${documentId}`, + started: false, + nextSequence: 1, + disconnect: () => {}, + }; + documents.set(documentId, doc); + live = doc; + return doc; + }, + /** `record-frame.content.ts` bootstrap at document_start. */ + async bootstrap(doc: FakeDocument): Promise { + let response: RecordFrameQueryResponse | undefined; + for (const listener of [...chromeApi.runtimeOnMessage.listeners]) { + listener({ type: RECORD_FRAME_QUERY }, sender(doc), (value) => { + response = value as RecordFrameQueryResponse; + }); + } + if (response?.active && response.requestId) connect(doc, response.requestId); + }, + /** `chrome.tabs.sendMessage(tabId, RECORD_FRAME_START, {documentId})`. */ + async deliverFrameStart( + target: { documentId?: string; frameId?: number }, + requestId: string, + ): Promise { + const doc = target.documentId + ? documents.get(target.documentId) + : [...documents.values()].find((d) => d === live && d.frameId === target.frameId); + if (!doc || doc !== live) throw new Error("no receiving end"); + if (doc.started && doc.requestId === requestId) return { ok: true }; + connect(doc, requestId); + return { ok: true }; + }, + /** A captured user action leaving the live Document. */ + emitStep(doc: FakeDocument, requestId: string, step: unknown): unknown { + const sequence = doc.nextSequence; + doc.nextSequence += 1; + let ack: unknown; + for (const listener of [...chromeApi.runtimeOnMessage.listeners]) { + listener( + { type: RECORD_STEP, requestId, producerId: doc.producerId, sequence, step }, + sender(doc), + (value) => { + ack = value; + }, + ); + } + return ack; + }, + frames(): { frameId: number; documentId?: string }[] { + return live ? [{ frameId: live.frameId, documentId: live.documentId }] : [{ frameId: 0 }]; + }, + }; +} + +function clickStep(url: string, name: string) { + return { + op: "click", + page_url: url, + target: { role: "button", name, tag: "button" }, + geometry: { rect: { x: 0, y: 0, w: 10, h: 10 }, tag: "button" }, + }; +} + +describe("recording survives a main-frame navigation", () => { + afterEach(() => { + resetBrowserObservationForTests(); + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + it("keeps capturing clicks in the Document that replaced the start page", async () => { + const chromeApi = installChrome(); + const manager = fakeManager(); + const tabsApi = makeTabsApi(); + const browser = makeBrowser(chromeApi); + + const coordinator = new RecordFrameCoordinator({ + getAllFrames: async () => browser.frames(), + sendToDocument: (_tabId, message, target) => + browser.deliverFrameStart(target, message.requestId), + subscribeFrameNavigation: () => () => {}, + }); + coordinator.attach(); + + let requestId = ""; + const sendToTab = vi.fn(async (_tabId: number, msg: unknown) => { + const typed = msg as { type?: string; requestId?: string }; + if (typed.type === RECORD_START && typed.requestId) requestId = typed.requestId; + return { ok: true }; + }); + + const deps = { tabsApi, sendToTab, cdp: makeFakeCdp(), frameCoordinator: coordinator }; + // The start page's Document exists before record_start arms the tab. + const first = browser.navigate("doc-1"); + const started = await handleRecordStart( + manager, + { session_id: "abcd", url: START_URL, trace_version: 3 as const }, + deps, + ); + expect(started).toEqual({ tab_id: TAB_ID, recording: true }); + expect(first.started).toBe(true); + + attachRecordStepListener(deps); + + expect(browser.emitStep(first, requestId, clickStep(START_URL, "Search"))).toEqual({ + ok: true, + sequence: 1, + }); + + // The click navigates: a fresh Document replaces the old one, the old port + // drops, and the browser reports the main-frame load. + tabsApi.goTo(NEXT_URL, "Article"); + const second = browser.navigate("doc-2"); + await browser.bootstrap(second); + chromeApi.webNavigationOnCommitted.emit({ + tabId: TAB_ID, + frameId: 0, + url: NEXT_URL, + transitionType: "link", + transitionQualifiers: [], + } as unknown as chrome.webNavigation.WebNavigationTransitionCallbackDetails); + chromeApi.webNavigationOnCompleted.emit({ + tabId: TAB_ID, + frameId: 0, + url: NEXT_URL, + } as unknown as chrome.webNavigation.WebNavigationFramedCallbackDetails); + await new Promise((resolve) => setTimeout(resolve, 400)); + + expect(second.started).toBe(true); + expect(browser.emitStep(second, requestId, clickStep(NEXT_URL, "Menu"))).toEqual({ + ok: true, + sequence: 1, + }); + + const stopped = await handleRecordStop(manager, { session_id: "abcd" }, deps); + const trace = (stopped as RecordStopResult).trace as TraceV3; + expect(trace.steps.map((step) => step.op)).toEqual(["click", "navigate", "click"]); + }, 20_000); + it("arms the new Document from the navigation listener when its own query is lost", async () => { + const chromeApi = installChrome(); + const manager = fakeManager(); + const tabsApi = makeTabsApi(); + const browser = makeBrowser(chromeApi); + + const coordinator = new RecordFrameCoordinator({ + getAllFrames: async () => browser.frames(), + sendToDocument: (_tabId, message, target) => + browser.deliverFrameStart(target, message.requestId), + subscribeFrameNavigation: () => () => {}, + }); + coordinator.attach(); + + let requestId = ""; + const sendToTab = vi.fn(async (_tabId: number, msg: unknown) => { + const typed = msg as { type?: string; requestId?: string }; + if (typed.type === RECORD_START && typed.requestId) requestId = typed.requestId; + return { ok: true }; + }); + + const deps = { tabsApi, sendToTab, cdp: makeFakeCdp(), frameCoordinator: coordinator }; + browser.navigate("doc-1"); + await handleRecordStart( + manager, + { session_id: "abcd", url: START_URL, trace_version: 3 as const }, + deps, + ); + attachRecordStepListener(deps); + + // A service worker that was asleep at document_start loses the query, so + // the Document never bootstraps itself. + tabsApi.goTo(NEXT_URL, "Article"); + const second = browser.navigate("doc-2"); + chromeApi.webNavigationOnCompleted.emit({ + tabId: TAB_ID, + frameId: 0, + url: NEXT_URL, + } as unknown as chrome.webNavigation.WebNavigationFramedCallbackDetails); + await new Promise((resolve) => setTimeout(resolve, 400)); + + expect(second.started).toBe(true); + expect(browser.emitStep(second, requestId, clickStep(NEXT_URL, "Menu"))).toEqual({ + ok: true, + sequence: 1, + }); + + const stopped = await handleRecordStop(manager, { session_id: "abcd" }, deps); + const trace = (stopped as RecordStopResult).trace as TraceV3; + expect(trace.steps.map((step) => step.op)).toEqual(["navigate", "click"]); + }, 20_000); +}); diff --git a/apps/extension/src/tools/navigation.ts b/apps/extension/src/tools/navigation.ts index e40d7241..47142e22 100644 --- a/apps/extension/src/tools/navigation.ts +++ b/apps/extension/src/tools/navigation.ts @@ -200,6 +200,30 @@ interface LifecycleWait { export interface LifecycleWaitGuard { loaderId?: string | (() => string | null | undefined); beforeLoaderId?: string | null; + /** + * Accept `Page.navigatedWithinDocument` as the end of the wait. A history + * entry that only differs in its fragment reuses the current Document, so no + * loader is created and no lifecycle event will ever arrive. + */ + acceptSameDocument?: boolean; +} + +/** `lastLifecycle` value reported when a navigation stayed in one Document. */ +export const SAME_DOCUMENT_LIFECYCLE = "same_document"; + +function fragmentlessUrl(url: string): string { + const hash = url.indexOf("#"); + return hash < 0 ? url : url.slice(0, hash); +} + +/** + * True when moving between these two history entries cannot create a Document: + * the URLs differ, but only after the `#`. Identical URLs stay out — a repeated + * entry can still be a real reload — so they keep the lifecycle wait. + */ +export function isSameDocumentHistoryHop(from?: string, to?: string): boolean { + if (!from || !to || from === to) return false; + return fragmentlessUrl(from) === fragmentlessUrl(to); } function currentLoaderId(guard: LifecycleWaitGuard | undefined): string { @@ -331,6 +355,14 @@ function startLifecycleWait( if (settled) return; if (source.tabId !== expectedTabId) return; + if (guard?.acceptSameDocument && method === "Page.navigatedWithinDocument") { + const p = params as { frameId?: string }; + if (!lifecycleEventMatchesFrame(p.frameId)) return; + noteRelevantLifecycle(SAME_DOCUMENT_LIFECYCLE); + finish({ reached: "match", lastLifecycle: SAME_DOCUMENT_LIFECYCLE }); + return; + } + if (targetName === "commit" && method === "Page.frameNavigated") { const p = params as { frame?: { id?: string; parentId?: string; loaderId?: string } }; const expectedFrameId = currentFrameId(); @@ -728,6 +760,10 @@ async function handleHistory( const beforeFrame = await readMainFrameInfo(deps.cdp, target.tabId); const expected = cdpLifecycleName(waitUntil); const beforeReadyState = await probeMainFrameReadyState(deps.cdp, target.tabId); + // A fragment-only hop reuses the Document, so no loader is created and the + // lifecycle event never arrives. Accepting the same-document event as well + // keeps the lifecycle path available for an entry that does load after all. + const acceptSameDocument = isSameDocumentHistoryHop(previousUrl, targetEntry?.url); const waitAbort = linkedAbortSignal(deps.signal); const wait = startLifecycleWait( deps.cdp, @@ -736,7 +772,10 @@ async function handleHistory( expected, timeoutMs, waitAbort.signal, - { beforeLoaderId: beforeFrame.loaderId }, + { + beforeLoaderId: beforeFrame.loaderId, + ...(acceptSameDocument ? { acceptSameDocument: true } : {}), + }, ); const waitPromise = wait.promise; try { @@ -759,7 +798,9 @@ async function handleHistory( tab_id: target.tabId, previous_url: previousUrl, final_url: finalUrl, - reached: waitUntil, + // Report what actually happened: a fragment hop never reaches `load`. + reached: + outcome.lastLifecycle === SAME_DOCUMENT_LIFECYCLE ? SAME_DOCUMENT_LIFECYCLE : waitUntil, }); } return attachDialogs(deps.cdp, target.tabId, dialogCursor, { diff --git a/crates/bsk-cli/Cargo.toml b/crates/bsk-cli/Cargo.toml index 565a7424..ab00bb10 100644 --- a/crates/bsk-cli/Cargo.toml +++ b/crates/bsk-cli/Cargo.toml @@ -34,6 +34,7 @@ tokio-tungstenite = { workspace = true } futures-util = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +schemars = { workspace = true } semver = { workspace = true } thiserror = { workspace = true } anyhow = { workspace = true } diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index 9a08643e..73acc46b 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -40,7 +40,9 @@ Every session-scoped command needs `--session `; `session stop` takes the ID positionally. For unfamiliar commands or flags, consult `bsk --help` or `bsk --help` instead of guessing; no need to read all help at startup. When following a trace, use its semantic targets and values in order, not its old -refs. Stop at the requested goal; a trace grants no additional authorization. +refs. Stop at the requested goal; a trace grants no additional authorization. A trace +already distilled into site memory reads better as numbered steps: check +`bsk site workflow show --host ` before working from the raw trace. ## Read and interact @@ -234,10 +236,60 @@ Use agent-local paths, not browser-internal staging paths. Use `console` / `network` for bounded read-only diagnostics; follow returned sequence cursors. `emulate --device iphone-14` affects one tab; `--off` restores it. `evaluate` is a last resort: inspect JSON `.ok`, since a script exception can have -CLI exit code 0. Never evaluate secrets. `record start` captures user actions; -read its help first and never record banking, SSO or password-manager pages. +CLI exit code 0. Never evaluate secrets. `record start` captures user actions, or +with `--detach` your own; read its help first and never record banking, SSO or +password-manager pages. Use `bsk --help` to find navigation/history, tab, wait and window commands. +## Site memory + +`bsk site` keeps local, per-host notes so an explored flow is not re-explored: a +constrained `SITE.md`, workflows derived from a recording, and candidate observations +awaiting evidence. It is private to this machine, needs no daemon, and never stores +credentials or recorded input values. + +```sh +bsk site context --host --task +bsk site workflow show --host +bsk site workflow save --from ./rec/trace.json --id --task +bsk site workflow verify --host --pass +bsk site candidate add --host --kind better_path --claim "" +bsk site checkpoint --host --task --reason direct_correction +``` + +- **Read before acting.** When a task names a site, run `bsk site context` first and + follow what it already knows. +- **Never explore to learn.** Record only what the task itself revealed; no extra + pages or detours to make memory "more complete". +- **Learning never fails the task.** These commands are advisory: on failure report + one line and continue the user's goal. Do not retry or let it block the task. + +To leave memory behind for the task you are already doing, record your own run: + +```sh +bsk record start --detach --url --output ./rec --json # prints session_id +bsk observe --session # do the real task +bsk fill @e4 --value "" --session +bsk press Enter --session +bsk record stop --output ./rec # exports trace.json +bsk site workflow save --from ./rec/trace.json --id --task +bsk site checkpoint --host --task --reason direct_correction +``` + +`--detach` returns once recording is armed, so the session accepts your commands; +without it `record start` holds the session and every command returns `session_busy`. +`record stop` also ends that session. Reusing a workflow needs no recording: read it, +run it, then `workflow verify --pass`, which clears NEEDS REVIEW. + +`workflow save` writes into a per-task draft; `checkpoint` publishes it. Revisions are +per host. A `conflict` result means another writer committed first: re-run +`bsk site context`, which re-seeds the draft's `SITE.md` from the published revision +(your draft edits are gone and must be replayed; staged workflows are kept), then +checkpoint again. Retry a conflict only once. Recorded `fill` values become named +parameters and are never stored; pass `--inline-values` only when the values belong +to the flow, not to the person who recorded it. `` is the exception: an option's `value` +//! attribute is a constant the site defines, not something the recorder +//! typed, so it is inlined and only a secret-looking one becomes a parameter. +//! +//! A recording is a raw artefact and contains noise — an `about:blank` left by +//! backing out past the start page, a first action that happens before any +//! navigation. Noise is dropped or repaired, never fatal: one stray step must +//! not cost the user the whole recording. + +use bsk_protocol::tools::{ + FillCommit, NavigationCause, SelectedOptionV3, StepV3, TargetDescriptorV3, TraceV3, +}; + +use super::model::{ + SecretsPolicy, SourceTrace, StepOp, StepTarget, WORKFLOW_SCHEMA_VERSION, Workflow, + WorkflowParam, WorkflowStep, WorkflowStrategy, +}; + +/// Outcome of a derivation, carrying the counts the CLI reports back. +#[derive(Debug, Clone)] +pub struct Derived { + pub workflow: Workflow, + /// Steps dropped as viewport / session side effects (`scroll`, `switch_tab`). + pub dropped_steps: usize, +} + +/// How recorded `fill` / `select` values are treated. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ValuePolicy { + /// Drop every recorded value and expose it as a parameter (the default). + #[default] + Parameterise, + /// Keep values that are demonstrably not personal data as constants. + Inline, +} + +/// Build a workflow from a v3 trace. +pub fn derive_workflow( + trace: &TraceV3, + id: &str, + host: &str, + purpose: Option<&str>, + policy: ValuePolicy, +) -> Derived { + let mut builder = Builder { + policy, + ..Builder::default() + }; + if !starts_with_navigate(&trace.steps) { + builder.push_entry_navigate(&trace.entry.start_url); + } + for step in &trace.steps { + builder.push(step); + } + let needs_review = builder.steps.iter().any(|step| step.needs_review); + let workflow = Workflow { + schema_version: WORKFLOW_SCHEMA_VERSION, + id: id.to_string(), + host: host.to_string(), + purpose: purpose + .map(str::to_string) + .or_else(|| trace.purpose.clone()), + strategy: WorkflowStrategy::UiOnly, + // Derivation is not verification: a fresh workflow is unverified until + // someone runs `bsk site workflow verify --pass`. + last_verified: None, + source_trace: Some(SourceTrace { + recorded_at: Some(trace.recorded_at.clone()), + recorder: Some(format!("bsk {}", trace.recorder.bsk)), + purpose: trace.purpose.clone(), + }), + params: builder.params, + preconditions: Vec::new(), + steps: builder.steps, + // Left empty on purpose: design §4.1 forbids lifting a postcondition + // out of `states[]` without human confirmation. + postcondition: None, + secrets_policy: SecretsPolicy { + store_values: false, + redacted_fields: builder.redacted_fields, + }, + needs_review, + review_notes: { + let mut notes = builder.notes; + if builder.effect_navigations > 0 { + notes.push(format!( + "{} navigation(s) the page made in response to a recorded click or submit \ + were dropped: replaying the action reaches the same page, and landing URLs \ + can embed typed values", + builder.effect_navigations + )); + } + notes + }, + }; + Derived { + workflow, + dropped_steps: builder.dropped, + } +} + +/// Host a trace belongs to: where the recording *started*. +/// +/// The entry URL comes first because that is the page a replay has to open. +/// Reading the first `navigate` step instead filed a search on +/// `www.wikipedia.org` under `zh.wikipedia.org` — the article the search landed +/// on — and then rejected `--host www.wikipedia.org` as a mismatch. A trace +/// whose entry URL is not an http(s) page (a recording armed on `about:blank`) +/// falls back to the first real navigation. +pub fn infer_host(trace: &TraceV3) -> Option { + if let Some(host) = host_of_url(&trace.entry.start_url) { + return Some(host); + } + for step in &trace.steps { + if let StepV3::Navigate { to, .. } = step + && let Some(host) = host_of_url(to) + { + return Some(host); + } + } + None +} + +/// Whether a recorded navigation target is a page a workflow can replay. +fn is_replayable_url(url: &str) -> bool { + url.starts_with("http://") || url.starts_with("https://") +} + +/// Whether the derived workflow will already open with a navigation. +/// +/// Mirrors what [`Builder::push`] keeps: scrolls, tab switches and +/// unreplayable navigations never become steps, so they cannot be the first +/// one either. +fn starts_with_navigate(steps: &[StepV3]) -> bool { + for step in steps { + match step { + StepV3::Scroll { .. } | StepV3::SwitchTab { .. } => {} + StepV3::Navigate { to, .. } if !is_replayable_url(to) => {} + StepV3::Navigate { .. } => return true, + _ => return false, + } + } + false +} + +/// Decode `%XX` escapes, keeping malformed sequences and non-UTF-8 bytes +/// as-is (lossily). Enough to see a typed value inside a query string. +fn percent_decode_lossy(input: &str) -> String { + let bytes = input.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' + && i + 2 < bytes.len() + && let Some(hex) = input.get(i + 1..i + 3) + && let Ok(byte) = u8::from_str_radix(hex, 16) + { + { + out.push(byte); + i += 3; + continue; + } + } + out.push(bytes[i]); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() +} + +/// Extract the host component of an absolute URL without a URL parser. +pub fn host_of_url(url: &str) -> Option { + let rest = url.split_once("://").map(|(_, rest)| rest)?; + let authority = rest.split(['/', '?', '#']).next()?; + let authority = authority.rsplit_once('@').map_or(authority, |(_, h)| h); + let host = match authority.rsplit_once(':') { + Some((head, port)) if !port.is_empty() && port.chars().all(|c| c.is_ascii_digit()) => head, + _ => authority, + }; + if host.is_empty() { + None + } else { + Some(host.to_ascii_lowercase()) + } +} + +#[derive(Default)] +struct Builder { + policy: ValuePolicy, + steps: Vec, + params: Vec, + redacted_fields: Vec, + /// Recorded values that were turned into parameters. A later navigation + /// whose URL embeds one of them (a search results page, say) would put + /// the value back on disk through the back door, so it is dropped. + kept_out: Vec<(String, String)>, + /// Navigations dropped as the effect of a recorded click / submit. + effect_navigations: usize, + effect_urls: Vec, + notes: Vec, + dropped: usize, +} + +impl Builder { + fn push(&mut self, step: &StepV3) { + match step { + StepV3::Navigate { common, to, cause } => self.push_navigate(to, *cause, common.id), + StepV3::Click { target, .. } => self.push_pointer(StepOp::Click, target), + StepV3::Hover { target, .. } => self.push_pointer(StepOp::Hover, target), + StepV3::Fill { + target, + value, + commit, + redacted, + .. + } => self.push_fill(target, value, *commit, *redacted), + StepV3::Select { + target, selection, .. + } => self.push_select(target, selection), + StepV3::Press { + key, + modifiers, + target, + .. + } => self.push_press(key, modifiers.as_deref(), target.as_ref()), + // Scrolling is a viewport side effect, not a semantic step, and tab + // switching is owned by the session in the Agent Window model. + StepV3::Scroll { .. } | StepV3::SwitchTab { .. } => self.dropped += 1, + } + } + + fn next_n(&self) -> u32 { + self.steps.len() as u32 + 1 + } + + fn blank(&self, op: StepOp) -> WorkflowStep { + WorkflowStep { + n: self.next_n(), + op, + to: None, + target: None, + value: None, + value_from: None, + commit: None, + key: None, + modifiers: None, + note: None, + needs_review: false, + } + } + + /// A navigation the recorder captured. Targets a workflow cannot replay — + /// `about:blank` from backing out past the recording's start page, + /// `chrome://` pages — are dropped like scrolls rather than failing the + /// whole derivation: they are ordinary by-products of recording, and one of + /// them used to make `workflow save` exit 1 and discard the entire bundle. + fn push_navigate(&mut self, to: &str, cause: NavigationCause, recorded_id: u32) { + // A navigation the page performed in response to the previous recorded + // action (a clicked link, a submitted form, a script redirect) is an + // effect, not a step: replaying the click reaches the same page, and + // the landing URL often embeds what was typed — the results page of a + // search, say — which is exactly what site memory must not keep. + // Address-bar entries, history moves and reloads are the agent's own + // actions and stay. + let is_effect = matches!( + cause, + NavigationCause::Link + | NavigationCause::FormSubmit + | NavigationCause::Script + | NavigationCause::Browser + ); + if is_effect && !self.steps.is_empty() { + self.dropped += 1; + self.effect_navigations += 1; + self.effect_urls.push(to.to_string()); + return; + } + // A history move back to a page the flow only reached through one of + // those effects carries the same URL, so it is kept out for the same + // reason; the agent replays it as `navigate-back`. + if matches!(cause, NavigationCause::History) && self.effect_urls.iter().any(|u| u == to) { + self.dropped += 1; + self.notes.push(format!( + "recorded step {recorded_id} was a history move back to a page reached by an \ + earlier action; it was dropped (use `bsk navigate-back` when replaying)" + )); + return; + } + if !is_replayable_url(to) { + self.dropped += 1; + self.notes.push(format!( + "recorded step {recorded_id} navigated to {to:?}, which is not an http(s) page; \ + the step was dropped" + )); + return; + } + if let Some(leak) = self.leaks_kept_out_value(to) { + self.dropped += 1; + self.notes.push(format!( + "recorded step {recorded_id} navigated to a URL that embeds the value typed into {leak:?}; the step was dropped because that value is kept out of site memory (replaying the fill reaches the same page)" + )); + return; + } + let mut step = self.blank(StepOp::Navigate); + step.to = Some(to.to_string()); + self.steps.push(step); + } + + /// Name of the field whose kept-out value appears in `url`, if any. + fn leaks_kept_out_value(&self, url: &str) -> Option { + if self.kept_out.is_empty() { + return None; + } + let decoded = percent_decode_lossy(url); + let plus_as_space = decoded.replace('+', " "); + self.kept_out + .iter() + .find(|(_, value)| { + decoded.contains(value.as_str()) || plus_as_space.contains(value.as_str()) + }) + .map(|(label, _)| label.clone()) + } + + fn remember_kept_out(&mut self, target: &TargetDescriptorV3, value: &str) { + let value = value.trim(); + // One or two characters match almost any URL; that is noise, not a leak. + if value.chars().count() < 3 { + return; + } + let label = target + .name + .as_deref() + .map(strip_state_annotations) + .filter(|n| !n.is_empty()) + .unwrap_or_else(|| "this field".to_string()); + self.kept_out.push((label, value.to_string())); + } + + /// Give the workflow a starting page when the recording's first captured + /// action was not a navigation. + /// + /// Without it a derived workflow opened with `select` or `fill` and never + /// said which page to be on — the one thing the next agent cannot guess. + /// The entry URL is kept verbatim: a query string is part of the address + /// the flow starts from. Called before the steps are pushed, so the step + /// numbers every review note quotes are the final ones. + fn push_entry_navigate(&mut self, start_url: &str) { + if !is_replayable_url(start_url) { + self.notes.push( + "the recording does not start on an http(s) page, so the workflow has no opening \ + navigate step; add one before relying on it" + .to_string(), + ); + return; + } + let mut step = self.blank(StepOp::Navigate); + step.to = Some(start_url.to_string()); + step.note = Some( + "added from the recording's entry URL; the recording's first captured action was not \ + a navigation" + .to_string(), + ); + self.steps.push(step); + self.notes.push(format!( + "step 1 navigate {start_url} was added from the recording's entry URL" + )); + } + + fn push_pointer(&mut self, op: StepOp, target: &TargetDescriptorV3) { + let mut step = self.blank(op); + self.apply_target(&mut step, target); + self.steps.push(step); + } + + fn push_fill( + &mut self, + target: &TargetDescriptorV3, + value: &str, + commit: FillCommit, + redacted: bool, + ) { + let mut step = self.blank(StepOp::Fill); + self.apply_target(&mut step, target); + step.commit = Some(commit_name(commit).to_string()); + if redacted { + let name = self.declare_secret(target); + step.value_from = Some(name.clone()); + self.redacted_fields.push(name); + } else { + self.apply_value(&mut step, target, value, "the recorded text"); + } + self.steps.push(step); + } + + /// Turn a recorded value into either a parameter or an inline constant. + fn apply_value( + &mut self, + step: &mut WorkflowStep, + target: &TargetDescriptorV3, + value: &str, + what: &str, + ) { + if self.policy == ValuePolicy::Inline + && let Some(reason) = must_not_inline(value) + { + let name = + self.declare_value_param(target, Some(&format!("{what} ({reason})")), step.n); + self.remember_kept_out(target, value); + step.value_from = Some(name); + step.needs_review = true; + step.note = Some(format!( + "{reason}, so it was kept out of site memory and turned into a parameter" + )); + return; + } + if self.policy == ValuePolicy::Inline { + step.value = Some(value.to_string()); + step.needs_review = true; + step.note = Some( + "value inlined from the recording with --inline-values; confirm it is a constant \ + of the flow and not someone's data" + .to_string(), + ); + return; + } + let name = self.declare_value_param(target, Some(what), step.n); + self.remember_kept_out(target, value); + step.value_from = Some(name); + step.needs_review = true; + step.note = Some( + "the recorded value is not stored; supply it through this parameter, or re-derive \ + with --inline-values if it really is a constant of the flow" + .to_string(), + ); + } + + fn push_select(&mut self, target: &TargetDescriptorV3, selection: &[SelectedOptionV3]) { + let mut step = self.blank(StepOp::Select); + self.apply_target(&mut step, target); + match selection.split_first() { + Some((first, rest)) => { + // `bsk select` matches an option's `value` attribute, not its + // label, and that attribute is a constant the site defines + // ("zh", "P2") rather than anything the recorder typed. So it + // is inlined: parameterising it forced the next agent to guess + // a value the recording already knew. A value that looks like + // credential material is still kept out of memory. + match super::validate::find_secret(first.value.trim()) { + Some(reason) => { + let name = self.declare_value_param( + target, + Some(&format!("the recorded option value ({reason})")), + step.n, + ); + step.value_from = Some(name); + step.needs_review = true; + step.note = Some(format!( + "{reason}, so it was kept out of site memory and turned into a \ + parameter" + )); + } + None => step.value = Some(first.value.clone()), + } + if !rest.is_empty() { + let extra: Vec<&str> = rest.iter().map(|o| o.value.as_str()).collect(); + self.notes.push(format!( + "step {} recorded a multi-select ({} options: {}); `bsk select` takes one \ + value per call, so split it into separate steps", + step.n, + rest.len() + 1, + extra.join(", ") + )); + } + } + None => { + step.needs_review = true; + step.note = Some("the recording captured no selected option".to_string()); + } + } + self.steps.push(step); + } + + fn push_press( + &mut self, + key: &str, + modifiers: Option<&[bsk_protocol::tools::KeyModifier]>, + target: Option<&TargetDescriptorV3>, + ) { + let mut step = self.blank(StepOp::Press); + step.key = Some(key.to_string()); + if let Some(modifiers) = modifiers + && !modifiers.is_empty() + { + step.modifiers = Some( + modifiers + .iter() + .map(|m| modifier_name(*m).to_string()) + .collect(), + ); + } + if let Some(target) = target { + self.apply_target(&mut step, target); + } + self.steps.push(step); + } + + /// Copy the semantic anchor, dropping `element_ref` and turning an + /// unmatched target into an explicit `null` that needs review. + fn apply_target(&mut self, step: &mut WorkflowStep, target: &TargetDescriptorV3) { + if target.unmatched { + step.target = None; + step.needs_review = true; + step.note = Some( + "the recorder could not match this element; supply a role/name anchor".to_string(), + ); + self.notes.push(format!( + "step {} has no anchor: the recording matched no unique element", + step.n + )); + return; + } + // The anchor must survive a state change: `搜索维基百科 [expanded]` at + // recording time is the same control as `搜索维基百科` on replay. + let anchor = StepTarget { + role: target.role.clone(), + name: target + .name + .as_deref() + .map(strip_state_annotations) + .filter(|name| !name.is_empty()), + ctx: target.ctx.clone(), + }; + if anchor.is_empty() { + step.target = None; + step.needs_review = true; + step.note = Some("the recording carried no semantic anchor".to_string()); + } else { + step.target = Some(anchor); + } + } + + /// Declare (or reuse) an ordinary parameter carrying a field's value. + /// + /// A label that slugs to nothing — any non-ASCII name, such as + /// `搜索维基百科` — falls back to `field_` rather than a shared `value`. + /// The old fallback silently merged every unnamed field in a flow into one + /// parameter, so filling two different boxes took the same input. + fn declare_value_param( + &mut self, + target: &TargetDescriptorV3, + what: Option<&str>, + n: u32, + ) -> String { + let base = target + .name + .as_deref() + .map(slug_param) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| format!("field_{n}")); + if let Some(existing) = self + .params + .iter() + .find(|p| !p.secret && p.name == base) + .map(|p| p.name.clone()) + { + return existing; + } + let name = self.unique_param_name(&base); + let label = target + .name + .as_deref() + .map(strip_state_annotations) + .filter(|n| !n.is_empty()) + .unwrap_or_else(|| "this field".to_string()); + self.params.push(WorkflowParam { + name: name.clone(), + required: true, + secret: false, + description: Some(match what { + Some(what) => format!("{what} for {label:?}; not stored on disk"), + None => format!("value for {label:?}; not stored on disk"), + }), + enum_values: None, + }); + name + } + + /// Declare (or reuse) a secret parameter for a redacted field. + fn declare_secret(&mut self, target: &TargetDescriptorV3) -> String { + let base = target + .name + .as_deref() + .map(slug_param) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| "secret".to_string()); + if let Some(existing) = self + .params + .iter() + .find(|p| p.secret && p.name == base) + .map(|p| p.name.clone()) + { + return existing; + } + let name = self.unique_param_name(&base); + self.params.push(WorkflowParam { + name: name.clone(), + required: true, + secret: true, + description: target + .name + .as_deref() + .map(strip_state_annotations) + .filter(|n| !n.is_empty()) + .map(|n| format!("value for the redacted field {n:?}; never stored on disk")), + enum_values: None, + }); + name + } + + fn unique_param_name(&self, base: &str) -> String { + if !self.params.iter().any(|p| p.name == base) { + return base.to_string(); + } + for suffix in 2..1000u32 { + let candidate = format!("{base}{suffix}"); + if !self.params.iter().any(|p| p.name == candidate) { + return candidate; + } + } + format!("{base}-{}", self.params.len()) + } +} + +/// Values that stay out of site memory even under `--inline-values`. +/// +/// `--inline-values` says "these constants belong to the flow", but the person +/// running it cannot vet every field, so the obvious personal and credential +/// shapes are still parameterised. +fn must_not_inline(value: &str) -> Option<&'static str> { + let trimmed = value.trim(); + if trimmed.is_empty() { + return None; + } + if super::validate::find_secret(trimmed).is_some() { + return Some("the value looks like credential material"); + } + if trimmed.contains('@') && trimmed.contains('.') && !trimmed.contains(' ') { + return Some("the value looks like an email address"); + } + let digits = trimmed.chars().filter(char::is_ascii_digit).count(); + if digits >= 6 + && trimmed + .chars() + .all(|c| c.is_ascii_digit() || "+-() ".contains(c)) + { + return Some("the value looks like a phone number or an identifier"); + } + if trimmed.chars().count() > 120 { + return Some("the value is long enough to be free-text someone typed"); + } + None +} + +fn commit_name(commit: FillCommit) -> &'static str { + match commit { + FillCommit::Enter => "enter", + FillCommit::Suggestion => "suggestion", + FillCommit::Blur => "blur", + } +} + +fn modifier_name(modifier: bsk_protocol::tools::KeyModifier) -> &'static str { + use bsk_protocol::tools::KeyModifier as K; + match modifier { + K::Alt => "alt", + K::Ctrl => "ctrl", + K::Meta => "meta", + K::Shift => "shift", + } +} + +/// Drop the bracketed state annotations the VOM appends to an accessible name. +/// +/// An observation renders a control as `ZH [has-submenu]` or +/// `搜索维基百科 [expanded]`: the bracketed part describes the widget's state at +/// recording time, not its identity, and carrying it into a parameter name +/// produced identifiers like `zh_has_submenu` that mean nothing on replay. +fn strip_state_annotations(name: &str) -> String { + let mut out = String::new(); + let mut depth = 0usize; + for ch in name.chars() { + match ch { + '[' => depth += 1, + ']' => depth = depth.saturating_sub(1), + _ if depth == 0 => out.push(ch), + _ => {} + } + } + out.trim().to_string() +} + +/// Turn an accessible name into a parameter identifier. Non-ASCII names (a +/// Chinese label, say) slug down to nothing; the caller supplies the fallback. +fn slug_param(name: &str) -> String { + let mut out = String::new(); + let mut prev_dash = false; + for ch in strip_state_annotations(name).chars() { + if ch.is_ascii_alphanumeric() { + out.push(ch.to_ascii_lowercase()); + prev_dash = false; + } else if !out.is_empty() && !prev_dash { + out.push('_'); + prev_dash = true; + } + } + let trimmed = out.trim_matches('_').to_string(); + let mut trimmed = trimmed; + trimmed.truncate(32); + trimmed.trim_matches('_').to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + use bsk_protocol::tools::{ + NavigationCause, RecorderInfo, StepCommonV3, StepResultV3, StopReason, TRACE_VERSION_V3, + TraceEntry, VOM_FORMAT_VERSION, + }; + + fn common(id: u32) -> StepCommonV3 { + StepCommonV3 { + id, + state: "s1".into(), + result: StepResultV3 { state: "s1".into() }, + } + } + + fn target(role: &str, name: &str) -> TargetDescriptorV3 { + TargetDescriptorV3 { + element_ref: Some("e12".into()), + role: Some(role.into()), + name: Some(name.into()), + ctx: Some("表单".into()), + unmatched: false, + } + } + + fn trace(steps: Vec) -> TraceV3 { + TraceV3 { + version: TRACE_VERSION_V3, + purpose: Some("提交工单".into()), + started_at: None, + recorded_at: "2026-09-12T09:31:00Z".into(), + stopped_by: StopReason::UserFinish, + entry: TraceEntry { + start_url: "https://ticket.corp.example/".into(), + }, + recorder: RecorderInfo { + bsk: "0.2.1".into(), + vom: VOM_FORMAT_VERSION, + }, + states: Vec::new(), + steps, + } + } + + #[test] + fn element_refs_never_survive_derivation() { + let derived = derive_workflow( + &trace(vec![StepV3::Click { + common: common(1), + target: target("button", "提交"), + }]), + "submit", + "ticket.corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains("\"ref\""), "{json}"); + assert!(!json.contains("e12"), "{json}"); + let step = &derived.workflow.steps[1]; + assert_eq!( + step.target.as_ref().unwrap().role.as_deref(), + Some("button") + ); + assert_eq!(step.target.as_ref().unwrap().ctx.as_deref(), Some("表单")); + } + + #[test] + fn redacted_fill_becomes_a_valueless_secret_parameter() { + let derived = derive_workflow( + &trace(vec![StepV3::Fill { + common: common(1), + target: target("textbox", "password"), + value: "hunter2".into(), + commit: FillCommit::Blur, + redacted: true, + }]), + "login", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains("hunter2"), "{json}"); + let param = &derived.workflow.params[0]; + assert_eq!(param.name, "password"); + assert!(param.secret && param.required); + assert_eq!(derived.workflow.steps[1].value, None); + assert_eq!( + derived.workflow.steps[1].value_from.as_deref(), + Some("password") + ); + assert!(!derived.workflow.secrets_policy.store_values); + assert_eq!( + derived.workflow.secrets_policy.redacted_fields, + vec!["password".to_string()] + ); + } + + fn fill_trace(name: &str, value: &str) -> TraceV3 { + trace(vec![StepV3::Fill { + common: common(1), + target: target("textbox", name), + value: value.into(), + commit: FillCommit::Enter, + redacted: false, + }]) + } + + /// Design §6: nothing the user typed is stored. The default policy turns + /// every recorded value into a parameter and drops the value itself. + #[test] + fn recorded_values_become_parameters_by_default() { + let derived = derive_workflow( + &fill_trace("title", "打印机坏了"), + "submit", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains("打印机坏了"), "{json}"); + let step = &derived.workflow.steps[1]; + assert_eq!(step.value, None); + assert_eq!(step.value_from.as_deref(), Some("title")); + assert!(step.needs_review); + assert_eq!(step.commit.as_deref(), Some("enter")); + let param = &derived.workflow.params[0]; + assert_eq!(param.name, "title"); + assert!(param.required && !param.secret); + } + + #[test] + fn inline_values_keeps_ordinary_constants() { + let derived = derive_workflow( + &fill_trace("title", "打印机坏了"), + "submit", + "corp.example", + None, + ValuePolicy::Inline, + ); + let step = &derived.workflow.steps[1]; + assert_eq!(step.value.as_deref(), Some("打印机坏了")); + assert!(step.value_from.is_none()); + assert!(step.needs_review); + assert!(derived.workflow.params.is_empty()); + } + + /// `--inline-values` is not a licence to store personal data: the obvious + /// shapes are parameterised even when inlining was requested. + #[test] + fn inline_values_still_refuses_personal_and_credential_shapes() { + for (label, value, reason) in [ + ("email", "someone@corp.example", "email address"), + ("phone", "+86 138 0013 8000", "phone number"), + ( + "token", + "Authorization: Bearer abc123def456", + "credential material", + ), + ] { + let derived = derive_workflow( + &fill_trace(label, value), + "x", + "corp.example", + None, + ValuePolicy::Inline, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains(value), "{label} leaked: {json}"); + let step = &derived.workflow.steps[1]; + assert_eq!(step.value, None, "{label}"); + assert!(step.value_from.is_some(), "{label}"); + assert!( + step.note.as_deref().unwrap_or_default().contains(reason), + "{label}: {:?}", + step.note + ); + } + } + + fn select_trace(name: &str, value: &str) -> TraceV3 { + use bsk_protocol::tools::SelectedOptionV3; + trace(vec![StepV3::Select { + common: common(1), + target: target("combobox", name), + selection: vec![SelectedOptionV3 { + value: value.into(), + label: Some("中文".into()), + }], + }]) + } + + /// S2: an option's `value` attribute is a constant the site defines, not + /// the recorder's data. Parameterising it made the next agent guess a value + /// the recording already knew, so a select inlines under either policy. + #[test] + fn select_values_are_inlined_under_either_policy() { + for policy in [ValuePolicy::Parameterise, ValuePolicy::Inline] { + let derived = derive_workflow( + &select_trace("priority", "P2"), + "x", + "corp.example", + None, + policy, + ); + let step = &derived.workflow.steps[1]; + assert_eq!(step.value.as_deref(), Some("P2"), "{policy:?}"); + assert_eq!(step.value_from, None, "{policy:?}"); + assert!(derived.workflow.params.is_empty(), "{policy:?}"); + assert!(!step.needs_review, "{policy:?}"); + } + } + + /// Inlining a select value is not a licence to store credential material. + #[test] + fn a_secret_looking_select_value_still_becomes_a_parameter() { + let derived = derive_workflow( + &select_trace("token", "Authorization: Bearer abc123def456"), + "x", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains("abc123def456"), "{json}"); + let step = &derived.workflow.steps[1]; + assert_eq!(step.value, None); + assert_eq!(step.value_from.as_deref(), Some("token")); + assert!(step.needs_review); + } + + /// S2: the VOM appends widget state to an accessible name. Carrying it into + /// the identifier produced `zh_has_submenu`, which describes nothing the + /// next agent can act on. + /// D4: the typed search term never lands on disk as a value, but the + /// results page URL the recorder captured next embeds it percent-encoded. + #[test] + fn a_navigation_that_embeds_a_kept_out_value_is_dropped() { + let derived = derive_workflow( + &trace(vec![ + StepV3::Fill { + common: common(1), + target: target("searchbox", "搜索维基百科"), + value: "珠穆朗玛峰".into(), + commit: FillCommit::Enter, + redacted: false, + }, + StepV3::Navigate { + common: common(2), + to: "https://zh.wikipedia.org/w/index.php?search=%E7%8F%A0%E7%A9%86%E6%9C%97%E7%8E%9B%E5%B3%B0&go=Go".into(), + cause: NavigationCause::FormSubmit, + }, + // Typed into the address bar, so it is the agent's own step — + // but the URL still carries the kept-out value. + StepV3::Navigate { + common: common(3), + to: "https://zh.wikipedia.org/w/index.php?search=珠穆朗玛峰".into(), + cause: NavigationCause::UserTyped, + }, + StepV3::Navigate { + common: common(4), + to: "https://zh.wikipedia.org/wiki/Main".into(), + cause: NavigationCause::UserTyped, + }, + ]), + "wf", + "ticket.corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(!json.contains("search="), "{json}"); + assert!(!json.contains("珠穆朗玛峰"), "{json}"); + assert!(json.contains("wiki/Main"), "{json}"); + // one effect navigation + one address-bar URL embedding the value + assert_eq!(derived.dropped_steps, 2); + let notes = &derived.workflow.review_notes; + assert!( + notes.iter().any(|n| n.contains("embeds the value")), + "{notes:?}" + ); + assert!( + notes + .iter() + .any(|n| n.contains("in response to a recorded click")), + "{notes:?}" + ); + } + + #[test] + fn state_annotations_never_reach_a_step_anchor() { + let derived = derive_workflow( + &fill_trace("搜索维基百科 [expanded]", "x"), + "wf", + "ticket.corp.example", + None, + ValuePolicy::Parameterise, + ); + let json = serde_json::to_string(&derived.workflow).unwrap(); + assert!(json.contains("\"name\":\"搜索维基百科\""), "{json}"); + assert!(!json.contains("[expanded]"), "{json}"); + } + + #[test] + fn state_annotations_never_reach_a_parameter_name() { + let derived = derive_workflow( + &fill_trace("ZH [has-submenu]", "x"), + "x", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + assert_eq!(derived.workflow.params[0].name, "zh"); + } + + /// A label that slugs to nothing gets a per-step name. The old shared + /// `value` fallback merged every unnamed field in a flow into one + /// parameter, so two different boxes took the same input. + #[test] + fn unsluggable_labels_get_one_parameter_per_step() { + let derived = derive_workflow( + &trace(vec![ + StepV3::Fill { + common: common(1), + target: target("searchbox", "搜索维基百科"), + value: "a".into(), + commit: FillCommit::Blur, + redacted: false, + }, + StepV3::Fill { + common: common(2), + target: target("textbox", "备注"), + value: "b".into(), + commit: FillCommit::Blur, + redacted: false, + }, + ]), + "x", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + let names: Vec<&str> = derived + .workflow + .params + .iter() + .map(|p| p.name.as_str()) + .collect(); + assert_eq!(names, vec!["field_2", "field_3"]); + assert_eq!( + derived.workflow.steps[1].value_from.as_deref(), + Some("field_2") + ); + assert_eq!( + derived.workflow.steps[2].value_from.as_deref(), + Some("field_3") + ); + } + + #[test] + fn unmatched_target_becomes_null_and_needs_review() { + let derived = derive_workflow( + &trace(vec![StepV3::Click { + common: common(1), + target: TargetDescriptorV3 { + element_ref: None, + role: Some("button".into()), + name: Some("提交".into()), + ctx: None, + unmatched: true, + }, + }]), + "submit", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + assert!(derived.workflow.steps[1].target.is_none()); + assert!(derived.workflow.steps[1].needs_review); + assert!( + derived + .workflow + .review_notes + .iter() + .any(|note| note.contains("step 2 has no anchor")), + "{:?}", + derived.workflow.review_notes + ); + } + + /// S2: a recording routinely ends up on `about:blank` — backing out past + /// the page it started on does exactly that. One such step used to fail the + /// whole derivation and throw the bundle away. + #[test] + fn unreplayable_navigations_are_dropped_not_fatal() { + let derived = derive_workflow( + &trace(vec![ + StepV3::Navigate { + common: common(1), + to: "https://corp.example/new".into(), + cause: NavigationCause::UserTyped, + }, + StepV3::Navigate { + common: common(2), + to: "about:blank".into(), + cause: NavigationCause::History, + }, + StepV3::Click { + common: common(3), + target: target("button", "提交"), + }, + ]), + "submit", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + assert_eq!(derived.dropped_steps, 1); + let ops: Vec = derived.workflow.steps.iter().map(|s| s.op).collect(); + assert_eq!(ops, vec![StepOp::Navigate, StepOp::Click]); + assert!( + derived + .workflow + .review_notes + .iter() + .any(|note| note.contains("about:blank")), + "{:?}", + derived.workflow.review_notes + ); + // The dropped step must not be validated as a navigate target either. + let raw = serde_json::to_value(&derived.workflow).unwrap(); + let report = + super::super::validate::validate_workflow(&derived.workflow, &raw, "corp.example"); + assert!(report.is_ok(), "{:?}", report.errors); + } + + /// S2: a workflow whose first step is a `select` never says which page to + /// be on. The entry URL is the one thing the recording knows and the next + /// agent cannot guess. + #[test] + fn a_workflow_that_does_not_start_on_a_navigation_gets_the_entry_url() { + let derived = derive_workflow( + &select_trace("lang", "zh"), + "x", + "wikipedia.org", + None, + ValuePolicy::Parameterise, + ); + let first = &derived.workflow.steps[0]; + assert_eq!(first.op, StepOp::Navigate); + assert_eq!(first.to.as_deref(), Some("https://ticket.corp.example/")); + assert_eq!(derived.workflow.steps[1].op, StepOp::Select); + + // A recording that already opens with a navigation is left alone. + let native = derive_workflow( + &trace(vec![StepV3::Navigate { + common: common(1), + to: "https://corp.example/new".into(), + cause: NavigationCause::UserTyped, + }]), + "x", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + assert_eq!(native.workflow.steps.len(), 1); + assert_eq!( + native.workflow.steps[0].to.as_deref(), + Some("https://corp.example/new") + ); + } + + #[test] + fn scroll_and_switch_tab_are_dropped_and_steps_renumbered() { + let derived = derive_workflow( + &trace(vec![ + StepV3::Navigate { + common: common(1), + to: "https://corp.example/new".into(), + cause: NavigationCause::UserTyped, + }, + StepV3::Scroll { common: common(2) }, + StepV3::SwitchTab { common: common(3) }, + StepV3::Click { + common: common(4), + target: target("button", "提交"), + }, + ]), + "submit", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + assert_eq!(derived.dropped_steps, 2); + let ns: Vec = derived.workflow.steps.iter().map(|s| s.n).collect(); + assert_eq!(ns, vec![1, 2]); + assert_eq!(derived.workflow.steps[1].op, StepOp::Click); + } + + #[test] + fn press_keeps_key_and_modifiers() { + use bsk_protocol::tools::KeyModifier; + let derived = derive_workflow( + &trace(vec![StepV3::Press { + common: common(1), + key: "Enter".into(), + modifiers: Some(vec![KeyModifier::Ctrl, KeyModifier::Shift]), + target: None, + }]), + "x", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + let step = &derived.workflow.steps[1]; + assert_eq!(step.key.as_deref(), Some("Enter")); + assert_eq!( + step.modifiers.as_deref(), + Some(["ctrl".to_string(), "shift".to_string()].as_slice()) + ); + } + + /// S2: the host is where the recording *started*. Reading the first + /// navigate instead filed a search begun on `www.wikipedia.org` under the + /// article host it landed on, and then rejected the real start host. + #[test] + fn host_comes_from_the_entry_url_before_any_navigation() { + let mut wandered = trace(vec![StepV3::Navigate { + common: common(1), + to: "https://zh.wikipedia.org/wiki/x".into(), + cause: NavigationCause::Browser, + }]); + wandered.entry.start_url = "https://www.wikipedia.org/".into(); + assert_eq!(infer_host(&wandered).as_deref(), Some("www.wikipedia.org")); + + assert_eq!( + infer_host(&trace(Vec::new())).as_deref(), + Some("ticket.corp.example") + ); + + // A recording armed on a blank tab falls back to the first real page. + let mut blank = trace(vec![StepV3::Navigate { + common: common(1), + to: "https://ticket.corp.example:8443/new?a=1".into(), + cause: NavigationCause::Link, + }]); + blank.entry.start_url = "about:blank".into(); + assert_eq!(infer_host(&blank).as_deref(), Some("ticket.corp.example")); + + let mut nothing = trace(Vec::new()); + nothing.entry.start_url = "about:blank".into(); + assert_eq!(infer_host(¬hing), None); + } + + #[test] + fn secret_parameter_names_are_unique_and_ascii() { + let derived = derive_workflow( + &trace(vec![ + StepV3::Fill { + common: common(1), + target: target("textbox", "密码"), + value: "a".into(), + commit: FillCommit::Blur, + redacted: true, + }, + StepV3::Fill { + common: common(2), + target: target("textbox", "确认密码"), + value: "a".into(), + commit: FillCommit::Blur, + redacted: true, + }, + ]), + "login", + "corp.example", + None, + ValuePolicy::Parameterise, + ); + // Both labels slug to nothing, so the second reuses the first's name. + assert_eq!(derived.workflow.params.len(), 1); + assert_eq!(derived.workflow.params[0].name, "secret"); + } + + #[test] + fn slug_param_keeps_ascii_words_only() { + assert_eq!(slug_param("Ticket Title!"), "ticket_title"); + assert_eq!(slug_param("密码"), ""); + assert_eq!(slug_param(" "), ""); + // VOM state annotations describe the widget, not the field. + assert_eq!(slug_param("ZH [has-submenu]"), "zh"); + assert_eq!(slug_param("Search [expanded] [empty]"), "search"); + assert_eq!(slug_param("搜索维基百科 [expanded]"), ""); + assert_eq!(strip_state_annotations("a [x] b"), "a b"); + // An unclosed bracket swallows the rest rather than leaking it. + assert_eq!(strip_state_annotations("name [oops"), "name"); + } +} diff --git a/crates/bsk-cli/src/cli/site/mod.rs b/crates/bsk-cli/src/cli/site/mod.rs new file mode 100644 index 00000000..61ddbc56 --- /dev/null +++ b/crates/bsk-cli/src/cli/site/mod.rs @@ -0,0 +1,999 @@ +//! `bsk site` — local, per-host memory distilled from recordings. +//! +//! Site memory turns one human exploration into an asset the next agent can +//! read: a constrained `SITE.md`, structured workflows derived from a recorded +//! `trace.json`, and append-only candidate observations. Design §4. +//! +//! Every command in this family is pure local file I/O under +//! `$BSK_HOME/sites`. None of them starts or needs the daemon, so they work +//! unchanged in a sandbox running with `BSK_AUTO_START=0`. +//! +//! Three disciplines the agent-facing skill repeats, enforced here: +//! +//! - **Read before acting.** `bsk site context` is the first call of a task. +//! - **Never explore to learn.** Memory records what a task already revealed. +//! - **Learning never fails the task.** These commands are advisory; a failure +//! is reported and the task continues. + +pub mod checkpoint; +pub mod derive; +pub mod model; +pub mod render; +pub mod store; +pub mod trace_input; +pub mod validate; + +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use clap::{Args, Subcommand}; + +use crate::cli::error::{CliError, Format}; + +use model::{ + Candidate, CandidateKind, CandidateStatus, CheckpointReason, StepOp, Workflow, today_utc, +}; +use render::{ + CandidateAddOutput, CheckpointOutput, ContextOutput, DraftInfo, SaveOutput, VerifyOutput, + WorkflowIndexEntry, +}; +use store::{Draft, HostKey, SiteStore}; + +// --------------------------------------------------------------------------- +// Command tree +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Args)] +pub struct SiteCmd { + #[command(subcommand)] + pub sub: SiteSub, +} + +#[derive(Debug, Clone, Subcommand)] +pub enum SiteSub { + /// Read everything known about a host: SITE.md, references, workflows, + /// pending candidates and the current revision. Call this first. + Context(ContextArgs), + + /// Inspect and record reusable workflows. + Workflow(WorkflowCmd), + + /// Record and read candidate observations. + Candidate(CandidateCmd), + + /// Promote a task's draft into active memory. + Checkpoint(CheckpointArgs), +} + +#[derive(Debug, Clone, Args)] +pub struct ContextArgs { + /// Site host, e.g. `ticket.corp.example`. A full URL is accepted. + #[arg(long)] + pub host: String, + + /// Task id. Supplying it stages a draft so edits stay isolated per task. + #[arg(long)] + pub task: Option, +} + +#[derive(Debug, Clone, Args)] +pub struct WorkflowCmd { + #[command(subcommand)] + pub sub: WorkflowSub, +} + +#[derive(Debug, Clone, Subcommand)] +pub enum WorkflowSub { + /// Derive a workflow draft from a recorded `trace.json`. + Save(WorkflowSaveArgs), + /// List stored workflows with their verification age. + List(WorkflowListArgs), + /// Print one workflow as numbered steps, or as JSON with `--json`. + Show(WorkflowShowArgs), + /// Record a pass/fail assertion against a stored workflow. + Verify(WorkflowVerifyArgs), +} + +#[derive(Debug, Clone, Args)] +pub struct WorkflowSaveArgs { + /// Path to a `trace.json` produced by `bsk record start --output `. + #[arg(long = "from")] + pub from: PathBuf, + + /// Workflow id: lowercase letters, digits, `-` and `_`. + #[arg(long)] + pub id: String, + + /// Host to file the workflow under. Inferred from the trace when omitted. + #[arg(long)] + pub host: Option, + + /// One line describing what the workflow accomplishes. + #[arg(long)] + pub purpose: Option, + + /// Task id owning the draft. Defaults to the workflow id. + #[arg(long)] + pub task: Option, + + /// Keep recorded field values as constants instead of turning them into + /// parameters. Use only when the values belong to the flow rather than to + /// whoever recorded it; values that look like credentials, email addresses + /// or identifiers stay parameters regardless. + #[arg(long = "inline-values")] + pub inline_values: bool, +} + +#[derive(Debug, Clone, Args)] +pub struct WorkflowListArgs { + /// Restrict to one host. Omit to list every host with stored workflows. + #[arg(long)] + pub host: Option, +} + +#[derive(Debug, Clone, Args)] +pub struct WorkflowShowArgs { + /// Workflow id. + pub id: String, + + /// Site host. + #[arg(long)] + pub host: String, +} + +#[derive(Debug, Clone, Args)] +#[command(group( + clap::ArgGroup::new("outcome").required(true).args(["pass", "fail"]) +))] +pub struct WorkflowVerifyArgs { + /// Workflow id. + pub id: String, + + /// Site host. + #[arg(long)] + pub host: String, + + /// The workflow reproduced the flow: refresh `lastVerified` to today. + #[arg(long)] + pub pass: bool, + + /// The workflow did not reproduce the flow: record a candidate. The + /// workflow itself is left untouched — one failure is not evidence. + #[arg(long)] + pub fail: bool, + + /// What went wrong. Required with `--fail`. + #[arg(long)] + pub note: Option, + + /// Candidate kind recorded for a failure. + #[arg(long, default_value = "repeated_mistake")] + pub kind: CandidateKind, +} + +#[derive(Debug, Clone, Args)] +pub struct CandidateCmd { + #[command(subcommand)] + pub sub: CandidateSub, +} + +#[derive(Debug, Clone, Subcommand)] +pub enum CandidateSub { + /// Record one observation. Written to active memory immediately. + Add(CandidateAddArgs), + /// List recorded observations. + List(CandidateListArgs), + /// Print one observation. + Show(CandidateShowArgs), +} + +#[derive(Debug, Clone, Args)] +pub struct CandidateAddArgs { + /// Site host. + #[arg(long)] + pub host: String, + + /// What kind of observation this is. + #[arg(long)] + pub kind: CandidateKind, + + /// The claim, in one sentence. + #[arg(long)] + pub claim: String, + + /// What was observed that supports the claim. + #[arg(long)] + pub evidence: Option, + + /// What happens if the claim is ignored. + #[arg(long)] + pub consequence: Option, +} + +#[derive(Debug, Clone, Args)] +pub struct CandidateListArgs { + /// Site host. + #[arg(long)] + pub host: String, + + /// Restrict to one kind. + #[arg(long)] + pub kind: Option, + + /// Restrict to one status. + #[arg(long)] + pub status: Option, +} + +#[derive(Debug, Clone, Args)] +pub struct CandidateShowArgs { + /// Candidate id. + pub id: String, + + /// Site host. + #[arg(long)] + pub host: String, +} + +#[derive(Debug, Clone, Args)] +pub struct CheckpointArgs { + /// Site host. + #[arg(long)] + pub host: String, + + /// Task id whose draft is being promoted. + #[arg(long)] + pub task: String, + + /// Why memory is changing. + #[arg(long)] + pub reason: CheckpointReason, + + /// Revision this edit was based on. Defaults to the draft's base revision. + #[arg(long = "expected-revision")] + pub expected_revision: Option, + + /// Candidate to promote. Repeatable. + #[arg(long = "ingest")] + pub ingest: Vec, + + /// Candidate to dismiss. Repeatable. + #[arg(long = "reject")] + pub reject: Vec, +} + +// --------------------------------------------------------------------------- +// Dispatch +// --------------------------------------------------------------------------- + +pub fn dispatch(cmd: SiteCmd, format: Format) -> Result<(), CliError> { + match cmd.sub { + SiteSub::Context(args) => run(|store| context(store, &args, format)), + SiteSub::Workflow(cmd) => match cmd.sub { + WorkflowSub::Save(args) => run(|store| workflow_save(store, &args, format)), + WorkflowSub::List(args) => run(|store| workflow_list(store, &args, format)), + WorkflowSub::Show(args) => run(|store| workflow_show(store, &args, format)), + WorkflowSub::Verify(args) => run(|store| workflow_verify(store, &args, format)), + }, + SiteSub::Candidate(cmd) => match cmd.sub { + CandidateSub::Add(args) => run(|store| candidate_add(store, &args, format)), + CandidateSub::List(args) => run(|store| candidate_list(store, &args, format)), + CandidateSub::Show(args) => run(|store| candidate_show(store, &args, format)), + }, + SiteSub::Checkpoint(args) => run(|store| checkpoint_cmd(store, &args, format)), + } +} + +/// Open the store and run one command, funnelling every failure through the +/// same renderer so `--json` callers always get a parseable body. +fn run(body: impl FnOnce(&SiteStore) -> Result) -> Result<(), CliError> { + let store = match SiteStore::open() { + Ok(store) => store, + Err(err) => return Err(fail(format!("{err:#}"))), + }; + match body(&store) { + Ok(Outcome::Done) => Ok(()), + Ok(Outcome::Unsuccessful) => Err(CliError::RenderedExit { exit_code: 1 }), + Err(err) => Err(fail(format!("{err:#}"))), + } +} + +/// Whether a command that already printed its result should exit non-zero. +enum Outcome { + Done, + /// The command ran and reported a negative answer (a conflict, a missing + /// workflow). Output is already rendered. + Unsuccessful, +} + +/// Render a site-memory failure through the CLI's standard error path. +/// +/// Routing through [`CliError::Rpc`] keeps `--json` consumers on the one +/// documented envelope (`{code, message, hint, exit_code, data}`) instead of a +/// second bespoke shape. `invalid_params` is the honest code: every failure +/// here is the caller asking for something the rules do not allow. +fn fail(message: String) -> CliError { + CliError::Rpc { + code: bsk_protocol::ErrorCode::InvalidParams, + message, + data: Some(serde_json::json!({ "reason": "site_memory" })), + source: None, + } +} + +fn emit(value: &T) -> Result<()> { + println!( + "{}", + serde_json::to_string_pretty(value).context("encode JSON output")? + ); + Ok(()) +} + +fn draft_info(draft: &Draft) -> DraftInfo { + DraftInfo { + path: draft.dir.display().to_string(), + task_id: draft.context.task_id.clone(), + base_revision: draft.context.base_revision, + read_only: draft.context.read_only, + created: draft.created, + rebased: draft.rebased, + hint: draft.rebased.then(|| { + format!( + "draft SITE.md re-seeded from revision {}; replay your edits before checkpoint", + draft.context.base_revision + ) + }), + } +} + +// --------------------------------------------------------------------------- +// context +// --------------------------------------------------------------------------- + +fn context(store: &SiteStore, args: &ContextArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + let active = store.host_dir(&host)?; + + // One lock around the whole read: the reported revision, the files read and + // the draft's base revision must all come from the same moment, or the + // agent checkpoints against a number that was stale when it was printed. + let _lock = store.lock()?; + let revision = store.revision(&host)?; + let draft = match &args.task { + Some(task) => { + let draft = store::ensure_draft(store, task, &host)?; + // Rebase an existing draft onto the revision being reported. This + // is the recovery step a conflict tells the agent to perform, and + // it re-seeds the staged prose from what is actually published. + Some(store::rebase_draft(store, &host, draft, revision)?) + } + None => None, + }; + + let (workflows, mut problems) = + store::load_workflows(&store::sub(&active, store::WORKFLOWS_DIR)?)?; + let (candidates, candidate_problems) = + store::load_candidates(&store::sub(&active, store::CANDIDATES_DIR)?)?; + problems.extend(candidate_problems); + + // A workflow saved this task but not yet checkpointed is real and usable; + // omitting it made `workflow save` followed by `context` report zero + // workflows, which reads as "the save did nothing". + let mut index: Vec = workflows + .iter() + .map(WorkflowIndexEntry::from_workflow) + .collect(); + if let Some(draft) = &draft { + let (staged, staged_problems) = + store::load_workflows(&store::sub(&draft.dir, store::WORKFLOWS_DIR)?)?; + problems.extend(staged_problems); + for workflow in &staged { + if !workflows.iter().any(|active| active.id == workflow.id) { + index.push(WorkflowIndexEntry::from_draft_workflow(workflow)); + } + } + index.sort_by(|a, b| a.id.cmp(&b.id)); + } + + let out = ContextOutput { + host: host.display.clone(), + revision, + read_only: host.read_only, + site_md: store::read_optional(&store::sub(&active, store::SITE_MD)?)?, + references: store::list_stems(&store::sub(&active, store::REFERENCES_DIR)?, "md")?, + workflows: index, + pending_candidates: candidates + .iter() + .filter(|c| c.status == CandidateStatus::Pending) + .count(), + draft: draft.as_ref().map(draft_info), + }; + + match format { + Format::Json => emit(&out)?, + Format::Human => render::print_context(&out), + } + for problem in problems { + eprintln!("warning: unreadable site-memory file — {problem}"); + } + Ok(Outcome::Done) +} + +// --------------------------------------------------------------------------- +// workflow save +// --------------------------------------------------------------------------- + +fn workflow_save(store: &SiteStore, args: &WorkflowSaveArgs, format: Format) -> Result { + store::validate_id(&args.id, "workflow")?; + let trace = trace_input::read_trace_v3(&args.from)?; + + let raw_host = match &args.host { + Some(host) => host.clone(), + None => derive::infer_host(&trace).context( + "could not infer a host from the trace: it has no navigate step and no entry URL. \ + Pass --host explicitly.", + )?, + }; + let host = store::normalize_host(&raw_host); + store::guard_host(&host)?; + // `--host` is a label for the directory, not a licence to refile a + // recording: a trace captured on an SSO domain must not be saved under an + // innocuous host. Compare against what the recording actually visited. + if let Some(recorded) = derive::infer_host(&trace) { + let recorded = store::normalize_host(&recorded); + store::guard_host(&recorded)?; + if recorded.dir != host.dir { + anyhow::bail!( + "the recording starts on {} but --host says {}. Save it under the host it was \ + recorded on, or re-record on the intended site.", + recorded.display, + host.display + ); + } + } + + let policy = if args.inline_values { + derive::ValuePolicy::Inline + } else { + derive::ValuePolicy::Parameterise + }; + let derived = derive::derive_workflow( + &trace, + &args.id, + &host.display, + args.purpose.as_deref(), + policy, + ); + let raw = serde_json::to_value(&derived.workflow).context("encode derived workflow")?; + let report = validate::validate_workflow(&derived.workflow, &raw, &host.display); + if !report.is_ok() { + anyhow::bail!( + "the derived workflow is not valid:\n - {}", + report.errors.join("\n - ") + ); + } + + let task = args.task.clone().unwrap_or_else(|| args.id.clone()); + let _lock = store.lock()?; + let draft = store::ensure_draft(store, &task, &host)?; + let name = format!("{}.json", args.id); + let path = store::resolve_in( + &store::sub(&draft.dir, store::WORKFLOWS_DIR)?, + &[name.as_str()], + )?; + store::write_json(&path, &derived.workflow)?; + + let out = SaveOutput { + id: args.id.clone(), + host: host.display.clone(), + path: path.display().to_string(), + steps: derived.workflow.steps.len(), + dropped_steps: derived.dropped_steps, + params: derived + .workflow + .params + .iter() + .map(|p| p.name.clone()) + .collect(), + needs_review: derived.workflow.needs_review, + review_notes: derived.workflow.review_notes.clone(), + draft: draft_info(&draft), + }; + match format { + Format::Json => emit(&out)?, + Format::Human => print_save(&out), + } + for warning in report.warnings { + eprintln!("warning: {warning}"); + } + Ok(Outcome::Done) +} + +fn print_save(out: &SaveOutput) { + println!("saved workflow {} for {}", out.id, out.host); + println!(" {}", out.path); + println!( + " {} steps derived, {} dropped (scroll / tab switches are not semantic steps)", + out.steps, out.dropped_steps + ); + if out.params.is_empty() { + println!(" parameters: (none)"); + } else { + println!(" parameters: {}", out.params.join(", ")); + } + for note in &out.review_notes { + println!(" note: {note}"); + } + if out.needs_review { + println!( + "\nThis is a draft and still needs review. Recorded values were turned into \ + parameters rather than stored, and every anchorless step is flagged. Edit {}, then \ + run `bsk site checkpoint --host {} --task {} --reason direct_correction`.", + out.path, out.host, out.draft.task_id + ); + } else { + println!( + "\nRun `bsk site checkpoint --host {} --task {} --reason direct_correction` to publish it.", + out.host, out.draft.task_id + ); + } +} + +// --------------------------------------------------------------------------- +// workflow list / show +// --------------------------------------------------------------------------- + +fn workflow_list(store: &SiteStore, args: &WorkflowListArgs, format: Format) -> Result { + let hosts = match &args.host { + Some(host) => { + let host = store::normalize_host(host); + store::guard_host(&host)?; + vec![host] + } + None => all_hosts(store)?, + }; + let mut rows = Vec::new(); + for host in &hosts { + let dir = store::sub(&store.host_dir(host)?, store::WORKFLOWS_DIR)?; + let (workflows, _) = store::load_workflows(&dir)?; + for workflow in workflows { + rows.push(( + host.display.clone(), + WorkflowIndexEntry::from_workflow(&workflow), + )); + } + } + match format { + Format::Json => { + let body: Vec = rows + .iter() + .map(|(host, entry)| { + let mut value = serde_json::to_value(entry).unwrap_or_default(); + if let Some(map) = value.as_object_mut() { + map.insert("host".into(), serde_json::Value::String(host.clone())); + } + value + }) + .collect(); + emit(&body)?; + } + Format::Human => { + if rows.is_empty() { + println!("(no workflows recorded yet)"); + } + for (host, entry) in &rows { + println!("{host} {}", render::render_index_entry(entry)); + } + } + } + Ok(Outcome::Done) +} + +/// Every host directory under `sites/`, skipping dot-prefixed bookkeeping. +fn all_hosts(store: &SiteStore) -> Result> { + let entries = match std::fs::read_dir(store.root()) { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(err) => { + return Err( + anyhow::Error::from(err).context(format!("read {}", store.root().display())) + ); + } + }; + let mut hosts = Vec::new(); + for entry in entries { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + let Some(name) = entry.file_name().to_str().map(str::to_string) else { + continue; + }; + if name.starts_with('.') { + continue; + } + hosts.push(HostKey { + display: name.clone(), + dir: name, + read_only: false, + }); + } + hosts.sort_by(|a, b| a.dir.cmp(&b.dir)); + Ok(hosts) +} + +fn workflow_show(store: &SiteStore, args: &WorkflowShowArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + let dir = store::sub(&store.host_dir(&host)?, store::WORKFLOWS_DIR)?; + let Some(workflow) = store::load_workflow(&dir, &args.id)? else { + return Err(anyhow::anyhow!( + "no workflow {} on {}. Run `bsk site workflow list --host {}` to see what exists.", + args.id, + host.display, + host.display + )); + }; + match format { + Format::Json => emit(&workflow)?, + Format::Human => render::print_workflow(&workflow), + } + Ok(Outcome::Done) +} + +// --------------------------------------------------------------------------- +// workflow verify +// --------------------------------------------------------------------------- + +fn workflow_verify( + store: &SiteStore, + args: &WorkflowVerifyArgs, + format: Format, +) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + if host.read_only { + anyhow::bail!( + "{} could not be normalised to a host directory, so it has no active memory to verify", + args.host + ); + } + let _lock = store.lock()?; + let active = store.host_dir(&host)?; + let dir = store::sub(&active, store::WORKFLOWS_DIR)?; + let Some(workflow) = store::load_workflow(&dir, &args.id)? else { + return Err(anyhow::anyhow!( + "no workflow {} on {}", + args.id, + host.display + )); + }; + + let out = if args.pass { + verify_pass(store, &host, &dir, workflow)? + } else { + verify_fail(store, &host, args)? + }; + match format { + Format::Json => emit(&out)?, + Format::Human => print_verify(&out), + } + Ok(Outcome::Done) +} + +fn verify_pass( + store: &SiteStore, + host: &HostKey, + dir: &std::path::Path, + mut workflow: Workflow, +) -> Result { + let today = today_utc(); + workflow.last_verified = Some(today.clone()); + // `--pass` means a human or an agent just ran this workflow end to end on + // the live site. That *is* the review the derived flags were asking for, so + // clearing them here is what stops a workflow containing a `fill` from + // carrying NEEDS REVIEW forever. `--fail` leaves every flag alone. + // An anchorless step stays flagged: a pass proves the flow works around + // it, not that it can be replayed, and the validator refuses to publish + // an unflagged anchorless step — clearing it would make the whole host's + // next checkpoint fail. + let mut cleared_review = workflow.needs_review; + workflow.needs_review = false; + for step in &mut workflow.steps { + if step.needs_review && step.target.is_none() && !matches!(step.op, StepOp::Navigate) { + continue; + } + cleared_review |= step.needs_review; + step.needs_review = false; + } + let name = format!("{}.json", workflow.id); + let path = store::resolve_in(dir, &[name.as_str()])?; + + // Refreshing `lastVerified` changes active memory, so it advances the CAS + // token and lands in the journal like any other commit — and all three + // writes move together, or none of them do. + let revision = store.revision(host)? + 1; + let entry = model::JournalEntry { + revision, + at: model::now_rfc3339(), + host: host.display.clone(), + task_id: None, + reason: "workflow_verify_pass".to_string(), + paths: vec![format!("{}/{}/{name}", host.dir, store::WORKFLOWS_DIR)], + ingested: Vec::new(), + rejected: Vec::new(), + }; + let mut tx = store::Transaction::default(); + if let Err(err) = write_verify_pass(store, host, &mut tx, &path, &workflow, revision, &entry) { + return match tx.rollback() { + Ok(()) => Err(err.context("verify --pass failed; active memory was rolled back")), + Err(rollback) => Err(err.context(format!( + "verify --pass failed and rollback was incomplete ({rollback:#})" + ))), + }; + } + Ok(VerifyOutput { + id: workflow.id, + host: host.display.clone(), + outcome: "pass", + last_verified: Some(today), + candidate_id: None, + revision: Some(revision), + cleared_review, + }) +} + +fn write_verify_pass( + store: &SiteStore, + host: &HostKey, + tx: &mut store::Transaction, + path: &std::path::Path, + workflow: &Workflow, + revision: u64, + entry: &model::JournalEntry, +) -> Result<()> { + tx.write_json(path, workflow)?; + tx.write( + &store.revision_path(host)?, + format!("{revision}\n").as_bytes(), + )?; + let line = serde_json::to_string(entry).context("encode journal entry")?; + tx.append_line(&store.journal_path(host)?, &line) +} + +fn verify_fail( + store: &SiteStore, + host: &HostKey, + args: &WorkflowVerifyArgs, +) -> Result { + let note = args.note.as_deref().unwrap_or_default().trim(); + if note.is_empty() { + anyhow::bail!("--fail needs --note describing what did not reproduce"); + } + if !validate::is_failure_kind(args.kind) { + anyhow::bail!( + "--kind for a failure must be repeated_mistake or access, not {}", + args.kind.as_str() + ); + } + let claim = format!("workflow {} did not reproduce: {note}", args.id); + let candidate = write_candidate( + store, + host, + args.kind, + &claim, + None, + None, + Some(args.id.clone()), + )?; + Ok(VerifyOutput { + id: args.id.clone(), + host: host.display.clone(), + outcome: "fail", + last_verified: None, + candidate_id: Some(candidate.id), + revision: None, + cleared_review: false, + }) +} + +fn print_verify(out: &VerifyOutput) { + match out.outcome { + "pass" => { + println!( + "workflow {} on {} verified; lastVerified is now {}", + out.id, + out.host, + out.last_verified.as_deref().unwrap_or("(unset)") + ); + if out.cleared_review { + println!("review cleared: the run you just made is the review"); + } + if let Some(revision) = out.revision { + println!("revision: {revision}"); + } + } + _ => { + println!( + "recorded candidate {} for workflow {} on {}", + out.candidate_id.as_deref().unwrap_or("(unknown)"), + out.id, + out.host + ); + println!( + "The workflow is unchanged. One failure is not evidence: record a second sighting \ + on a different day before changing site memory." + ); + } + } +} + +// --------------------------------------------------------------------------- +// candidates +// --------------------------------------------------------------------------- + +fn candidate_add(store: &SiteStore, args: &CandidateAddArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + if host.read_only { + anyhow::bail!( + "{} could not be normalised to a host directory; candidates are written to active \ + memory and need a plain hostname", + args.host + ); + } + for (label, text) in [ + ("--claim", Some(args.claim.as_str())), + ("--evidence", args.evidence.as_deref()), + ("--consequence", args.consequence.as_deref()), + ] { + if let Some(text) = text + && let Some(reason) = validate::find_secret(text) + { + anyhow::bail!("{label}: {reason}"); + } + } + if args.claim.trim().is_empty() { + anyhow::bail!("--claim must not be empty"); + } + + let _lock = store.lock()?; + let candidate = write_candidate( + store, + &host, + args.kind, + &args.claim, + args.evidence.clone(), + args.consequence.clone(), + None, + )?; + let path = store + .host_dir(&host)? + .join(store::CANDIDATES_DIR) + .join(format!("{}.json", candidate.id)); + + let out = CandidateAddOutput { + id: candidate.id.clone(), + host: host.display.clone(), + path: path.display().to_string(), + kind: candidate.kind.as_str(), + observed_date_utc: candidate.observed_date_utc.clone(), + }; + match format { + Format::Json => emit(&out)?, + Format::Human => { + println!("recorded candidate {} on {}", out.id, out.host); + println!(" {}", out.path); + if candidate.kind.needs_corroboration() { + println!( + " This kind needs a second observation on a different UTC date before it may \ + change site memory." + ); + } + } + } + Ok(Outcome::Done) +} + +/// Write one candidate into active memory. The caller holds the lock. +fn write_candidate( + store: &SiteStore, + host: &HostKey, + kind: CandidateKind, + claim: &str, + evidence: Option, + consequence: Option, + workflow_id: Option, +) -> Result { + let dir = store::sub(&store.host_dir(host)?, store::CANDIDATES_DIR)?; + let date = today_utc(); + let id = store::next_candidate_id(&dir, &date)?; + let candidate = Candidate { + id, + host: host.display.clone(), + observed_date_utc: date, + kind, + claim: claim.trim().to_string(), + evidence, + consequence, + status: CandidateStatus::Pending, + workflow_id, + }; + let name = format!("{}.json", candidate.id); + let path = store::resolve_in(&dir, &[name.as_str()])?; + store::write_json(&path, &candidate)?; + Ok(candidate) +} + +fn candidate_list(store: &SiteStore, args: &CandidateListArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + let dir = store::sub(&store.host_dir(&host)?, store::CANDIDATES_DIR)?; + let (candidates, problems) = store::load_candidates(&dir)?; + let filtered: Vec<&Candidate> = candidates + .iter() + .filter(|c| args.kind.is_none_or(|kind| c.kind == kind)) + .filter(|c| args.status.is_none_or(|status| c.status == status)) + .collect(); + match format { + Format::Json => emit(&filtered)?, + Format::Human => { + if filtered.is_empty() { + println!("(no candidates match)"); + } + for candidate in &filtered { + println!("{}", render::render_candidate_line(candidate)); + } + } + } + for problem in problems { + eprintln!("warning: unreadable candidate — {problem}"); + } + Ok(Outcome::Done) +} + +fn candidate_show(store: &SiteStore, args: &CandidateShowArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + let dir = store::sub(&store.host_dir(&host)?, store::CANDIDATES_DIR)?; + let (candidates, _) = store::load_candidates(&dir)?; + let Some(candidate) = candidates.iter().find(|c| c.id == args.id) else { + return Err(anyhow::anyhow!( + "no candidate {} on {}", + args.id, + host.display + )); + }; + match format { + Format::Json => emit(candidate)?, + Format::Human => render::print_candidate(candidate), + } + Ok(Outcome::Done) +} + +// --------------------------------------------------------------------------- +// checkpoint +// --------------------------------------------------------------------------- + +fn checkpoint_cmd(store: &SiteStore, args: &CheckpointArgs, format: Format) -> Result { + let host = store::normalize_host(&args.host); + store::guard_host(&host)?; + let out = checkpoint::run(&checkpoint::CheckpointRequest { + store, + host: &host, + task: &args.task, + reason: args.reason, + expected_revision: args.expected_revision, + ingest: &args.ingest, + reject: &args.reject, + })?; + match format { + Format::Json => emit(&out)?, + Format::Human => render::print_checkpoint(&out), + } + match out { + CheckpointOutput::Committed { .. } => Ok(Outcome::Done), + CheckpointOutput::Conflict { .. } => Ok(Outcome::Unsuccessful), + } +} diff --git a/crates/bsk-cli/src/cli/site/model.rs b/crates/bsk-cli/src/cli/site/model.rs new file mode 100644 index 00000000..e62c6098 --- /dev/null +++ b/crates/bsk-cli/src/cli/site/model.rs @@ -0,0 +1,550 @@ +//! Serde model for the local site-memory tree (`$BSK_HOME/sites`). +//! +//! Every type here is persisted verbatim as JSON, so the wire names are +//! camelCase to match design §3.2 / §3.4. `deny_unknown_fields` is deliberate: +//! it is the schema-level half of the "no `@eN` refs in site memory" rule — +//! a derived workflow that still carries a record-local `ref` fails to parse +//! instead of silently persisting a dead anchor. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use time::format_description::well_known::Rfc3339; +use time::{Date, Month, OffsetDateTime}; + +/// Version stamped into every `workflows/.json`. +pub const WORKFLOW_SCHEMA_VERSION: u32 = 1; + +/// A workflow unverified for longer than this is reported as stale. +pub const STALE_AFTER_DAYS: i64 = 30; + +/// `SITE.md` hard budget: a longer file is rejected at checkpoint. +pub const SITE_MD_HARD_LINE_BUDGET: usize = 500; + +/// `SITE.md` soft budget: past this, checkpoint warns and asks for references. +pub const SITE_MD_SOFT_LINE_BUDGET: usize = 200; + +// --------------------------------------------------------------------------- +// Workflow +// --------------------------------------------------------------------------- + +/// Replay strategy. Only `ui-only` exists by design (§6): a workflow may only +/// be reproduced through observe/act inside the Agent Window, never by +/// replaying cookies or intercepting requests. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, Default)] +#[serde(rename_all = "kebab-case")] +pub enum WorkflowStrategy { + #[default] + UiOnly, +} + +/// Semantic anchor for a step target: role + accessible name + context text. +/// +/// The record-local `ref` of [`bsk_protocol::tools::TargetDescriptorV3`] is +/// stripped during derivation and rejected here. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct StepTarget { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub role: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ctx: Option, +} + +impl StepTarget { + pub fn is_empty(&self) -> bool { + self.role.is_none() && self.name.is_none() && self.ctx.is_none() + } + + /// One-line human rendering, e.g. `role=button name="提交" ctx="底部操作栏"`. + pub fn describe(&self) -> String { + let mut parts = Vec::new(); + if let Some(role) = &self.role { + parts.push(format!("role={role}")); + } + if let Some(name) = &self.name { + parts.push(format!("name={name:?}")); + } + if let Some(ctx) = &self.ctx { + parts.push(format!("ctx={ctx:?}")); + } + if parts.is_empty() { + "(no anchor)".to_string() + } else { + parts.join(" ") + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum StepOp { + Navigate, + Click, + Hover, + Fill, + Select, + Press, +} + +impl StepOp { + pub fn as_str(self) -> &'static str { + match self { + StepOp::Navigate => "navigate", + StepOp::Click => "click", + StepOp::Hover => "hover", + StepOp::Fill => "fill", + StepOp::Select => "select", + StepOp::Press => "press", + } + } +} + +/// One replayable step. Flat rather than an internally tagged enum so the +/// on-disk shape matches design §3.2 and so validation can report a precise +/// "op `fill` requires `value` or `valueFrom`" style message. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct WorkflowStep { + /// 1-based position; validated to be dense and ordered. + pub n: u32, + pub op: StepOp, + /// `navigate` destination. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub to: Option, + /// Semantic anchor. `null` when the recorder could not match the element; + /// such a step always carries `needsReview`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub target: Option, + /// Inlined constant for `fill` / `select`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub value: Option, + /// Name of a declared parameter supplying this step's value. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub value_from: Option, + /// How a `fill` is committed (`enter` / `suggestion` / `blur`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub commit: Option, + /// `press` key name. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub key: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub modifiers: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub note: Option, + /// Set by derivation wherever a human must confirm the projection. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub needs_review: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct WorkflowParam { + pub name: String, + #[serde(default)] + pub required: bool, + /// A secret parameter never carries a value on disk (design §3.2). + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub secret: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + #[serde(default, rename = "enum", skip_serializing_if = "Option::is_none")] + pub enum_values: Option>, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct Precondition { + pub kind: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub value: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub evidence: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct PostconditionExpect { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub role: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name_pattern: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct Postcondition { + pub kind: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub expect: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct SourceTrace { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub recorded_at: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub recorder: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub purpose: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, Default)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct SecretsPolicy { + /// Always `false`: recorded input values for redacted fields never land on + /// disk (design §6, mirroring the operation-audit promise). + #[serde(default)] + pub store_values: bool, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub redacted_fields: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct Workflow { + pub schema_version: u32, + pub id: String, + pub host: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub purpose: Option, + #[serde(default)] + pub strategy: WorkflowStrategy, + /// `YYYY-MM-DD` — the structured equivalent of webcmd's `[verified date]`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_verified: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_trace: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub params: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub preconditions: Vec, + pub steps: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub postcondition: Option, + #[serde(default)] + pub secrets_policy: SecretsPolicy, + /// True while any derived step still needs a human pass. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub needs_review: bool, + /// Free-form derivation notes for the agent reading this workflow. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub review_notes: Vec, +} + +impl Workflow { + /// Days since `lastVerified`, or `None` when never verified / unparseable. + pub fn days_since_verified(&self) -> Option { + let verified = parse_date(self.last_verified.as_deref()?)?; + Some(days_between(verified, OffsetDateTime::now_utc().date())) + } + + /// True when a workflow *was* verified, but too long ago. + /// + /// A never-verified workflow is [`Self::is_unverified`], not stale. The two + /// used to be the same flag, which made every freshly saved workflow read + /// back as STALE + NEEDS REVIEW — an agent has no reason to distrust + /// memory it just wrote, and conflating the two taught it to. + pub fn is_stale(&self) -> bool { + match self.days_since_verified() { + Some(days) => days > STALE_AFTER_DAYS, + None => false, + } + } + + /// True when the workflow has never been confirmed against the live site + /// (no `lastVerified`, or one that does not parse). + pub fn is_unverified(&self) -> bool { + self.days_since_verified().is_none() + } +} + +// --------------------------------------------------------------------------- +// Candidates +// --------------------------------------------------------------------------- + +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, clap::ValueEnum, +)] +#[serde(rename_all = "snake_case")] +#[clap(rename_all = "snake_case")] +pub enum CandidateKind { + ActionSpace, + BetterPath, + Access, + /// Exempt from the two-observation rule: one sighting is enough (§3.4). + HighConsequence, + RepeatedMistake, +} + +impl CandidateKind { + pub fn as_str(self) -> &'static str { + match self { + CandidateKind::ActionSpace => "action_space", + CandidateKind::BetterPath => "better_path", + CandidateKind::Access => "access", + CandidateKind::HighConsequence => "high_consequence", + CandidateKind::RepeatedMistake => "repeated_mistake", + } + } + + /// Whether promoting this candidate needs corroboration from a second day. + pub fn needs_corroboration(self) -> bool { + !matches!(self, CandidateKind::HighConsequence) + } +} + +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, clap::ValueEnum, +)] +#[serde(rename_all = "snake_case")] +#[clap(rename_all = "snake_case")] +pub enum CandidateStatus { + Pending, + Ingested, + Rejected, +} + +impl CandidateStatus { + pub fn as_str(self) -> &'static str { + match self { + CandidateStatus::Pending => "pending", + CandidateStatus::Ingested => "ingested", + CandidateStatus::Rejected => "rejected", + } + } +} + +/// Append-only observation. Candidates are never deleted — only their status +/// moves from `pending` to `ingested` / `rejected` at checkpoint (§3.4). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct Candidate { + pub id: String, + pub host: String, + /// `YYYY-MM-DD` in UTC. Two distinct values are what corroboration means. + pub observed_date_utc: String, + pub kind: CandidateKind, + pub claim: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub evidence: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub consequence: Option, + pub status: CandidateStatus, + /// Set when the candidate came from `workflow verify --fail`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workflow_id: Option, +} + +/// Claims compare after trimming, lowercasing and collapsing whitespace so +/// "Submit needs the P2 dropdown" and "submit needs the p2 dropdown" +/// corroborate each other. +pub fn normalize_claim(claim: &str) -> String { + claim + .split_whitespace() + .collect::>() + .join(" ") + .to_lowercase() +} + +// --------------------------------------------------------------------------- +// Draft context and journal +// --------------------------------------------------------------------------- + +/// `.drafts///.context.json`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct DraftContext { + /// True when the host could not be normalised: the draft may be written + /// but never promoted into active memory (§3.1 provisional fallback). + pub read_only: bool, + /// Revision the draft branched from; the default `expectedRevision`. + pub base_revision: u64, + pub host: String, + pub task_id: String, + pub created_at: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct JournalEntry { + pub revision: u64, + pub at: String, + pub host: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub task_id: Option, + pub reason: String, + pub paths: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub ingested: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub rejected: Vec, +} + +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, clap::ValueEnum, +)] +#[serde(rename_all = "snake_case")] +#[clap(rename_all = "snake_case")] +pub enum CheckpointReason { + CandidateIngestion, + DirectCorrection, + MajorRewrite, +} + +impl CheckpointReason { + pub fn as_str(self) -> &'static str { + match self { + CheckpointReason::CandidateIngestion => "candidate_ingestion", + CheckpointReason::DirectCorrection => "direct_correction", + CheckpointReason::MajorRewrite => "major_rewrite", + } + } +} + +// --------------------------------------------------------------------------- +// Dates +// --------------------------------------------------------------------------- + +/// Today in UTC as `YYYY-MM-DD`. +pub fn today_utc() -> String { + format_date(OffsetDateTime::now_utc().date()) +} + +pub fn format_date(date: Date) -> String { + format!( + "{:04}-{:02}-{:02}", + date.year(), + u8::from(date.month()), + date.day() + ) +} + +/// Parse a strict `YYYY-MM-DD` date. Anything looser is rejected so a typo in +/// `[verified …]` surfaces at checkpoint instead of silently ageing out. +pub fn parse_date(text: &str) -> Option { + let bytes = text.as_bytes(); + if bytes.len() != 10 || bytes[4] != b'-' || bytes[7] != b'-' { + return None; + } + if !bytes + .iter() + .enumerate() + .all(|(i, b)| i == 4 || i == 7 || b.is_ascii_digit()) + { + return None; + } + let year: i32 = text[0..4].parse().ok()?; + let month: u8 = text[5..7].parse().ok()?; + let day: u8 = text[8..10].parse().ok()?; + Date::from_calendar_date(year, Month::try_from(month).ok()?, day).ok() +} + +pub fn days_between(from: Date, to: Date) -> i64 { + i64::from(to.to_julian_day()) - i64::from(from.to_julian_day()) +} + +pub fn now_rfc3339() -> String { + OffsetDateTime::now_utc() + .format(&Rfc3339) + .unwrap_or_else(|_| today_utc()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strategy_serialises_as_ui_only() { + let json = serde_json::to_value(WorkflowStrategy::UiOnly).unwrap(); + assert_eq!(json, serde_json::json!("ui-only")); + } + + #[test] + fn step_target_rejects_record_local_ref() { + let err = serde_json::from_value::(serde_json::json!({ + "role": "button", + "ref": "e12" + })) + .unwrap_err(); + assert!(err.to_string().contains("ref"), "{err}"); + } + + #[test] + fn parse_date_is_strict() { + assert!(parse_date("2026-09-15").is_some()); + assert!(parse_date("2026-9-15").is_none()); + assert!(parse_date("2026-13-01").is_none()); + assert!(parse_date("2026-02-30").is_none()); + assert!(parse_date("not-a-date").is_none()); + assert!(parse_date("2026-09-15T00:00:00Z").is_none()); + } + + #[test] + fn days_between_counts_calendar_days() { + let a = parse_date("2026-01-01").unwrap(); + let b = parse_date("2026-03-01").unwrap(); + assert_eq!(days_between(a, b), 59); + assert_eq!(days_between(b, a), -59); + } + + #[test] + fn never_verified_workflow_is_unverified_not_stale() { + let wf = Workflow { + schema_version: WORKFLOW_SCHEMA_VERSION, + id: "x".into(), + host: "example.com".into(), + purpose: None, + strategy: WorkflowStrategy::UiOnly, + last_verified: None, + source_trace: None, + params: Vec::new(), + preconditions: Vec::new(), + steps: Vec::new(), + postcondition: None, + secrets_policy: SecretsPolicy::default(), + needs_review: false, + review_notes: Vec::new(), + }; + assert!(wf.is_unverified(), "no lastVerified means unverified"); + assert!( + !wf.is_stale(), + "a workflow saved a minute ago is not stale; it is simply unverified" + ); + assert_eq!(wf.days_since_verified(), None); + + let fresh = Workflow { + last_verified: Some(today_utc()), + ..wf.clone() + }; + assert!(!fresh.is_unverified()); + assert!(!fresh.is_stale()); + + let old = Workflow { + last_verified: Some(format_date( + OffsetDateTime::now_utc().date() - time::Duration::days(STALE_AFTER_DAYS + 1), + )), + ..wf + }; + assert!(old.is_stale()); + assert!(!old.is_unverified()); + } + + #[test] + fn normalize_claim_collapses_case_and_whitespace() { + assert_eq!( + normalize_claim(" Submit needs\tthe P2 dropdown "), + "submit needs the p2 dropdown" + ); + } + + #[test] + fn high_consequence_skips_corroboration() { + assert!(!CandidateKind::HighConsequence.needs_corroboration()); + assert!(CandidateKind::ActionSpace.needs_corroboration()); + } +} diff --git a/crates/bsk-cli/src/cli/site/render.rs b/crates/bsk-cli/src/cli/site/render.rs new file mode 100644 index 00000000..e01fa5e8 --- /dev/null +++ b/crates/bsk-cli/src/cli/site/render.rs @@ -0,0 +1,592 @@ +//! Output shapes for `bsk site`. +//! +//! Human and `--json` renderings are built from the same payload structs so +//! the two never drift, following the split in `crates/bsk-cli/src/cli/network.rs`. + +use serde::Serialize; + +use super::model::{ + Candidate, STALE_AFTER_DAYS, Workflow, WorkflowStep, WorkflowStrategy, today_utc, +}; + +// --------------------------------------------------------------------------- +// Payloads +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct WorkflowIndexEntry { + pub id: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub purpose: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub last_verified: Option, + /// Verified once, but longer than `STALE_AFTER_DAYS` ago. + pub stale: bool, + /// Never verified against the live site. Distinct from `stale`: a workflow + /// saved a minute ago is unverified, not out of date. + pub unverified: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub days_since_verified: Option, + pub needs_review: bool, + pub steps: usize, + /// True for a workflow that only exists in this task's draft. It is real + /// and readable, but it is not part of active memory until a checkpoint, + /// and the agent that just saved it would otherwise not see it listed. + #[serde(skip_serializing_if = "is_false")] + pub draft: bool, +} + +fn is_false(value: &bool) -> bool { + !*value +} + +impl WorkflowIndexEntry { + pub fn from_workflow(workflow: &Workflow) -> Self { + Self { + id: workflow.id.clone(), + purpose: workflow.purpose.clone(), + last_verified: workflow.last_verified.clone(), + stale: workflow.is_stale(), + unverified: workflow.is_unverified(), + days_since_verified: workflow.days_since_verified(), + needs_review: workflow.needs_review, + steps: workflow.steps.len(), + draft: false, + } + } + + /// Same entry, marked as staged rather than published. + pub fn from_draft_workflow(workflow: &Workflow) -> Self { + Self { + draft: true, + ..Self::from_workflow(workflow) + } + } +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DraftInfo { + pub path: String, + pub task_id: String, + pub base_revision: u64, + pub read_only: bool, + pub created: bool, + /// True when this call re-seeded the draft from a newer published revision + /// because another writer committed first. The staged prose is now theirs, + /// not this task's: `hint` says what to do about it. + #[serde(skip_serializing_if = "is_false")] + pub rebased: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub hint: Option, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ContextOutput { + pub host: String, + pub revision: u64, + pub read_only: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub site_md: Option, + pub references: Vec, + pub workflows: Vec, + pub pending_candidates: usize, + #[serde(skip_serializing_if = "Option::is_none")] + pub draft: Option, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SaveOutput { + pub id: String, + pub host: String, + pub path: String, + pub steps: usize, + pub dropped_steps: usize, + pub params: Vec, + pub needs_review: bool, + pub review_notes: Vec, + pub draft: DraftInfo, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct VerifyOutput { + pub id: String, + pub host: String, + pub outcome: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + pub last_verified: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub candidate_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub revision: Option, + /// True when `--pass` also cleared `needsReview` on the workflow or a step. + pub cleared_review: bool, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct CandidateAddOutput { + pub id: String, + pub host: String, + pub path: String, + pub kind: &'static str, + pub observed_date_utc: String, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(tag = "status", rename_all = "camelCase")] +pub enum CheckpointOutput { + #[serde(rename = "committed")] + Committed { + revision: u64, + host: String, + reason: String, + paths: Vec, + ingested: Vec, + rejected: Vec, + warnings: Vec, + }, + #[serde(rename = "conflict")] + Conflict { expected: u64, actual: u64 }, +} + +// --------------------------------------------------------------------------- +// Human rendering +// --------------------------------------------------------------------------- + +pub fn print_context(out: &ContextOutput) { + println!("host: {} revision: {}", out.host, out.revision); + if out.read_only { + println!( + "read-only: this host could not be normalised, so drafts cannot be promoted into \ + active memory" + ); + } + match &out.site_md { + Some(text) if !text.trim().is_empty() => { + println!("\n--- SITE.md ---"); + print!("{text}"); + if !text.ends_with('\n') { + println!(); + } + println!("--- end SITE.md ---"); + } + _ => println!("\nSITE.md: (empty — nothing has been learned about this host yet)"), + } + + println!("\nreferences ({}):", out.references.len()); + if out.references.is_empty() { + println!(" (none)"); + } else { + for name in &out.references { + println!(" references/{name}.md"); + } + } + + println!("\nworkflows ({}):", out.workflows.len()); + if out.workflows.is_empty() { + println!(" (none)"); + } else { + for entry in &out.workflows { + println!(" {}", render_index_entry(entry)); + } + } + + println!("\npending candidates: {}", out.pending_candidates); + match &out.draft { + Some(draft) => { + println!( + "draft: {} (task {}, base revision {}{})", + draft.path, + draft.task_id, + draft.base_revision, + if draft.created { ", created now" } else { "" } + ); + if let Some(hint) = &draft.hint { + println!(" {hint}"); + } + } + None => println!("draft: (none — pass --task to stage edits)"), + } +} + +pub fn render_index_entry(entry: &WorkflowIndexEntry) -> String { + let verified = match (&entry.last_verified, entry.days_since_verified) { + (Some(date), Some(days)) => format!("verified {date} ({days}d ago)"), + (Some(date), None) => format!("verified {date}"), + _ => "never verified".to_string(), + }; + let mut flags = Vec::new(); + if entry.draft { + flags.push("DRAFT"); + } + if entry.stale { + flags.push("STALE"); + } else if entry.unverified { + // Not the same claim as STALE: nothing here has expired, it has simply + // never been run back. Say so, so the agent still trusts what it saved. + flags.push("UNVERIFIED"); + } + if entry.needs_review { + flags.push("NEEDS REVIEW"); + } + let suffix = if flags.is_empty() { + String::new() + } else { + format!(" [{}]", flags.join(", ")) + }; + format!( + "{} {} steps, {verified}{suffix}{}", + entry.id, + entry.steps, + entry + .purpose + .as_deref() + .map(|p| format!("\n purpose: {p}")) + .unwrap_or_default() + ) +} + +pub fn print_workflow(workflow: &Workflow) { + println!("workflow: {}", workflow.id); + println!("host: {}", workflow.host); + if let Some(purpose) = &workflow.purpose { + println!("purpose: {purpose}"); + } + println!( + "strategy: {}", + match workflow.strategy { + WorkflowStrategy::UiOnly => "ui-only (replay by observe/act only)", + } + ); + match (&workflow.last_verified, workflow.days_since_verified()) { + (Some(date), Some(days)) => { + println!("last verified: {date} ({days} days ago)"); + if days > STALE_AFTER_DAYS { + println!( + "warning: unverified for more than {STALE_AFTER_DAYS} days. Confirm each step \ + against the live page, then run `bsk site workflow verify {} --host {} \ + --pass`.", + workflow.id, workflow.host + ); + } + } + _ => println!( + "last verified: never — treat every step as unconfirmed (today is {})", + today_utc() + ), + } + + if !workflow.params.is_empty() { + println!("\nparameters:"); + for param in &workflow.params { + let mut tags = Vec::new(); + if param.required { + tags.push("required".to_string()); + } + if param.secret { + tags.push("secret, never stored".to_string()); + } + if let Some(values) = ¶m.enum_values { + tags.push(format!("one of {}", values.join(" | "))); + } + println!( + " {}{}{}", + param.name, + if tags.is_empty() { + String::new() + } else { + format!(" ({})", tags.join("; ")) + }, + param + .description + .as_deref() + .map(|d| format!("\n {d}")) + .unwrap_or_default() + ); + } + } + + if !workflow.preconditions.is_empty() { + println!("\npreconditions:"); + for pre in &workflow.preconditions { + let detail = pre + .value + .as_deref() + .or(pre.evidence.as_deref()) + .map(|d| format!(": {d}")) + .unwrap_or_default(); + println!(" {}{detail}", pre.kind); + } + } + + println!("\nsteps:"); + for step in &workflow.steps { + println!("{}", render_step(step)); + } + + if let Some(post) = &workflow.postcondition { + println!("\npostcondition ({}):", post.kind); + for expect in &post.expect { + let mut parts = Vec::new(); + if let Some(role) = &expect.role { + parts.push(format!("role={role}")); + } + if let Some(name) = &expect.name { + parts.push(format!("name={name:?}")); + } + if let Some(pattern) = &expect.name_pattern { + parts.push(format!("namePattern={pattern:?}")); + } + println!(" {}", parts.join(" ")); + } + } + + if !workflow.review_notes.is_empty() { + println!("\nreview notes:"); + for note in &workflow.review_notes { + println!(" - {note}"); + } + } + if workflow.needs_review { + println!( + "\nThis workflow still needs review. Confirm the flagged steps against the live page \ + before relying on them." + ); + } +} + +pub fn render_step(step: &WorkflowStep) -> String { + let mut line = format!(" {}. {}", step.n, step.op.as_str()); + if let Some(to) = &step.to { + line.push_str(&format!(" {to}")); + } + match &step.target { + Some(target) => line.push_str(&format!(" {}", target.describe())), + None if !matches!(step.op, super::model::StepOp::Navigate) => { + line.push_str(" (no anchor)"); + } + None => {} + } + if let Some(key) = &step.key { + line.push_str(&format!(" key={key}")); + if let Some(modifiers) = &step.modifiers { + line.push_str(&format!(" modifiers={}", modifiers.join("+"))); + } + } + if let Some(from) = &step.value_from { + line.push_str(&format!(" <- ${from}")); + } else if let Some(value) = &step.value { + line.push_str(&format!(" = {value:?}")); + } + if let Some(commit) = &step.commit { + line.push_str(&format!(" (commit: {commit})")); + } + if step.needs_review { + line.push_str(" [needs review]"); + } + if let Some(note) = &step.note { + line.push_str(&format!("\n note: {note}")); + } + line +} + +pub fn print_candidate(candidate: &Candidate) { + println!("candidate: {}", candidate.id); + println!("host: {}", candidate.host); + println!("kind: {}", candidate.kind.as_str()); + println!("status: {}", candidate.status.as_str()); + println!("observed (UTC): {}", candidate.observed_date_utc); + if let Some(id) = &candidate.workflow_id { + println!("workflow: {id}"); + } + println!("claim: {}", candidate.claim); + if let Some(evidence) = &candidate.evidence { + println!("evidence: {evidence}"); + } + if let Some(consequence) = &candidate.consequence { + println!("consequence: {consequence}"); + } +} + +pub fn render_candidate_line(candidate: &Candidate) -> String { + format!( + "{} [{}] {} {} {}", + candidate.id, + candidate.status.as_str(), + candidate.kind.as_str(), + candidate.observed_date_utc, + candidate.claim + ) +} + +pub fn print_checkpoint(out: &CheckpointOutput) { + match out { + CheckpointOutput::Committed { + revision, + host, + reason, + paths, + ingested, + rejected, + warnings, + } => { + println!("committed {host} at revision {revision} ({reason})"); + for path in paths { + println!(" {path}"); + } + if !ingested.is_empty() { + println!("ingested candidates: {}", ingested.join(", ")); + } + if !rejected.is_empty() { + println!("rejected candidates: {}", rejected.join(", ")); + } + for warning in warnings { + eprintln!("warning: {warning}"); + } + } + CheckpointOutput::Conflict { expected, actual } => { + eprintln!( + "conflict: expected revision {expected}, active memory is at {actual}. Another \ + writer committed first. Re-run `bsk site context` with the same --task: it \ + re-seeds the draft's SITE.md and references from revision {actual}, so your own \ + edits are gone from the draft and must be replayed on top before you checkpoint \ + again." + ); + } + } +} + +#[cfg(test)] +mod tests { + use super::super::model::{StepOp, StepTarget, WorkflowStep}; + use super::*; + + fn step() -> WorkflowStep { + WorkflowStep { + n: 2, + op: StepOp::Fill, + to: None, + target: Some(StepTarget { + role: Some("textbox".into()), + name: Some("标题".into()), + ctx: Some("工单信息".into()), + }), + value: None, + value_from: Some("title".into()), + commit: Some("blur".into()), + key: None, + modifiers: None, + note: None, + needs_review: false, + } + } + + #[test] + fn step_renders_anchor_parameter_and_commit() { + let line = render_step(&step()); + assert_eq!( + line, + " 2. fill role=textbox name=\"标题\" ctx=\"工单信息\" <- $title (commit: blur)" + ); + } + + #[test] + fn step_without_anchor_is_marked_for_review() { + let mut step = step(); + step.target = None; + step.needs_review = true; + step.note = Some("supply an anchor".into()); + let line = render_step(&step); + assert!(line.contains("(no anchor)")); + assert!(line.contains("[needs review]")); + assert!(line.contains("note: supply an anchor")); + } + + #[test] + fn navigate_step_does_not_claim_a_missing_anchor() { + let mut step = step(); + step.op = StepOp::Navigate; + step.target = None; + step.value_from = None; + step.commit = None; + step.to = Some("https://corp.example/new".into()); + assert_eq!(render_step(&step), " 2. navigate https://corp.example/new"); + } + + #[test] + fn index_entry_flags_stale_and_review() { + let entry = WorkflowIndexEntry { + id: "submit-ticket".into(), + purpose: None, + last_verified: Some("2026-01-01".into()), + stale: true, + unverified: false, + days_since_verified: Some(90), + needs_review: true, + steps: 4, + draft: false, + }; + let line = render_index_entry(&entry); + assert!(line.contains("STALE")); + assert!(!line.contains("UNVERIFIED"), "{line}"); + assert!(line.contains("NEEDS REVIEW")); + assert!(line.contains("90d ago")); + } + + /// A workflow saved a minute ago has not expired — calling it STALE told + /// the agent to distrust the memory it had just written. + #[test] + fn a_never_verified_entry_reads_unverified_not_stale() { + let entry = WorkflowIndexEntry { + id: "submit-ticket".into(), + purpose: None, + last_verified: None, + stale: false, + unverified: true, + days_since_verified: None, + needs_review: false, + steps: 4, + draft: false, + }; + let line = render_index_entry(&entry); + assert!(line.contains("UNVERIFIED"), "{line}"); + assert!(!line.contains("STALE"), "{line}"); + assert!(line.contains("never verified"), "{line}"); + } + + /// Once it is verified and the review is cleared, nothing is flagged. + #[test] + fn a_verified_reviewed_entry_carries_no_flags() { + let entry = WorkflowIndexEntry { + id: "submit-ticket".into(), + purpose: None, + last_verified: Some(today_utc()), + stale: false, + unverified: false, + days_since_verified: Some(0), + needs_review: false, + steps: 4, + draft: false, + }; + let line = render_index_entry(&entry); + assert!(!line.contains('['), "{line}"); + } + + #[test] + fn checkpoint_conflict_serialises_the_documented_shape() { + let json = serde_json::to_value(CheckpointOutput::Conflict { + expected: 3, + actual: 5, + }) + .unwrap(); + assert_eq!( + json, + serde_json::json!({ "status": "conflict", "expected": 3, "actual": 5 }) + ); + } +} diff --git a/crates/bsk-cli/src/cli/site/store.rs b/crates/bsk-cli/src/cli/site/store.rs new file mode 100644 index 00000000..c4191618 --- /dev/null +++ b/crates/bsk-cli/src/cli/site/store.rs @@ -0,0 +1,1200 @@ +//! Filesystem layer for `$BSK_HOME/sites`. +//! +//! Layout (design §3.1): +//! +//! ```text +//! sites/ +//! .lock repository-wide advisory lock +//! .drafts/// task-isolated staging area +//! .context.json +//! / +//! .revision monotonic CAS token, per host +//! .journal.jsonl one line per committed checkpoint, per host +//! SITE.md references/ workflows/ candidates/ +//! ``` +//! +//! The revision counter is **per host**. A single global counter made every +//! host's checkpoint invalidate every other host's draft: creating memory for +//! `example.org` pushed an untouched `wikipedia.org` draft straight into the +//! conflict path. The lock stays repository-wide because it is cheap and +//! guards the whole tree. +//! +//! Everything in this module is pure local file I/O: no `bsk site` command +//! needs the daemon, so the whole family works under `BSK_AUTO_START=0`. + +use std::fs::{self, File, OpenOptions}; +use std::io::Write; +use std::path::{Component, Path, PathBuf}; +use std::time::{Duration, Instant}; + +use anyhow::{Context, Result, anyhow, bail}; +use fs2::FileExt; +use serde::Serialize; +use serde::de::DeserializeOwned; + +use super::model::{Candidate, DraftContext, JournalEntry, Workflow, now_rfc3339}; +use crate::daemon::paths; + +pub const SITE_MD: &str = "SITE.md"; +pub const REFERENCES_DIR: &str = "references"; +pub const WORKFLOWS_DIR: &str = "workflows"; +pub const CANDIDATES_DIR: &str = "candidates"; +pub const DRAFTS_DIR: &str = ".drafts"; + +const REVISION_FILE: &str = ".revision"; +const JOURNAL_FILE: &str = ".journal.jsonl"; +const LOCK_FILE: &str = ".lock"; +const CONTEXT_FILE: &str = ".context.json"; + +/// How long a command waits for the repository lock before giving up. Long +/// enough to ride out a concurrent checkpoint, short enough that a wedged +/// agent reports a real error instead of hanging the task. +const LOCK_TIMEOUT: Duration = Duration::from_secs(10); +const LOCK_POLL: Duration = Duration::from_millis(50); + +// --------------------------------------------------------------------------- +// Host normalisation +// --------------------------------------------------------------------------- + +/// A host after registrable-domain-ish normalisation. +/// +/// No public-suffix list is pulled in: folding `www.` / `m.` and lowercasing +/// covers the cases that actually collide in practice, and a wrong PSL answer +/// would silently merge two sites' memory. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HostKey { + /// Canonical host as reported to the user. + pub display: String, + /// Directory name under `sites/`. Equals `display` unless `read_only`. + pub dir: String, + /// True when normalisation failed: drafts are allowed, promotion is not. + pub read_only: bool, +} + +/// Windows reserves these names at every path level, extension or not. +const WINDOWS_RESERVED: &[&str] = &[ + "con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8", + "com9", "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", +]; + +/// Normalise a `--host` value. Accepts a bare host or a full URL. +pub fn normalize_host(raw: &str) -> HostKey { + let candidate = strip_to_host(raw); + if is_valid_host(&candidate) { + // A dotless host (`localhost`, an internal short name) is legal but its + // bare label is exactly what Windows reserves, so it is filed under a + // prefixed directory. `display` keeps the name the user typed. + let dir = if candidate.contains('.') { + candidate.clone() + } else { + format!("host-{candidate}") + }; + return HostKey { + display: candidate, + dir, + read_only: false, + }; + } + let slug = slugify(&candidate); + HostKey { + display: raw.trim().to_string(), + dir: format!("provisional-{slug}"), + read_only: true, + } +} + +fn strip_to_host(raw: &str) -> String { + let mut text = raw.trim(); + if let Some(idx) = text.find("://") { + text = &text[idx + 3..]; + } + // Cut the authority off FIRST. Searching the whole string for `@` lets a + // path or query smuggle in a different host: + // `https://login.microsoftonline.com/x@ticket.corp.example` would otherwise + // normalise to `ticket.corp.example` and slip past the sensitive-host gate. + text = text.split(['/', '?', '#']).next().unwrap_or_default(); + // Userinfo lives inside the authority; the host follows the last `@`. + if let Some((_, host)) = text.rsplit_once('@') { + text = host; + } + text = text.trim_end_matches('.'); + // Strip a trailing :port, but leave bracketed IPv6 literals alone (they are + // rejected by `is_valid_host` anyway). + if !text.starts_with('[') + && let Some(idx) = text.rfind(':') + && text[idx + 1..].chars().all(|c| c.is_ascii_digit()) + { + text = &text[..idx]; + } + let lower = text.to_ascii_lowercase(); + for prefix in ["www.", "m."] { + if let Some(rest) = lower.strip_prefix(prefix) + && rest.contains('.') + { + return rest.to_string(); + } + } + lower +} + +fn is_valid_host(host: &str) -> bool { + if host.is_empty() || host.len() > 253 || host.contains("..") { + return false; + } + if !host + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '.') + { + return false; + } + let labels: Vec<&str> = host.split('.').collect(); + if labels + .iter() + .any(|label| label.is_empty() || label.len() > 63) + { + return false; + } + // A reserved device name as the first label would produce a directory + // Windows refuses to create, and only a dotted host can hit it: a dotless + // one is prefixed by `normalize_host`. + if host.contains('.') && WINDOWS_RESERVED.contains(&labels[0]) { + return false; + } + true +} + +/// Short, stable digest keeping truncated slugs from colliding. +fn short_hash(text: &str) -> String { + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; + + let mut hasher = DefaultHasher::new(); + text.hash(&mut hasher); + format!("{:08x}", hasher.finish() as u32) +} + +fn slugify(text: &str) -> String { + let mut out: String = text + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '.' { + c.to_ascii_lowercase() + } else { + '-' + } + }) + .collect(); + out = out.trim_matches(['-', '.']).to_string(); + if out.is_empty() { + return format!("unknown-{}", short_hash(text)); + } + if out.len() > 48 { + // Two long inputs sharing a 48-character prefix must not land in the + // same directory, so the digest of the full value is appended. + let digest = short_hash(text); + out.truncate(48); + out = format!("{}-{digest}", out.trim_end_matches(['-', '.'])); + } + out +} + +/// Refuse a host that must never become durable site memory. +/// +/// Every entry point calls this right after [`normalize_host`], so the +/// credential-surface rule cannot be reached around by choosing a different +/// subcommand. The check runs on the normalised host, which is why +/// [`strip_to_host`] must cut the authority before looking for userinfo. +pub fn guard_host(host: &HostKey) -> Result<()> { + if let Some(reason) = super::validate::sensitive_host_reason(&host.display) { + bail!("{reason}"); + } + Ok(()) +} + +/// Sanitise a task id into one safe path segment. +pub fn task_slug(task: &str) -> Result { + let trimmed = task.trim(); + if trimmed.is_empty() { + bail!("--task must not be empty"); + } + let slug = slugify(trimmed); + if WINDOWS_RESERVED.contains(&slug.as_str()) { + bail!("--task {trimmed:?} is a reserved name on Windows"); + } + Ok(slug) +} + +/// Validate a workflow / candidate id: one lowercase path segment. +pub fn validate_id(id: &str, what: &str) -> Result<()> { + if id.is_empty() || id.len() > 64 { + bail!("{what} id must be 1-64 characters, got {id:?}"); + } + if !id + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-' || c == '_') + { + bail!("{what} id {id:?} must use only lowercase letters, digits, '-' and '_'"); + } + if id.starts_with('-') || id.starts_with('_') { + bail!("{what} id {id:?} must not start with '-' or '_'"); + } + if WINDOWS_RESERVED.contains(&id) { + bail!("{what} id {id:?} is a reserved name on Windows"); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Path containment +// --------------------------------------------------------------------------- + +/// One child of a host or draft directory (`workflows`, `SITE.md`, ...). +/// +/// Every subdirectory goes through here rather than a bare `join` so a +/// symlinked `candidates/` cannot redirect writes outside `sites/`. +pub fn sub(base: &Path, name: &str) -> Result { + resolve_in(base, &[name]) +} + +/// Join `segments` under `base`, rejecting traversal and symlinked components. +/// +/// Site memory is written from agent-supplied ids and host names, so every +/// path is rebuilt segment by segment rather than trusted. A symlink anywhere +/// along the chain is refused outright: following one would let a crafted +/// draft write outside `sites/`. +pub fn resolve_in(base: &Path, segments: &[&str]) -> Result { + let mut path = base.to_path_buf(); + for segment in segments { + if segment.is_empty() || *segment == "." || *segment == ".." { + bail!("unsafe path segment {segment:?}"); + } + let as_path = Path::new(segment); + let mut components = as_path.components(); + match (components.next(), components.next()) { + (Some(Component::Normal(_)), None) => {} + _ => bail!("path segment {segment:?} must be a single plain name"), + } + path.push(segment); + if let Ok(meta) = fs::symlink_metadata(&path) + && meta.file_type().is_symlink() + { + bail!("refusing to follow symlink at {}", path.display()); + } + } + Ok(path) +} + +// --------------------------------------------------------------------------- +// Repository handle +// --------------------------------------------------------------------------- + +/// Handle on `$BSK_HOME/sites`. +#[derive(Debug, Clone)] +pub struct SiteStore { + root: PathBuf, +} + +impl SiteStore { + /// Open (and create) the site-memory root. + pub fn open() -> Result { + let root = paths::ensure_sites_root().context("prepare site-memory root")?; + Ok(Self { root }) + } + + #[cfg(test)] + pub fn at(root: PathBuf) -> Result { + paths::ensure_private_dir(&root)?; + Ok(Self { root }) + } + + pub fn root(&self) -> &Path { + &self.root + } + + /// Active memory directory for a host. Not created. + pub fn host_dir(&self, host: &HostKey) -> Result { + resolve_in(&self.root, &[host.dir.as_str()]) + } + + /// Draft directory for `(task, host)`. Not created. + pub fn draft_dir(&self, task_slug: &str, host: &HostKey) -> Result { + resolve_in(&self.root, &[DRAFTS_DIR, task_slug, host.dir.as_str()]) + } + + // -- revision --------------------------------------------------------- + + /// `sites//.revision` — the CAS token for one host. + /// + /// Per host, not per repository: `wikipedia.org` publishing a workflow is + /// not a reason for a `ticket.corp.example` draft to conflict. Any + /// leftover `sites/.revision` from the pre-release global scheme is simply + /// never read. + pub fn revision_path(&self, host: &HostKey) -> Result { + Ok(self.host_dir(host)?.join(REVISION_FILE)) + } + + /// `sites//.journal.jsonl` — that host's append-only commit log. + pub fn journal_path(&self, host: &HostKey) -> Result { + Ok(self.host_dir(host)?.join(JOURNAL_FILE)) + } + + pub fn revision(&self, host: &HostKey) -> Result { + let path = self.revision_path(host)?; + match fs::read_to_string(&path) { + Ok(text) => text + .trim() + .parse::() + .with_context(|| format!("{} is not a revision number", path.display())), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(0), + Err(err) => Err(anyhow!(err).context(format!("read {}", path.display()))), + } + } + + pub fn set_revision(&self, host: &HostKey, value: u64) -> Result<()> { + write_atomic(&self.revision_path(host)?, format!("{value}\n").as_bytes()) + } + + pub fn append_journal(&self, host: &HostKey, entry: &JournalEntry) -> Result<()> { + let path = self.journal_path(host)?; + paths::ensure_private_dir(self.host_dir(host)?.as_path())?; + let line = serde_json::to_string(entry).context("encode journal entry")?; + let mut file = OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .with_context(|| format!("open {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(&path, fs::Permissions::from_mode(0o600)); + } + writeln!(file, "{line}").with_context(|| format!("append {}", path.display())) + } + + // -- locking ---------------------------------------------------------- + + /// Take the repository-wide exclusive lock. + /// + /// `fs2` advisory locks are released by the OS when a holder dies, so no + /// pid/mtime stale-breaking dance is needed: a crashed agent cannot wedge + /// the tree. The bounded wait turns a live deadlock into a clear error. + pub fn lock(&self) -> Result { + let path = self.root.join(LOCK_FILE); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + .with_context(|| format!("open {}", path.display()))?; + // The lock file sits beside private memory and is created by whichever + // command runs first, so it gets the same 0600 promise as the rest. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(&path, fs::Permissions::from_mode(0o600)); + } + let deadline = Instant::now() + LOCK_TIMEOUT; + loop { + match file.try_lock_exclusive() { + Ok(()) => return Ok(SiteLock { file }), + Err(err) if err.raw_os_error() == fs2::lock_contended_error().raw_os_error() => { + if Instant::now() >= deadline { + bail!( + "another bsk site command is holding {} (waited {}s)", + path.display(), + LOCK_TIMEOUT.as_secs() + ); + } + std::thread::sleep(LOCK_POLL); + } + Err(err) => { + return Err(anyhow!(err).context(format!("lock {}", path.display()))); + } + } + } + } +} + +/// Guard releasing the repository lock on drop. +pub struct SiteLock { + file: File, +} + +impl Drop for SiteLock { + fn drop(&mut self) { + let _ = FileExt::unlock(&self.file); + } +} + +// --------------------------------------------------------------------------- +// Atomic IO helpers +// --------------------------------------------------------------------------- + +/// Replace `path` with `bytes` through a same-directory temporary file. +pub fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> { + let parent = path + .parent() + .ok_or_else(|| anyhow!("{} has no parent directory", path.display()))?; + paths::ensure_private_dir(parent)?; + let mut temp = tempfile::Builder::new() + .prefix(".bsk-site-") + .tempfile_in(parent) + .with_context(|| format!("stage a temporary file next to {}", path.display()))?; + temp.write_all(bytes) + .with_context(|| format!("write staged content for {}", path.display()))?; + temp.flush() + .with_context(|| format!("flush staged content for {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o600)) + .with_context(|| format!("chmod 0600 staged content for {}", path.display()))?; + } + temp.persist(path) + .map_err(|err| anyhow!(err.error)) + .with_context(|| format!("replace {}", path.display()))?; + Ok(()) +} + +pub fn write_json(path: &Path, value: &T) -> Result<()> { + let mut text = serde_json::to_string_pretty(value) + .with_context(|| format!("encode {}", path.display()))?; + text.push('\n'); + write_atomic(path, text.as_bytes()) +} + +pub fn read_json(path: &Path) -> Result { + let text = fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?; + serde_json::from_str(&text).with_context(|| format!("parse {}", path.display())) +} + +/// Read a file, mapping "missing" to `None`. +pub fn read_optional(path: &Path) -> Result> { + match fs::read_to_string(path) { + Ok(text) => Ok(Some(text)), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(anyhow!(err).context(format!("read {}", path.display()))), + } +} + +/// List `*.` file stems in `dir`, sorted. A missing directory is empty. +pub fn list_stems(dir: &Path, ext: &str) -> Result> { + let entries = match fs::read_dir(dir) { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(err) => return Err(anyhow!(err).context(format!("read {}", dir.display()))), + }; + let mut out = Vec::new(); + for entry in entries { + let entry = entry.with_context(|| format!("read entry in {}", dir.display()))?; + if !entry.file_type().map(|t| t.is_file()).unwrap_or(false) { + continue; + } + let path = entry.path(); + if path.extension().and_then(|s| s.to_str()) != Some(ext) { + continue; + } + if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) { + out.push(stem.to_string()); + } + } + out.sort(); + Ok(out) +} + +// --------------------------------------------------------------------------- +// Transactions +// --------------------------------------------------------------------------- + +enum Undo { + /// Put a replaced file back, or delete one that did not exist before. + Restore { + path: PathBuf, + previous: Option>, + }, + /// Cut an appended line back off the journal. + Truncate { + path: PathBuf, + len: u64, + existed: bool, + }, +} + +/// Groups several file writes so a failure part-way through leaves the tree +/// exactly as it was. +/// +/// The previous content of each touched file is held in memory; site-memory +/// files are prose and small JSON, so this stays cheap. Undo actions replay in +/// reverse order. +#[derive(Default)] +pub struct Transaction { + undo: Vec, +} + +impl Transaction { + pub fn write(&mut self, path: &Path, bytes: &[u8]) -> Result<()> { + let previous = match fs::read(path) { + Ok(bytes) => Some(bytes), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => None, + Err(err) => { + return Err(anyhow!(err).context(format!("read {}", path.display()))); + } + }; + self.undo.push(Undo::Restore { + path: path.to_path_buf(), + previous, + }); + write_atomic(path, bytes) + } + + pub fn write_json(&mut self, path: &Path, value: &T) -> Result<()> { + let mut text = serde_json::to_string_pretty(value) + .with_context(|| format!("encode {}", path.display()))?; + text.push('\n'); + self.write(path, text.as_bytes()) + } + + /// Append one line, remembering the original length so a rollback can trim + /// it. A journal entry must never outlive the commit it describes. + pub fn append_line(&mut self, path: &Path, line: &str) -> Result<()> { + let (len, existed) = match fs::metadata(path) { + Ok(meta) => (meta.len(), true), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => (0, false), + Err(err) => { + return Err(anyhow!(err).context(format!("stat {}", path.display()))); + } + }; + self.undo.push(Undo::Truncate { + path: path.to_path_buf(), + len, + existed, + }); + let mut file = OpenOptions::new() + .create(true) + .append(true) + .open(path) + .with_context(|| format!("open {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(path, fs::Permissions::from_mode(0o600)); + } + writeln!(file, "{line}").with_context(|| format!("append {}", path.display())) + } + + /// Undo everything written so far. Returns the paths it could not restore. + pub fn rollback(&mut self) -> Result<()> { + let mut failures = Vec::new(); + for action in self.undo.drain(..).rev() { + let outcome = match action { + Undo::Restore { + path, + previous: Some(bytes), + } => write_atomic(&path, &bytes), + Undo::Restore { + path, + previous: None, + } => remove_if_present(&path), + Undo::Truncate { + path, + len, + existed: true, + } => OpenOptions::new() + .write(true) + .open(&path) + .and_then(|file| file.set_len(len)) + .map_err(|err| anyhow!(err).context(format!("truncate {}", path.display()))), + Undo::Truncate { + path, + existed: false, + .. + } => remove_if_present(&path), + }; + if let Err(err) = outcome { + failures.push(format!("{err:#}")); + } + } + if failures.is_empty() { + Ok(()) + } else { + bail!("{}", failures.join("; ")) + } + } +} + +fn remove_if_present(path: &Path) -> Result<()> { + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(anyhow!(err).context(format!("remove {}", path.display()))), + } +} + +// --------------------------------------------------------------------------- +// Drafts +// --------------------------------------------------------------------------- + +/// A draft staged for one `(task, host)` pair. +#[derive(Debug, Clone)] +pub struct Draft { + pub dir: PathBuf, + pub context: DraftContext, + /// True when this call created the draft rather than reusing it. + pub created: bool, + /// True when this call re-seeded the draft from a newer active revision. + pub rebased: bool, +} + +/// Open the draft for `(task, host)`, seeding it from active memory on first +/// use. Seeding copies `SITE.md`, `references/` and `workflows/` — candidates +/// stay out of drafts because they are append-only evidence written straight +/// to active memory (§3.4). +pub fn ensure_draft(store: &SiteStore, task: &str, host: &HostKey) -> Result { + let slug = task_slug(task)?; + let dir = store.draft_dir(&slug, host)?; + let context_path = dir.join(CONTEXT_FILE); + if context_path.is_file() { + let context: DraftContext = read_json(&context_path)?; + return Ok(Draft { + dir, + context, + created: false, + rebased: false, + }); + } + // Each level is created through `ensure_private_dir` in turn so `.drafts/` + // and `.drafts//` are 0700 too, not just the `/` leaf: the task + // id itself is information about what the user is doing. + paths::ensure_private_dir(&resolve_in(store.root(), &[DRAFTS_DIR])?)?; + paths::ensure_private_dir(&resolve_in(store.root(), &[DRAFTS_DIR, slug.as_str()])?)?; + paths::ensure_private_dir(&dir)?; + seed_draft(store, host, &dir, SeedScope::Everything)?; + let context = DraftContext { + read_only: host.read_only, + base_revision: store.revision(host)?, + host: host.display.clone(), + task_id: task.trim().to_string(), + created_at: now_rfc3339(), + }; + write_json(&context_path, &context)?; + Ok(Draft { + dir, + context, + created: true, + rebased: false, + }) +} + +/// Which parts of active memory a seeding pass copies into the draft. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SeedScope { + /// First use of a draft: copy prose and workflows alike. + Everything, + /// A rebase: copy only the prose the agent is expected to replay its edits + /// on top of. Workflows staged in the draft are new work this task has not + /// published yet, so overwriting them would destroy it. + ProseOnly, +} + +fn seed_draft(store: &SiteStore, host: &HostKey, draft_dir: &Path, scope: SeedScope) -> Result<()> { + let active = store.host_dir(host)?; + if let Some(site_md) = read_optional(&active.join(SITE_MD))? { + write_atomic(&draft_dir.join(SITE_MD), site_md.as_bytes())?; + } + let subs: &[(&str, &str)] = match scope { + SeedScope::Everything => &[(REFERENCES_DIR, "md"), (WORKFLOWS_DIR, "json")], + SeedScope::ProseOnly => &[(REFERENCES_DIR, "md")], + }; + for (sub, ext) in subs { + let src = active.join(sub); + let dst = draft_dir.join(sub); + let stems = list_stems(&src, ext)?; + if stems.is_empty() { + continue; + } + paths::ensure_private_dir(&dst)?; + for stem in stems { + let name = format!("{stem}.{ext}"); + let from = resolve_in(&src, &[name.as_str()])?; + let to = resolve_in(&dst, &[name.as_str()])?; + let bytes = fs::read(&from).with_context(|| format!("read {}", from.display()))?; + write_atomic(&to, &bytes)?; + } + } + Ok(()) +} + +/// Path to a draft's `.context.json`. +pub fn draft_context_path(draft_dir: &Path) -> PathBuf { + draft_dir.join(CONTEXT_FILE) +} + +/// Move a draft onto `revision`, re-seeding its prose from active memory. +/// +/// This is the recovery `context` performs after a checkpoint conflict, and it +/// is the whole point of the conflict. Moving only `.context.json` used to let +/// the retry publish a `SITE.md` copied from the *old* base, silently deleting +/// whatever the writer who won the race had just committed. Re-seeding means +/// the draft's `SITE.md` and `references/` become exactly what is published +/// now; the agent replays its own edits on top and checkpoints again. +/// +/// `workflows/` staged in the draft survive: they are this task's unpublished +/// work, not a stale copy of someone else's. +pub fn rebase_draft( + store: &SiteStore, + host: &HostKey, + mut draft: Draft, + revision: u64, +) -> Result { + if draft.context.base_revision == revision { + return Ok(draft); + } + seed_draft(store, host, &draft.dir, SeedScope::ProseOnly)?; + draft.context.base_revision = revision; + write_json(&draft_context_path(&draft.dir), &draft.context)?; + draft.rebased = true; + Ok(draft) +} + +/// Read an existing draft without creating one. +pub fn load_draft(store: &SiteStore, task: &str, host: &HostKey) -> Result> { + let slug = task_slug(task)?; + let dir = store.draft_dir(&slug, host)?; + let context_path = dir.join(CONTEXT_FILE); + if !context_path.is_file() { + return Ok(None); + } + let context: DraftContext = read_json(&context_path)?; + Ok(Some(Draft { + dir, + context, + created: false, + rebased: false, + })) +} + +// --------------------------------------------------------------------------- +// Typed readers over the active tree +// --------------------------------------------------------------------------- + +/// Read every workflow stored for a host, skipping unreadable files. +/// +/// A corrupt file must not hide the rest of a site's memory, so parse failures +/// are returned alongside the workflows rather than aborting the read. +pub fn load_workflows(dir: &Path) -> Result<(Vec, Vec)> { + let mut workflows = Vec::new(); + let mut problems = Vec::new(); + for stem in list_stems(dir, "json")? { + let name = format!("{stem}.json"); + let path = resolve_in(dir, &[name.as_str()])?; + match read_json::(&path) { + Ok(workflow) => workflows.push(workflow), + Err(err) => problems.push(format!("{name}: {err:#}")), + } + } + workflows.sort_by(|a, b| a.id.cmp(&b.id)); + Ok((workflows, problems)) +} + +/// Read one workflow by id, or `None` when it does not exist. +pub fn load_workflow(dir: &Path, id: &str) -> Result> { + validate_id(id, "workflow")?; + let name = format!("{id}.json"); + let path = resolve_in(dir, &[name.as_str()])?; + if !path.is_file() { + return Ok(None); + } + read_json(&path).map(Some) +} + +/// Read every candidate stored for a host, skipping unreadable files. +pub fn load_candidates(dir: &Path) -> Result<(Vec, Vec)> { + let mut candidates = Vec::new(); + let mut problems = Vec::new(); + for stem in list_stems(dir, "json")? { + let name = format!("{stem}.json"); + let path = resolve_in(dir, &[name.as_str()])?; + match read_json::(&path) { + Ok(candidate) => candidates.push(candidate), + Err(err) => problems.push(format!("{name}: {err:#}")), + } + } + candidates.sort_by(|a, b| { + a.observed_date_utc + .cmp(&b.observed_date_utc) + .then_with(|| a.id.cmp(&b.id)) + }); + Ok((candidates, problems)) +} + +/// Allocate a candidate id that is unique within `dir`. +/// +/// Shape is `-` so a directory listing sorts chronologically and the +/// id itself carries the observation date. +pub fn next_candidate_id(dir: &Path, date: &str) -> Result { + let existing = list_stems(dir, "json")?; + for n in 1..10_000u32 { + let candidate = format!("{date}-{n:03}"); + if !existing.contains(&candidate) { + return Ok(candidate); + } + } + bail!("more than 9999 candidates recorded for {date}") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn store() -> (tempfile::TempDir, SiteStore) { + let tmp = tempfile::tempdir().unwrap(); + let store = SiteStore::at(tmp.path().join("sites")).unwrap(); + (tmp, store) + } + + #[test] + fn host_normalisation_folds_prefixes_and_case() { + assert_eq!(normalize_host("WWW.Example.COM").dir, "example.com"); + assert_eq!(normalize_host("m.example.com").dir, "example.com"); + assert_eq!( + normalize_host("https://ticket.corp.example/new?x=1").dir, + "ticket.corp.example" + ); + assert_eq!(normalize_host("example.com:8443").dir, "example.com"); + assert_eq!(normalize_host("example.com.").dir, "example.com"); + // A bare `m` / `www` label with nothing behind it is kept as-is, but a + // dotless host is filed under a prefixed directory (see L15). + let bare = normalize_host("www."); + assert_eq!(bare.display, "www"); + assert_eq!(bare.dir, "host-www"); + assert!(!bare.read_only); + } + + #[test] + fn unnormalisable_host_falls_back_to_read_only_slug() { + let key = normalize_host("../../etc/passwd"); + assert!(key.read_only); + assert!(key.dir.starts_with("provisional-")); + assert!(!key.dir.contains("..")); + + let empty = normalize_host(" "); + assert!(empty.read_only); + assert!(empty.dir.starts_with("provisional-unknown-")); + } + + #[test] + fn windows_reserved_names_never_become_directory_names() { + // A dotted host whose first label is reserved has no safe directory. + assert!(normalize_host("nul.example.com").read_only); + assert!(normalize_host("con.corp.example").read_only); + // A dotless one is legal and gets the `host-` prefix instead (L15). + let con = normalize_host("con"); + assert!(!con.read_only); + assert_eq!(con.dir, "host-con"); + } + + /// H2: userinfo is only meaningful inside the authority. Searching the + /// whole URL for `@` let a path or query name a different host and slip + /// past the sensitive-host gate. + #[test] + fn userinfo_in_a_path_or_query_cannot_rewrite_the_host() { + assert_eq!( + normalize_host("https://login.microsoftonline.com/x@ticket.corp.example").dir, + "login.microsoftonline.com" + ); + assert_eq!( + normalize_host("https://a.example.com/r?to=x@okta.com").dir, + "a.example.com" + ); + // Real userinfo inside the authority still resolves to the host. + assert_eq!( + normalize_host("https://user:pw@ticket.corp.example/new").dir, + "ticket.corp.example" + ); + assert!( + guard_host(&normalize_host( + "https://login.microsoftonline.com/x@ticket.corp.example" + )) + .is_err() + ); + } + + /// L15: two long values sharing a 48-character prefix must not collide. + #[test] + fn long_slugs_stay_distinct_after_truncation() { + let prefix = "x".repeat(60); + let a = normalize_host(&format!("{prefix} one!")); + let b = normalize_host(&format!("{prefix} two!")); + assert!(a.read_only && b.read_only); + assert_ne!(a.dir, b.dir); + assert!(a.dir.len() <= 48 + "provisional-".len() + 9); + } + + #[cfg(unix)] + #[test] + fn symlinked_subdirectories_cannot_redirect_writes() { + let (tmp, store) = store(); + let host = normalize_host("example.com"); + let active = store.host_dir(&host).unwrap(); + paths::ensure_private_dir(&active).unwrap(); + let outside = tmp.path().join("outside"); + fs::create_dir_all(&outside).unwrap(); + std::os::unix::fs::symlink(&outside, active.join(CANDIDATES_DIR)).unwrap(); + let err = sub(&active, CANDIDATES_DIR).unwrap_err(); + assert!(err.to_string().contains("symlink"), "{err}"); + } + + #[test] + fn rebase_draft_moves_the_base_revision() { + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + let draft = ensure_draft(&store, "t1", &host).unwrap(); + assert_eq!(draft.context.base_revision, 0); + let rebased = rebase_draft(&store, &host, draft, 5).unwrap(); + assert_eq!(rebased.context.base_revision, 5); + assert!(rebased.rebased); + let reread = load_draft(&store, "t1", &host).unwrap().unwrap(); + assert_eq!(reread.context.base_revision, 5); + } + + /// S7: a rebase must hand the draft the prose that is actually published, + /// or replaying edits on it republishes the old base and deletes whatever + /// the writer who won the race committed. + #[test] + fn rebase_reseeds_prose_but_keeps_staged_workflows() { + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + let active = store.host_dir(&host).unwrap(); + write_atomic(&active.join(SITE_MD), b"base line\n").unwrap(); + + let draft = ensure_draft(&store, "t1", &host).unwrap(); + // This task edits the prose and stages a brand-new workflow. + write_atomic(&draft.dir.join(SITE_MD), b"base line\nmy line\n").unwrap(); + let staged = sub(&draft.dir, WORKFLOWS_DIR).unwrap().join("mine.json"); + write_atomic(&staged, b"{}").unwrap(); + + // Someone else publishes first. + write_atomic(&active.join(SITE_MD), b"base line\ntheir line\n").unwrap(); + store.set_revision(&host, 1).unwrap(); + + let rebased = rebase_draft(&store, &host, draft, 1).unwrap(); + assert!(rebased.rebased); + assert_eq!( + fs::read_to_string(rebased.dir.join(SITE_MD)).unwrap(), + "base line\ntheir line\n", + "the draft must show what is published now" + ); + assert!(staged.is_file(), "unpublished workflow work must survive"); + } + + /// A per-host counter: one host's checkpoint is not another host's problem. + #[test] + fn revisions_are_per_host() { + let (_tmp, store) = store(); + let a = normalize_host("a.example"); + let b = normalize_host("b.example"); + store.set_revision(&a, 3).unwrap(); + assert_eq!(store.revision(&a).unwrap(), 3); + assert_eq!(store.revision(&b).unwrap(), 0); + assert!( + store + .revision_path(&a) + .unwrap() + .starts_with(store.host_dir(&a).unwrap()) + ); + } + + /// S10: `.drafts/` and `.drafts//` used to land at the process umask, + /// letting any other account on the machine list the user's task ids. + #[cfg(unix)] + #[test] + fn draft_directories_and_the_lock_are_private() { + use std::os::unix::fs::PermissionsExt; + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + let draft = ensure_draft(&store, "task-1", &host).unwrap(); + let mode = |path: &Path| fs::metadata(path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode(&store.root().join(DRAFTS_DIR)), 0o700); + assert_eq!(mode(&store.root().join(DRAFTS_DIR).join("task-1")), 0o700); + assert_eq!(mode(&draft.dir), 0o700); + assert_eq!(mode(&draft_context_path(&draft.dir)), 0o600); + + let held = store.lock().unwrap(); + assert_eq!(mode(&store.root().join(LOCK_FILE)), 0o600); + drop(held); + + store.set_revision(&host, 1).unwrap(); + store + .append_journal( + &host, + &JournalEntry { + revision: 1, + at: "2026-09-16T00:00:00Z".into(), + host: host.display.clone(), + task_id: None, + reason: "direct_correction".into(), + paths: Vec::new(), + ingested: Vec::new(), + rejected: Vec::new(), + }, + ) + .unwrap(); + assert_eq!(mode(&store.revision_path(&host).unwrap()), 0o600); + assert_eq!(mode(&store.journal_path(&host).unwrap()), 0o600); + assert_eq!(mode(&store.host_dir(&host).unwrap()), 0o700); + } + + #[test] + fn transaction_rollback_restores_files_and_trims_the_journal() { + let (_tmp, store) = store(); + let kept = store.root().join("kept.txt"); + let created = store.root().join("created.txt"); + let journal = store.root().join("scratch.jsonl"); + write_atomic(&kept, b"original").unwrap(); + let mut tx = Transaction::default(); + tx.append_line(&journal, "{\"first\":true}").unwrap(); + + let mut tx2 = Transaction::default(); + tx2.write(&kept, b"replaced").unwrap(); + tx2.write(&created, b"new").unwrap(); + tx2.append_line(&journal, "{\"second\":true}").unwrap(); + assert_eq!(fs::read_to_string(&journal).unwrap().lines().count(), 2); + + tx2.rollback().unwrap(); + assert_eq!(fs::read_to_string(&kept).unwrap(), "original"); + assert!(!created.exists(), "a file the transaction created must go"); + let journal_text = fs::read_to_string(&journal).unwrap(); + assert_eq!(journal_text.lines().count(), 1, "{journal_text}"); + assert!(journal_text.contains("first")); + drop(tx); + } + + #[test] + fn resolve_in_rejects_traversal_and_separators() { + let (_tmp, store) = store(); + assert!(resolve_in(store.root(), &[".."]).is_err()); + assert!(resolve_in(store.root(), &["a/b"]).is_err()); + assert!(resolve_in(store.root(), &[""]).is_err()); + assert!(resolve_in(store.root(), &["ok"]).is_ok()); + } + + #[cfg(unix)] + #[test] + fn resolve_in_refuses_symlinked_components() { + let (tmp, store) = store(); + let outside = tmp.path().join("outside"); + fs::create_dir_all(&outside).unwrap(); + std::os::unix::fs::symlink(&outside, store.root().join("evil.com")).unwrap(); + let err = resolve_in(store.root(), &["evil.com", "SITE.md"]).unwrap_err(); + assert!(err.to_string().contains("symlink"), "{err}"); + } + + #[test] + fn revision_starts_at_zero_and_round_trips() { + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + assert_eq!(store.revision(&host).unwrap(), 0); + store.set_revision(&host, 7).unwrap(); + assert_eq!(store.revision(&host).unwrap(), 7); + // A stray global counter left by the pre-release layout is ignored. + write_atomic(&store.root().join(REVISION_FILE), b"99\n").unwrap(); + assert_eq!(store.revision(&host).unwrap(), 7); + } + + #[test] + fn lock_is_exclusive_across_handles_and_released_on_drop() { + let (_tmp, store) = store(); + let held = store.lock().unwrap(); + let path = store.root().join(LOCK_FILE); + let probe = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&path) + .unwrap(); + assert!(probe.try_lock_exclusive().is_err()); + drop(probe); + drop(held); + let again = OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .unwrap(); + assert!(again.try_lock_exclusive().is_ok()); + } + + #[test] + fn draft_seeds_from_active_memory_then_reuses() { + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + let active = store.host_dir(&host).unwrap(); + paths::ensure_private_dir(&active.join(WORKFLOWS_DIR)).unwrap(); + write_atomic(&active.join(SITE_MD), b"# example [verified 2026-09-15]\n").unwrap(); + write_atomic(&active.join(WORKFLOWS_DIR).join("a.json"), b"{}").unwrap(); + store.set_revision(&host, 4).unwrap(); + + let draft = ensure_draft(&store, "task-1", &host).unwrap(); + assert!(draft.created); + assert_eq!(draft.context.base_revision, 4); + assert!(!draft.context.read_only); + assert!(draft.dir.join(SITE_MD).is_file()); + assert!(draft.dir.join(WORKFLOWS_DIR).join("a.json").is_file()); + + // A second call reuses the staged copy rather than re-seeding. + write_atomic(&draft.dir.join(SITE_MD), b"edited\n").unwrap(); + let again = ensure_draft(&store, "task-1", &host).unwrap(); + assert!(!again.created); + assert_eq!( + fs::read_to_string(again.dir.join(SITE_MD)).unwrap(), + "edited\n" + ); + } + + #[test] + fn draft_for_unnormalisable_host_is_read_only() { + let (_tmp, store) = store(); + let host = normalize_host("not a host!!"); + let draft = ensure_draft(&store, "task-1", &host).unwrap(); + assert!(draft.context.read_only); + } + + #[test] + fn validate_id_rejects_path_tricks() { + assert!(validate_id("submit-ticket", "workflow").is_ok()); + assert!(validate_id("../escape", "workflow").is_err()); + assert!(validate_id("Upper", "workflow").is_err()); + assert!(validate_id("", "workflow").is_err()); + assert!(validate_id("con", "workflow").is_err()); + } + + #[test] + fn journal_appends_one_line_per_entry() { + let (_tmp, store) = store(); + let host = normalize_host("example.com"); + for revision in 1..=2 { + store + .append_journal( + &host, + &JournalEntry { + revision, + at: "2026-09-15T00:00:00Z".into(), + host: "example.com".into(), + task_id: Some("t".into()), + reason: "direct_correction".into(), + paths: vec!["example.com/SITE.md".into()], + ingested: Vec::new(), + rejected: Vec::new(), + }, + ) + .unwrap(); + } + let text = fs::read_to_string(store.journal_path(&host).unwrap()).unwrap(); + assert_eq!(text.lines().count(), 2); + assert!( + text.lines() + .all(|l| serde_json::from_str::(l).is_ok()) + ); + } +} diff --git a/crates/bsk-cli/src/cli/site/trace_input.rs b/crates/bsk-cli/src/cli/site/trace_input.rs new file mode 100644 index 00000000..3393473a --- /dev/null +++ b/crates/bsk-cli/src/cli/site/trace_input.rs @@ -0,0 +1,249 @@ +//! Read a recorded trace from either shape `bsk record` can produce. +//! +//! `tool.record_stop` returns the **wire** trace, where each state carries its +//! observation inline as `body`. `bsk record start --output ` writes a +//! **bundle** instead (`crates/bsk-cli/src/cli/record/export.rs`): the page +//! text moves to `states/.txt` and `trace.json` references it as `page`. +//! `TraceV3` denies unknown fields, so a bundle never parses as a wire trace — +//! which is exactly the file an agent has on disk after a recording. +//! +//! The observation text is read back only so the reconstructed `TraceV3` is +//! faithful. Nothing derived from it reaches site memory: `states[]` holds +//! full page text including URLs, account names and order numbers, and design +//! §4.1 keeps that out of the store. + +use std::path::Path; + +use anyhow::{Context, Result, bail}; +use bsk_protocol::tools::{RecordedTrace, TraceStateV3, TraceV3}; +use serde::Deserialize; + +/// A `states[]` entry as written into a bundle: `page` instead of `body`. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct BundleState { + id: String, + url: String, + #[serde(default)] + title: Option, + page: String, + #[serde(default)] + truncated: bool, +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct BundleTrace { + version: u32, + #[serde(default)] + purpose: Option, + #[serde(default)] + started_at: Option, + recorded_at: String, + stopped_by: bsk_protocol::tools::StopReason, + entry: bsk_protocol::tools::TraceEntry, + recorder: bsk_protocol::tools::RecorderInfo, + states: Vec, + steps: Vec, +} + +/// Load a v3 trace from `path`, accepting the wire and bundle shapes. +pub fn read_trace_v3(path: &Path) -> Result { + let text = + std::fs::read_to_string(path).with_context(|| format!("read trace {}", path.display()))?; + + // A bundle and a wire trace are told apart by `states[].page`, so try the + // wire shape first and keep its error for the failure message. + let wire_error = match serde_json::from_str::(&text) { + Ok(RecordedTrace::V3(trace)) => return Ok(trace), + Ok(RecordedTrace::V2(_)) => bail!( + "{} is a v2 trace, which has no per-step semantic targets. Re-record it with a \ + v3-capable extension before deriving a workflow.", + path.display() + ), + Err(err) => err, + }; + + match serde_json::from_str::(&text) { + Ok(bundle) => rebuild(bundle, path), + Err(bundle_error) => Err(anyhow::anyhow!( + "{} is neither a recorded trace ({wire_error}) nor a record bundle ({bundle_error})", + path.display() + )), + } +} + +fn rebuild(bundle: BundleTrace, trace_path: &Path) -> Result { + let states_dir = trace_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("states"); + let mut states = Vec::with_capacity(bundle.states.len()); + for state in bundle.states { + // `page` is a filename the bundle writer produced; refuse anything that + // could read outside the bundle's own states directory. + let name = Path::new(&state.page); + let mut components = name.components(); + match (components.next(), components.next()) { + (Some(std::path::Component::Normal(_)), None) => {} + _ => bail!( + "trace bundle {} references the state file {:?}, which is not a plain filename", + trace_path.display(), + state.page + ), + } + // A missing page file costs nothing here: the body is never stored, so + // an incomplete bundle should still yield a usable workflow. + let body = std::fs::read_to_string(states_dir.join(name)).unwrap_or_default(); + states.push(TraceStateV3 { + id: state.id, + url: state.url, + title: state.title, + body, + truncated: state.truncated, + }); + } + + let trace = TraceV3 { + version: bundle.version, + purpose: bundle.purpose, + started_at: bundle.started_at, + recorded_at: bundle.recorded_at, + stopped_by: bundle.stopped_by, + entry: bundle.entry, + recorder: bundle.recorder, + states, + steps: bundle.steps, + }; + if trace.version != bsk_protocol::tools::TRACE_VERSION_V3 { + bail!( + "trace bundle {} has version {}, expected {}", + trace_path.display(), + trace.version, + bsk_protocol::tools::TRACE_VERSION_V3 + ); + } + Ok(trace) +} + +#[cfg(test)] +mod tests { + use super::*; + use bsk_protocol::tools::{ + NavigationCause, RecorderInfo, StepCommonV3, StepResultV3, StepV3, StopReason, + TRACE_VERSION_V3, TraceEntry, VOM_FORMAT_VERSION, + }; + + fn sample() -> TraceV3 { + TraceV3 { + version: TRACE_VERSION_V3, + purpose: Some("提交工单".into()), + started_at: None, + recorded_at: "2026-09-12T09:31:00Z".into(), + stopped_by: StopReason::UserFinish, + entry: TraceEntry { + start_url: "https://ticket.corp.example/".into(), + }, + recorder: RecorderInfo { + bsk: "0.2.1".into(), + vom: VOM_FORMAT_VERSION, + }, + states: vec![TraceStateV3 { + id: "s1".into(), + url: "https://ticket.corp.example/new".into(), + title: Some("新建工单".into()), + body: "@vom 1\nRootWebArea \"新建工单\"".into(), + truncated: false, + }], + steps: vec![StepV3::Navigate { + common: StepCommonV3 { + id: 1, + state: "s1".into(), + result: StepResultV3 { state: "s1".into() }, + }, + to: "https://ticket.corp.example/new".into(), + cause: NavigationCause::UserTyped, + }], + } + } + + /// The exact file `bsk record start --output ` leaves on disk must + /// load. This uses the real exporter so the two cannot drift. + #[test] + fn reads_a_bundle_written_by_the_real_exporter() { + let dir = tempfile::tempdir().unwrap(); + let out = dir.path().join("rec"); + crate::cli::record::export::write_trace_bundle(&out, &sample()).unwrap(); + + let path = out.join("trace.json"); + let raw = std::fs::read_to_string(&path).unwrap(); + assert!(raw.contains("\"page\""), "exporter shape changed: {raw}"); + assert!(!raw.contains("\"body\"")); + + let trace = read_trace_v3(&path).unwrap(); + assert_eq!(trace, sample(), "body must be restored from states/s1.txt"); + } + + #[test] + fn reads_a_wire_trace_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("trace.json"); + std::fs::write(&path, serde_json::to_string(&sample()).unwrap()).unwrap(); + assert_eq!(read_trace_v3(&path).unwrap(), sample()); + } + + #[test] + fn missing_state_files_still_yield_a_usable_trace() { + let dir = tempfile::tempdir().unwrap(); + let out = dir.path().join("rec"); + crate::cli::record::export::write_trace_bundle(&out, &sample()).unwrap(); + std::fs::remove_dir_all(out.join("states")).unwrap(); + let trace = read_trace_v3(&out.join("trace.json")).unwrap(); + assert_eq!(trace.steps.len(), 1); + assert_eq!(trace.states[0].body, ""); + } + + #[test] + fn bundle_page_pointing_outside_the_states_dir_is_refused() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("trace.json"); + let mut value = serde_json::to_value(sample()).unwrap(); + value["states"][0].as_object_mut().unwrap().remove("body"); + value["states"][0]["page"] = serde_json::json!("../../etc/passwd"); + std::fs::write(&path, value.to_string()).unwrap(); + let err = read_trace_v3(&path).unwrap_err(); + assert!(err.to_string().contains("not a plain filename"), "{err:#}"); + } + + #[test] + fn a_v2_trace_is_rejected_with_a_re_record_hint() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("trace.json"); + std::fs::write( + &path, + serde_json::json!({ + "recorded_at": "2026-09-12T09:31:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://corp.example/" }, + "recorder": { "bsk": "0.1.0", "vom": 1 }, + "pages": [], + "steps": [] + }) + .to_string(), + ) + .unwrap(); + let err = read_trace_v3(&path).unwrap_err(); + assert!(err.to_string().contains("v2 trace"), "{err:#}"); + } + + #[test] + fn unrelated_json_reports_both_parse_failures() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("trace.json"); + std::fs::write(&path, "{\"hello\":1}").unwrap(); + let err = read_trace_v3(&path).unwrap_err(); + let text = err.to_string(); + assert!(text.contains("neither a recorded trace"), "{text}"); + assert!(text.contains("record bundle"), "{text}"); + } +} diff --git a/crates/bsk-cli/src/cli/site/validate.rs b/crates/bsk-cli/src/cli/site/validate.rs new file mode 100644 index 00000000..0f4c77ea --- /dev/null +++ b/crates/bsk-cli/src/cli/site/validate.rs @@ -0,0 +1,1133 @@ +//! Policy checks that gate what may enter active site memory (design §3.3, +//! §3.4, §6). +//! +//! The checks split in two: *promotion* rules run at checkpoint and decide +//! whether a draft may be published, while *capture* rules (sensitive hosts, +//! secret-looking text) run at write time so a bad value never lands on disk +//! in the first place. + +use std::collections::BTreeSet; +use std::path::Path; + +use super::model::{ + Candidate, CandidateKind, CandidateStatus, SITE_MD_HARD_LINE_BUDGET, SITE_MD_SOFT_LINE_BUDGET, + StepOp, WORKFLOW_SCHEMA_VERSION, Workflow, WorkflowStrategy, normalize_claim, parse_date, +}; +use super::store; + +/// Per-file cap for `references/*.md`. Generous for prose, small enough that a +/// pasted page dump is refused. +pub const REFERENCE_MAX_BYTES: usize = 64 * 1024; + +/// Result of a validation pass. Warnings are reported but never block. +#[derive(Debug, Default, Clone)] +pub struct Report { + pub errors: Vec, + pub warnings: Vec, +} + +impl Report { + pub fn is_ok(&self) -> bool { + self.errors.is_empty() + } + + pub fn merge(&mut self, other: Report) { + self.errors.extend(other.errors); + self.warnings.extend(other.warnings); + } +} + +// --------------------------------------------------------------------------- +// Sensitive hosts +// --------------------------------------------------------------------------- + +/// Hosts whose recordings must never be distilled into a reusable asset. +/// +/// `skill/SKILL.md` already forbids *recording* banking, SSO and +/// password-manager pages; this list is the machine-checked half, so a trace +/// captured against the rule cannot become a durable, shareable workflow. +const SENSITIVE_DOMAIN_SUFFIXES: &[&str] = &[ + // Password managers and secret stores + "1password.com", + "bitwarden.com", + "dashlane.com", + "keepersecurity.com", + "lastpass.com", + "vaultproject.io", + // Identity providers / SSO + "auth0.com", + "duosecurity.com", + "okta.com", + "onelogin.com", + "accounts.google.com", + "login.microsoftonline.com", + "login.live.com", + "login.yahoo.com", + "id.apple.com", + // Payments and banking + "paypal.com", + "stripe.com", + "chase.com", + "bankofamerica.com", + "wellsfargo.com", + "citibank.com", + "hsbc.com", + "barclays.co.uk", + "icbc.com.cn", + "ccb.com", + "abchina.com", + "boc.cn", + "bankcomm.com", + "cmbchina.com", + "psbc.com", + "unionpay.com", + "alipay.com", +]; + +/// Host labels that mark a login / secret surface regardless of domain. +/// +/// Matched on whole labels (or a `keyword-…` / `…-keyword` label) so +/// `authoring.corp.example` is not mistaken for an auth endpoint. +const SENSITIVE_LABELS: &[&str] = &[ + "auth", "authn", "bank", "banking", "ebank", "netbank", "idp", "keychain", "login", "logon", + "mfa", "oauth", "openid", "otp", "passwd", "password", "signin", "sso", "2fa", "vault", +]; + +/// Why a host is refused, in a sentence the agent can relay to the user. +pub fn sensitive_host_reason(host: &str) -> Option { + let host = host.to_ascii_lowercase(); + for suffix in SENSITIVE_DOMAIN_SUFFIXES { + if host == *suffix || host.ends_with(&format!(".{suffix}")) { + return Some(format!( + "{host} matches the blocked domain {suffix}: banking, SSO and password-manager \ + flows must not be distilled into reusable site memory" + )); + } + } + for label in host.split('.') { + if let Some(keyword) = SENSITIVE_LABELS.iter().find(|k| label_matches(label, k)) { + return Some(format!( + "{host} carries the credential-surface label {keyword:?}: banking, SSO and \ + password-manager flows must not be distilled into reusable site memory" + )); + } + } + None +} + +fn label_matches(label: &str, keyword: &str) -> bool { + if label == keyword { + return true; + } + label + .strip_prefix(keyword) + .is_some_and(|rest| rest.starts_with('-')) + || label + .strip_suffix(keyword) + .is_some_and(|rest| rest.ends_with('-')) +} + +// --------------------------------------------------------------------------- +// Secret-looking text +// --------------------------------------------------------------------------- + +/// Substrings that mark a value as credential material. Matched +/// case-insensitively against the whole text. +const SECRET_MARKERS: &[(&str, &str)] = &[ + ("bearer ", "a bearer token"), + ("authorization:", "an Authorization header"), + ("set-cookie:", "a Set-Cookie header"), + ("cookie=", "a cookie value"), + ("password=", "a password"), + ("passwd=", "a password"), + ("pwd=", "a password"), + ("token=", "a token"), + ("access_token", "an access token"), + ("refresh_token", "a refresh token"), + ("client_secret", "a client secret"), + ("api_key=", "an API key"), + ("apikey=", "an API key"), + ("secret=", "a secret"), + ("-----begin ", "a PEM private key"), + ("eyj", "a JWT"), + ("ghp_", "a GitHub token"), + ("xoxb-", "a Slack token"), + ("sk_live_", "a live API key"), +]; + +/// Detect obvious credential material. Returns a human-readable reason. +/// +/// Deliberately conservative pattern matching rather than entropy scoring: +/// false positives cost one rephrased claim, a false negative writes a token +/// into a file that is never deleted. +pub fn find_secret(text: &str) -> Option { + let lower = text.to_ascii_lowercase(); + for (marker, what) in SECRET_MARKERS { + if !lower.contains(marker) { + continue; + } + // `eyJ` only means a JWT when a base64 segment and a dot follow it. + if *marker == "eyj" && !looks_like_jwt(&lower) { + continue; + } + return Some(format!( + "the text looks like it contains {what} ({marker:?}); site memory records what to do, \ + never credentials" + )); + } + None +} + +fn looks_like_jwt(lower: &str) -> bool { + lower.match_indices("eyj").any(|(idx, _)| { + let tail = &lower[idx..]; + let segment: String = tail + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '_' || *c == '-') + .collect(); + segment.len() >= 12 && tail[segment.len()..].starts_with('.') + }) +} + +// --------------------------------------------------------------------------- +// SITE.md +// --------------------------------------------------------------------------- + +/// Validate a `SITE.md` body (design §3.3). +/// +/// `references` lists the reference file names available next to it, so +/// pointers can be checked for dangling targets. +pub fn validate_site_md(text: &str, references: &[String]) -> Report { + let mut report = Report::default(); + let lines: Vec<&str> = text.lines().collect(); + + if lines.len() > SITE_MD_HARD_LINE_BUDGET { + report.errors.push(format!( + "SITE.md is {} lines, over the {SITE_MD_HARD_LINE_BUDGET}-line budget. Rewrite it to \ + {SITE_MD_SOFT_LINE_BUDGET} lines or fewer and move the detail into \ + references/.md, linked as `](references/.md)`.", + lines.len() + )); + } else if lines.len() > SITE_MD_SOFT_LINE_BUDGET { + report.warnings.push(format!( + "SITE.md is {} lines, past the {SITE_MD_SOFT_LINE_BUDGET}-line target. Move detail \ + into references/.md before it reaches {SITE_MD_HARD_LINE_BUDGET}.", + lines.len() + )); + } + + report.merge(check_verified_dates(&lines)); + report.merge(check_reference_pointers(text, references)); + report.merge(check_no_secrets(&lines, "SITE.md")); + report +} + +/// Refuse credential material anywhere in a prose memory file. +/// +/// `SITE.md` and `references/*.md` are free text an agent writes, which is +/// exactly where a pasted header or token ends up. They live next to +/// `~/.bsk/audit/` and must keep the same promise. +pub fn check_no_secrets(lines: &[&str], label: &str) -> Report { + let mut report = Report::default(); + for (index, line) in lines.iter().enumerate() { + if let Some(reason) = find_secret(line) { + report + .errors + .push(format!("{label} line {}: {reason}", index + 1)); + } + } + report +} + +/// Validate one `references/.md` file. +pub fn validate_reference(name: &str, text: &str) -> Report { + let mut report = Report::default(); + if text.len() > REFERENCE_MAX_BYTES { + report.errors.push(format!( + "references/{name}.md is {} bytes, over the {REFERENCE_MAX_BYTES}-byte limit. A \ + reference holds detail a human reads, not a page dump.", + text.len() + )); + } + let lines: Vec<&str> = text.lines().collect(); + report.merge(check_no_secrets(&lines, &format!("references/{name}.md"))); + report +} + +/// Every persistent fact carries `[verified YYYY-MM-DD]`. +/// +/// Facts are folded before they are judged: an indented line continues the +/// fact above it, so a markdown list item wrapped over several lines is one +/// claim carrying one stamp, not N unstamped lines. Exempt entirely: +/// frontmatter, fenced code, table separator and header rows, headings and +/// horizontal rules — a section title is structure, not a claim about the site. +fn check_verified_dates(lines: &[&str]) -> Report { + let mut report = Report::default(); + let mut state = ScanState::default(); + let mut pending: Option = None; + for (index, raw) in lines.iter().enumerate() { + match state.classify(index, raw, lines) { + LineKind::Exempt => { + if let Some(fact) = pending.take() { + fact.check(&mut report); + } + } + // An indented line with nothing above it is not a continuation of + // anything — it is a claim that tried to hide behind its indent. + LineKind::Continuation(text) => match pending.as_mut() { + Some(fact) => fact.push(text), + None => pending = Some(Fact::new(index, text)), + }, + LineKind::Fact(text) => { + if let Some(fact) = pending.replace(Fact::new(index, text)) { + fact.check(&mut report); + } + } + } + } + if let Some(fact) = pending.take() { + fact.check(&mut report); + } + if let Some(problem) = state.unclosed_fence() { + report.errors.push(problem); + } + report +} + +/// One logical claim: a line plus any indented lines that continue it. +struct Fact { + /// Zero-based index of the line the fact starts on. + line: usize, + text: String, +} + +impl Fact { + fn new(line: usize, text: &str) -> Self { + Self { + line, + text: text.to_string(), + } + } + + fn push(&mut self, text: &str) { + self.text.push(' '); + self.text.push_str(text); + } + + fn check(&self, report: &mut Report) { + match extract_verified_date(&self.text) { + None => report.errors.push(format!( + "SITE.md line {}: every persistent fact needs a `[verified YYYY-MM-DD]` stamp — \ + keep each fact on one line or indent continuation lines — {}", + self.line + 1, + truncate(&self.text, 72) + )), + Some(date) => { + if let Some(problem) = bad_date(date) { + report.errors.push(format!( + "SITE.md line {}: {problem} in `[verified {date}]`", + self.line + 1 + )); + } + } + } + } +} + +/// What one raw line contributes to the fact stream. +enum LineKind<'a> { + /// Structure or skipped content. Ends any open fact. + Exempt, + /// Indented: continues the fact above it. + Continuation(&'a str), + /// Starts a new fact. + Fact(&'a str), +} + +#[derive(Default)] +struct ScanState { + in_frontmatter: bool, + fence: Option, +} + +impl ScanState { + fn classify<'a>(&mut self, index: usize, raw: &'a str, lines: &[&str]) -> LineKind<'a> { + let trimmed = raw.trim(); + if index == 0 && trimmed == "---" && lines.len() > 1 { + self.in_frontmatter = true; + return LineKind::Exempt; + } + if self.in_frontmatter { + if trimmed == "---" || trimmed == "..." { + self.in_frontmatter = false; + } + return LineKind::Exempt; + } + if let Some(fence) = &self.fence { + if trimmed.starts_with(fence.as_str()) { + self.fence = None; + } + return LineKind::Exempt; + } + for marker in ["```", "~~~"] { + if trimmed.starts_with(marker) { + self.fence = Some(marker.to_string()); + return LineKind::Exempt; + } + } + if trimmed.is_empty() { + return LineKind::Exempt; + } + let structural = trimmed.starts_with('#') + || is_horizontal_rule(trimmed) + || is_table_separator(trimmed) + // A table header is structure; the separator row below it says so. + || lines + .get(index + 1) + .is_some_and(|next| is_table_separator(next.trim())); + if structural { + return LineKind::Exempt; + } + // Strip blockquote markers so `> a fact [verified …]` is judged on its + // content rather than its quoting. A blockquote is quoted material, but + // it still asserts something about the site, so the rule applies. + let content = trimmed.trim_start_matches(['>', ' ']).trim(); + if content.is_empty() { + return LineKind::Exempt; + } + if raw.starts_with(" ") || raw.starts_with('\t') { + return LineKind::Continuation(content); + } + LineKind::Fact(content) + } + + /// Report an unterminated fence at end of input: silently swallowing the + /// rest of the file would exempt every fact below it. + fn unclosed_fence(&self) -> Option { + self.fence + .as_ref() + .map(|fence| format!("SITE.md has an unclosed `{fence}` code fence")) + } +} + +fn is_horizontal_rule(line: &str) -> bool { + line.len() >= 3 + && (line.chars().all(|c| c == '-') + || line.chars().all(|c| c == '*') + || line.chars().all(|c| c == '_')) +} + +fn is_table_separator(line: &str) -> bool { + line.starts_with('|') + && line.contains('-') + && line + .chars() + .all(|c| matches!(c, '|' | '-' | ':' | ' ' | '+')) +} + +/// Pull the date out of a `[verified YYYY-MM-DD]` stamp. +fn extract_verified_date(line: &str) -> Option<&str> { + let start = line.find("[verified ")? + "[verified ".len(); + let rest = &line[start..]; + let end = rest.find(']')?; + Some(&rest[..end]) +} + +fn bad_date(date: &str) -> Option { + let Some(parsed) = parse_date(date) else { + return Some(format!("{date:?} is not a YYYY-MM-DD date")); + }; + if parsed > time::OffsetDateTime::now_utc().date() { + return Some(format!("{date:?} is in the future")); + } + None +} + +/// `](references/.md)` pointers must resolve. +fn check_reference_pointers(text: &str, references: &[String]) -> Report { + let mut report = Report::default(); + let known: BTreeSet<&str> = references.iter().map(String::as_str).collect(); + let mut rest = text; + while let Some(idx) = rest.find("](references/") { + rest = &rest[idx + "](references/".len()..]; + let Some(end) = rest.find(')') else { break }; + let target = &rest[..end]; + rest = &rest[end..]; + let Some(stem) = target.strip_suffix(".md") else { + report.errors.push(format!( + "SITE.md points at references/{target}, but reference files must end in .md" + )); + continue; + }; + if !known.contains(stem) { + report.errors.push(format!( + "SITE.md points at references/{target}, which does not exist in this draft" + )); + } + } + report +} + +fn truncate(text: &str, max: usize) -> String { + if text.chars().count() <= max { + return text.to_string(); + } + let head: String = text.chars().take(max).collect(); + format!("{head}…") +} + +// --------------------------------------------------------------------------- +// Workflows +// --------------------------------------------------------------------------- + +/// Validate one workflow document plus its raw JSON. +/// +/// `raw` is scanned separately so a rejected field such as `ref` is reported +/// with an actionable message rather than only as a serde parse failure. +pub fn validate_workflow(workflow: &Workflow, raw: &serde_json::Value, host: &str) -> Report { + let mut report = Report::default(); + let id = &workflow.id; + + if workflow.schema_version != WORKFLOW_SCHEMA_VERSION { + report.errors.push(format!( + "workflow {id}: schemaVersion must be {WORKFLOW_SCHEMA_VERSION}, found {}", + workflow.schema_version + )); + } + if let Err(err) = store::validate_id(id, "workflow") { + report.errors.push(format!("workflow {id}: {err}")); + } + if workflow.host != host { + report.errors.push(format!( + "workflow {id}: host is {:?} but it lives under {host:?}", + workflow.host + )); + } + if workflow.strategy != WorkflowStrategy::UiOnly { + report + .errors + .push(format!("workflow {id}: strategy must be \"ui-only\"")); + } + if workflow.secrets_policy.store_values { + report.errors.push(format!( + "workflow {id}: secretsPolicy.storeValues must be false — site memory never stores \ + recorded input values" + )); + } + if let Some(date) = &workflow.last_verified + && let Some(problem) = bad_date(date) + { + report + .errors + .push(format!("workflow {id}: lastVerified {problem}")); + } + if let Some(reason) = find_secret(&raw.to_string()) { + report.errors.push(format!("workflow {id}: {reason}")); + } + if let Some(path) = find_ref_key(raw) { + report.errors.push(format!( + "workflow {id}: `{path}` carries a record-local element ref. Refs die with the \ + recording session; keep only role/name/ctx." + )); + } + report.merge(validate_steps(workflow)); + report +} + +fn validate_steps(workflow: &Workflow) -> Report { + let mut report = Report::default(); + let id = &workflow.id; + if workflow.steps.is_empty() { + report + .errors + .push(format!("workflow {id}: steps must not be empty")); + return report; + } + let params: BTreeSet<&str> = workflow.params.iter().map(|p| p.name.as_str()).collect(); + let secrets: BTreeSet<&str> = workflow + .params + .iter() + .filter(|p| p.secret) + .map(|p| p.name.as_str()) + .collect(); + + for (index, step) in workflow.steps.iter().enumerate() { + let expected = index as u32 + 1; + if step.n != expected { + report.errors.push(format!( + "workflow {id}: step {} is numbered {}, expected {expected}", + index + 1, + step.n + )); + } + if let Some(name) = &step.value_from { + if !params.contains(name.as_str()) { + report.errors.push(format!( + "workflow {id} step {expected}: valueFrom {name:?} is not a declared parameter" + )); + } + if step.value.is_some() && secrets.contains(name.as_str()) { + report.errors.push(format!( + "workflow {id} step {expected}: a secret parameter must not carry an inline \ + value" + )); + } + } + report.merge(validate_step_shape(id, step, expected)); + } + report +} + +fn validate_step_shape(id: &str, step: &super::model::WorkflowStep, n: u32) -> Report { + let mut report = Report::default(); + let mut require_value = |what: &str| { + if step.value.is_none() && step.value_from.is_none() { + report.errors.push(format!( + "workflow {id} step {n}: op {what} requires `value` or `valueFrom`" + )); + } + }; + match step.op { + StepOp::Navigate => match step.to.as_deref() { + None => report + .errors + .push(format!("workflow {id} step {n}: op navigate requires `to`")), + Some(to) if !to.starts_with("http://") && !to.starts_with("https://") => { + report.errors.push(format!( + "workflow {id} step {n}: navigate target {to:?} must be an http(s) URL" + )); + } + Some(_) => {} + }, + StepOp::Click | StepOp::Hover => { + if step.target.is_none() && !step.needs_review { + report.errors.push(format!( + "workflow {id} step {n}: op {} has no target; an anchorless step must be \ + flagged needsReview", + step.op.as_str() + )); + } + } + StepOp::Fill => require_value("fill"), + StepOp::Select => require_value("select"), + StepOp::Press => { + if step.key.as_deref().unwrap_or_default().is_empty() { + report + .errors + .push(format!("workflow {id} step {n}: op press requires `key`")); + } + } + } + report +} + +/// Locate any `ref` key anywhere in the document, reporting its JSON path. +fn find_ref_key(value: &serde_json::Value) -> Option { + fn walk(value: &serde_json::Value, path: &str) -> Option { + match value { + serde_json::Value::Object(map) => { + for (key, child) in map { + let next = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") + }; + if key == "ref" { + return Some(next); + } + if let Some(found) = walk(child, &next) { + return Some(found); + } + } + None + } + serde_json::Value::Array(items) => items + .iter() + .enumerate() + .find_map(|(i, child)| walk(child, &format!("{path}[{i}]"))), + _ => None, + } + } + walk(value, "") +} + +/// Parse and validate every workflow JSON file in a directory. +pub fn validate_workflow_dir(dir: &Path, host: &str) -> Report { + let mut report = Report::default(); + let stems = match store::list_stems(dir, "json") { + Ok(stems) => stems, + Err(err) => { + report.errors.push(format!("{}: {err:#}", dir.display())); + return report; + } + }; + for stem in stems { + let name = format!("{stem}.json"); + let path = match store::resolve_in(dir, &[name.as_str()]) { + Ok(path) => path, + Err(err) => { + report.errors.push(format!("{name}: {err:#}")); + continue; + } + }; + let raw: serde_json::Value = match store::read_json(&path) { + Ok(raw) => raw, + Err(err) => { + report.errors.push(format!("{name}: {err:#}")); + continue; + } + }; + match serde_json::from_value::(raw.clone()) { + Ok(workflow) => { + if workflow.id != stem { + report.errors.push(format!( + "{name}: workflow id is {:?} but the file is named {stem}.json", + workflow.id + )); + } + report.merge(validate_workflow(&workflow, &raw, host)); + } + Err(err) => report.errors.push(format!("{name}: {err}")), + } + } + report +} + +// --------------------------------------------------------------------------- +// Candidate promotion +// --------------------------------------------------------------------------- + +/// Whether `candidate` may be ingested, given every candidate on this host. +/// +/// Ordinary kinds need a second independent observation on a different UTC +/// day; `high_consequence` is exempt because waiting for a repeat means +/// letting the damage happen twice (design §3.4). +pub fn check_corroboration(candidate: &Candidate, all: &[Candidate]) -> Result<(), String> { + if !candidate.kind.needs_corroboration() { + return Ok(()); + } + let claim = normalize_claim(&candidate.claim); + let days: BTreeSet<&str> = all + .iter() + .filter(|other| { + other.host == candidate.host + && other.status != CandidateStatus::Rejected + && normalize_claim(&other.claim) == claim + }) + .map(|other| other.observed_date_utc.as_str()) + .collect(); + if days.len() >= 2 { + return Ok(()); + } + Err(format!( + "candidate {} ({}) was observed on {} day(s); an ordinary claim needs two independent \ + observations on different UTC dates before it may change site memory. Record the second \ + sighting with `bsk site candidate add`, or use kind high_consequence when one sighting is \ + already decisive.", + candidate.id, + candidate.kind.as_str(), + days.len().max(1) + )) +} + +/// Kinds accepted by `workflow verify --fail`. +pub fn is_failure_kind(kind: CandidateKind) -> bool { + matches!(kind, CandidateKind::RepeatedMistake | CandidateKind::Access) +} + +#[cfg(test)] +mod tests { + use super::super::model::{Candidate, SecretsPolicy, StepTarget, WorkflowParam, WorkflowStep}; + use super::*; + + fn workflow(steps: Vec, params: Vec) -> Workflow { + Workflow { + schema_version: WORKFLOW_SCHEMA_VERSION, + id: "submit-ticket".into(), + host: "corp.example".into(), + purpose: None, + strategy: WorkflowStrategy::UiOnly, + last_verified: None, + source_trace: None, + params, + preconditions: Vec::new(), + steps, + postcondition: None, + secrets_policy: SecretsPolicy::default(), + needs_review: false, + review_notes: Vec::new(), + } + } + + fn step(n: u32, op: StepOp) -> WorkflowStep { + WorkflowStep { + n, + op, + to: None, + target: Some(StepTarget { + role: Some("button".into()), + name: Some("提交".into()), + ctx: None, + }), + value: None, + value_from: None, + commit: None, + key: None, + modifiers: None, + note: None, + needs_review: false, + } + } + + fn check(wf: &Workflow) -> Report { + let raw = serde_json::to_value(wf).unwrap(); + validate_workflow(wf, &raw, "corp.example") + } + + #[test] + fn sensitive_hosts_are_refused_without_blocking_lookalikes() { + assert!(sensitive_host_reason("login.microsoftonline.com").is_some()); + assert!(sensitive_host_reason("www.paypal.com").is_some()); + assert!(sensitive_host_reason("sso.corp.example").is_some()); + assert!(sensitive_host_reason("my-bank.corp.example").is_some()); + assert!(sensitive_host_reason("netbank.example.co.uk").is_some()); + + assert!(sensitive_host_reason("ticket.corp.example").is_none()); + // "authoring" merely starts with "auth"; label matching must not fire. + assert!(sensitive_host_reason("authoring.corp.example").is_none()); + assert!(sensitive_host_reason("notpaypal.com.example").is_none()); + } + + #[test] + fn secret_markers_are_detected_and_plain_text_passes() { + assert!(find_secret("Authorization: Bearer abc123").is_some()); + assert!(find_secret("set the cookie=session_id").is_some()); + assert!(find_secret("password=hunter2").is_some()); + assert!(find_secret("eyJhbGciOiJIUzI1NiJ9.payload").is_some()); + assert!(find_secret("-----BEGIN RSA PRIVATE KEY-----").is_some()); + + assert!(find_secret("the submit button lives in the bottom bar").is_none()); + // A bare `eyj` without a JWT-shaped tail is not a token. + assert!(find_secret("eyj is not a token").is_none()); + } + + #[test] + fn site_md_requires_verified_stamps_outside_exempt_blocks() { + let text = "---\ntitle: x\n---\n\n# Heading\n\n\ + Submitting needs the P2 dropdown. [verified 2026-09-15]\n\n\ + ```sh\nbsk observe\n```\n\n\ + | a | b |\n|---|---|\n| one | two | [verified 2026-09-15]\n\n\ + > quoted note [verified 2026-09-15]\n\ + - a fact with no stamp\n"; + let report = validate_site_md(text, &[]); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!(report.errors[0].contains("a fact with no stamp")); + } + + /// M10: a blockquote asserts something about the site too, so quoting a + /// claim must not exempt it. + #[test] + fn blockquotes_still_need_a_stamp() { + let report = validate_site_md("> submit is at the bottom\n", &[]); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!(report.errors[0].contains("submit is at the bottom")); + assert!(validate_site_md("> submit is at the bottom [verified 2026-09-15]\n", &[]).is_ok()); + } + + /// M10: indenting a claim must not exempt it — only a continuation of a + /// line that already carried a stamp is exempt. + #[test] + fn indented_lines_only_continue_a_stamped_fact() { + let continuation = "a fact [verified 2026-09-15]\n and its continuation\n"; + assert!(validate_site_md(continuation, &[]).is_ok()); + + let smuggled = "# Heading\n\n an indented claim with no stamp\n"; + let report = validate_site_md(smuggled, &[]); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!(report.errors[0].contains("an indented claim")); + } + + /// A markdown list item wrapped over several lines is one fact carrying + /// one stamp. Validating line-by-line used to make that impossible to write. + #[test] + fn a_wrapped_list_item_is_one_fact() { + let wrapped = "- the search box is in the header, and submitting it\n jumps straight to the article [verified 2026-09-15]\n"; + assert!( + validate_site_md(wrapped, &[]).is_ok(), + "{:?}", + validate_site_md(wrapped, &[]) + ); + + let stamped_on_the_first_line = "- the search box is in the header [verified 2026-09-15]\n and submitting it jumps to the article\n"; + assert!(validate_site_md(stamped_on_the_first_line, &[]).is_ok()); + } + + /// Folding must not become an escape hatch: a wrapped fact with no stamp + /// anywhere still fails, and it is reported once, on the line it starts on. + #[test] + fn a_wrapped_fact_without_a_stamp_fails_once() { + let report = validate_site_md( + "# Heading\n\n- the search box is in the header\n and submitting it jumps\n", + &[], + ); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!( + report.errors[0].contains("SITE.md line 3"), + "{:?}", + report.errors + ); + assert!( + report.errors[0].contains("keep each fact on one line or indent continuation lines"), + "{:?}", + report.errors + ); + assert!( + report.errors[0].contains("and submitting it jumps"), + "{:?}", + report.errors + ); + } + + /// Two separate list items stay two separate facts. + #[test] + fn sibling_list_items_are_judged_separately() { + let report = validate_site_md( + "- first fact [verified 2026-09-15]\n- second fact with no stamp\n", + &[], + ); + assert_eq!(report.errors.len(), 1, "{:?}", report.errors); + assert!( + report.errors[0].contains("second fact"), + "{:?}", + report.errors + ); + } + + /// M10: an unclosed fence used to swallow the rest of the file. + #[test] + fn an_unclosed_code_fence_is_an_error() { + let report = validate_site_md("```sh\nbsk observe\na fact with no stamp\n", &[]); + assert!( + report.errors.iter().any(|e| e.contains("unclosed")), + "{:?}", + report.errors + ); + } + + /// H4: SITE.md is free text an agent writes; it must not carry secrets. + #[test] + fn site_md_refuses_credential_material() { + let report = validate_site_md( + "send Authorization: Bearer abc123def456 [verified 2026-09-15]\n", + &[], + ); + assert!( + report.errors.iter().any(|e| e.contains("bearer token")), + "{:?}", + report.errors + ); + } + + #[test] + fn references_are_size_capped_and_secret_scanned() { + assert!(validate_reference("checkout", "plain prose").is_ok()); + + let secret = validate_reference("checkout", "cookie=session_id=abc"); + assert!( + secret.errors.iter().any(|e| e.contains("cookie")), + "{:?}", + secret.errors + ); + + let huge = "x".repeat(REFERENCE_MAX_BYTES + 1); + let big = validate_reference("dump", &huge); + assert!( + big.errors.iter().any(|e| e.contains("over the")), + "{:?}", + big.errors + ); + } + + #[test] + fn site_md_rejects_bad_and_future_verified_dates() { + let report = validate_site_md("a fact [verified 2026-13-40]\n", &[]); + assert!( + report.errors[0].contains("not a YYYY-MM-DD"), + "{:?}", + report + ); + + let report = validate_site_md("a fact [verified 2099-01-01]\n", &[]); + assert!(report.errors[0].contains("future"), "{:?}", report); + } + + #[test] + fn site_md_line_budget_warns_then_rejects() { + let line = "a fact [verified 2026-09-15]\n"; + let under = line.repeat(SITE_MD_SOFT_LINE_BUDGET); + assert!(validate_site_md(&under, &[]).is_ok()); + assert!(validate_site_md(&under, &[]).warnings.is_empty()); + + let soft = line.repeat(SITE_MD_SOFT_LINE_BUDGET + 1); + let report = validate_site_md(&soft, &[]); + assert!(report.is_ok()); + assert_eq!(report.warnings.len(), 1); + + let hard = line.repeat(SITE_MD_HARD_LINE_BUDGET + 1); + let report = validate_site_md(&hard, &[]); + assert!(!report.is_ok()); + assert!(report.errors[0].contains("references/.md")); + } + + #[test] + fn site_md_reference_pointers_must_resolve() { + let text = "See [detail](references/checkout.md) [verified 2026-09-15]\n"; + assert!(validate_site_md(text, &["checkout".to_string()]).is_ok()); + let report = validate_site_md(text, &[]); + assert!(report.errors[0].contains("does not exist"), "{:?}", report); + } + + #[test] + fn workflow_rejects_refs_anywhere_in_the_document() { + let wf = workflow(vec![step(1, StepOp::Click)], Vec::new()); + let mut raw = serde_json::to_value(&wf).unwrap(); + raw["steps"][0]["target"]["ref"] = serde_json::json!("e12"); + let report = validate_workflow(&wf, &raw, "corp.example"); + assert!( + report.errors[0].contains("record-local element ref"), + "{report:?}" + ); + } + + #[test] + fn workflow_rejects_non_ui_only_and_stored_values() { + let mut wf = workflow(vec![step(1, StepOp::Click)], Vec::new()); + wf.secrets_policy.store_values = true; + let report = check(&wf); + assert!(report.errors.iter().any(|e| e.contains("storeValues"))); + } + + #[test] + fn workflow_requires_dense_numbering_and_declared_parameters() { + let mut fill = step(2, StepOp::Fill); + fill.value_from = Some("title".into()); + let wf = workflow(vec![step(1, StepOp::Click), fill], Vec::new()); + let report = check(&wf); + assert!( + report + .errors + .iter() + .any(|e| e.contains("not a declared parameter")) + ); + + let mut misnumbered = + workflow(vec![step(1, StepOp::Click), step(3, StepOp::Click)], vec![]); + misnumbered.steps[1].n = 3; + let report = check(&misnumbered); + assert!(report.errors.iter().any(|e| e.contains("expected 2"))); + } + + #[test] + fn workflow_requires_op_specific_fields() { + let wf = workflow(vec![step(1, StepOp::Navigate)], Vec::new()); + assert!( + check(&wf) + .errors + .iter() + .any(|e| e.contains("requires `to`")) + ); + + let mut nav = step(1, StepOp::Navigate); + nav.to = Some("ftp://corp.example".into()); + let wf = workflow(vec![nav], Vec::new()); + assert!(check(&wf).errors.iter().any(|e| e.contains("http(s) URL"))); + + let wf = workflow(vec![step(1, StepOp::Fill)], Vec::new()); + assert!( + check(&wf) + .errors + .iter() + .any(|e| e.contains("`value` or `valueFrom`")) + ); + + let wf = workflow(vec![step(1, StepOp::Press)], Vec::new()); + assert!( + check(&wf) + .errors + .iter() + .any(|e| e.contains("requires `key`")) + ); + } + + #[test] + fn anchorless_step_must_be_flagged_for_review() { + let mut anchorless = step(1, StepOp::Click); + anchorless.target = None; + let wf = workflow(vec![anchorless.clone()], Vec::new()); + assert!(check(&wf).errors.iter().any(|e| e.contains("needsReview"))); + + anchorless.needs_review = true; + let wf = workflow(vec![anchorless], Vec::new()); + assert!(check(&wf).is_ok()); + } + + fn candidate(id: &str, date: &str, kind: CandidateKind, claim: &str) -> Candidate { + Candidate { + id: id.into(), + host: "corp.example".into(), + observed_date_utc: date.into(), + kind, + claim: claim.into(), + evidence: None, + consequence: None, + status: CandidateStatus::Pending, + workflow_id: None, + } + } + + #[test] + fn ordinary_candidates_need_two_distinct_days() { + let one = candidate( + "c1", + "2026-09-15", + CandidateKind::BetterPath, + "Use the P2 dropdown", + ); + assert!(check_corroboration(&one, std::slice::from_ref(&one)).is_err()); + + let same_day = candidate( + "c2", + "2026-09-15", + CandidateKind::BetterPath, + "use the p2 dropdown", + ); + assert!(check_corroboration(&one, &[one.clone(), same_day]).is_err()); + + let other_day = candidate( + "c3", + "2026-09-16", + CandidateKind::BetterPath, + "use the P2 DROPDOWN", + ); + assert!(check_corroboration(&one, &[one.clone(), other_day]).is_ok()); + } + + #[test] + fn high_consequence_candidates_skip_corroboration() { + let one = candidate( + "c1", + "2026-09-15", + CandidateKind::HighConsequence, + "deletes silently", + ); + assert!(check_corroboration(&one, std::slice::from_ref(&one)).is_ok()); + } + + #[test] + fn rejected_candidates_do_not_corroborate() { + let one = candidate("c1", "2026-09-15", CandidateKind::Access, "needs VPN"); + let mut rejected = candidate("c2", "2026-09-16", CandidateKind::Access, "needs VPN"); + rejected.status = CandidateStatus::Rejected; + assert!(check_corroboration(&one, &[one.clone(), rejected]).is_err()); + } +} diff --git a/crates/bsk-cli/src/daemon/ipc.rs b/crates/bsk-cli/src/daemon/ipc.rs index 69daf58c..12f7309a 100644 --- a/crates/bsk-cli/src/daemon/ipc.rs +++ b/crates/bsk-cli/src/daemon/ipc.rs @@ -472,6 +472,17 @@ async fn handle_tool_dispatch( .await }; drop(inflight_guard); + // Keep the idle reaper away from a session whose recording is armed + // (detached `record start` has no inflight command to protect it). + match (&method, &outcome) { + (Method::ToolRecordStart, Ok(_)) => { + state.sessions.set_recording(&session_id, true); + } + (Method::ToolRecordStop | Method::ToolRecordAwait, _) => { + state.sessions.set_recording(&session_id, false); + } + _ => {} + } match outcome { Ok(v) if method == Method::ToolDownload => { let id = download_transfer_id.expect("download transfer allocated"); diff --git a/crates/bsk-cli/src/daemon/paths.rs b/crates/bsk-cli/src/daemon/paths.rs index 0c42a030..0784f38d 100644 --- a/crates/bsk-cli/src/daemon/paths.rs +++ b/crates/bsk-cli/src/daemon/paths.rs @@ -75,17 +75,59 @@ fn prepare_home(home: &Path) -> Result<()> { } fn ensure_run_dir(home: &Path) -> Result<()> { - let run = home.join("run"); - if !run.exists() { - std::fs::create_dir_all(&run).with_context(|| format!("create {}", run.display()))?; + ensure_private_dir(&home.join("run")) +} + +/// Create `dir` (and parents) if missing and tighten it to 0700 on Unix. +/// +/// Shared by every BSK_HOME subtree that may hold user-derived data +/// (`run/`, `sites/`) so they all carry the same permission promise as the +/// home itself. +/// +/// Every level this call brings into existence is created and chmodded +/// individually rather than through `create_dir_all`, which would leave the +/// intermediate levels at the process umask: `sites/.drafts//` +/// used to end up with a world-readable `.drafts/` and `.drafts//`, +/// letting any other account on the machine enumerate task ids. Directories +/// that already existed keep their own permissions except for `dir` itself. +pub fn ensure_private_dir(dir: &Path) -> Result<()> { + if !dir.exists() { + let mut missing: Vec<&Path> = Vec::new(); + let mut cursor = Some(dir); + while let Some(path) = cursor { + if path.as_os_str().is_empty() || path.exists() { + break; + } + missing.push(path); + cursor = path.parent(); + } + for path in missing.into_iter().rev() { + match std::fs::create_dir(path) { + Ok(()) => {} + // A concurrent agent may have created the same level first. + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(err) => { + return Err( + anyhow::Error::from(err).context(format!("create {}", path.display())) + ); + } + } + set_private_mode(path)?; + } } + set_private_mode(dir) +} + +fn set_private_mode(dir: &Path) -> Result<()> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; let perms = std::fs::Permissions::from_mode(0o700); - std::fs::set_permissions(&run, perms) - .with_context(|| format!("chmod 0700 {}", run.display()))?; + std::fs::set_permissions(dir, perms) + .with_context(|| format!("chmod 0700 {}", dir.display()))?; } + #[cfg(not(unix))] + let _ = dir; Ok(()) } @@ -128,6 +170,36 @@ pub fn record_recovery_path() -> Result { Ok(bsk_home()?.join("record-recovery.json")) } +/// Root of the local site-memory tree (`$BSK_HOME/sites`). +/// +/// Holds the same kind of user-derived data as `audit/`, so it inherits the +/// 0700 directory promise from [`ensure_private_dir`]. +pub fn sites_root() -> Result { + Ok(bsk_home()?.join("sites")) +} + +/// Ensure the site-memory root exists with restrictive permissions. +pub fn ensure_sites_root() -> Result { + let home = ensure_bsk_home()?; + let root = home.join("sites"); + ensure_private_dir(&root)?; + Ok(root) +} + +/// Active memory directory for one already-normalised host (design §5). +/// +/// `host` must be a single safe path segment; callers go through +/// `cli::site::store::normalize_host`, which guarantees that. The directory is +/// not created here. +pub fn site_dir(host: &str) -> Result { + Ok(sites_root()?.join(host)) +} + +/// Draft root for one task (design §5). Not created here. +pub fn drafts_dir(task: &str) -> Result { + Ok(sites_root()?.join(".drafts").join(task)) +} + /// Windows named-pipe name. Include the resolved `BSK_HOME` path in the /// token so test homes and custom installs do not share a predictable /// per-username pipe. @@ -202,6 +274,21 @@ mod tests { }); } + #[test] + fn ensure_sites_root_creates_private_directory() { + with_temp_home(|_| { + let root = ensure_sites_root().unwrap(); + assert!(root.is_dir()); + assert_eq!(root, sites_root().unwrap()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&root).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o700); + } + }); + } + #[test] fn computes_expected_paths() { with_temp_home(|_| { @@ -219,6 +306,15 @@ mod tests { record_recovery_path().unwrap(), home.join("record-recovery.json") ); + assert_eq!(sites_root().unwrap(), home.join("sites")); + assert_eq!( + site_dir("corp.example").unwrap(), + home.join("sites").join("corp.example") + ); + assert_eq!( + drafts_dir("t1").unwrap(), + home.join("sites").join(".drafts").join("t1") + ); }); } diff --git a/crates/bsk-cli/src/daemon/sessions.rs b/crates/bsk-cli/src/daemon/sessions.rs index 48863cff..12b2c264 100644 --- a/crates/bsk-cli/src/daemon/sessions.rs +++ b/crates/bsk-cli/src/daemon/sessions.rs @@ -3,7 +3,7 @@ //! `bsk session stop`, `bsk session list` and the matching //! `tool.session_start` / `tool.session_stop` round-trips. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use std::sync::Mutex; use std::time::{Instant, SystemTime, UNIX_EPOCH}; @@ -73,6 +73,10 @@ pub struct SessionRegistry { /// Operational metadata kept outside the public `Session` wire/domain /// shape so idle enforcement does not break external struct users. last_activity: Mutex>, + /// Sessions with an armed recording. A detached `record start` leaves + /// no inflight tool behind, so the idle reaper must skip these + /// explicitly or a long agent pause silently discards the recording. + recording: Mutex>, } impl SessionRegistry { @@ -198,6 +202,10 @@ impl SessionRegistry { .lock() .expect("session activity registry poisoned") .remove(session_id); + self.recording + .lock() + .expect("session recording registry poisoned") + .remove(session_id); } pub fn remove(&self, id: &SessionId) -> Option { @@ -210,6 +218,10 @@ impl SessionRegistry { .lock() .expect("session activity registry poisoned") .remove(id); + self.recording + .lock() + .expect("session recording registry poisoned") + .remove(id); if removed.is_some() && let Some(audit) = &self.audit { @@ -256,20 +268,57 @@ impl SessionRegistry { true } + /// Mark or clear an armed recording on a live session. Returns false + /// when the session is unknown. + pub fn set_recording(&self, id: &SessionId, recording: bool) -> bool { + if !self + .inner + .lock() + .expect("session registry poisoned") + .contains_key(id) + { + return false; + } + let mut guard = self + .recording + .lock() + .expect("session recording registry poisoned"); + if recording { + guard.insert(id.clone()); + } else { + guard.remove(id); + } + true + } + + pub fn is_recording(&self, id: &SessionId) -> bool { + self.recording + .lock() + .expect("session recording registry poisoned") + .contains(id) + } + /// Return sessions whose last tool activity is at least `idle_for` - /// old. The caller supplies `now` to keep boundary tests deterministic. + /// old. Sessions with an armed recording are never reported: their + /// idleness is the user (or a paused agent) thinking, not abandonment. + /// The caller supplies `now` to keep boundary tests deterministic. pub fn idle_ids_at(&self, idle_for: Duration, now: Instant) -> Vec { let sessions = self.inner.lock().expect("session registry poisoned"); let activity = self .last_activity .lock() .expect("session activity registry poisoned"); + let recording = self + .recording + .lock() + .expect("session recording registry poisoned"); sessions .values() .filter(|session| { - activity - .get(&session.id) - .is_some_and(|last| now.saturating_duration_since(*last) >= idle_for) + !recording.contains(&session.id) + && activity + .get(&session.id) + .is_some_and(|last| now.saturating_duration_since(*last) >= idle_for) }) .map(|session| session.id.clone()) .collect() @@ -290,9 +339,15 @@ impl SessionRegistry { .last_activity .lock() .expect("session activity registry poisoned"); + let mut recording = self + .recording + .lock() + .expect("session recording registry poisoned"); for session in &drained { activity.remove(&session.id); + recording.remove(&session.id); } + drop(recording); drop(activity); drop(guard); if let Some(audit) = &self.audit { diff --git a/crates/bsk-cli/src/main.rs b/crates/bsk-cli/src/main.rs index 1fd447e4..dcdee687 100644 --- a/crates/bsk-cli/src/main.rs +++ b/crates/bsk-cli/src/main.rs @@ -108,6 +108,7 @@ fn dispatch(cli: Cli, format: Format) -> Result<(), CliError> { Command::WaitMs(args) => cli::waits::dispatch_wait_ms(args, format), Command::RequestHelp(args) => cli::human_loop::dispatch(args, format), Command::Record(cmd) => cli::record::dispatch(cmd, format), + Command::Site(cmd) => cli::site::dispatch(cmd, format), } } diff --git a/crates/bsk-cli/tests/cli_parse.rs b/crates/bsk-cli/tests/cli_parse.rs index 59773de5..29f6a8a2 100644 --- a/crates/bsk-cli/tests/cli_parse.rs +++ b/crates/bsk-cli/tests/cli_parse.rs @@ -472,6 +472,38 @@ fn parses_record_start_without_url() { assert!(args.url.is_none()); } +#[test] +fn parses_record_start_detach() { + let cli = parse(&[ + "bsk", + "record", + "start", + "--detach", + "--url", + "https://x", + "--output", + "rec", + ]); + let Command::Record(RecordCmd { + sub: RecordSub::Start(args), + }) = cli.command + else { + panic!("expected record start subcommand"); + }; + assert!(args.detach); + assert_eq!(args.output, std::path::PathBuf::from("rec")); + + // The blocking path stays the default so existing use is unchanged. + let cli = parse(&["bsk", "record", "start"]); + let Command::Record(RecordCmd { + sub: RecordSub::Start(args), + }) = cli.command + else { + panic!("expected record start subcommand"); + }; + assert!(!args.detach); +} + #[test] fn parses_session_start_with_window_size() { use bsk::cli::session::{SessionCmd, SessionSub}; @@ -804,3 +836,305 @@ fn canvas_click_requires_complete_capture_coordinates() { assert!(Cli::try_parse_from(argv).is_err()); } } + +// --- bsk site --------------------------------------------------------------- + +#[test] +fn parses_site_context_with_optional_task() { + use bsk::cli::site::{SiteSub, WorkflowSub}; + + let Command::Site(cmd) = parse(&["bsk", "site", "context", "--host", "corp.example"]).command + else { + panic!("expected site command"); + }; + let SiteSub::Context(args) = cmd.sub else { + panic!("expected site context"); + }; + assert_eq!(args.host, "corp.example"); + assert_eq!(args.task, None); + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "context", + "--host", + "corp.example", + "--task", + "T1", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Context(args) = cmd.sub else { + panic!("expected site context"); + }; + assert_eq!(args.task.as_deref(), Some("T1")); + + // `--host` is the only required flag; `workflow list` may omit it. + let Command::Site(cmd) = parse(&["bsk", "site", "workflow", "list"]).command else { + panic!("expected site command"); + }; + let SiteSub::Workflow(cmd) = cmd.sub else { + panic!("expected site workflow"); + }; + let WorkflowSub::List(args) = cmd.sub else { + panic!("expected workflow list"); + }; + assert_eq!(args.host, None); +} + +#[test] +fn parses_site_workflow_save_and_show() { + use bsk::cli::site::{SiteSub, WorkflowSub}; + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "workflow", + "save", + "--from", + "rec/trace.json", + "--id", + "submit-ticket", + "--host", + "ticket.corp.example", + "--purpose", + "submit a ticket", + "--task", + "T1", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Workflow(cmd) = cmd.sub else { + panic!("expected site workflow"); + }; + let WorkflowSub::Save(args) = cmd.sub else { + panic!("expected workflow save"); + }; + assert_eq!(args.from, std::path::PathBuf::from("rec/trace.json")); + assert_eq!(args.id, "submit-ticket"); + assert_eq!(args.host.as_deref(), Some("ticket.corp.example")); + assert_eq!(args.purpose.as_deref(), Some("submit a ticket")); + assert_eq!(args.task.as_deref(), Some("T1")); + + // `--from` and `--id` are both required. + assert!( + Cli::try_parse_from(["bsk", "site", "workflow", "save", "--id", "x"]).is_err(), + "workflow save must require --from" + ); + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "workflow", + "show", + "submit-ticket", + "--host", + "corp.example", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Workflow(cmd) = cmd.sub else { + panic!("expected site workflow"); + }; + let WorkflowSub::Show(args) = cmd.sub else { + panic!("expected workflow show"); + }; + assert_eq!(args.id, "submit-ticket"); + assert_eq!(args.host, "corp.example"); +} + +#[test] +fn site_workflow_verify_requires_exactly_one_outcome() { + use bsk::cli::site::model::CandidateKind; + use bsk::cli::site::{SiteSub, WorkflowSub}; + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "workflow", + "verify", + "submit-ticket", + "--host", + "corp.example", + "--pass", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Workflow(cmd) = cmd.sub else { + panic!("expected site workflow"); + }; + let WorkflowSub::Verify(args) = cmd.sub else { + panic!("expected workflow verify"); + }; + assert!(args.pass && !args.fail); + assert_eq!(args.kind, CandidateKind::RepeatedMistake); + + assert!( + Cli::try_parse_from([ + "bsk", + "site", + "workflow", + "verify", + "x", + "--host", + "corp.example", + ]) + .is_err(), + "verify must require --pass or --fail" + ); + assert!( + Cli::try_parse_from([ + "bsk", + "site", + "workflow", + "verify", + "x", + "--host", + "corp.example", + "--pass", + "--fail", + ]) + .is_err(), + "--pass and --fail are mutually exclusive" + ); +} + +#[test] +fn parses_site_candidate_commands_with_enum_values() { + use bsk::cli::site::model::{CandidateKind, CandidateStatus}; + use bsk::cli::site::{CandidateSub, SiteSub}; + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "candidate", + "add", + "--host", + "corp.example", + "--kind", + "high_consequence", + "--claim", + "delete has no confirm", + "--evidence", + "the row vanished", + "--consequence", + "data loss", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Candidate(cmd) = cmd.sub else { + panic!("expected site candidate"); + }; + let CandidateSub::Add(args) = cmd.sub else { + panic!("expected candidate add"); + }; + assert_eq!(args.kind, CandidateKind::HighConsequence); + assert_eq!(args.claim, "delete has no confirm"); + assert_eq!(args.evidence.as_deref(), Some("the row vanished")); + assert_eq!(args.consequence.as_deref(), Some("data loss")); + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "candidate", + "list", + "--host", + "corp.example", + "--kind", + "better_path", + "--status", + "pending", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Candidate(cmd) = cmd.sub else { + panic!("expected site candidate"); + }; + let CandidateSub::List(args) = cmd.sub else { + panic!("expected candidate list"); + }; + assert_eq!(args.kind, Some(CandidateKind::BetterPath)); + assert_eq!(args.status, Some(CandidateStatus::Pending)); + + assert!( + Cli::try_parse_from([ + "bsk", + "site", + "candidate", + "add", + "--host", + "corp.example", + "--kind", + "not_a_kind", + "--claim", + "x", + ]) + .is_err(), + "an unknown candidate kind must be rejected" + ); +} + +#[test] +fn parses_site_checkpoint_with_repeatable_dispositions() { + use bsk::cli::site::SiteSub; + use bsk::cli::site::model::CheckpointReason; + + let Command::Site(cmd) = parse(&[ + "bsk", + "site", + "checkpoint", + "--host", + "corp.example", + "--task", + "T1", + "--reason", + "candidate_ingestion", + "--expected-revision", + "7", + "--ingest", + "c1", + "--ingest", + "c2", + "--reject", + "c3", + ]) + .command + else { + panic!("expected site command"); + }; + let SiteSub::Checkpoint(args) = cmd.sub else { + panic!("expected site checkpoint"); + }; + assert_eq!(args.reason, CheckpointReason::CandidateIngestion); + assert_eq!(args.expected_revision, Some(7)); + assert_eq!(args.ingest, vec!["c1".to_string(), "c2".to_string()]); + assert_eq!(args.reject, vec!["c3".to_string()]); + + assert!( + Cli::try_parse_from([ + "bsk", + "site", + "checkpoint", + "--host", + "corp.example", + "--task", + "T1", + "--reason", + "nope", + ]) + .is_err(), + "an unknown checkpoint reason must be rejected" + ); +} diff --git a/crates/bsk-cli/tests/sessions_ipc.rs b/crates/bsk-cli/tests/sessions_ipc.rs index b94724ee..f488e166 100644 --- a/crates/bsk-cli/tests/sessions_ipc.rs +++ b/crates/bsk-cli/tests/sessions_ipc.rs @@ -1505,3 +1505,48 @@ async fn borrow_reports_unknown_outcome_when_cancel_cleanup_never_finishes() { drop(ws); handle.shutdown().await; } + +#[test] +fn armed_recording_keeps_the_idle_reaper_away_until_cleared() { + use bsk::daemon::{ + browsers::BrowserId, + sessions::{Session, SessionId, SessionRegistry}, + }; + use std::time::Instant; + let registry = SessionRegistry::new(); + let owner = BrowserId("owner".into()); + let recording = SessionId("recd".into()); + let plain = SessionId("plan".into()); + for id in [&recording, &plain] { + registry.insert(Session { + id: id.clone(), + browser_id: owner.clone(), + agent_window_id: Some(1), + created_at_ms: 0, + interaction: None, + }); + } + assert!(registry.set_recording(&recording, true)); + assert!(!registry.set_recording(&SessionId("nope".into()), true)); + assert!(registry.is_recording(&recording)); + + let later = Instant::now() + Duration::from_secs(600); + let idle = registry.idle_ids_at(Duration::from_secs(300), later); + assert_eq!( + idle, + vec![plain.clone()], + "recording session must not be reaped" + ); + + assert!(registry.set_recording(&recording, false)); + let mut idle = registry.idle_ids_at(Duration::from_secs(300), later); + idle.sort_by(|a, b| a.0.cmp(&b.0)); + assert_eq!(idle, vec![plain.clone(), recording.clone()]); + + registry.set_recording(&recording, true); + registry.remove(&recording); + assert!( + !registry.is_recording(&recording), + "removal clears the flag" + ); +} diff --git a/crates/bsk-cli/tests/site_cli.rs b/crates/bsk-cli/tests/site_cli.rs new file mode 100644 index 00000000..214b9304 --- /dev/null +++ b/crates/bsk-cli/tests/site_cli.rs @@ -0,0 +1,986 @@ +//! End-to-end `bsk site` coverage against a real `BSK_HOME`. +//! +//! These run the built binary rather than the library so the exit codes, the +//! `--json` envelope and the on-disk layout are all exercised the way an agent +//! sees them. None of the commands may start or need the daemon, so the whole +//! file runs with `BSK_AUTO_START=0`. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +use serde_json::Value; + +fn bin() -> PathBuf { + // `CARGO_BIN_EXE_` is set by cargo for integration tests. + PathBuf::from(env!("CARGO_BIN_EXE_bsk")) +} + +struct Env { + home: tempfile::TempDir, +} + +impl Env { + fn new() -> Self { + Self { + home: tempfile::tempdir().expect("temp home"), + } + } + + fn sites(&self) -> PathBuf { + self.home.path().join("sites") + } + + fn run(&self, args: &[&str]) -> Output { + Command::new(bin()) + .args(args) + .env("BSK_HOME", self.home.path()) + .env("BSK_AUTO_START", "0") + // Keep the update-check probe and tracing out of the captured output. + .env("BSK_NO_UPDATE_CHECK", "1") + .output() + .expect("run bsk") + } + + fn ok(&self, args: &[&str]) -> String { + let out = self.run(args); + assert!( + out.status.success(), + "expected success from {args:?}\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).into_owned() + } + + fn json(&self, args: &[&str]) -> Value { + let mut argv = vec!["--json"]; + argv.extend_from_slice(args); + let text = self.ok(&argv); + serde_json::from_str(&text).unwrap_or_else(|e| panic!("{args:?} -> {text}: {e}")) + } + + /// Run a command expected to fail, returning `(exit code, parsed --json)`. + fn json_err(&self, args: &[&str]) -> (i32, Value) { + let mut argv = vec!["--json"]; + argv.extend_from_slice(args); + let out = self.run(&argv); + assert!(!out.status.success(), "expected failure from {args:?}"); + let text = String::from_utf8_lossy(&out.stdout); + let value = serde_json::from_str(&text) + .unwrap_or_else(|e| panic!("{args:?} produced non-JSON {text}: {e}")); + (out.status.code().unwrap_or(-1), value) + } +} + +/// A trace bundle shaped exactly like `bsk record start --output ` writes +/// one: page text lives in `states/`, and `trace.json` references it as `page`. +fn write_bundle(dir: &Path) -> PathBuf { + std::fs::create_dir_all(dir.join("states")).unwrap(); + std::fs::write( + dir.join("states").join("s1.txt"), + "# bsk-observation 1\nRootWebArea \"新建工单\"\n", + ) + .unwrap(); + let trace = serde_json::json!({ + "version": 3, + "purpose": "提交一张工单", + "recorded_at": "2026-09-12T09:31:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://ticket.corp.example/" }, + "recorder": { "bsk": "0.2.1", "vom": 1 }, + "states": [{ + "id": "s1", + "url": "https://ticket.corp.example/new", + "title": "新建工单", + "page": "s1.txt" + }], + "steps": [ + { "op": "navigate", "id": 1, "state": "s1", "result": { "state": "s1" }, + "to": "https://ticket.corp.example/new", "cause": "user_typed" }, + { "op": "scroll", "id": 2, "state": "s1", "result": { "state": "s1" } }, + { "op": "fill", "id": 3, "state": "s1", "result": { "state": "s1" }, + "target": { "ref": "e12", "role": "textbox", "name": "title", "ctx": "工单信息" }, + "value": "打印机坏了", "commit": "blur" }, + { "op": "fill", "id": 4, "state": "s1", "result": { "state": "s1" }, + "target": { "ref": "e13", "role": "textbox", "name": "password" }, + "value": "***", "commit": "blur", "redacted": true }, + { "op": "click", "id": 5, "state": "s1", "result": { "state": "s1" }, + "target": { "ref": "e21", "role": "button", "name": "提交", "ctx": "底部操作栏" } } + ] + }); + let path = dir.join("trace.json"); + std::fs::write(&path, serde_json::to_string_pretty(&trace).unwrap()).unwrap(); + path +} + +const SITE_MD: &str = "# ticket.corp.example\n\n\ +需要公司 VPN 才能打开。 [verified 2026-09-15]\n\n\ +新建工单入口是 /new。 [verified 2026-09-15]\n"; + +#[test] +fn full_chain_from_a_record_bundle_to_verified_memory() { + let env = Env::new(); + let rec = env.home.path().join("rec"); + let trace = write_bundle(&rec); + let trace = trace.to_string_lossy().into_owned(); + let host = "ticket.corp.example"; + + // 1. context on an unknown host is empty but succeeds, and stages a draft. + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + assert_eq!(context["revision"], 0); + assert_eq!(context["readOnly"], false); + assert_eq!(context["workflows"].as_array().unwrap().len(), 0); + assert_eq!(context["pendingCandidates"], 0); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + assert!(draft.is_dir(), "context must stage a draft"); + assert_eq!(context["draft"]["baseRevision"], context["revision"]); + + // 2. save derives a workflow from the bundle the recorder actually wrote. + let saved = env.json(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "submit-ticket", + "--task", + "T1", + ]); + assert_eq!(saved["host"], host); + assert_eq!(saved["steps"], 4, "scroll is dropped, 5 steps become 4"); + assert_eq!(saved["droppedSteps"], 1); + assert_eq!(saved["needsReview"], true); + + // Neither the record-local ref nor any recorded value may reach disk. + let workflow_json = + std::fs::read_to_string(draft.join("workflows/submit-ticket.json")).unwrap(); + assert!(!workflow_json.contains("\"ref\""), "{workflow_json}"); + assert!(!workflow_json.contains("e12"), "{workflow_json}"); + assert!(!workflow_json.contains("打印机坏了"), "{workflow_json}"); + assert!(!workflow_json.contains("***"), "{workflow_json}"); + + // 3. checkpoint publishes the draft. + std::fs::write(draft.join("SITE.md"), SITE_MD).unwrap(); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + assert_eq!(committed["status"], "committed"); + assert_eq!(committed["revision"], 1); + assert!(env.sites().join(host).join("SITE.md").is_file()); + + // 4. show renders the published workflow. + let shown = env.json(&["site", "workflow", "show", "submit-ticket", "--host", host]); + assert_eq!(shown["strategy"], "ui-only"); + assert_eq!(shown["lastVerified"], Value::Null); + let human = env.ok(&["site", "workflow", "show", "submit-ticket", "--host", host]); + assert!( + human.contains("1. navigate https://ticket.corp.example/new"), + "{human}" + ); + assert!(human.contains("last verified: never"), "{human}"); + + // Freshly saved memory is UNVERIFIED, not STALE: nothing has expired. + let listed = env.json(&["site", "workflow", "list", "--host", host]); + assert_eq!( + listed[0]["stale"], false, + "a workflow saved now is not stale" + ); + assert_eq!(listed[0]["unverified"], true); + assert_eq!(listed[0]["needsReview"], true); + let listed_human = env.ok(&["site", "workflow", "list", "--host", host]); + assert!(listed_human.contains("UNVERIFIED"), "{listed_human}"); + assert!(!listed_human.contains("STALE"), "{listed_human}"); + + // 5. verify --pass stamps it, clears the review flags, and advances the revision. + let verified = env.json(&[ + "site", + "workflow", + "verify", + "submit-ticket", + "--host", + host, + "--pass", + ]); + assert_eq!(verified["outcome"], "pass"); + assert_eq!(verified["revision"], 2); + assert_eq!(verified["clearedReview"], true); + let listed = env.json(&["site", "workflow", "list", "--host", host]); + assert_eq!(listed[0]["stale"], false); + assert_eq!(listed[0]["unverified"], false); + assert_eq!( + listed[0]["needsReview"], false, + "running the workflow through IS the review" + ); + assert_eq!(listed[0]["daysSinceVerified"], 0); + let listed_human = env.ok(&["site", "workflow", "list", "--host", host]); + assert!(!listed_human.contains("NEEDS REVIEW"), "{listed_human}"); + assert!(!listed_human.contains("UNVERIFIED"), "{listed_human}"); + let shown_human = env.ok(&["site", "workflow", "show", "submit-ticket", "--host", host]); + assert!(!shown_human.contains("[needs review]"), "{shown_human}"); + assert!(!shown_human.contains("still needs review"), "{shown_human}"); + + // 6. candidates accumulate as pending evidence without touching memory. + let added = env.json(&[ + "site", + "candidate", + "add", + "--host", + host, + "--kind", + "better_path", + "--claim", + "priority matches the option value, not the label", + ]); + let candidate_id = added["id"].as_str().unwrap().to_string(); + let shown = env.json(&["site", "candidate", "show", &candidate_id, "--host", host]); + assert_eq!(shown["status"], "pending"); + let pending = env.json(&[ + "site", + "candidate", + "list", + "--host", + host, + "--status", + "pending", + ]); + assert_eq!(pending.as_array().unwrap().len(), 1); +} + +#[test] +fn a_stale_expected_revision_conflicts_then_recovers_after_context() { + let env = Env::new(); + let host = "ticket.corp.example"; + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + std::fs::write(draft.join("SITE.md"), SITE_MD).unwrap(); + env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + + // A second task branched from revision 0 now collides. + let other = env.json(&["site", "context", "--host", host, "--task", "T2"]); + let other_draft = PathBuf::from(other["draft"]["path"].as_str().unwrap()); + std::fs::write(other_draft.join("SITE.md"), SITE_MD).unwrap(); + let (code, conflict) = env.json_err(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T2", + "--reason", + "direct_correction", + "--expected-revision", + "0", + ]); + assert_eq!(conflict["status"], "conflict"); + assert_eq!(conflict["expected"], 0); + assert_eq!(conflict["actual"], 1); + assert_ne!(code, 0, "a conflict must not report success"); + + // The documented recovery: re-read context, then checkpoint again. + let refreshed = env.json(&["site", "context", "--host", host, "--task", "T2"]); + assert_eq!(refreshed["draft"]["baseRevision"], 1); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T2", + "--reason", + "direct_correction", + ]); + assert_eq!(committed["status"], "committed"); + assert_eq!(committed["revision"], 2); +} + +/// Every entry point applies the same credential-surface gate, and each failure +/// uses the CLI's standard `--json` error envelope. +#[test] +fn sensitive_hosts_are_refused_at_every_entry_point() { + let env = Env::new(); + let rec = env.home.path().join("rec"); + let trace = write_bundle(&rec); + let trace = trace.to_string_lossy().into_owned(); + + let cases: Vec> = vec![ + vec!["site", "context", "--host", "okta.com"], + vec![ + "site", "workflow", "save", "--from", &trace, "--id", "x", "--host", "okta.com", + ], + vec!["site", "workflow", "show", "x", "--host", "okta.com"], + vec![ + "site", "workflow", "verify", "x", "--host", "okta.com", "--pass", + ], + vec![ + "site", + "candidate", + "add", + "--host", + "okta.com", + "--kind", + "access", + "--claim", + "x", + ], + vec!["site", "candidate", "list", "--host", "okta.com"], + vec![ + "site", + "checkpoint", + "--host", + "okta.com", + "--task", + "T1", + "--reason", + "direct_correction", + ], + ]; + for args in cases { + let (code, body) = env.json_err(&args); + assert_eq!(code, 1, "{args:?}"); + assert_eq!(body["code"], "invalid_params", "{args:?} -> {body}"); + assert_eq!(body["exit_code"], 1, "{args:?}"); + let message = body["message"].as_str().unwrap_or_default(); + assert!( + message.contains("blocked domain") || message.contains("credential-surface"), + "{args:?} -> {message}" + ); + } + assert!(!env.sites().join("okta.com").exists()); +} + +/// H2: a path or query must not be able to name a different host. +#[test] +fn userinfo_in_a_path_cannot_smuggle_a_host_past_the_gate() { + let env = Env::new(); + let (_, body) = env.json_err(&[ + "site", + "context", + "--host", + "https://login.microsoftonline.com/x@ticket.corp.example", + ]); + assert!( + body["message"] + .as_str() + .unwrap_or_default() + .contains("login.microsoftonline.com"), + "{body}" + ); + + // The benign case still resolves to the authority's host. + let context = env.json(&[ + "site", + "context", + "--host", + "https://a.example.com/r?to=x@okta.com", + ]); + assert_eq!(context["host"], "a.example.com"); +} + +#[test] +fn a_recording_cannot_be_refiled_under_a_different_host() { + let env = Env::new(); + let rec = env.home.path().join("rec"); + let trace = write_bundle(&rec); + let trace = trace.to_string_lossy().into_owned(); + let (_, body) = env.json_err(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "x", + "--host", + "elsewhere.example", + ]); + assert!( + body["message"] + .as_str() + .unwrap_or_default() + .contains("recording starts on ticket.corp.example"), + "{body}" + ); +} + +#[test] +fn secrets_are_refused_in_candidates_and_in_site_md() { + let env = Env::new(); + let host = "ticket.corp.example"; + let (_, body) = env.json_err(&[ + "site", + "candidate", + "add", + "--host", + host, + "--kind", + "access", + "--claim", + "send Authorization: Bearer abc123def456", + ]); + assert!( + body["message"] + .as_str() + .unwrap_or_default() + .contains("bearer token"), + "{body}" + ); + + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + std::fs::write( + draft.join("SITE.md"), + "the session cookie=abc123 works [verified 2026-09-15]\n", + ) + .unwrap(); + let (_, body) = env.json_err(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + assert!( + body["message"] + .as_str() + .unwrap_or_default() + .contains("cookie"), + "{body}" + ); + assert!(!env.sites().join(host).join("SITE.md").exists()); +} + +/// Ordinary candidates need a second sighting on a different UTC day; +/// `high_consequence` does not. +#[test] +fn candidate_ingestion_enforces_the_evidence_rule() { + let env = Env::new(); + let host = "ticket.corp.example"; + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + std::fs::write(draft.join("SITE.md"), SITE_MD).unwrap(); + + let first = env.json(&[ + "site", + "candidate", + "add", + "--host", + host, + "--kind", + "better_path", + "--claim", + "use the priority dropdown", + ]); + let first_id = first["id"].as_str().unwrap().to_string(); + + let (_, body) = env.json_err(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "candidate_ingestion", + "--ingest", + &first_id, + ]); + assert!( + body["message"] + .as_str() + .unwrap_or_default() + .contains("two independent observations"), + "{body}" + ); + + // A sighting recorded on another day corroborates it. + let candidates = env.sites().join(host).join("candidates"); + std::fs::write( + candidates.join("2026-01-02-001.json"), + serde_json::json!({ + "id": "2026-01-02-001", + "host": host, + "observedDateUtc": "2026-01-02", + "kind": "better_path", + "claim": "Use the priority DROPDOWN", + "status": "pending" + }) + .to_string(), + ) + .unwrap(); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "candidate_ingestion", + "--ingest", + &first_id, + ]); + assert_eq!(committed["status"], "committed"); + assert_eq!( + committed["ingested"].as_array().unwrap(), + &vec![Value::String(first_id.clone())] + ); + let settled = env.json(&["site", "candidate", "show", &first_id, "--host", host]); + assert_eq!(settled["status"], "ingested"); +} + +/// No `bsk site` command may start the daemon: agents run these under +/// `BSK_AUTO_START=0` in a sandbox. +#[test] +fn site_commands_never_start_the_daemon() { + let env = Env::new(); + env.json(&[ + "site", + "context", + "--host", + "ticket.corp.example", + "--task", + "T1", + ]); + env.json(&["site", "workflow", "list"]); + assert!(!env.home.path().join("daemon.json").exists()); + assert!(!env.home.path().join("daemon.lock").exists()); +} + +/// A trace shaped like the real Wikipedia recording that broke derivation: +/// it opens on a `select` rather than a navigation, carries an `about:blank` +/// left by backing out past the start page, and names its controls with the +/// VOM's bracketed state annotations. +fn write_noisy_bundle(dir: &Path) -> PathBuf { + std::fs::create_dir_all(dir).unwrap(); + let trace = serde_json::json!({ + "version": 3, + "recorded_at": "2026-09-16T01:00:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://www.wikipedia.org/" }, + "recorder": { "bsk": "0.2.1", "vom": 1 }, + "states": [], + "steps": [ + { "op": "select", "id": 1, "state": "s1", "result": { "state": "s1" }, + "target": { "ref": "e12", "role": "combobox", "name": "ZH [has-submenu]" }, + "selection": [{ "value": "zh", "label": "中文" }] }, + { "op": "fill", "id": 2, "state": "s1", "result": { "state": "s2" }, + "target": { "ref": "e11", "role": "searchbox", "name": "Search Wikipedia" }, + "value": "珠穆朗玛峰", "commit": "blur" }, + { "op": "navigate", "id": 3, "state": "s2", "result": { "state": "s3" }, + "to": "https://zh.wikipedia.org/wiki/x", "cause": "browser" }, + { "op": "navigate", "id": 4, "state": "s3", "result": { "state": "s4" }, + "to": "about:blank", "cause": "history" } + ] + }); + let path = dir.join("trace.json"); + std::fs::write(&path, serde_json::to_string_pretty(&trace).unwrap()).unwrap(); + path +} + +/// S2: a recording full of ordinary noise must still derive. One +/// `about:blank` used to exit 1 and discard the whole bundle. +#[test] +fn a_noisy_recording_still_derives_a_usable_workflow() { + let env = Env::new(); + let trace = write_noisy_bundle(&env.home.path().join("rec")); + let trace = trace.to_string_lossy().into_owned(); + + let saved = env.json(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "wiki-search", + "--task", + "T1", + ]); + // The host is where the recording started, not the article it landed on. + assert_eq!(saved["host"], "wikipedia.org"); + // about:blank is dropped, not fatal; the article navigation the page + // made after the search is an effect of the fill, not a step. + assert_eq!(saved["droppedSteps"], 2, "{saved}"); + // navigate(entry) + select + fill + assert_eq!(saved["steps"], 3); + // The select's option value is a site constant, so it is not a parameter; + // the typed search term still is, under a name free of `[has-submenu]`. + assert_eq!( + saved["params"].as_array().unwrap(), + &[Value::String("search_wikipedia".into())] + ); + + let path = PathBuf::from(saved["path"].as_str().unwrap()); + let workflow: Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let steps = workflow["steps"].as_array().unwrap(); + assert_eq!(steps[0]["op"], "navigate"); + assert_eq!(steps[0]["to"], "https://www.wikipedia.org/"); + assert_eq!(steps[1]["op"], "select"); + assert_eq!(steps[1]["value"], "zh"); + assert_eq!(steps[2]["valueFrom"], "search_wikipedia"); + let raw = std::fs::read_to_string(&path).unwrap(); + assert!(!raw.contains("珠穆朗玛峰"), "{raw}"); + assert!( + steps.iter().all(|step| step["to"] != "about:blank"), + "{raw}" + ); + // It is reported as dropped rather than silently vanishing. + assert!( + workflow["reviewNotes"] + .as_array() + .unwrap() + .iter() + .any(|note| note.as_str().unwrap().contains("about:blank")), + "{raw}" + ); + // The widget-state annotation describes recording-time state, not the + // control's identity: it is dropped from the anchor and the parameter. + assert_eq!(steps[1]["target"]["name"], "ZH"); + for param in workflow["params"].as_array().unwrap() { + let name = param["name"].as_str().unwrap(); + assert!(!name.contains("submenu"), "{name}"); + } + + // The host the recording started on is the one `--host` must name. + env.json(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "wiki-search", + "--task", + "T1", + "--host", + "www.wikipedia.org", + ]); + let (_, err) = env.json_err(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "wiki-search", + "--task", + "T1", + "--host", + "zh.wikipedia.org", + ]); + assert!( + err["message"] + .as_str() + .unwrap() + .contains("starts on wikipedia.org"), + "{err}" + ); +} + +/// A workflow staged but not yet checkpointed is still this task's workflow. +/// Hiding it made `workflow save` look like it had done nothing. +#[test] +fn context_lists_the_drafts_own_unpublished_workflows() { + let env = Env::new(); + let trace = write_bundle(&env.home.path().join("rec")); + let trace = trace.to_string_lossy().into_owned(); + let host = "ticket.corp.example"; + env.json(&[ + "site", + "workflow", + "save", + "--from", + &trace, + "--id", + "submit-ticket", + "--task", + "T1", + ]); + + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + let listed = context["workflows"].as_array().unwrap(); + assert_eq!(listed.len(), 1, "{context}"); + assert_eq!(listed[0]["id"], "submit-ticket"); + assert_eq!(listed[0]["draft"], true); + let human = env.ok(&["site", "context", "--host", host, "--task", "T1"]); + assert!(human.contains("DRAFT"), "{human}"); + + // Another task must not see it, and nor must a context without --task. + let other = env.json(&["site", "context", "--host", host, "--task", "T2"]); + assert_eq!(other["workflows"].as_array().unwrap().len(), 0); + + // Once published it is ordinary active memory, no longer flagged. + env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + assert_eq!(context["workflows"][0].get("draft"), None, "{context}"); +} + +/// S7: the documented conflict recovery must not discard the commit that won +/// the race. `context` re-seeds the draft from what is published; the agent +/// replays its own edit on top, and both lines survive. +#[test] +fn conflict_recovery_preserves_the_other_writers_commit() { + let env = Env::new(); + let host = "ticket.corp.example"; + let a_line = "A 的事实。 [verified 2026-09-15]\n"; + let b_line = "B 的事实。 [verified 2026-09-15]\n"; + + let a = env.json(&["site", "context", "--host", host, "--task", "TA"]); + let b = env.json(&["site", "context", "--host", host, "--task", "TB"]); + let a_draft = PathBuf::from(a["draft"]["path"].as_str().unwrap()); + let b_draft = PathBuf::from(b["draft"]["path"].as_str().unwrap()); + std::fs::write(a_draft.join("SITE.md"), a_line).unwrap(); + std::fs::write(b_draft.join("SITE.md"), b_line).unwrap(); + + env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "TA", + "--reason", + "direct_correction", + ]); + let (_, conflict) = env.json_err(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "TB", + "--reason", + "direct_correction", + ]); + assert_eq!(conflict["status"], "conflict"); + + // Recovery: re-read context. The draft now holds A's published prose and + // says so, so B knows its own edit has to be replayed. + let refreshed = env.json(&["site", "context", "--host", host, "--task", "TB"]); + assert_eq!(refreshed["draft"]["baseRevision"], 1); + assert_eq!(refreshed["draft"]["rebased"], true); + assert!( + refreshed["draft"]["hint"] + .as_str() + .unwrap() + .contains("replay your edits"), + "{refreshed}" + ); + let staged = std::fs::read_to_string(b_draft.join("SITE.md")).unwrap(); + assert_eq!(staged, a_line, "the draft must show what is published now"); + + std::fs::write(b_draft.join("SITE.md"), format!("{a_line}{b_line}")).unwrap(); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "TB", + "--reason", + "direct_correction", + ]); + assert_eq!(committed["revision"], 2); + let published = std::fs::read_to_string(env.sites().join(host).join("SITE.md")).unwrap(); + assert!(published.contains("A 的事实"), "{published}"); + assert!(published.contains("B 的事实"), "{published}"); +} + +/// S7: one host's checkpoint must not push an untouched host's draft into the +/// conflict path. The revision counter lives under each host. +#[test] +fn revisions_are_scoped_to_one_host() { + let env = Env::new(); + let site_md = "一条事实。 [verified 2026-09-15]\n"; + for host in ["ticket.corp.example", "example.org"] { + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + assert_eq!(context["revision"], 0, "{host} starts at 0"); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + std::fs::write(draft.join("SITE.md"), site_md).unwrap(); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + assert_eq!( + committed["revision"], 1, + "{host} commits its own revision 1" + ); + } + assert!(!env.sites().join(".revision").exists(), "no global counter"); + for host in ["ticket.corp.example", "example.org"] { + assert!(env.sites().join(host).join(".revision").is_file()); + assert!(env.sites().join(host).join(".journal.jsonl").is_file()); + } +} + +/// S10: the draft tree used to be world-listable, exposing task ids to any +/// other account on the machine, and the lock file was world-readable. +#[cfg(unix)] +#[test] +fn site_memory_is_private_on_disk() { + use std::os::unix::fs::PermissionsExt; + let env = Env::new(); + let host = "ticket.corp.example"; + let context = env.json(&["site", "context", "--host", host, "--task", "T1"]); + let draft = PathBuf::from(context["draft"]["path"].as_str().unwrap()); + std::fs::write(draft.join("SITE.md"), SITE_MD).unwrap(); + env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "T1", + "--reason", + "direct_correction", + ]); + env.json(&[ + "site", + "candidate", + "add", + "--host", + host, + "--kind", + "better_path", + "--claim", + "the dropdown is faster", + ]); + + let mode = |path: PathBuf| { + let meta = std::fs::metadata(&path).unwrap_or_else(|e| panic!("{}: {e}", path.display())); + (path, meta.permissions().mode() & 0o777) + }; + let sites = env.sites(); + for (path, mode) in [ + mode(sites.clone()), + mode(sites.join(".drafts")), + mode(sites.join(".drafts").join("t1")), + mode(draft.clone()), + mode(sites.join(host)), + mode(sites.join(host).join("candidates")), + ] { + assert_eq!(mode, 0o700, "{} must be 0700", path.display()); + } + for (path, mode) in [ + mode(sites.join(".lock")), + mode(sites.join(host).join(".revision")), + mode(sites.join(host).join(".journal.jsonl")), + mode(sites.join(host).join("SITE.md")), + mode(draft.join(".context.json")), + ] { + assert_eq!(mode, 0o600, "{} must be 0600", path.display()); + } +} + +/// D1: `verify --pass` used to clear `needsReview` on an anchorless step too. +/// The validator rightly refuses an unflagged anchorless step, so every later +/// checkpoint on that host failed. A pass now leaves those steps flagged. +#[test] +fn verify_pass_keeps_anchorless_steps_flagged_so_later_checkpoints_still_pass() { + let env = Env::new(); + let dir = env.home.path().join("rec"); + std::fs::create_dir_all(&dir).unwrap(); + let trace = serde_json::json!({ + "version": 3, + "recorded_at": "2026-09-16T01:00:00Z", + "stopped_by": "user_finish", + "entry": { "start_url": "https://www.wikipedia.org/" }, + "recorder": { "bsk": "0.2.1", "vom": 1 }, + "states": [], + "steps": [ + { "op": "fill", "id": 1, "state": "s1", "result": { "state": "s1" }, + "target": { "ref": "e11", "role": "searchbox", "name": "Search Wikipedia" }, + "value": "珠穆朗玛峰", "commit": "blur" }, + { "op": "click", "id": 2, "state": "s1", "result": { "state": "s2" }, + "target": { "unmatched": true } } + ] + }); + let path = dir.join("trace.json"); + std::fs::write(&path, serde_json::to_string_pretty(&trace).unwrap()).unwrap(); + let trace = path.to_string_lossy().into_owned(); + let host = "wikipedia.org"; + + env.json(&["site", "context", "--host", host, "--task", "A"]); + env.json(&[ + "site", "workflow", "save", "--from", &trace, "--id", "wf", "--task", "A", + ]); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "A", + "--reason", + "direct_correction", + ]); + assert_eq!(committed["status"], "committed", "{committed}"); + + let verified = env.json(&["site", "workflow", "verify", "wf", "--host", host, "--pass"]); + assert_eq!(verified["outcome"], "pass"); + let listed = env.json(&["site", "workflow", "list", "--host", host]); + assert_eq!( + listed[0]["needsReview"], false, + "workflow-level flag clears" + ); + + // A later task on the same host must still be able to publish. + env.json(&["site", "context", "--host", host, "--task", "B"]); + let site_md = env.sites().join(".drafts/B").join(host).join("SITE.md"); + std::fs::write( + &site_md, + "# wikipedia.org\n\n- fact. [verified 2026-09-16]\n", + ) + .unwrap(); + let committed = env.json(&[ + "site", + "checkpoint", + "--host", + host, + "--task", + "B", + "--reason", + "direct_correction", + ]); + assert_eq!(committed["status"], "committed", "{committed}"); + + let shown = env.json(&["site", "workflow", "show", "wf", "--host", host]); + let steps = shown["steps"].as_array().unwrap(); + let anchorless = steps.iter().find(|s| s["op"] == "click").unwrap(); + assert_eq!(anchorless["needsReview"], true, "{shown}"); +} diff --git a/docs/operation-audit.md b/docs/operation-audit.md index 87915bd5..b502b749 100644 --- a/docs/operation-audit.md +++ b/docs/operation-audit.md @@ -53,6 +53,16 @@ bsk session start --name "整理本周待办" 审计接口仅由扩展页面经后台服务访问,网页内容脚本不能调用。服务按已连接的浏览器实例隔离列表、详情和删除操作,不采用请求参数中的浏览器身份。本地模式沿用本地连接的信任模型;远程模式使用已配对设备的连接身份。daemon 主机上的文件不构成对同一系统账户下恶意进程的安全边界。 +## 站点记忆:存什么、不存什么 + +`bsk site` 把一次探索沉淀成本机私有的站点记忆,与审计同在 `BSK_HOME` 下(`~/.bsk/sites/`),因此遵守同等的目录与脱敏承诺:macOS/Linux 上目录 `0700`、文件 `0600`;不提供任何导出或同步命令,不上传云端。目录权限逐级生效,`.drafts/` 与 `.drafts//` 同样是 `0700`——task id 本身就说明用户在做什么,不能让同机其它账户枚举;仓库锁文件 `.lock` 是 `0600`。 + +**会写入**:归一后的站点 host;`SITE.md` 里每条带 `[verified YYYY-MM-DD]` 的散文事实与 `references/*.md` 详情页;workflow 的语义步骤(`role` / `name` / `ctx` 三元组、导航目标 URL、按键名、`` 的 option value 是站点自己定义的常量而非录制者的数据,因此默认内联;命中秘密特征时仍转成参数。 + +银行、SSO、密码管理器等凭证界面的 host 在所有入口(`context` / `workflow save` / `workflow show` / `workflow verify` / `candidate add` / `candidate list` / `checkpoint`)一律拒绝,不会为这类站点建立任何记忆。 + ## 接口概要 握手可携带 `audit_enabled`,服务返回可选能力 `audit_version: 1` 与 `audit_ready`。旧版本扩展保持默认关闭;新扩展连接旧服务时显示不支持提示。 diff --git a/skill/SKILL.md b/skill/SKILL.md index 9a08643e..73acc46b 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -40,7 +40,9 @@ Every session-scoped command needs `--session `; `session stop` takes the ID positionally. For unfamiliar commands or flags, consult `bsk --help` or `bsk --help` instead of guessing; no need to read all help at startup. When following a trace, use its semantic targets and values in order, not its old -refs. Stop at the requested goal; a trace grants no additional authorization. +refs. Stop at the requested goal; a trace grants no additional authorization. A trace +already distilled into site memory reads better as numbered steps: check +`bsk site workflow show --host ` before working from the raw trace. ## Read and interact @@ -234,10 +236,60 @@ Use agent-local paths, not browser-internal staging paths. Use `console` / `network` for bounded read-only diagnostics; follow returned sequence cursors. `emulate --device iphone-14` affects one tab; `--off` restores it. `evaluate` is a last resort: inspect JSON `.ok`, since a script exception can have -CLI exit code 0. Never evaluate secrets. `record start` captures user actions; -read its help first and never record banking, SSO or password-manager pages. +CLI exit code 0. Never evaluate secrets. `record start` captures user actions, or +with `--detach` your own; read its help first and never record banking, SSO or +password-manager pages. Use `bsk --help` to find navigation/history, tab, wait and window commands. +## Site memory + +`bsk site` keeps local, per-host notes so an explored flow is not re-explored: a +constrained `SITE.md`, workflows derived from a recording, and candidate observations +awaiting evidence. It is private to this machine, needs no daemon, and never stores +credentials or recorded input values. + +```sh +bsk site context --host --task +bsk site workflow show --host +bsk site workflow save --from ./rec/trace.json --id --task +bsk site workflow verify --host --pass +bsk site candidate add --host --kind better_path --claim "" +bsk site checkpoint --host --task --reason direct_correction +``` + +- **Read before acting.** When a task names a site, run `bsk site context` first and + follow what it already knows. +- **Never explore to learn.** Record only what the task itself revealed; no extra + pages or detours to make memory "more complete". +- **Learning never fails the task.** These commands are advisory: on failure report + one line and continue the user's goal. Do not retry or let it block the task. + +To leave memory behind for the task you are already doing, record your own run: + +```sh +bsk record start --detach --url --output ./rec --json # prints session_id +bsk observe --session # do the real task +bsk fill @e4 --value "" --session +bsk press Enter --session +bsk record stop --output ./rec # exports trace.json +bsk site workflow save --from ./rec/trace.json --id --task +bsk site checkpoint --host --task --reason direct_correction +``` + +`--detach` returns once recording is armed, so the session accepts your commands; +without it `record start` holds the session and every command returns `session_busy`. +`record stop` also ends that session. Reusing a workflow needs no recording: read it, +run it, then `workflow verify --pass`, which clears NEEDS REVIEW. + +`workflow save` writes into a per-task draft; `checkpoint` publishes it. Revisions are +per host. A `conflict` result means another writer committed first: re-run +`bsk site context`, which re-seeds the draft's `SITE.md` from the published revision +(your draft edits are gone and must be replayed; staged workflows are kept), then +checkpoint again. Retry a conflict only once. Recorded `fill` values become named +parameters and are never stored; pass `--inline-values` only when the values belong +to the flow, not to the person who recorded it. `