Complete AgentTab credential and task lifecycle - #49
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Core RPC schema/tooling has inconsistencies (missing browser_credentials method/schema wiring and mismatched browser_finish response shape/docs counts) that will break compatibility and documentation correctness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR completes AgentTab’s lifecycle boundaries by adding policy-gated 1Password credential filling and an explicit task finalization (agenttab.finish / adapter browser_finish) that cleans up task-created tabs while preserving user-adopted tabs, with consistent deadline handling across transports and updated docs.
Changes:
- Added
browser_credentialsend-to-end (schemas, host broker, extension fill path, SDKs, MCP/OMP/Pi adapters) with origin-bound opaque tokens. - Added
agenttab.finish/browser_finishend-to-end (schemas, host/native protocol, SDKs, MCP/OMP adapters, installer CLI), including cleanup policy controls. - Updated extension state/model to track tab provenance and to expose cleanup policy + per-task “Keep/Finish” controls in the popup; updated documentation accordingly.
File summaries
| File | Description |
|---|---|
| schemas/rpc/v1/request.schema.json | Adds agenttab.finish request wiring (but needs browser_credentials schema integration fixes). |
| schemas/rpc/v1/browser-credentials.schema.json | New Core RPC schema for credential preparation/fill/next. |
| schemas/rpc/v1/agenttab-finish.schema.json | New Core RPC schema for task finalization parameters. |
| README.md | Updates product boundary and tool-surface documentation to include credentials + finish. |
| packages/sdk-typescript/test/client.test.ts | Adds transport-timeout coverage for browser_credentials and behavior tests for finishTask. |
| packages/sdk-typescript/src/index.ts | Adds browser_credentials types/timeout + finishTask() API for agenttab.finish. |
| packages/sdk-python/tests/test_client.py | Adds transport-timeout coverage and finish_task behavior test. |
| packages/sdk-python/agenttab/client.py | Adds default credentials timeout and finish_task() implementation. |
| packages/sdk-python/agenttab/init.py | Exposes DEFAULT_BROWSER_CREDENTIALS_TIMEOUT. |
| packages/omp/test/extension.test.ts | Expands tool-surface tests and adds browser_finish finalization/orphan-cleanup tests. |
| packages/omp/src/tool-method.ts | Adjusts tool-method typing to include adapter-only browser_finish. |
| packages/omp/src/render.ts | Adds rendering/summary for browser_credentials and browser_finish. |
| packages/omp/src/pi-schema.ts | Adds TypeBox schemas for browser_credentials and browser_finish. |
| packages/omp/src/index.ts | Registers new tools + adds session lifecycle hooks and orphan cleanup timer; implements browser_finish. |
| packages/mcp/test/server.test.ts | Updates Standard tool surface tests and adds browser_finish / browser_credentials assertions. |
| packages/mcp/src/server.ts | Adds browser_credentials and browser_finish tools and routes finish via SDK path. |
| packages/mcp/package.json | Updates package description (currently mismatched vs actual tool count). |
| packages/installer/src/cli.ts | Adds agenttab finish CLI command with disposition + keep-tab-ids parsing. |
| packages/extension/test/extension.test.ts | Adds coverage for debugger reattach recovery and finish/cleanup provenance behavior. |
| packages/extension/src/storage.ts | Adds cleanup policy + createdTabIds to durable state and parsing/validation. |
| packages/extension/src/protocol.ts | Adds native finish_task and browser_credentials_fill parameter validation/parsing. |
| packages/extension/src/popup.ts | Adds cleanup policy UI wiring and per-task “Keep” action. |
| packages/extension/src/popup.html | Adds cleanup policy selector to the popup UI. |
| packages/extension/src/ownership.ts | Implements finishTask() cleanup with provenance-aware tab retention/closure. |
| packages/extension/src/browser.ts | Adds credential field filling and robust debugger “detached session” recovery. |
| packages/extension/src/background.ts | Dispatches new native commands (finish_task, browser_credentials_fill) and popup messages for cleanup policy/keep. |
| host-rs/crates/agenttab-protocol/src/lib.rs | Adds protocol structs/enums for credentials + finish (Core + native), plus validation/tests. |
| host-rs/crates/agenttab-host/src/runtime.rs | Wires agenttab.finish handling and browser_credentials dispatch + origin gating. |
| host-rs/crates/agenttab-host/src/native.rs | Adds native finish_task transport request/response handling with consistency checks. |
| host-rs/crates/agenttab-host/src/lib.rs | Exposes new credentials module. |
| host-rs/crates/agenttab-host/src/journal.rs | Adjusts staged-commit schema migration ordering; adds regression test. |
| host-rs/crates/agenttab-host/src/guardrails.rs | Adds one_password policy parsing/validation + authorization gate for credentials. |
| host-rs/crates/agenttab-host/src/credentials.rs | Implements the 1Password CLI broker with bounded candidate/attempt semantics and tests. |
| docs/setup.md | Documents enabling/configuring the optional 1Password broker. |
| docs/security.md | Updates boundary + adds broker section (but tool count/list is currently inconsistent). |
| docs/roadmap.md | Updates roadmap gate language to include finish + broker. |
| docs/mcp.md | Updates MCP/OMP/Pi docs to reflect expanded Standard tool surface and credential flow. |
| docs/launch/show-hn.md | Updates launch draft messaging around credentials/boundary. |
| docs/launch/reddit-posts.md | Updates launch draft messaging around credentials/boundary. |
| docs/launch/directory-listings.md | Updates launch draft messaging around finish + credentials. |
| docs/launch/chrome-web-store.md | Updates store draft messaging around finish + credentials. |
| docs/commands.md | Updates MCP command docs to reflect expanded Standard tool surface. |
| docs/adr/0001-agenttab-runtime.md | Updates ADR to include browser_credentials + browser_finish in the Standard surface definition. |
| CHANGELOG.md | Records the new finish lifecycle and 1Password broker in Unreleased notes. |
Review details
Suppressed comments (2)
schemas/rpc/v1/request.schema.json:43
browser_credentialsis a Core RPC method (used by the SDK/MCP/host changes in this PR) but it is missing from the request schema's allowedmethodenum and from the idempotency-key-required method set. This makes valid credential requests fail schema validation and also fails to requireidempotency_keyfor a mutating method.
This issue also appears on line 64 of the same file.
schemas/rpc/v1/request.schema.json:82
- The request schema does not associate
browser_credentialswith its parameter schema (browser-credentials.schema.json). Without this conditional$ref,paramsremains an unconstrained object for that method, which defeats the purpose of the Core RPC schemas being normative.
- Files reviewed: 44/44 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete API/documentation mismatches (notably OMP browser_finish fallback result keys and inconsistent tool-surface counts) that should be corrected to avoid breaking consumers and shipping contradictory docs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
host-rs/crates/agenttab-host/src/runtime.rs:558
- When the connection has no bound task_id,
agenttab.finishcurrently returnsOutcome::Completedbutfinished: false. Elsewhere in this handler, outcome is derived fromresult.finished, so this branch is inconsistent and can confuse adapters/clients. If there is no task to finish, treating it as an already-finished no-op keeps outcome/result consistent and lets SDK clients close/clear state.
packages/mcp/package.json:5
- Package metadata says "Eight-tool MCP adapter" but Standard mode now exposes nine tools (including
browser_finish). This should be updated to avoid shipping inconsistent package descriptions.
"name": "agenttab-mcp",
"version": "2.0.0-rc.1",
"description": "Nine-tool MCP adapter for AgentTab",
"type": "module",
packages/omp/src/index.ts:517
browser_finishreturns a fallback result with non-existent keys (released_tab_ids,kept_tab_ids) instead of the Core/SDK result fields (retained_tab_ids). This will break consumers and makes the OMP tool output inconsistent with the rest of the stack.
- Files reviewed: 44/44 changed files
- Comments generated: 3
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The Core RPC request schema and Rust protocol schema assets are not fully aligned with the new browser_credentials surface, and the MCP adapter misses browser_credentials in its mutation/idempotency handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (5)
Previously missed (3) — in code that hasn't changed since the last review.
packages/omp/src/index.ts:271
- The OMP Zod schema for
browser_credentialsallowsfill/nextcalls with no*_reffields, but Core RPC and the JSON schema require at least one ofusername_ref,password_ref, orotp_ref. Tightening this prevents generating requests that will be rejected downstream.
packages/omp/src/pi-schema.ts:122 - The Pi/TypeBox schema for
browser_credentialsalso permitsfill/nextwith none of the*_reffields set, which conflicts with the Core RPC schema requirement that at least one ref is provided. Aligning adapter schemas avoids invalid tool calls reaching Core.
packages/sdk-typescript/src/index.ts:99 - The TypeScript SDK type for
BrowserCredentialsParamsallowsfill/nextwith nousername_ref/password_ref/otp_ref, but the Core RPC schema rejects that. Encoding the "at least one ref" rule in the type helps callers avoid compile-time-valid but runtime-invalid requests.
docs/launch/show-hn.md:30
- The copy says "eight-tool MCP surface", but with
browser_finishadded Standard mode is now nine tools. Keeping the count consistent avoids confusion in launch collateral.
I am preparing the v2 design for controlled review, not public use. I would eventually welcome feedback on the task-workspace boundary, the Your Turn blackout, the best-effort Commit model, and whether the nine-tool MCP surface is the right default. There is no stable install path or launch link in this draft.
docs/launch/reddit-posts.md:26
- The draft refers to an "eight-tool MCP default", but this PR's Standard surface is nine tools once
browser_finishis included. The count should match the current surface described elsewhere.
AgentTab `v2.0.0-rc.1` is unreleased and this is not a launch post. I am preserving the draft for later feedback on task ownership, the global handoff blackout, Commit's best-effort semantics, and the nine-tool MCP default.
- Files reviewed: 44/44 changed files
- Comments generated: 6
- Review effort level: Lite
Comment on lines
+51
to
55
| ( | ||
| "agenttab_finish", | ||
| include_str!("../../../../schemas/rpc/v1/agenttab-finish.schema.json"), | ||
| ), | ||
| ( |
Comment on lines
37
to
+41
| "browser_commit", | ||
| "browser_developer", | ||
| ]); | ||
| const UUID_V7 = /^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
| type ToolMethod = RpcMethod | "browser_finish"; |
Comment on lines
22
to
+26
| "browser_handoff", | ||
| "browser_commit", | ||
| "browser_developer", | ||
| "agenttab.status", | ||
| "agenttab.finish", |
Comment on lines
+79
to
+82
| { | ||
| "if": { "properties": { "method": { "const": "agenttab.finish" } }, "required": ["method"] }, | ||
| "then": { "properties": { "params": { "$ref": "agenttab-finish.schema.json" } } } | ||
| }, |
There was a problem hiding this comment.
🟡 Changes recommended
The Core RPC request schema and embedded schema assets are missing browser_credentials, and agenttab.finish returns an inconsistent outcome when no task is bound, which can break validation and adapter behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
host-rs/crates/agenttab-host/src/runtime.rs:570
- When no task is bound to the connection,
agenttab.finishreturnsOutcome::Completedwithfinished: false. This is internally inconsistent (nothing was finished) and makes it hard for adapters/SDK users to distinguish "no task started" from a real deferred cleanup. ReturningOutcome::NotStarted(and optionally a small sentinel likedeferred: "not_started") better matches the existing outcome vocabulary in response.schema.json.
schemas/rpc/v1/request.schema.json:82
- The request schema has a params mapping for
agenttab.finish, but there is no corresponding mapping for the newbrowser_credentialsmethod. Without it,paramswill remain an unvalidated generic object for credential requests, which defeats the purpose of the per-method schemas.
host-rs/crates/agenttab-protocol/src/lib.rs:54 RPC_SCHEMA_ASSETSembeds the newagenttab_finishschema, but it does not embed the newbrowser-credentials.schema.json. Consumers relying on the embedded schema bundle will be missing the credential params schema.
(
"agenttab_finish",
include_str!("../../../../schemas/rpc/v1/agenttab-finish.schema.json"),
),
- Files reviewed: 44/44 changed files
- Comments generated: 3
- Review effort level: Lite
Comment on lines
37
to
+41
| "browser_commit", | ||
| "browser_developer", | ||
| ]); | ||
| const UUID_V7 = /^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
| type ToolMethod = RpcMethod | "browser_finish"; |
Comment on lines
525
to
530
| const response = await invocationClient.request( | ||
| definition.name, | ||
| params as MethodParams[ToolMethod], | ||
| params as MethodParams[Exclude<ToolMethod, "browser_finish">], | ||
| isMutation(definition.name) | ||
| ? { idempotencyKey: invocationKeys.for(invocationId) } | ||
| : {}, |
Comment on lines
23
to
27
| "browser_commit", | ||
| "browser_developer", | ||
| "agenttab.status", | ||
| "agenttab.finish", | ||
| "agenttab.close" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
AgentTab needed a complete lifecycle boundary for credential-assisted browser work. Credentials must remain origin-bound and opaque to extensions and clients, while completed or abandoned tasks must release ownership without closing tabs the user adopted.
What changed
agenttab.finishacross schemas, native protocol, SDKs, MCP, and OMPValidation