diff --git a/AGENTS.md b/AGENTS.md index c797069..e95c3e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,8 @@ the implementation, and where should a change land first. - `automation.toml` packages and configures a task snapshot; it must not duplicate task logic. - `browser-cli task` is the first-class local authoring UX. - `browser-cli automation publish` creates immutable snapshots under the Browser CLI home and auto-imports them into the automation service. +- Published automation snapshot truth lives under `~/.browser-cli/automations//versions//automation.toml`; current service state is a separate live view. +- If source `automation.toml` exists, `browser-cli automation publish` must preserve its supported fields in the published snapshot; generated defaults are allowed only when the source manifest is absent and must be reported explicitly. - Do not introduce a public `browser-cli explore` surface or a second browser runtime. Exploration remains an agent activity layered on top of Browser CLI. - The extension popup is a human-facing runtime observer and light recovery surface. Agent feedback still flows through command responses and `runtime-status`. @@ -181,8 +183,16 @@ the implementation, and where should a change land first. start at `src/browser_cli/automation/service/*`, `src/browser_cli/automation/persistence/*`, `src/browser_cli/automation/scheduler/*`, and `src/browser_cli/automation/api/*`. - If the user reports confusion about what publish created or which version is active: inspect `src/browser_cli/commands/automation.py`, `src/browser_cli/automation/publisher.py`, `src/browser_cli/automation/api/server.py`, and the snapshot directories under `~/.browser-cli/automations//versions/`. +- If the user reports publish/config drift between a version and current service state: + inspect the snapshot manifest under `~/.browser-cli/automations//versions//automation.toml` first, then compare it with `browser_cli.commands.automation` live inspect payload assembly and `browser_cli.automation.api.server` persisted automation serialization. +- If the user reports that `browser-cli read` stopped explaining fallback profile use: + inspect `src/browser_cli/daemon/browser_service.py::read_page`, `src/browser_cli/task_runtime/read.py`, and `src/browser_cli/commands/read.py`; fallback metadata is only guaranteed on the read path. - If the user mentions extension capability gaps, artifacts, or real Chrome behavior: inspect both `src/browser_cli/extension/*` and `browser-cli-extension/src/*`; many bugs live in protocol drift between Python and extension JS. + Symptom -> root cause -> where to inspect: + `Tab is not debuggable: unknown` during extension-mode `read` or `open-tab` + -> extension tried to attach Chrome debugger before the new tab reported a debuggable URL + -> inspect `browser-cli-extension/src/background.js::openTab` and `browser-cli-extension/src/debugger.js::ensureAttached` - If the user reports popup/runtime observer drift: start at `src/browser_cli/daemon/runtime_presentation.py`, then `src/browser_cli/extension/session.py`, then `browser-cli-extension/src/background.js`, `browser-cli-extension/src/popup_view.js`, and `browser-cli-extension/src/popup.js`. - If the user reports long-run stability drift across repeated reloads, reconnects, or artifact runs: @@ -235,6 +245,8 @@ public interactive commands. - `task.py` contains reusable automation logic. `automation.toml` packages and configures that logic; it should not become a second implementation surface. - Runtime automation state belongs to the automation service persistence layer, not `automation.toml`. `automation.toml` remains an import/export and reviewable packaging artifact. - Published automations are immutable snapshots. Versioning should append a new snapshot rather than mutating an older one. +- `browser-cli automation inspect --version ` must render snapshot config and live config separately; do not collapse them into one synthesized automation view. +- `runtime.timeout_seconds` is enforced only by the automation service and applies to the total wall-clock budget for one automation run; timeout failures must not auto-retry. - When adapting third-party logic from `third_party/bridgic-browser`, keep provenance clear and the adapted code understandable. Do not add a runtime dependency on the external package just to shortcut implementation. ## Testing And Validation diff --git a/README.md b/README.md index 969b64a..2d7a435 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,12 @@ browser-cli automation status browser-cli automation ui ``` +Publication semantics: + +- `automation publish` snapshots `task.py`, `task.meta.json`, and `automation.toml` together under `~/.browser-cli/automations//versions//` +- if source `automation.toml` exists, Browser CLI uses it as the publish-time configuration truth +- if source `automation.toml` is absent, Browser CLI publishes generated defaults and reports that explicitly via `manifest_source` + Export a persisted automation back to `automation.toml`: ```bash @@ -206,6 +212,12 @@ browser-cli automation inspect douyin_video_download browser-cli automation status ``` +Inspect semantics: + +- `browser-cli automation inspect ` shows the current live automation-service configuration +- `browser-cli automation inspect --version ` shows `snapshot_config` for the immutable published version and `live_config` for the current service state +- `latest_run` remains a separate operational view + ## Output Contracts ### `read` @@ -241,6 +253,7 @@ Exit codes: - Managed profile mode is the default backend. - Extension mode is the preferred real-Chrome backend when connected and healthy. - Driver rebinding happens only at safe idle points and is reported as `state_reset`. +- `runtime.timeout_seconds` is the total wall-clock timeout for one automation run in the automation service. ## Documentation diff --git a/browser-cli-extension/src/debugger.js b/browser-cli-extension/src/debugger.js index ee1b2e2..f1ca030 100644 --- a/browser-cli-extension/src/debugger.js +++ b/browser-cli-extension/src/debugger.js @@ -10,6 +10,10 @@ const INLINE_TEXT_MAX_BYTES = 128 * 1024; const INLINE_BINARY_MAX_BYTES = 64 * 1024; const MAX_CAPTURE_BODY_BYTES = 8 * 1024 * 1024; const STATIC_RESOURCE_TYPES = new Set(['image', 'stylesheet', 'script', 'font', 'media']); +const DEFAULT_ATTACH_WAIT_TIMEOUT_MS = 1500; +const DEFAULT_ATTACH_POLL_INTERVAL_MS = 50; +const DEFAULT_ATTACH_RETRY_COUNT = 2; +const DEFAULT_ATTACH_RETRY_DELAY_MS = 100; const textEncoder = new TextEncoder(); const textDecoder = new TextDecoder(); @@ -18,6 +22,26 @@ function isDebuggableUrl(url = '') { return url.startsWith('http://') || url.startsWith('https://') || url.startsWith('file://') || url === 'about:blank'; } +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +async function waitForDebuggableTab( + tabId, + { + timeoutMs = DEFAULT_ATTACH_WAIT_TIMEOUT_MS, + pollIntervalMs = DEFAULT_ATTACH_POLL_INTERVAL_MS, + } = {}, +) { + const deadline = Date.now() + Math.max(0, Number(timeoutMs) || 0); + let tab = await chrome.tabs.get(tabId); + while (!isDebuggableUrl(tab.url || '') && Date.now() < deadline) { + await sleep(Math.max(0, Number(pollIntervalMs) || 0)); + tab = await chrome.tabs.get(tabId); + } + return tab; +} + function encodeUtf8(value = '') { return textEncoder.encode(String(value || '')); } @@ -401,16 +425,54 @@ function finalizeFailedRequest(tabId, requestId, failureReason) { ); } -export async function ensureAttached(tabId) { - const tab = await chrome.tabs.get(tabId); +export async function ensureAttached( + tabId, + { + waitTimeoutMs = DEFAULT_ATTACH_WAIT_TIMEOUT_MS, + pollIntervalMs = DEFAULT_ATTACH_POLL_INTERVAL_MS, + retryCount = DEFAULT_ATTACH_RETRY_COUNT, + retryDelayMs = DEFAULT_ATTACH_RETRY_DELAY_MS, + } = {}, +) { + const tab = await waitForDebuggableTab(tabId, { + timeoutMs: waitTimeoutMs, + pollIntervalMs, + }); if (!isDebuggableUrl(tab.url || '')) { throw new Error(`Tab ${tabId} is not debuggable: ${tab.url || 'unknown'}`); } if (attached.has(tabId)) { return; } - await chrome.debugger.attach({ tabId }, '1.3'); - attached.add(tabId); + const attempts = Math.max(1, Number(retryCount) || 0); + let attachError = null; + for (let attempt = 1; attempt <= attempts; attempt += 1) { + if (attached.has(tabId)) { + return; + } + try { + await chrome.debugger.attach({ tabId }, '1.3'); + attached.add(tabId); + return; + } catch (error) { + if (attached.has(tabId)) { + return; + } + attachError = error; + if (attempt >= attempts) { + break; + } + await sleep(Math.max(0, Number(retryDelayMs) || 0)); + const refreshed = await waitForDebuggableTab(tabId, { + timeoutMs: waitTimeoutMs, + pollIntervalMs, + }); + if (!isDebuggableUrl(refreshed.url || '')) { + throw new Error(`Tab ${tabId} is not debuggable: ${refreshed.url || 'unknown'}`); + } + } + } + throw attachError; } export async function sendCommand(tabId, method, params = {}) { diff --git a/browser-cli-extension/tests/debugger.test.js b/browser-cli-extension/tests/debugger.test.js new file mode 100644 index 0000000..cb8ec3c --- /dev/null +++ b/browser-cli-extension/tests/debugger.test.js @@ -0,0 +1,99 @@ +import test from 'node:test' +import assert from 'node:assert/strict' + +function installChromeStub({ + urls, + attachImpl = async () => {}, +}) { + const sequence = Array.isArray(urls) && urls.length ? [...urls] : ['about:blank'] + let getCallCount = 0 + const attachCalls = [] + + globalThis.chrome = { + tabs: { + async get(tabId) { + const index = Math.min(getCallCount, sequence.length - 1) + const url = sequence[index] + getCallCount += 1 + return { id: tabId, url } + }, + }, + debugger: { + async attach(target, version) { + attachCalls.push({ target, version }) + return await attachImpl(target, version) + }, + }, + } + + return { + attachCalls, + getCallCount: () => getCallCount, + } +} + +async function loadDebuggerModule() { + const cacheBust = `${Date.now()}-${Math.random()}` + return await import(new URL(`../src/debugger.js?test=${cacheBust}`, import.meta.url)) +} + +test('ensureAttached waits for an initially blank tab URL before attaching', async (t) => { + const chromeStub = installChromeStub({ urls: ['', '', 'about:blank'] }) + t.after(() => { + delete globalThis.chrome + }) + + const { ensureAttached } = await loadDebuggerModule() + await ensureAttached(17, { + waitTimeoutMs: 25, + pollIntervalMs: 0, + retryCount: 1, + }) + + assert.equal(chromeStub.attachCalls.length, 1) + assert.equal(chromeStub.attachCalls[0].target.tabId, 17) + assert.equal(chromeStub.getCallCount(), 3) +}) + +test('ensureAttached retries a transient debugger attach failure', async (t) => { + let attachAttempts = 0 + installChromeStub({ + urls: ['about:blank'], + attachImpl: async () => { + attachAttempts += 1 + if (attachAttempts === 1) { + throw new Error('transient attach failure') + } + }, + }) + t.after(() => { + delete globalThis.chrome + }) + + const { ensureAttached } = await loadDebuggerModule() + await ensureAttached(23, { + waitTimeoutMs: 0, + pollIntervalMs: 0, + retryCount: 2, + retryDelayMs: 0, + }) + + assert.equal(attachAttempts, 2) +}) + +test('ensureAttached still rejects truly non-debuggable tabs', async (t) => { + installChromeStub({ urls: ['', 'chrome://newtab/'] }) + t.after(() => { + delete globalThis.chrome + }) + + const { ensureAttached } = await loadDebuggerModule() + await assert.rejects( + ensureAttached(29, { + waitTimeoutMs: 1, + pollIntervalMs: 0, + retryCount: 1, + }), + /Tab 29 is not debuggable: chrome:\/\/newtab\//, + ) +}) diff --git a/docs/superpowers/plans/2026-04-14-browser-cli-extension-attach-race-implementation-plan.md b/docs/superpowers/plans/2026-04-14-browser-cli-extension-attach-race-implementation-plan.md new file mode 100644 index 0000000..2567bd3 --- /dev/null +++ b/docs/superpowers/plans/2026-04-14-browser-cli-extension-attach-race-implementation-plan.md @@ -0,0 +1,48 @@ +# Browser CLI Extension Attach Race Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Prevent extension-mode `open-tab` from failing with `Tab is not debuggable: unknown` when Chrome has created the tab but has not populated a debuggable URL yet. + +**Architecture:** Keep the existing extension-mode workflow and network-capture timing, but harden `ensureAttached()` so it waits briefly for a debuggable URL and retries one transient attach failure before surfacing an error. Cover the behavior with Node-based extension unit tests and keep repository lint wired to run all extension tests. + +**Tech Stack:** Browser extension JavaScript, Node `node:test`, repository shell lint/test/guard scripts + +--- + +### Task 1: Harden Extension Attach Timing + +**Files:** +- Modify: `browser-cli-extension/src/debugger.js` + +- [ ] Add a short polling helper that waits for `chrome.tabs.get(tabId).url` to become `about:blank`, `http(s)`, or `file`. +- [ ] Update `ensureAttached()` to use the helper before `chrome.debugger.attach()`. +- [ ] Retry one transient attach failure after a short delay without changing the public error wording for truly non-debuggable tabs. + +### Task 2: Add Regression Tests + +**Files:** +- Create: `browser-cli-extension/tests/debugger.test.js` +- Modify: `scripts/lint.sh` + +- [ ] Add a Node test that proves `ensureAttached()` waits through an empty tab URL and then attaches once the URL becomes `about:blank`. +- [ ] Add a Node test that proves `ensureAttached()` retries a transient attach failure and succeeds on the second attempt. +- [ ] Add a Node test that proves truly non-debuggable URLs still fail with a clear message. +- [ ] Update lint to run all extension `*.test.js` files, not just the popup test. + +### Task 3: Capture Durable Repo Guidance + +**Files:** +- Modify: `AGENTS.md` + +- [ ] Add a concise failure-driven note pointing future agents to the extension `openTab` and `ensureAttached` path when they see `Tab is not debuggable: unknown`. + +### Task 4: Validate + +**Files:** +- Modify: none + +- [ ] Run `node --test browser-cli-extension/tests/*.test.js`. +- [ ] Run `scripts/lint.sh`. +- [ ] Run `scripts/test.sh`. +- [ ] Run `scripts/guard.sh`. diff --git a/docs/superpowers/plans/2026-04-14-browser-cli-release-readiness-implementation-plan.md b/docs/superpowers/plans/2026-04-14-browser-cli-release-readiness-implementation-plan.md new file mode 100644 index 0000000..8d65e95 --- /dev/null +++ b/docs/superpowers/plans/2026-04-14-browser-cli-release-readiness-implementation-plan.md @@ -0,0 +1,860 @@ +# Browser CLI Release Readiness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make automation publish/import/export/inspect preserve supported manifest fields, enforce automation run timeouts, and restore one-shot read fallback reporting before the first public release. + +**Architecture:** Keep the current task/automation/daemon architecture and tighten the release contract around one rule: a published automation version is defined by the immutable snapshot manifest in `~/.browser-cli/automations//versions//`. Align loader, models, publisher, commands, API, and service runtime to that truth model, while keeping local `task run` semantics unchanged and limiting fallback-profile reporting changes to the read path. + +**Tech Stack:** Python 3.10+, argparse CLI handlers, TOML via `tomllib`/`tomli`, SQLite-backed automation persistence, pytest unit/integration tests + +--- + +## File Structure + +### Files to modify + +- `src/browser_cli/automation/models.py` + - Add missing runtime schema fields so manifest loading, persistence conversion, export, and inspect all share one supported field set. +- `src/browser_cli/automation/loader.py` + - Parse the full supported manifest schema, especially `runtime.retry_backoff_seconds`. +- `src/browser_cli/automation/publisher.py` + - Load source `automation.toml` when present, generate defaults only when absent, snapshot the resolved manifest, and return `manifest_source`. +- `src/browser_cli/commands/automation.py` + - Surface `manifest_source` on publish and split inspect output into `snapshot_config` and `live_config`. +- `src/browser_cli/automation/api/server.py` + - Accept and serialize the full supported automation field set, including `stdout_mode` and `retry_backoff_seconds`. +- `src/browser_cli/automation/service/runtime.py` + - Enforce run-level timeout behavior and prevent timeout-triggered retries. +- `src/browser_cli/errors.py` + - Add a dedicated timeout error or code path suitable for automation-run timeout failures. +- `src/browser_cli/daemon/browser_service.py` + - Restore fallback profile metadata on `read_page`. +- `README.md` + - Update durable release-model and inspect/timeout guidance. +- `AGENTS.md` + - Update durable navigation and debugging guidance for publish truth and inspect dual-view behavior. + +### Files to create or extend in tests + +- `tests/unit/test_automation_publish.py` + - Add publish tests covering source manifest preservation and generated-defaults fallback. +- `tests/unit/test_task_runtime_automation.py` + - Add loader/model tests for runtime schema parity. +- `tests/unit/test_automation_commands.py` + - Add publish/inspect command output tests for `manifest_source`, `snapshot_config`, and `live_config`. +- `tests/unit/test_automation_api.py` + - Add import/export/API parity tests for `stdout_mode`, `retry_backoff_seconds`, and timeout fields. +- `tests/unit/test_automation_service.py` + - Add runtime timeout and no-retry-on-timeout coverage. +- `tests/unit/test_task_runtime_client_read.py` + - Add read fallback metadata propagation coverage for the daemon-backed path. +- `tests/integration/test_task_runtime_read.py` + - Extend daemon-backed read integration coverage for fallback metadata on the stable fixture path. + +## Task 1: Align Manifest Schema And Persistence Payloads + +**Files:** +- Modify: `src/browser_cli/automation/models.py` +- Modify: `src/browser_cli/automation/loader.py` +- Modify: `src/browser_cli/commands/automation.py` +- Modify: `src/browser_cli/automation/api/server.py` +- Test: `tests/unit/test_task_runtime_automation.py` +- Test: `tests/unit/test_automation_api.py` + +- [ ] **Step 1: Write the failing schema-parity tests** + +```python +def test_load_automation_manifest_preserves_retry_backoff_and_stdout(tmp_path: Path) -> None: + manifest_path = tmp_path / "automation.toml" + manifest_path.write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[outputs]\n" + 'stdout = "text"\n' + "[runtime]\n" + "retry_attempts = 2\n" + "retry_backoff_seconds = 7\n" + "timeout_seconds = 12.5\n", + encoding="utf-8", + ) + (tmp_path / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (tmp_path / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + + manifest = load_automation_manifest(manifest_path) + persisted = manifest_to_persisted_definition(manifest) + + assert manifest.outputs.stdout == "text" + assert manifest.runtime.retry_backoff_seconds == 7 + assert manifest.runtime.timeout_seconds == 12.5 + assert persisted.stdout_mode == "text" + assert persisted.retry_backoff_seconds == 7 + assert persisted.timeout_seconds == 12.5 +``` + +```python +def test_payload_to_automation_preserves_stdout_and_retry_backoff() -> None: + persisted = _payload_to_automation( + { + "id": "demo", + "name": "Demo", + "task_path": "/tmp/task.py", + "task_meta_path": "/tmp/task.meta.json", + "output_dir": "/tmp/out", + "stdout_mode": "text", + "retry_attempts": 1, + "retry_backoff_seconds": 9, + "timeout_seconds": 4.0, + } + ) + + assert persisted.stdout_mode == "text" + assert persisted.retry_backoff_seconds == 9 + assert persisted.timeout_seconds == 4.0 +``` + +- [ ] **Step 2: Run the targeted tests to verify they fail** + +Run: `uv run pytest tests/unit/test_task_runtime_automation.py tests/unit/test_automation_api.py -k "retry_backoff or stdout" -v` + +Expected: FAIL because `AutomationRuntime` does not expose `retry_backoff_seconds` and one or more command/API payload paths still drop `stdout_mode` or retry-backoff data. + +- [ ] **Step 3: Implement the schema and payload alignment** + +Update `src/browser_cli/automation/models.py`: + +```python +@dataclass(slots=True, frozen=True) +class AutomationRuntime: + timeout_seconds: float | None = None + retry_attempts: int = 0 + retry_backoff_seconds: int = 0 + log_level: str = "info" +``` + +```python +def manifest_to_persisted_definition( + manifest: AutomationManifest, + *, + enabled: bool = False, +) -> PersistedAutomationDefinition: + return PersistedAutomationDefinition( + id=manifest.automation.id, + name=manifest.automation.name, + task_path=manifest.task.path, + task_meta_path=manifest.task.meta_path, + output_dir=manifest.outputs.artifact_dir, + description=manifest.automation.description, + version=str(manifest.automation.version), + entrypoint=manifest.task.entrypoint, + enabled=enabled, + schedule_kind=str(manifest.schedule.get("mode") or "manual"), + schedule_payload=dict(manifest.schedule), + timezone=str(manifest.schedule.get("timezone") or "UTC"), + result_json_path=manifest.outputs.result_json_path, + stdout_mode=manifest.outputs.stdout, + input_overrides=dict(manifest.inputs), + before_run_hooks=manifest.hooks.before_run, + after_success_hooks=manifest.hooks.after_success, + after_failure_hooks=manifest.hooks.after_failure, + retry_attempts=manifest.runtime.retry_attempts, + retry_backoff_seconds=manifest.runtime.retry_backoff_seconds, + timeout_seconds=manifest.runtime.timeout_seconds, + ) +``` + +Update `src/browser_cli/automation/loader.py`: + +```python + runtime=AutomationRuntime( + timeout_seconds=float((data.get("runtime") or {})["timeout_seconds"]) + if (data.get("runtime") or {}).get("timeout_seconds") is not None + else None, + retry_attempts=int((data.get("runtime") or {}).get("retry_attempts") or 0), + retry_backoff_seconds=int( + (data.get("runtime") or {}).get("retry_backoff_seconds") or 0 + ), + log_level=str((data.get("runtime") or {}).get("log_level") or "info"), + ), +``` + +Update `src/browser_cli/commands/automation.py` and `src/browser_cli/automation/api/server.py` so all automation payload builders include: + +```python + "stdout_mode": manifest.outputs.stdout, + "retry_backoff_seconds": int(manifest.runtime.retry_backoff_seconds or 0), +``` + +and retain those values when building snapshot/live inspect payloads and persisted automations. + +- [ ] **Step 4: Run the targeted tests to verify they pass** + +Run: `uv run pytest tests/unit/test_task_runtime_automation.py tests/unit/test_automation_api.py -k "retry_backoff or stdout" -v` + +Expected: PASS with the new runtime field and payload parity tests green. + +- [ ] **Step 5: Commit the schema-alignment change** + +```bash +git add tests/unit/test_task_runtime_automation.py tests/unit/test_automation_api.py src/browser_cli/automation/models.py src/browser_cli/automation/loader.py src/browser_cli/commands/automation.py src/browser_cli/automation/api/server.py +git commit -m "fix: align automation manifest schema across payload paths" +``` + +## Task 2: Make Publish Snapshot The Source Manifest Truth + +**Files:** +- Modify: `src/browser_cli/automation/publisher.py` +- Modify: `src/browser_cli/commands/automation.py` +- Test: `tests/unit/test_automation_publish.py` +- Test: `tests/unit/test_automation_commands.py` + +- [ ] **Step 1: Write the failing publish-contract tests** + +```python +def test_publish_task_dir_preserves_source_manifest_fields(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (task_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[inputs]\n" + 'url = "https://example.com"\n' + "[schedule]\n" + 'mode = "manual"\n' + 'timezone = "Asia/Shanghai"\n' + "[outputs]\n" + 'artifact_dir = "artifacts"\n' + 'result_json_path = "artifacts/result.json"\n' + 'stdout = "text"\n' + "[runtime]\n" + "retry_attempts = 1\n" + "retry_backoff_seconds = 7\n", + encoding="utf-8", + ) + + published = publish_task_dir(task_dir, app_paths=get_app_paths()) + manifest = load_automation_manifest(published.manifest_path) + + assert published.manifest_source == "task_dir" + assert manifest.inputs == {"url": "https://example.com"} + assert manifest.schedule["timezone"] == "Asia/Shanghai" + assert manifest.outputs.result_json_path is not None + assert manifest.outputs.stdout == "text" + assert manifest.runtime.retry_backoff_seconds == 7 +``` + +```python +def test_publish_task_dir_generates_defaults_when_manifest_is_absent( + tmp_path: Path, monkeypatch +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + + published = publish_task_dir(task_dir, app_paths=get_app_paths()) + manifest = load_automation_manifest(published.manifest_path) + + assert published.manifest_source == "generated_defaults" + assert manifest.schedule["timezone"] == "UTC" + assert manifest.inputs == {} +``` + +```python +def test_run_automation_publish_returns_manifest_source(monkeypatch, tmp_path: Path) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (task_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n', + encoding="utf-8", + ) + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "browser_cli.commands.automation.ensure_automation_service_running", + lambda: None, + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: {"ok": True, "data": {"id": "demo"}}, + ) + args = Namespace(automation_subcommand="publish", path=str(task_dir)) + payload = json.loads(run_automation_command(args)) + assert payload["data"]["published"]["manifest_source"] == "task_dir" +``` + +- [ ] **Step 2: Run the targeted publish tests to verify they fail** + +Run: `uv run pytest tests/unit/test_automation_publish.py tests/unit/test_automation_commands.py -k "manifest_source or preserves_source_manifest or generates_defaults" -v` + +Expected: FAIL because publish currently regenerates the snapshot manifest from defaults and does not return `manifest_source`. + +- [ ] **Step 3: Implement source-manifest publish behavior** + +Update `src/browser_cli/automation/publisher.py`: + +```python +@dataclass(slots=True, frozen=True) +class PublishedAutomation: + automation_id: str + automation_name: str + version: int + snapshot_dir: Path + manifest_path: Path + output_dir: Path + manifest_source: str +``` + +```python +def publish_task_dir(task_dir: Path, *, app_paths: AppPaths) -> PublishedAutomation: + metadata = validate_task_dir(task_dir) + source_manifest_path = task_dir / "automation.toml" + if source_manifest_path.exists(): + source_manifest = load_automation_manifest(source_manifest_path) + manifest_source = "task_dir" + rendered_manifest = render_automation_manifest_from_manifest( + source_manifest, + version=version, + task_path=task_path, + task_meta_path=task_meta_path, + output_dir=automation_root, + ) + else: + manifest_source = "generated_defaults" + rendered_manifest = render_automation_manifest( + automation_id=automation_id, + name=automation_name, + version=version, + task_path=task_path, + task_meta_path=task_meta_path, + output_dir=automation_root, + ) + manifest_path.write_text(rendered_manifest, encoding="utf-8") + return PublishedAutomation( + automation_id=automation_id, + automation_name=automation_name, + version=version, + snapshot_dir=snapshot_dir, + manifest_path=manifest_path, + output_dir=automation_root, + manifest_source=manifest_source, + ) +``` + +Update `src/browser_cli/commands/automation.py` publish response: + +```python + "published": { + "automation_id": published.automation_id, + "automation_name": published.automation_name, + "version": published.version, + "manifest_source": published.manifest_source, + "source_task_dir": str(source_task_dir), + "snapshot_dir": str(published.snapshot_dir), + "manifest_path": str(published.manifest_path), + }, +``` + +- [ ] **Step 4: Run the targeted publish tests to verify they pass** + +Run: `uv run pytest tests/unit/test_automation_publish.py tests/unit/test_automation_commands.py -k "manifest_source or preserves_source_manifest or generates_defaults" -v` + +Expected: PASS with snapshot manifests preserving source configuration and CLI publish output reporting the manifest source. + +- [ ] **Step 5: Commit the publish-contract change** + +```bash +git add tests/unit/test_automation_publish.py tests/unit/test_automation_commands.py src/browser_cli/automation/publisher.py src/browser_cli/commands/automation.py +git commit -m "fix: preserve source manifest during automation publish" +``` + +## Task 3: Split Inspect Into Snapshot And Live Configuration Views + +**Files:** +- Modify: `src/browser_cli/commands/automation.py` +- Test: `tests/unit/test_automation_commands.py` + +- [ ] **Step 1: Write the failing inspect dual-view tests** + +```python +def test_automation_inspect_version_returns_snapshot_and_live_config( + monkeypatch, tmp_path: Path +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + version_dir = tmp_path / "home" / "automations" / "demo" / "versions" / "1" + version_dir.mkdir(parents=True) + (version_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (version_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (version_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo Snapshot"\n' + 'version = "1"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[outputs]\n" + 'stdout = "text"\n', + encoding="utf-8", + ) + (version_dir / "publish.json").write_text( + f'{{"automation_id":"demo","version":1,"source_task_path":"/tmp/task","snapshot_dir":"{version_dir}"}}', + encoding="utf-8", + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: { + "ok": True, + "data": { + "id": "demo", + "name": "Demo Live", + "version": "2", + "task_path": str(version_dir / "task.py"), + "task_meta_path": str(version_dir / "task.meta.json"), + "schedule_kind": "manual", + "schedule_payload": {"mode": "manual"}, + "stdout_mode": "json", + "latest_run": {"status": "success"}, + }, + }, + ) + args = Namespace(automation_subcommand="inspect", automation_id="demo", version=1) + payload = json.loads(run_automation_command(args)) + assert payload["data"]["snapshot_config"]["name"] == "Demo Snapshot" + assert payload["data"]["snapshot_config"]["stdout_mode"] == "text" + assert payload["data"]["live_config"]["name"] == "Demo Live" +``` + +```python +def test_automation_inspect_version_reports_snapshot_config_error( + monkeypatch, tmp_path: Path +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + version_dir = tmp_path / "home" / "automations" / "demo" / "versions" / "1" + version_dir.mkdir(parents=True) + (version_dir / "automation.toml").write_text("[automation]\n", encoding="utf-8") + (version_dir / "publish.json").write_text( + f'{{"automation_id":"demo","version":1,"source_task_path":"/tmp/task","snapshot_dir":"{version_dir}"}}', + encoding="utf-8", + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: { + "ok": True, + "data": {"id": "demo", "name": "Demo Live", "latest_run": None}, + }, + ) + args = Namespace(automation_subcommand="inspect", automation_id="demo", version=1) + payload = json.loads(run_automation_command(args)) + assert payload["data"]["snapshot_config"] is None + assert "snapshot_config_error" in payload["data"] +``` + +- [ ] **Step 2: Run the inspect tests to verify they fail** + +Run: `uv run pytest tests/unit/test_automation_commands.py -k "snapshot_config or live_config" -v` + +Expected: FAIL because inspect currently merges snapshot data into a single automation payload and falls back to live config when snapshot loading fails. + +- [ ] **Step 3: Implement inspect dual-view behavior** + +Update `src/browser_cli/commands/automation.py` to return separate sections: + +```python + return render_json_payload( + { + "ok": True, + "data": { + "snapshot_config": snapshot_config, + "snapshot_config_error": snapshot_config_error, + "live_config": live_automation_data, + "versions": versions, + "selected_version": selected, + "latest_run": selected_latest_run, + "summary": _build_inspect_summary( + args.automation_id, + live_automation_data, + versions, + selected, + selected_latest_run, + ), + }, + "meta": {"action": "automation-inspect"}, + } + ) +``` + +Update snapshot loading helpers so they report snapshot-manifest failures explicitly instead of silently falling back: + +```python +def _load_snapshot_manifest(path: Path) -> tuple[AutomationManifest | None, str | None]: + if not path.exists(): + return None, None + try: + return load_automation_manifest(path), None + except InvalidInputError as exc: + return None, str(exc) +``` + +- [ ] **Step 4: Run the inspect tests to verify they pass** + +Run: `uv run pytest tests/unit/test_automation_commands.py -k "snapshot_config or live_config" -v` + +Expected: PASS with inspect returning separate `snapshot_config`, `live_config`, and `snapshot_config_error` fields. + +- [ ] **Step 5: Commit the inspect split** + +```bash +git add tests/unit/test_automation_commands.py src/browser_cli/commands/automation.py +git commit -m "feat: split automation inspect into snapshot and live views" +``` + +## Task 4: Enforce Run-Level Timeout And Disable Timeout Retries + +**Files:** +- Modify: `src/browser_cli/automation/service/runtime.py` +- Modify: `src/browser_cli/errors.py` +- Test: `tests/unit/test_automation_service.py` + +- [ ] **Step 1: Write the failing timeout tests** + +```python +def test_automation_service_marks_run_failed_on_timeout(tmp_path: Path) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "import time\n" + "def run(flow, inputs):\n" + " time.sleep(0.3)\n" + " return {'ok': True}\n", + encoding="utf-8", + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + store = AutomationStore(tmp_path / "automations.db") + store.upsert_automation( + PersistedAutomationDefinition( + id="demo", + name="Demo", + task_path=task_dir / "task.py", + task_meta_path=task_dir / "task.meta.json", + output_dir=tmp_path / "out", + timeout_seconds=0.05, + ) + ) + run = store.create_run("demo", trigger_type="manual") + runtime = AutomationServiceRuntime(store=store) + + runtime._execute_run(run.run_id) + updated = store.get_run(run.run_id) + + assert updated.status == "failed" + assert updated.error_code == "AUTOMATION_RUN_TIMEOUT" + assert "timed out" in str(updated.error_message).lower() +``` + +```python +def test_automation_service_does_not_retry_timeout_failure(tmp_path: Path) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "import time\n" + "def run(flow, inputs):\n" + " time.sleep(0.3)\n" + " return {'ok': True}\n", + encoding="utf-8", + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + store = AutomationStore(tmp_path / "automations.db") + store.upsert_automation( + PersistedAutomationDefinition( + id="demo", + name="Demo", + task_path=task_dir / "task.py", + task_meta_path=task_dir / "task.meta.json", + output_dir=tmp_path / "out", + timeout_seconds=0.05, + retry_attempts=3, + ) + ) + run = store.create_run("demo", trigger_type="manual") + runtime = AutomationServiceRuntime(store=store) + runtime._execute_run(run.run_id) + runs = store.list_runs("demo", limit=10) + assert [run.status for run in runs] == ["failed"] +``` + +- [ ] **Step 2: Run the timeout tests to verify they fail** + +Run: `uv run pytest tests/unit/test_automation_service.py -k "timeout" -v` + +Expected: FAIL because `timeout_seconds` is currently not enforced and timed-out runs still complete successfully. + +- [ ] **Step 3: Implement run-level timeout enforcement** + +Add a dedicated timeout error in `src/browser_cli/errors.py`: + +```python +class AutomationRunTimeoutError(OperationFailedError): + def __init__(self, message: str = "Automation run timed out.") -> None: + super().__init__(message, error_code="AUTOMATION_RUN_TIMEOUT") +``` + +Update `src/browser_cli/automation/service/runtime.py` to run the main execution path under a timeout: + +```python +def _run_with_timeout(self, automation, fn): + if automation.timeout_seconds is None or automation.timeout_seconds <= 0: + return fn() + result_box: dict[str, Any] = {} + error_box: dict[str, BaseException] = {} + + def _target() -> None: + try: + result_box["value"] = fn() + except BaseException as exc: # noqa: BLE001 + error_box["error"] = exc + + thread = threading.Thread(target=_target, daemon=True) + thread.start() + thread.join(timeout=automation.timeout_seconds) + if thread.is_alive(): + raise AutomationRunTimeoutError( + f"Automation run timed out after {automation.timeout_seconds} seconds." + ) + if "error" in error_box: + raise error_box["error"] + return result_box.get("value") +``` + +Wrap the existing execution path so timeout is enforced around daemon readiness, hooks, and task execution, and gate retry scheduling: + +```python + self._run_with_timeout( + automation, + lambda: self._execute_run_main(run_id, automation, run, log_handle, run_dir), + ) +``` + +```python + timed_out = error_code == "AUTOMATION_RUN_TIMEOUT" + if not timed_out and automation.retry_attempts > run.attempt_number: + retry_run = self.store.retry_run(run_id) + self.store.add_run_event( + retry_run.run_id, + AutomationRunEvent( + run_id=retry_run.run_id, + event_type="retry_scheduled", + message=f"Retry scheduled after failed run {run_id}.", + ), + ) +``` + +- [ ] **Step 4: Run the timeout tests to verify they pass** + +Run: `uv run pytest tests/unit/test_automation_service.py -k "timeout" -v` + +Expected: PASS with timed-out runs marked failed and no extra retry runs created. + +- [ ] **Step 5: Commit the timeout enforcement** + +```bash +git add tests/unit/test_automation_service.py src/browser_cli/automation/service/runtime.py src/browser_cli/errors.py +git commit -m "fix: enforce automation run timeouts" +``` + +## Task 5: Restore Read Fallback Metadata And Update Durable Docs + +**Files:** +- Modify: `src/browser_cli/daemon/browser_service.py` +- Modify: `README.md` +- Modify: `AGENTS.md` +- Test: `tests/unit/test_task_runtime_client_read.py` +- Test: `tests/integration/test_task_runtime_read.py` + +- [ ] **Step 1: Write the failing fallback-reporting tests** + +```python +def test_run_read_request_preserves_daemon_fallback_metadata() -> None: + payload = { + "ok": True, + "data": { + "body": "", + "used_fallback_profile": True, + "fallback_profile_dir": "/tmp/browser-cli/default-profile", + "fallback_reason": "Chrome profile appears to be in use.", + }, + } + with patch("browser_cli.task_runtime.read.send_command", return_value=payload): + result = asyncio.run( + run_read_request(ReadRequest(url="https://example.com", output_mode="html")) + ) + + assert result.used_fallback_profile is True + assert result.fallback_profile_dir == "/tmp/browser-cli/default-profile" + assert result.fallback_reason == "Chrome profile appears to be in use." +``` + +```python +def test_task_runtime_read_daemon_path_reports_fallback_metadata( + monkeypatch, tmp_path: Path +) -> None: + _configure_runtime(monkeypatch, tmp_path) + chrome_environment = _build_chrome_environment(tmp_path) + with run_fixture_server() as base_url: + payload = send_command( + "read-page", + { + "url": f"{base_url}/static", + "output_mode": "html", + "chrome_environment": { + "executable_path": None, + "user_data_dir": str(chrome_environment.user_data_dir), + "profile_directory": chrome_environment.profile_directory, + "profile_name": chrome_environment.profile_name, + "source": "fallback", + "fallback_reason": "Chrome profile appears to be in use.", + }, + }, + ) + assert payload["data"]["used_fallback_profile"] is True + assert payload["data"]["fallback_reason"] == "Chrome profile appears to be in use." + send_command("stop", start_if_needed=False) +``` + +- [ ] **Step 2: Run the fallback tests to verify they fail** + +Run: `uv run pytest tests/unit/test_task_runtime_client_read.py tests/integration/test_task_runtime_read.py -k "fallback" -v` + +Expected: FAIL because daemon `read_page` currently drops fallback profile metadata on the daemon-backed read path. + +- [ ] **Step 3: Restore read fallback metadata and update docs** + +Update `src/browser_cli/daemon/browser_service.py`: + +```python + response = { + "page_id": page_id, + "body": body, + "output_mode": output_mode, + "url": str(page["url"]), + } + chrome_environment = self._playwright.chrome_environment + if chrome_environment is not None: + response["used_fallback_profile"] = chrome_environment.source == "fallback" + if chrome_environment.source == "fallback": + response["fallback_profile_dir"] = str(chrome_environment.user_data_dir) + response["fallback_reason"] = chrome_environment.fallback_reason + if chrome_environment.profile_name: + response["profile_name"] = chrome_environment.profile_name + response["profile_directory"] = chrome_environment.profile_directory + return response +``` + +Update `README.md` with durable release-model notes such as: + +```markdown +- `automation publish` snapshots source `automation.toml` when present; if it is + absent, Browser CLI publishes generated defaults and reports that explicitly. +- `automation inspect --version ` shows the immutable snapshot configuration + separately from the current live automation-service configuration. +- `runtime.timeout_seconds` is the total wall-clock timeout for one automation + run, not just the `task.py` function body. +``` + +Update `AGENTS.md` with durable navigation notes such as: + +```markdown +- If the user reports publish/config drift, inspect `src/browser_cli/automation/publisher.py`, + `src/browser_cli/commands/automation.py`, and the snapshot manifest under + `~/.browser-cli/automations//versions//automation.toml`. +- `inspect --version` must treat the snapshot manifest as historical truth and + render current persisted automation state separately. +``` + +- [ ] **Step 4: Run the fallback and repository validation tests** + +Run: `uv run pytest tests/unit/test_task_runtime_client_read.py tests/integration/test_task_runtime_read.py -k "fallback" -v` + +Expected: PASS with daemon-backed read preserving fallback metadata again. + +Run: `./scripts/check.sh` + +Expected: PASS with lint, test, and guard all green. + +- [ ] **Step 5: Commit the fallback and docs update** + +```bash +git add tests/unit/test_task_runtime_client_read.py tests/integration/test_task_runtime_read.py src/browser_cli/daemon/browser_service.py README.md AGENTS.md +git commit -m "fix: restore read fallback reporting and document release contract" +``` + +## Self-Review + +### Spec Coverage + +- Publish preserves source manifest truth: covered by Task 2. +- Generated-defaults fallback when source manifest is absent: covered by Task 2. +- Manifest schema parity and round-trip support for supported fields: covered by Task 1. +- Inspect separation between snapshot and live config: covered by Task 3. +- Timeout is a run-level total timeout with no retry-on-timeout: covered by Task 4. +- Read fallback reporting restored only for the read path: covered by Task 5. +- Durable docs and agent guidance updates: covered by Task 5. + +### Placeholder Scan + +- No unresolved placeholder markers remain. +- Each task includes concrete files, test code, commands, expected outcomes, and commit commands. + +### Type Consistency + +- `retry_backoff_seconds`, `stdout_mode`, `manifest_source`, `snapshot_config`, `live_config`, and `AUTOMATION_RUN_TIMEOUT` are used consistently across the tasks. +- Timeout scope remains explicitly limited to automation-service execution and does not leak into `task run`. diff --git a/docs/superpowers/specs/2026-04-14-browser-cli-release-readiness-design.md b/docs/superpowers/specs/2026-04-14-browser-cli-release-readiness-design.md new file mode 100644 index 0000000..9bc8462 --- /dev/null +++ b/docs/superpowers/specs/2026-04-14-browser-cli-release-readiness-design.md @@ -0,0 +1,443 @@ +# Browser CLI Release Readiness Design + +Date: 2026-04-14 +Status: Drafted for review +Repo: `browser-cli` + +## Summary + +This design closes a set of release-blocking consistency gaps across the +automation publish chain, automation manifest round-tripping, automation run +timeouts, and one-shot read fallback reporting. + +The unifying rule is: + +- a published automation version is defined by the immutable snapshot under + `~/.browser-cli/automations//versions//` +- that snapshot truth includes `task.py`, `task.meta.json`, and the + version-specific `automation.toml` +- current automation-service state is a separate live view and must not be + confused with historical snapshot truth + +This design does not introduce a new product surface. It tightens the existing +contracts so the first public release behaves consistently with the current +documentation and user model. + +## Problem Statement + +The repository is close to release quality, but several mismatches remain: + +- `automation publish` snapshots task code but currently regenerates automation + configuration from defaults, which discards reviewed source configuration +- automation manifest fields are not modeled consistently across load, import, + export, inspect, and persistence paths +- `runtime.timeout_seconds` exists as public configuration but is not enforced + by automation execution +- `inspect --version` does not cleanly distinguish immutable snapshot config + from current live automation state +- one-shot `read` fallback profile reporting was preserved in lower layers but + was lost on the daemon-backed read path + +These failures all have the same user-visible effect: Browser CLI exposes a +configuration surface that appears stable, but some paths silently change or +discard information. + +## Goals + +- Preserve reviewed `automation.toml` configuration when publishing a task. +- Keep published automation versions immutable and self-describing. +- Make snapshot truth and live service state clearly separable in inspect + output. +- Make supported automation manifest fields round-trip without silent loss + across import, export, publish, and inspect. +- Enforce `runtime.timeout_seconds` as a real automation-service behavior. +- Keep timeout failure semantics explicit: run-level total timeout, no automatic + retry on timeout. +- Restore fallback profile reporting for `browser-cli read`. + +## Non-Goals + +- No change to `task run` semantics. `task.meta.json` remains a reference and + sidecar artifact, not a runtime requirement for execution. +- No expansion of daemon-wide response metadata for all commands. +- No automation timeout support for local `browser-cli task run`. +- No broader daemon/runtime architecture changes beyond the targeted fixes. +- No new public `automation` subcommands. + +## Chosen Direction + +Browser CLI should treat the snapshot manifest as the configuration truth for a +published version, while treating the automation-service database as the truth +for current live state. + +That produces five explicit rules: + +1. `automation publish` snapshots task code and configuration together. +2. If source `automation.toml` exists, publish uses it as the configuration + truth. +3. If source `automation.toml` does not exist, publish may still proceed, but + the generated defaults must be explicit in the published metadata. +4. `inspect --version N` shows the immutable snapshot configuration for version + `N`, while current live configuration is shown separately. +5. `runtime.timeout_seconds` is a run-level total timeout budget enforced by + the automation service, not a dead configuration field. + +## Options Considered + +### 1. Minimal Bug Fixes Only + +Patch each reported bug independently without introducing a tighter release +model. + +Advantages: + +- smallest patch set +- lowest immediate implementation risk + +Disadvantages: + +- leaves the publish/import/export/inspect flow conceptually fragmented +- makes future regressions more likely because there is no single truth model + +Rejected. + +### 2. Snapshot-Truth Consistency Repair + +Define a single release model around immutable snapshot manifests, align +publish/import/export/inspect to that model, and enforce timeout/fallback +behavior where the public contract already promises it. + +Advantages: + +- matches the existing task/automation design +- fixes the current release blockers without widening scope unnecessarily +- gives inspect and publish a clear mental model + +Disadvantages: + +- requires coordinated updates across several automation modules +- increases the amount of contract-focused testing that must be updated + +Chosen direction. + +### 3. Broader Runtime Contract Unification + +Use this work to redesign more of the daemon and automation metadata model. + +Advantages: + +- could simplify future work further + +Disadvantages: + +- too large for the current pre-release window +- risks mixing release fixes with unrelated contract redesign + +Rejected. + +## Publish Contract + +### Source Inputs + +`browser-cli automation publish ` consumes: + +- required: `task.py` +- required: `task.meta.json` +- optional: `automation.toml` + +### Publish Behavior + +Publish must validate `task.py` and `task.meta.json` first. + +Then publish determines the manifest source: + +- if `automation.toml` exists in the source task directory, load and validate it + and use it as the release configuration truth +- if `automation.toml` does not exist, generate the minimal default manifest and + mark the published result as `manifest_source=generated_defaults` +- if `automation.toml` exists but is invalid, fail publish; do not silently fall + back to defaults + +The publish output snapshot must contain: + +- `task.py` +- `task.meta.json` +- the final resolved `automation.toml` +- `publish.json` + +The canonical snapshot location remains: + +```text +~/.browser-cli/automations//versions// +``` + +### Publish Result Metadata + +Publish output should explicitly include: + +- `manifest_source` + - `task_dir` + - `generated_defaults` +- snapshot path +- version +- automation id +- a concise summary of the resolved published configuration + +This is an additive contract improvement. It does not change the public command +name or the broader task/automation model. + +## Manifest Modeling And Round-Trip Rules + +### Supported Fields + +The automation manifest loader, persistence conversion, export path, and inspect +path must agree on the same supported field set. + +At minimum, the currently supported fields that must no longer be silently lost +are: + +- `inputs` +- schedule fields including `mode` and `timezone` +- `outputs.artifact_dir` +- `outputs.result_json_path` +- `outputs.stdout` +- `hooks.before_run` +- `hooks.after_success` +- `hooks.after_failure` +- `runtime.retry_attempts` +- `runtime.retry_backoff_seconds` +- `runtime.timeout_seconds` +- `runtime.log_level` + +### Round-Trip Expectations + +The round-trip requirements are: + +- `automation import` must preserve all supported fields into persisted + automation state +- `automation export` must faithfully render supported persisted fields back to + `automation.toml` +- `automation publish` must preserve supported source manifest fields when + producing the versioned snapshot manifest +- snapshot inspect must reflect the supported fields stored in the versioned + snapshot manifest + +Silent field loss is not acceptable for supported fields. + +Unknown-field preservation is not a goal in this design. The contract is based +on complete handling of the supported schema, not lossless passthrough of +arbitrary TOML. + +## Inspect Contract + +### Live Inspect + +`browser-cli automation inspect ` without `--version` should show +the current persisted automation definition plus the latest run summary. + +This is the live operational view. + +### Versioned Inspect + +`browser-cli automation inspect --version N` should show two +separate configuration views: + +- `snapshot_config` + - loaded from + `~/.browser-cli/automations//versions//automation.toml` + - represents the immutable configuration that was published for that version +- `live_config` + - loaded from the current persisted automation definition in the automation + service + - represents the current service configuration, which may reflect a newer + publish or an import/update path + +`latest_run` remains a separate operational section. + +This separation is required so users can answer three different questions +without ambiguity: + +- what was published in version `N` +- what is currently configured in the automation service +- what happened in the most recent run + +### Error Handling + +If the requested snapshot version exists but its manifest cannot be loaded, +inspect must not silently replace the broken snapshot view with live config. +Instead it should expose a dedicated error field such as +`snapshot_config_error`. + +## Timeout Contract + +### Semantics + +`runtime.timeout_seconds` must mean the total wall-clock time budget for one +automation run while it occupies the executor. + +This is a run-level timeout, not a task-function-only timeout. + +The timeout budget covers the main run path, including: + +- daemon readiness checks performed as part of run execution +- `before_run` hooks +- `task.py` execution +- main result handling before final success/failure state is recorded + +### Failure Semantics + +If the timeout budget is exceeded: + +- the run is marked `failed` +- the run receives a dedicated timeout error code +- timeout does not participate in automatic retry, even if + `retry_attempts > 0` + +This makes timeout behavior explicit and avoids turning “the run exceeded its +budget” into a silent retry loop. + +### Failure Hooks + +`after_failure` hooks may still run after timeout, but they must not be allowed +to hang indefinitely. They should run under a separate bounded timeout budget. + +The design does not require one specific implementation strategy for that hook +timeout, but it does require bounded behavior. + +### Timeout Scope Boundary + +Timeout remains an automation-service concern only. + +It should not be pushed into: + +- `browser-cli task run` +- `browser_cli.task_runtime` +- general daemon command handling + +This keeps local task execution behavior stable and prevents automation-service +policy from leaking into the reusable task runtime. + +## Read Fallback Reporting + +The fix scope is intentionally narrow. + +Fallback profile reporting should be restored only for the one-shot read path: + +- daemon `read-page` +- task runtime `read` +- CLI `browser-cli read` + +The daemon-backed read response must again include the fallback profile metadata +already used by the user-facing read command: + +- `used_fallback_profile` +- `fallback_profile_dir` +- `fallback_reason` + +The CLI should continue printing the user-facing stderr notice only on the read +surface. + +This design explicitly does not promote fallback/profile details into the +general response contract of every daemon-backed action. + +## Module Responsibilities + +### `browser_cli.automation.loader` + +- fully parse the supported manifest schema +- stop dropping supported runtime fields such as + `retry_backoff_seconds` + +### `browser_cli.automation.models` + +- model the same supported runtime fields that loader/import/export/persistence + support +- keep manifest-to-persisted conversion aligned with the supported schema + +### `browser_cli.automation.publisher` + +- determine manifest source during publish +- load source `automation.toml` when present +- generate defaults only when the source manifest is absent +- write the final resolved manifest into the snapshot directory +- return `manifest_source` metadata + +### `browser_cli.commands.automation` + +- present publish results clearly +- keep import/export/inspect response shapes aligned with the supported field + set +- expose snapshot/live separation in inspect output + +### `browser_cli.automation.api.server` + +- accept and return the full supported field set for persisted automations +- avoid silently defaulting supported fields when they were supplied upstream + +### `browser_cli.automation.service.runtime` + +- enforce run-level timeout semantics +- enforce “timeout does not auto-retry” +- emit explicit timeout failure information in run events/status + +### `browser_cli.daemon.browser_service` + +- restore fallback metadata on `read_page` + +## Testing Requirements + +The release fix is not complete without contract-focused tests. + +At minimum, tests should cover: + +- publish preserves source `automation.toml` fields into the snapshot manifest +- publish without source `automation.toml` produces defaults and marks + `manifest_source=generated_defaults` +- import/export preserve the supported manifest fields +- inspect with `--version` returns separate snapshot and live config views +- timeout fails a run when the total wall-clock budget is exceeded +- timeout failure does not schedule automatic retry +- read fallback metadata reaches `browser-cli read` again + +The full repository validation bar remains: + +- `scripts/lint.sh` +- `scripts/test.sh` +- `scripts/guard.sh` + +or equivalently: + +- `scripts/check.sh` + +## Documentation Requirements + +The documentation updates in this design are part of the implementation scope, +not follow-up polish. + +At minimum, docs should make the following durable statements explicit: + +- published automation versions live under + `~/.browser-cli/automations/...` +- `automation publish` snapshots configuration from source + `automation.toml` when present +- publish may use generated defaults only when source `automation.toml` is + absent, and this is made explicit +- `inspect --version` shows snapshot config separately from live config +- `runtime.timeout_seconds` is a run-level total timeout + +`AGENTS.md` should also be updated if any navigation or debugging guidance needs +to reflect the tightened snapshot/live truth rules. + +## Expected Outcome + +After this design is implemented: + +- publishing a task no longer discards reviewed automation configuration +- supported manifest fields stop drifting across publish/import/export/inspect +- users can inspect historical snapshot truth and current live state separately +- automation timeout becomes a real supported behavior rather than a dead + setting +- `browser-cli read` again explains when fallback profile mode was used + +That is the minimum release-ready consistency bar for Browser CLI’s first public +version. diff --git a/scripts/lint.sh b/scripts/lint.sh index 7387942..15f52ea 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -15,5 +15,5 @@ if command -v node >/dev/null 2>&1; then while IFS= read -r js_file; do node --check "$js_file" done < <(find browser-cli-extension -type f -name '*.js' | sort) - node --test browser-cli-extension/tests/popup_view.test.js + node --test browser-cli-extension/tests/*.test.js fi diff --git a/src/browser_cli/automation/api/server.py b/src/browser_cli/automation/api/server.py index aab8b0a..cae86d0 100644 --- a/src/browser_cli/automation/api/server.py +++ b/src/browser_cli/automation/api/server.py @@ -316,6 +316,7 @@ def _payload_to_automation(payload: dict) -> PersistedAutomationDefinition: timezone=str(payload.get("timezone") or "UTC"), output_dir=output_dir, result_json_path=Path(result_json_raw) if result_json_raw else None, + stdout_mode=str(payload.get("stdout_mode") or "json"), input_overrides=dict(payload.get("input_overrides") or {}), before_run_hooks=tuple(payload.get("before_run_hooks") or []), after_success_hooks=tuple(payload.get("after_success_hooks") or []), diff --git a/src/browser_cli/automation/hooks.py b/src/browser_cli/automation/hooks.py index b546473..ee328a2 100644 --- a/src/browser_cli/automation/hooks.py +++ b/src/browser_cli/automation/hooks.py @@ -13,6 +13,7 @@ def run_hook_commands( *, cwd: Path, extra_env: dict[str, Any] | None = None, + timeout_seconds: float | None = None, ) -> list[dict[str, Any]]: results: list[dict[str, Any]] = [] env = os.environ.copy() @@ -26,6 +27,7 @@ def run_hook_commands( env=env, capture_output=True, text=True, + timeout=timeout_seconds, ) results.append( { diff --git a/src/browser_cli/automation/loader.py b/src/browser_cli/automation/loader.py index 2c4a443..0cd1d1d 100644 --- a/src/browser_cli/automation/loader.py +++ b/src/browser_cli/automation/loader.py @@ -88,6 +88,9 @@ def load_automation_manifest(path: str | Path) -> AutomationManifest: if (data.get("runtime") or {}).get("timeout_seconds") is not None else None, retry_attempts=int((data.get("runtime") or {}).get("retry_attempts") or 0), + retry_backoff_seconds=int( + (data.get("runtime") or {}).get("retry_backoff_seconds") or 0 + ), log_level=str((data.get("runtime") or {}).get("log_level") or "info"), ), ) diff --git a/src/browser_cli/automation/models.py b/src/browser_cli/automation/models.py index 62ea550..a65bd16 100644 --- a/src/browser_cli/automation/models.py +++ b/src/browser_cli/automation/models.py @@ -40,6 +40,7 @@ class AutomationHooks: class AutomationRuntime: timeout_seconds: float | None = None retry_attempts: int = 0 + retry_backoff_seconds: int = 0 log_level: str = "info" @@ -140,5 +141,6 @@ def manifest_to_persisted_definition( after_success_hooks=manifest.hooks.after_success, after_failure_hooks=manifest.hooks.after_failure, retry_attempts=manifest.runtime.retry_attempts, + retry_backoff_seconds=manifest.runtime.retry_backoff_seconds, timeout_seconds=manifest.runtime.timeout_seconds, ) diff --git a/src/browser_cli/automation/publisher.py b/src/browser_cli/automation/publisher.py index 72b9066..c73df9a 100644 --- a/src/browser_cli/automation/publisher.py +++ b/src/browser_cli/automation/publisher.py @@ -8,6 +8,8 @@ from pathlib import Path from typing import Any +from browser_cli.automation.loader import load_automation_manifest +from browser_cli.automation.models import AutomationManifest from browser_cli.automation.toml import dumps_toml_sections from browser_cli.constants import AppPaths from browser_cli.task_runtime import validate_task_dir @@ -21,6 +23,7 @@ class PublishedAutomation: snapshot_dir: Path manifest_path: Path output_dir: Path + manifest_source: str def publish_task_dir(task_dir: Path, *, app_paths: AppPaths) -> PublishedAutomation: @@ -44,24 +47,37 @@ def publish_task_dir(task_dir: Path, *, app_paths: AppPaths) -> PublishedAutomat shutil.copy2(task_dir / "task.py", task_path) shutil.copy2(task_dir / "task.meta.json", task_meta_path) - manifest_path = snapshot_dir / "automation.toml" - manifest_path.write_text( - render_automation_manifest( + source_manifest_path = task_dir / "automation.toml" + if source_manifest_path.exists(): + manifest_source = "task_dir" + source_manifest = load_automation_manifest(source_manifest_path) + rendered_manifest = render_automation_manifest_from_manifest( + source_manifest, + version=version, + task_path=task_path, + task_meta_path=task_meta_path, + output_dir=automation_root, + ) + else: + manifest_source = "generated_defaults" + rendered_manifest = render_automation_manifest( automation_id=automation_id, name=automation_name, version=version, task_path=task_path, task_meta_path=task_meta_path, output_dir=automation_root, - ), - encoding="utf-8", - ) + ) + + manifest_path = snapshot_dir / "automation.toml" + manifest_path.write_text(rendered_manifest, encoding="utf-8") (snapshot_dir / "publish.json").write_text( json.dumps( { "automation_id": automation_id, "name": automation_name, "version": version, + "manifest_source": manifest_source, "source_task_path": str(task_dir), "snapshot_dir": str(snapshot_dir), }, @@ -77,6 +93,7 @@ def publish_task_dir(task_dir: Path, *, app_paths: AppPaths) -> PublishedAutomat snapshot_dir=snapshot_dir, manifest_path=manifest_path, output_dir=automation_root, + manifest_source=manifest_source, ) @@ -129,6 +146,84 @@ def render_automation_manifest( return dumps_toml_sections(sections) +def render_automation_manifest_from_manifest( + manifest: AutomationManifest, + *, + version: int, + task_path: Path, + task_meta_path: Path, + output_dir: Path, +) -> str: + schedule = dict(manifest.schedule) + artifact_dir = output_dir + result_json_path = _remap_result_json_path( + manifest.outputs.artifact_dir, + manifest.outputs.result_json_path, + artifact_dir, + ) + sections: list[tuple[str, dict[str, Any]]] = [ + ( + "automation", + { + "id": str(manifest.automation.id), + "name": str(manifest.automation.name), + "description": str(manifest.automation.description), + "version": str(version), + }, + ), + ( + "task", + { + "path": str(task_path), + "meta_path": str(task_meta_path), + "entrypoint": str(manifest.task.entrypoint), + }, + ), + ("inputs", dict(manifest.inputs)), + ("schedule", schedule), + ( + "outputs", + { + "artifact_dir": str(artifact_dir), + "result_json_path": str(result_json_path) if result_json_path else None, + "stdout": str(manifest.outputs.stdout), + }, + ), + ( + "hooks", + { + "before_run": list(manifest.hooks.before_run), + "after_success": list(manifest.hooks.after_success), + "after_failure": list(manifest.hooks.after_failure), + }, + ), + ( + "runtime", + { + "retry_attempts": int(manifest.runtime.retry_attempts), + "retry_backoff_seconds": int(manifest.runtime.retry_backoff_seconds), + "timeout_seconds": manifest.runtime.timeout_seconds, + "log_level": str(manifest.runtime.log_level), + }, + ), + ] + return dumps_toml_sections(sections) + + +def _remap_result_json_path( + source_artifact_dir: Path, + source_result_json_path: Path | None, + target_artifact_dir: Path, +) -> Path | None: + if source_result_json_path is None: + return None + try: + relative = source_result_json_path.relative_to(source_artifact_dir) + except ValueError: + return target_artifact_dir / source_result_json_path.name + return target_artifact_dir / relative + + def _next_version(versions_dir: Path) -> int: versions = [ int(path.name) for path in versions_dir.iterdir() if path.is_dir() and path.name.isdigit() diff --git a/src/browser_cli/automation/service/runtime.py b/src/browser_cli/automation/service/runtime.py index 88dcf2b..ebf86c2 100644 --- a/src/browser_cli/automation/service/runtime.py +++ b/src/browser_cli/automation/service/runtime.py @@ -4,8 +4,10 @@ import json import logging +import multiprocessing import threading import time +from pathlib import Path from typing import Any from browser_cli import __version__, error_codes @@ -13,13 +15,53 @@ from browser_cli.automation.models import AutomationRunEvent from browser_cli.automation.persistence import AutomationStore from browser_cli.constants import get_app_paths -from browser_cli.errors import OperationFailedError +from browser_cli.errors import AutomationRunTimeoutError, OperationFailedError from browser_cli.task_runtime import run_task_entrypoint from browser_cli.task_runtime.client import BrowserCliTaskClient logger = logging.getLogger(__name__) AUTOMATION_SERVICE_RUNTIME_VERSION = "2026-04-12-automation-service-v1" +FAILURE_HOOK_TIMEOUT_SECONDS = 5.0 + + +def _run_task_entrypoint_child( + queue, + *, + task_path: str, + entrypoint: str, + inputs: dict[str, Any], + artifacts_dir: str, + automation_path: str | None, + automation_name: str | None, + log_path: str, +) -> None: + try: + with open(log_path, "a", encoding="utf-8") as log_handle: + result = run_task_entrypoint( + task_path=Path(task_path), + entrypoint=entrypoint, + inputs=inputs, + artifacts_dir=Path(artifacts_dir), + automation_path=Path(automation_path) if automation_path else None, + automation_name=automation_name, + client=BrowserCliTaskClient(), + stdout_handle=log_handle, + stderr_handle=log_handle, + ) + queue.put({"ok": True, "result": result}) + except BaseException as exc: # noqa: BLE001 + queue.put( + { + "ok": False, + "error_message": str(exc), + "error_code": ( + exc.error_code + if isinstance(exc, OperationFailedError) + else error_codes.AUTOMATION_RUN_FAILED + ), + } + ) class AutomationServiceRuntime: @@ -91,6 +133,8 @@ def _execute_run(self, run_id: str) -> None: run_dir.mkdir(parents=True, exist_ok=True) log_path = run_dir / "run.log" result_path = run_dir / "result.json" + deadline = self._deadline_for_run(automation.timeout_seconds) + stage = "daemon_ready" self.store.add_run_event( run_id, AutomationRunEvent( @@ -101,7 +145,14 @@ def _execute_run(self, run_id: str) -> None: ) try: client = BrowserCliTaskClient() - client.command("runtime-status") + self._run_with_timeout( + lambda: client.command("runtime-status"), + timeout_seconds=self._remaining_timeout(deadline, stage="daemon_ready"), + timeout_message=( + f"Automation run timed out during daemon readiness after " + f"{automation.timeout_seconds} seconds." + ), + ) self.store.add_run_event( run_id, AutomationRunEvent( @@ -128,40 +179,47 @@ def _execute_run(self, run_id: str) -> None: "BROWSER_CLI_TASK_PATH": str(automation.task_path), "BROWSER_CLI_AUTOMATION_STATUS": "running", } + stage = "before_hooks" before_hooks = run_hook_commands( automation.before_run_hooks, cwd=automation.task_path.parent, extra_env=hook_env, + timeout_seconds=self._remaining_timeout(deadline, stage=stage), ) if before_hooks: log_handle.write( json.dumps({"before_hooks": before_hooks}, ensure_ascii=False) + "\n" ) - result = run_task_entrypoint( + stage = "task_execution" + result = self._run_task_entrypoint_with_timeout( task_path=automation.task_path, entrypoint=automation.entrypoint, inputs=run.effective_inputs, artifacts_dir=run_dir, automation_path=automation.output_dir, automation_name=automation.name, - client=BrowserCliTaskClient(), - stdout_handle=log_handle, - stderr_handle=log_handle, + log_path=log_path, + timeout_seconds=self._remaining_timeout(deadline, stage=stage), ) + stage = "persist_result" + self._ensure_time_remaining(deadline, stage=stage) result_path.write_text( json.dumps(result, ensure_ascii=False, indent=2), encoding="utf-8", ) if automation.result_json_path is not None: + self._ensure_time_remaining(deadline, stage=stage) automation.result_json_path.parent.mkdir(parents=True, exist_ok=True) automation.result_json_path.write_text( json.dumps(result, ensure_ascii=False, indent=2), encoding="utf-8", ) + stage = "after_success_hooks" success_hooks = run_hook_commands( automation.after_success_hooks, cwd=automation.task_path.parent, extra_env={**hook_env, "BROWSER_CLI_AUTOMATION_STATUS": "success"}, + timeout_seconds=self._remaining_timeout(deadline, stage=stage), ) if success_hooks: log_handle.write( @@ -176,6 +234,17 @@ def _execute_run(self, run_id: str) -> None: log_path=log_path, ) except Exception as exc: + timed_out = isinstance(exc, AutomationRunTimeoutError) + if timed_out: + self.store.add_run_event( + run_id, + AutomationRunEvent( + run_id=run_id, + event_type="run_timed_out", + message=str(exc), + payload={"stage": stage}, + ), + ) try: failure_hooks = run_hook_commands( automation.after_failure_hooks, @@ -186,6 +255,7 @@ def _execute_run(self, run_id: str) -> None: "BROWSER_CLI_TASK_PATH": str(automation.task_path), "BROWSER_CLI_AUTOMATION_STATUS": "failure", }, + timeout_seconds=FAILURE_HOOK_TIMEOUT_SECONDS, ) if failure_hooks: log_handle.write( @@ -210,7 +280,7 @@ def _execute_run(self, run_id: str) -> None: artifacts_dir=run_dir, log_path=log_path, ) - if automation.retry_attempts > run.attempt_number: + if not timed_out and automation.retry_attempts > run.attempt_number: retry_run = self.store.retry_run(run_id) self.store.add_run_event( retry_run.run_id, @@ -220,3 +290,99 @@ def _execute_run(self, run_id: str) -> None: message=f"Retry scheduled after failed run {run_id}.", ), ) + + @staticmethod + def _deadline_for_run(timeout_seconds: float | None) -> float | None: + if timeout_seconds is None or timeout_seconds <= 0: + return None + return time.monotonic() + timeout_seconds + + @staticmethod + def _remaining_timeout(deadline: float | None, *, stage: str) -> float | None: + if deadline is None: + return None + remaining = deadline - time.monotonic() + if remaining <= 0: + raise AutomationRunTimeoutError( + f"Automation run timed out during {stage} before work could continue." + ) + return remaining + + @staticmethod + def _ensure_time_remaining(deadline: float | None, *, stage: str) -> None: + _ = AutomationServiceRuntime._remaining_timeout(deadline, stage=stage) + + @staticmethod + def _run_with_timeout( + fn, + *, + timeout_seconds: float | None, + timeout_message: str, + ): + if timeout_seconds is None: + return fn() + result_box: dict[str, Any] = {} + error_box: dict[str, BaseException] = {} + + def _target() -> None: + try: + result_box["value"] = fn() + except BaseException as exc: # noqa: BLE001 + error_box["error"] = exc + + thread = threading.Thread(target=_target, daemon=True) + thread.start() + thread.join(timeout=timeout_seconds) + if thread.is_alive(): + raise AutomationRunTimeoutError(timeout_message) + if "error" in error_box: + raise error_box["error"] + return result_box.get("value") + + @staticmethod + def _run_task_entrypoint_with_timeout( + *, + task_path: Path, + entrypoint: str, + inputs: dict[str, Any], + artifacts_dir: Path, + automation_path: Path | None, + automation_name: str | None, + log_path: Path, + timeout_seconds: float | None, + ) -> dict[str, Any]: + if timeout_seconds is None: + timeout_seconds = None + context = multiprocessing.get_context("spawn") + queue = context.Queue() + process = context.Process( + target=_run_task_entrypoint_child, + kwargs={ + "queue": queue, + "task_path": str(task_path), + "entrypoint": entrypoint, + "inputs": inputs, + "artifacts_dir": str(artifacts_dir), + "automation_path": str(automation_path) if automation_path else None, + "automation_name": automation_name, + "log_path": str(log_path), + }, + daemon=True, + ) + process.start() + process.join(timeout=timeout_seconds) + if process.is_alive(): + process.terminate() + process.join(timeout=1.0) + raise AutomationRunTimeoutError( + f"Automation run timed out during task execution after {timeout_seconds} seconds." + ) + if queue.empty(): + raise OperationFailedError("Task worker exited without returning a result.") + payload = queue.get() + if bool(payload.get("ok")): + return dict(payload.get("result") or {}) + raise OperationFailedError( + str(payload.get("error_message") or "Task worker failed."), + error_code=str(payload.get("error_code") or error_codes.AUTOMATION_RUN_FAILED), + ) diff --git a/src/browser_cli/commands/automation.py b/src/browser_cli/commands/automation.py index 2136c4b..5fde78a 100644 --- a/src/browser_cli/commands/automation.py +++ b/src/browser_cli/commands/automation.py @@ -43,6 +43,7 @@ def run_automation_command(args: Namespace) -> str: "automation_id": published.automation_id, "automation_name": published.automation_name, "version": published.version, + "manifest_source": published.manifest_source, "source_task_dir": str(source_task_dir), "snapshot_dir": str(published.snapshot_dir), "manifest_path": str(published.manifest_path), @@ -84,29 +85,44 @@ def run_automation_command(args: Namespace) -> str: "GET", f"/api/automations/{args.automation_id}", start_if_needed=True ) versions = _load_snapshot_versions(args.automation_id) - selected = _select_snapshot_version( - args.automation_id, - versions, - version=getattr(args, "version", None), - ) live_automation_data = dict(payload.get("data") or {}) - selected_automation_data, selected_latest_run = _selected_snapshot_payload( - selected, fallback_automation=live_automation_data + selected = ( + _select_snapshot_version( + args.automation_id, + versions, + version=getattr(args, "version", None), + ) + if getattr(args, "version", None) is not None + else None + ) + snapshot_config = ( + selected.get("snapshot_automation") + if isinstance(selected, dict) and isinstance(selected.get("snapshot_automation"), dict) + else None ) + snapshot_config_error = ( + str(selected.get("snapshot_config_error") or "") + if isinstance(selected, dict) and selected.get("snapshot_config_error") + else None + ) + latest_run = live_automation_data.get("latest_run") + latest_run_payload = latest_run if isinstance(latest_run, dict) else None return render_json_payload( { "ok": True, "data": { - "automation": selected_automation_data, + "snapshot_config": snapshot_config, + "snapshot_config_error": snapshot_config_error, + "live_config": live_automation_data, "versions": versions, "selected_version": selected, - "latest_run": selected_latest_run, + "latest_run": latest_run_payload, "summary": _build_inspect_summary( args.automation_id, - selected_automation_data, + live_automation_data, versions, selected, - selected_latest_run, + latest_run_payload, ), }, "meta": {"action": "automation-inspect"}, @@ -186,7 +202,9 @@ def _load_snapshot_versions(automation_id: str) -> list[dict[str, object]]: continue publish_path = entry / "publish.json" publish_data = _read_json_file(publish_path) - snapshot_manifest = _load_snapshot_manifest(entry / "automation.toml") + snapshot_manifest, snapshot_config_error = _load_snapshot_manifest( + entry / "automation.toml" + ) versions.append( { "version": int(entry.name), @@ -197,6 +215,7 @@ def _load_snapshot_versions(automation_id: str) -> list[dict[str, object]]: if snapshot_manifest is not None else None ), + "snapshot_config_error": snapshot_config_error, "snapshot_latest_run": publish_data.get("latest_run") if isinstance(publish_data.get("latest_run"), dict) else None, @@ -256,27 +275,13 @@ def _build_inspect_summary( } -def _selected_snapshot_payload( - selected: dict[str, object] | None, - *, - fallback_automation: dict[str, object], -) -> tuple[dict[str, object], dict[str, object] | None]: - if selected is None: - latest_run = fallback_automation.get("latest_run") - return fallback_automation, latest_run if isinstance(latest_run, dict) else None - snapshot_automation = selected.get("snapshot_automation") - snapshot_latest_run = selected.get("snapshot_latest_run") - if isinstance(snapshot_automation, dict): - latest_run = snapshot_latest_run if isinstance(snapshot_latest_run, dict) else None - return snapshot_automation, latest_run - latest_run = fallback_automation.get("latest_run") - return fallback_automation, latest_run if isinstance(latest_run, dict) else None - - -def _load_snapshot_manifest(path: Path) -> AutomationManifest | None: +def _load_snapshot_manifest(path: Path) -> tuple[AutomationManifest | None, str | None]: if not path.exists(): - return None - return load_automation_manifest(path) + return None, None + try: + return load_automation_manifest(path), None + except InvalidInputError as exc: + return None, str(exc) def _snapshot_manifest_to_automation_payload(manifest: AutomationManifest) -> dict[str, object]: @@ -305,7 +310,7 @@ def _snapshot_manifest_to_automation_payload(manifest: AutomationManifest) -> di "after_success_hooks": list(manifest.hooks.after_success), "after_failure_hooks": list(manifest.hooks.after_failure), "retry_attempts": int(manifest.runtime.retry_attempts or 0), - "retry_backoff_seconds": 0, + "retry_backoff_seconds": int(manifest.runtime.retry_backoff_seconds or 0), "timeout_seconds": manifest.runtime.timeout_seconds, "created_at": None, "updated_at": None, @@ -331,10 +336,12 @@ def _manifest_to_automation_payload(manifest, *, enabled: bool) -> dict[str, obj "timezone": str(schedule.get("timezone") or "UTC"), "output_dir": str(manifest.outputs.artifact_dir), "result_json_path": str(manifest.outputs.result_json_path or ""), + "stdout_mode": manifest.outputs.stdout, "input_overrides": dict(manifest.inputs), "before_run_hooks": list(manifest.hooks.before_run), "after_success_hooks": list(manifest.hooks.after_success), "after_failure_hooks": list(manifest.hooks.after_failure), "retry_attempts": int(manifest.runtime.retry_attempts or 0), + "retry_backoff_seconds": int(manifest.runtime.retry_backoff_seconds or 0), "timeout_seconds": manifest.runtime.timeout_seconds, } diff --git a/src/browser_cli/daemon/browser_service.py b/src/browser_cli/daemon/browser_service.py index 99bb09f..31ab1dd 100644 --- a/src/browser_cli/daemon/browser_service.py +++ b/src/browser_cli/daemon/browser_service.py @@ -352,12 +352,22 @@ async def read_page( body = str((await self.capture_html(page_id))["html"]) if not body.strip(): raise EmptyContentError() - return { + payload = { "page_id": page_id, "body": body, "output_mode": output_mode, "url": str(page["url"]), } + chrome_environment = self._playwright.chrome_environment + if chrome_environment is not None: + payload["used_fallback_profile"] = chrome_environment.source == "fallback" + if chrome_environment.source == "fallback": + payload["fallback_profile_dir"] = str(chrome_environment.user_data_dir) + payload["fallback_reason"] = chrome_environment.fallback_reason + if chrome_environment.profile_name: + payload["profile_name"] = chrome_environment.profile_name + payload["profile_directory"] = chrome_environment.profile_directory + return payload finally: with contextlib.suppress(Exception): await self.close_tab(page_id) diff --git a/src/browser_cli/error_codes.py b/src/browser_cli/error_codes.py index e72d072..5de5c8c 100644 --- a/src/browser_cli/error_codes.py +++ b/src/browser_cli/error_codes.py @@ -24,3 +24,4 @@ AUTOMATION_SERVICE_NOT_AVAILABLE = "AUTOMATION_SERVICE_NOT_AVAILABLE" AUTOMATION_INVALID = "AUTOMATION_INVALID" AUTOMATION_RUN_FAILED = "AUTOMATION_RUN_FAILED" +AUTOMATION_RUN_TIMEOUT = "AUTOMATION_RUN_TIMEOUT" diff --git a/src/browser_cli/errors.py b/src/browser_cli/errors.py index 9e58e2b..a1bff3b 100644 --- a/src/browser_cli/errors.py +++ b/src/browser_cli/errors.py @@ -139,3 +139,8 @@ def __init__(self, payload: dict[str, object]) -> None: class AutomationInvalidError(BrowserCliError): def __init__(self, message: str) -> None: super().__init__(message, exit_codes.USAGE_ERROR, error_codes.AUTOMATION_INVALID) + + +class AutomationRunTimeoutError(OperationFailedError): + def __init__(self, message: str = "Automation run timed out.") -> None: + super().__init__(message, error_code=error_codes.AUTOMATION_RUN_TIMEOUT) diff --git a/tests/integration/test_task_runtime_read.py b/tests/integration/test_task_runtime_read.py index fc21b22..08372bc 100644 --- a/tests/integration/test_task_runtime_read.py +++ b/tests/integration/test_task_runtime_read.py @@ -143,3 +143,34 @@ def test_task_runtime_read_scroll_bottom_loads_more_content_without_leaking_tabs tabs = send_command("tabs", start_if_needed=False) assert [tab["page_id"] for tab in tabs["data"]["tabs"]] == [existing_page_id] send_command("stop", start_if_needed=False) + + +@pytest.mark.skipif( + not _can_launch_playwright_browser(), reason="Playwright browser runtime unavailable" +) +def test_task_runtime_read_daemon_path_reports_fallback_metadata( + monkeypatch, tmp_path: Path +) -> None: + _configure_runtime(monkeypatch, tmp_path) + chrome_environment = ChromeEnvironment( + executable_path=None, + user_data_dir=tmp_path / "fallback-user-data", + profile_directory="Default", + source="fallback", + fallback_reason="Chrome profile appears to be in use.", + ) + (chrome_environment.user_data_dir / "Default").mkdir(parents=True) + + with run_fixture_server() as base_url: + payload = send_command( + "read-page", + { + "url": f"{base_url}/static", + "output_mode": "html", + "chrome_environment": _serialize_environment(chrome_environment), + }, + ) + assert payload["data"]["used_fallback_profile"] is True + assert payload["data"]["fallback_profile_dir"] == str(chrome_environment.user_data_dir) + assert payload["data"]["fallback_reason"] == "Chrome profile appears to be in use." + send_command("stop", start_if_needed=False) diff --git a/tests/unit/test_automation_api.py b/tests/unit/test_automation_api.py index 0940442..110a64a 100644 --- a/tests/unit/test_automation_api.py +++ b/tests/unit/test_automation_api.py @@ -6,6 +6,7 @@ from pathlib import Path from browser_cli.automation.api import AutomationHttpServer, AutomationRequestHandler +from browser_cli.automation.api.server import _payload_to_automation from browser_cli.automation.persistence import AutomationStore from browser_cli.automation.service.runtime import AutomationServiceRuntime @@ -105,3 +106,23 @@ def test_automation_api_returns_not_found_for_missing_run(tmp_path: Path) -> Non server.shutdown() server.server_close() thread.join(timeout=2.0) + + +def test_payload_to_automation_preserves_stdout_and_retry_backoff() -> None: + persisted = _payload_to_automation( + { + "id": "demo", + "name": "Demo", + "task_path": "/tmp/task.py", + "task_meta_path": "/tmp/task.meta.json", + "output_dir": "/tmp/out", + "stdout_mode": "text", + "retry_attempts": 1, + "retry_backoff_seconds": 9, + "timeout_seconds": 4.0, + } + ) + + assert persisted.stdout_mode == "text" + assert persisted.retry_backoff_seconds == 9 + assert persisted.timeout_seconds == 4.0 diff --git a/tests/unit/test_automation_commands.py b/tests/unit/test_automation_commands.py index 7208041..0f8e4b5 100644 --- a/tests/unit/test_automation_commands.py +++ b/tests/unit/test_automation_commands.py @@ -57,6 +57,40 @@ def test_automation_publish_reports_next_commands(monkeypatch, tmp_path: Path) - assert payload["data"]["next_commands"]["inspect"] == "browser-cli automation inspect demo" +def test_automation_publish_returns_manifest_source(monkeypatch, tmp_path: Path) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (task_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n', + encoding="utf-8", + ) + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + monkeypatch.setattr( + "browser_cli.commands.automation.ensure_automation_service_running", + lambda: None, + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: {"ok": True, "data": {"id": "demo"}}, + ) + payload = json.loads( + run_automation_command(Namespace(automation_subcommand="publish", path=str(task_dir))) + ) + assert payload["data"]["published"]["manifest_source"] == "task_dir" + + def test_automation_list_returns_service_items(monkeypatch) -> None: monkeypatch.setattr( "browser_cli.commands.automation.request_automation_service", @@ -83,7 +117,9 @@ def test_automation_versions_reads_snapshot_versions(monkeypatch, tmp_path: Path assert payload["data"]["versions"][0]["version"] == 3 -def test_automation_inspect_combines_service_and_snapshot_data(monkeypatch, tmp_path: Path) -> None: +def test_automation_inspect_without_version_returns_live_config( + monkeypatch, tmp_path: Path +) -> None: monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) version_dir = tmp_path / "home" / "automations" / "demo" / "versions" / "2" version_dir.mkdir(parents=True) @@ -131,10 +167,100 @@ def test_automation_inspect_combines_service_and_snapshot_data(monkeypatch, tmp_ Namespace(automation_subcommand="inspect", automation_id="demo", version=None) ) ) - assert payload["data"]["automation"]["id"] == "demo" - assert payload["data"]["automation"]["name"] == "Demo Snapshot" + assert payload["data"]["snapshot_config"] is None + assert payload["data"]["live_config"]["id"] == "demo" assert payload["data"]["versions"][0]["version"] == 2 - assert payload["data"]["latest_run"] is None + assert payload["data"]["latest_run"] == {"status": "success"} assert payload["data"]["summary"]["automation_id"] == "demo" - assert payload["data"]["summary"]["selected_version"] == 2 - assert payload["data"]["summary"]["latest_run_status"] is None + assert payload["data"]["summary"]["selected_version"] is None + assert payload["data"]["summary"]["latest_run_status"] == "success" + + +def test_automation_inspect_version_returns_snapshot_and_live_config( + monkeypatch, tmp_path: Path +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + version_dir = tmp_path / "home" / "automations" / "demo" / "versions" / "1" + version_dir.mkdir(parents=True) + (version_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (version_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (version_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo Snapshot"\n' + 'version = "1"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[outputs]\n" + 'stdout = "text"\n', + encoding="utf-8", + ) + (version_dir / "publish.json").write_text( + f'{{"automation_id":"demo","version":1,"source_task_path":"/tmp/task","snapshot_dir":"{version_dir}"}}', + encoding="utf-8", + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: { + "ok": True, + "data": { + "id": "demo", + "name": "Demo Live", + "version": "2", + "task_path": str(version_dir / "task.py"), + "task_meta_path": str(version_dir / "task.meta.json"), + "schedule_kind": "manual", + "schedule_payload": {"mode": "manual"}, + "stdout_mode": "json", + "latest_run": {"status": "success"}, + }, + }, + ) + + payload = json.loads( + run_automation_command( + Namespace(automation_subcommand="inspect", automation_id="demo", version=1) + ) + ) + + assert payload["data"]["snapshot_config"]["name"] == "Demo Snapshot" + assert payload["data"]["snapshot_config"]["stdout_mode"] == "text" + assert payload["data"]["live_config"]["name"] == "Demo Live" + assert payload["data"]["latest_run"] == {"status": "success"} + assert payload["data"]["summary"]["selected_version"] == 1 + + +def test_automation_inspect_version_reports_snapshot_config_error( + monkeypatch, tmp_path: Path +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + version_dir = tmp_path / "home" / "automations" / "demo" / "versions" / "1" + version_dir.mkdir(parents=True) + (version_dir / "automation.toml").write_text("[automation]\n", encoding="utf-8") + (version_dir / "publish.json").write_text( + f'{{"automation_id":"demo","version":1,"source_task_path":"/tmp/task","snapshot_dir":"{version_dir}"}}', + encoding="utf-8", + ) + monkeypatch.setattr( + "browser_cli.commands.automation.request_automation_service", + lambda method, path, body=None, start_if_needed=True: { + "ok": True, + "data": {"id": "demo", "name": "Demo Live", "latest_run": None}, + }, + ) + + payload = json.loads( + run_automation_command( + Namespace(automation_subcommand="inspect", automation_id="demo", version=1) + ) + ) + + assert payload["data"]["snapshot_config"] is None + assert "snapshot_config_error" in payload["data"] + assert payload["data"]["live_config"]["name"] == "Demo Live" diff --git a/tests/unit/test_automation_publish.py b/tests/unit/test_automation_publish.py index d1f5dac..98098eb 100644 --- a/tests/unit/test_automation_publish.py +++ b/tests/unit/test_automation_publish.py @@ -48,6 +48,72 @@ def test_publish_task_dir_creates_versioned_snapshot(tmp_path: Path, monkeypatch assert (published.snapshot_dir / "automation.toml").exists() +def test_publish_task_dir_preserves_source_manifest_fields(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + (task_dir / "automation.toml").write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[inputs]\n" + 'url = "https://example.com"\n' + "[schedule]\n" + 'mode = "manual"\n' + 'timezone = "Asia/Shanghai"\n' + "[outputs]\n" + 'artifact_dir = "artifacts"\n' + 'result_json_path = "artifacts/result.json"\n' + 'stdout = "text"\n' + "[runtime]\n" + "retry_attempts = 1\n" + "retry_backoff_seconds = 7\n", + encoding="utf-8", + ) + + published = publish_task_dir(task_dir, app_paths=get_app_paths()) + manifest = load_automation_manifest(published.manifest_path) + + assert published.manifest_source == "task_dir" + assert manifest.inputs == {"url": "https://example.com"} + assert manifest.schedule["timezone"] == "Asia/Shanghai" + assert manifest.outputs.result_json_path is not None + assert manifest.outputs.stdout == "text" + assert manifest.runtime.retry_backoff_seconds == 7 + + +def test_publish_task_dir_generates_defaults_when_manifest_is_absent( + tmp_path: Path, monkeypatch +) -> None: + monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + + published = publish_task_dir(task_dir, app_paths=get_app_paths()) + manifest = load_automation_manifest(published.manifest_path) + + assert published.manifest_source == "generated_defaults" + assert manifest.schedule["timezone"] == "UTC" + assert manifest.inputs == {} + + def test_publish_task_dir_accepts_douyin_example(monkeypatch, tmp_path: Path) -> None: monkeypatch.setenv("BROWSER_CLI_HOME", str(tmp_path / "home")) published = publish_task_dir( diff --git a/tests/unit/test_automation_service.py b/tests/unit/test_automation_service.py index 65b3ca0..847a27d 100644 --- a/tests/unit/test_automation_service.py +++ b/tests/unit/test_automation_service.py @@ -6,6 +6,7 @@ from browser_cli.automation.models import PersistedAutomationDefinition from browser_cli.automation.persistence import AutomationStore from browser_cli.automation.scheduler import compute_next_run_at, normalize_schedule +from browser_cli.automation.service.runtime import AutomationServiceRuntime REPO_ROOT = Path(__file__).resolve().parents[2] @@ -142,3 +143,86 @@ def test_douyin_automation_manifest_exists() -> None: payload = manifest_path.read_text(encoding="utf-8") assert "[automation]" in payload assert 'id = "douyin_video_download"' in payload + + +def test_automation_service_marks_run_failed_on_timeout(tmp_path: Path, monkeypatch) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "import time\ndef run(flow, inputs):\n time.sleep(0.3)\n return {'ok': True}\n", + encoding="utf-8", + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + store = AutomationStore(tmp_path / "automations.db") + store.upsert_automation( + PersistedAutomationDefinition( + id="demo", + name="Demo", + task_path=task_dir / "task.py", + task_meta_path=task_dir / "task.meta.json", + output_dir=tmp_path / "out", + timeout_seconds=0.05, + ) + ) + run = store.create_run("demo", trigger_type="manual") + runtime = AutomationServiceRuntime(store=store) + + class _FakeClient: + def command(self, action: str) -> dict: + assert action == "runtime-status" + return {} + + monkeypatch.setattr("browser_cli.automation.service.runtime.BrowserCliTaskClient", _FakeClient) + + runtime._execute_run(run.run_id) + updated = store.get_run(run.run_id) + events = store.list_run_events(run.run_id) + + assert updated.status == "failed" + assert updated.error_code == "AUTOMATION_RUN_TIMEOUT" + assert "timed out" in str(updated.error_message).lower() + assert any( + event.event_type == "run_timed_out" and event.payload.get("stage") == "task_execution" + for event in events + ) + + +def test_automation_service_does_not_retry_timeout_failure(tmp_path: Path, monkeypatch) -> None: + task_dir = tmp_path / "task" + task_dir.mkdir() + (task_dir / "task.py").write_text( + "import time\ndef run(flow, inputs):\n time.sleep(0.3)\n return {'ok': True}\n", + encoding="utf-8", + ) + (task_dir / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + store = AutomationStore(tmp_path / "automations.db") + store.upsert_automation( + PersistedAutomationDefinition( + id="demo", + name="Demo", + task_path=task_dir / "task.py", + task_meta_path=task_dir / "task.meta.json", + output_dir=tmp_path / "out", + timeout_seconds=0.05, + retry_attempts=3, + ) + ) + run = store.create_run("demo", trigger_type="manual") + runtime = AutomationServiceRuntime(store=store) + + class _FakeClient: + def command(self, action: str) -> dict: + assert action == "runtime-status" + return {} + + monkeypatch.setattr("browser_cli.automation.service.runtime.BrowserCliTaskClient", _FakeClient) + + runtime._execute_run(run.run_id) + runs = store.list_runs("demo", limit=10) + assert [item.status for item in runs] == ["failed"] diff --git a/tests/unit/test_task_runtime_automation.py b/tests/unit/test_task_runtime_automation.py index 4b552e4..8291e3d 100644 --- a/tests/unit/test_task_runtime_automation.py +++ b/tests/unit/test_task_runtime_automation.py @@ -6,6 +6,7 @@ from browser_cli.automation.hooks import run_hook_commands from browser_cli.automation.loader import load_automation_manifest +from browser_cli.automation.models import manifest_to_persisted_definition from browser_cli.task_runtime import parse_input_overrides from browser_cli.task_runtime.models import TaskMetadataError, validate_task_metadata @@ -44,6 +45,42 @@ def test_load_automation_manifest_resolves_douyin_example() -> None: assert manifest.task.meta_path.name == "task.meta.json" +def test_load_automation_manifest_preserves_retry_backoff_and_stdout(tmp_path: Path) -> None: + manifest_path = tmp_path / "automation.toml" + manifest_path.write_text( + "[automation]\n" + 'id = "demo"\n' + 'name = "Demo"\n' + "[task]\n" + 'path = "task.py"\n' + 'meta_path = "task.meta.json"\n' + "[outputs]\n" + 'stdout = "text"\n' + "[runtime]\n" + "retry_attempts = 2\n" + "retry_backoff_seconds = 7\n" + "timeout_seconds = 12.5\n", + encoding="utf-8", + ) + (tmp_path / "task.py").write_text( + "def run(flow, inputs):\n return {'ok': True}\n", encoding="utf-8" + ) + (tmp_path / "task.meta.json").write_text( + '{"task":{"id":"demo","name":"Demo","goal":"Run"},"environment":{},"success_path":{},"recovery_hints":{},"failures":[],"knowledge":{}}', + encoding="utf-8", + ) + + manifest = load_automation_manifest(manifest_path) + persisted = manifest_to_persisted_definition(manifest) + + assert manifest.outputs.stdout == "text" + assert manifest.runtime.retry_backoff_seconds == 7 + assert manifest.runtime.timeout_seconds == 12.5 + assert persisted.stdout_mode == "text" + assert persisted.retry_backoff_seconds == 7 + assert persisted.timeout_seconds == 12.5 + + def test_run_hook_commands_executes_shell_commands(tmp_path: Path) -> None: marker = tmp_path / "hook.txt" results = run_hook_commands((f'printf done > "{marker}"',), cwd=tmp_path) diff --git a/tests/unit/test_task_runtime_client_read.py b/tests/unit/test_task_runtime_client_read.py index 38c01c0..40b742b 100644 --- a/tests/unit/test_task_runtime_client_read.py +++ b/tests/unit/test_task_runtime_client_read.py @@ -1,5 +1,6 @@ from __future__ import annotations +import asyncio from pathlib import Path from unittest.mock import patch @@ -10,6 +11,7 @@ from browser_cli.task_runtime.client import BrowserCliTaskClient from browser_cli.task_runtime.flow import Flow from browser_cli.task_runtime.models import FlowContext +from browser_cli.task_runtime.read import ReadRequest, run_read_request def _chrome_environment(tmp_path: Path) -> ChromeEnvironment: @@ -89,6 +91,29 @@ def test_client_read_raises_empty_content_error() -> None: BrowserCliTaskClient().read("https://example.com") +def test_run_read_request_preserves_daemon_fallback_metadata() -> None: + payload = { + "ok": True, + "data": { + "body": "", + "used_fallback_profile": True, + "fallback_profile_dir": "/tmp/browser-cli/default-profile", + "fallback_reason": "Chrome profile appears to be in use.", + }, + } + with ( + patch("browser_cli.task_runtime.read.probe_socket", return_value=True), + patch("browser_cli.task_runtime.read.send_command", return_value=payload), + ): + result = asyncio.run( + run_read_request(ReadRequest(url="https://example.com", output_mode="html")) + ) + + assert result.used_fallback_profile is True + assert result.fallback_profile_dir == "/tmp/browser-cli/default-profile" + assert result.fallback_reason == "Chrome profile appears to be in use." + + def test_flow_read_delegates_to_client(tmp_path: Path) -> None: from browser_cli.task_runtime.read import ReadResult