diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md index c74e459a06f..378c9b68799 100644 --- a/.agents/skills/add-block/SKILL.md +++ b/.agents/skills/add-block/SKILL.md @@ -52,6 +52,12 @@ export const {ServiceName}Block: BlockConfig = { // Auth mode authMode: AuthMode.OAuth, // or AuthMode.ApiKey + // Card summary sentences — see "Canvas Sentences" below + canvasPresentation: { + defaultTitle: '{Default Operation}', + sentences: { byOperation: { /* one per operation dropdown option id */ } }, + }, + subBlocks: [ // Define all UI fields here ], @@ -945,6 +951,35 @@ Derive templates from the service's real use cases. Each prompt should name a co - **Ground every skill in operations the block actually exposes** — cross-check each skill's steps against `tools.access`. Never describe an action the integration cannot perform. - **Derive skills from real, popular use cases found online — never invent them.** Web-search the service's documented use cases (vendor use-case/solutions pages, official docs describing the workflow, reputable "top automations for X" articles) and only add a skill you can source as something people genuinely do with the service. Do not hallucinate skills. +## Canvas Sentences + +Every block declares a one-line prose summary that replaces its card's field rows: + +``` +Slack ← header (already names the block) +Posts ⟨Ship it 🚀⟩ to ⟨#eng⟩ ← the sentence; ⟨…⟩ are live value chips +``` + +Write one `byOperation` entry per operation dropdown option (or a single `default` +when the block has no operation dropdown). + +**The full authoring contract — voice, structure, and the two mistakes that break +cards silently — is `apps/sim/blocks/AGENTS.md` → "Canvas sentences". Read it +before writing any.** The two failures worth repeating here, because both are +invisible at runtime: + +1. A clause naming only one member of a `canonicalParamId` pair drops the sentence + for every advanced-mode user. List all members: + `field: ['channelSelector', 'manualChannel']`. +2. A clause referencing a subblock whose `condition` excludes that operation can + never render. + +Validate before finishing: + +```bash +bun run apps/sim/scripts/check-canvas-sentences.ts --block={service} +``` + ## Generated artifacts Adding a block on its own needs no **tool metadata** regeneration — a block references existing @@ -963,7 +998,6 @@ bun run integration-catalog:check The catalog check independently derives deployment metadata from the executable block registry and compares it with the committed `apps/sim/lib/integrations/integrations.json`. Review the generated diff and keep only intentional changes. - ## Checklist Before Finishing - [ ] `integrationType` is set to the correct `IntegrationType` enum value @@ -991,6 +1025,7 @@ diff and keep only intentional changes. - [ ] Exported `{Service}BlockMeta` with at least 7 templates - [ ] `url` set on `{Service}BlockMeta` to the external service's verified homepage (omit only for first-party blocks with no external service) - [ ] `skills` added to `{Service}BlockMeta`, each grounded in `tools.access` and sourced from a real online use case (not invented) +- [ ] `canvasPresentation.sentences` covers every operation, and `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` passes with 100% coverage ## Final Validation (Required) diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index e72dd878bbe..3369828d6f2 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -768,9 +768,11 @@ tools: { } ``` -#### 3. Create Internal API Route +#### 3. Create Special Internal Tool Execution Route -Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. +Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. This raw route pattern is only for an integration's provider-execution boundary when it needs special file normalization, large-body handling, or protocol behavior. It is not the pattern for CRUD or other operations on protected Sim resources. For those, use the `migrate-application-operation` skill and an authorized application use case with the ordinary internal/v2 route builders. + +Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. Authenticate and perform cheap admission before parsing or downloading files. ```typescript // apps/sim/lib/api/contracts/tools/{service}.ts diff --git a/.agents/skills/migrate-application-operation/SKILL.md b/.agents/skills/migrate-application-operation/SKILL.md new file mode 100644 index 00000000000..e7f0039e84b --- /dev/null +++ b/.agents/skills/migrate-application-operation/SKILL.md @@ -0,0 +1,304 @@ +--- +name: migrate-application-operation +description: Create or migrate a protected Sim resource operation in the shared Principal and application-use-case architecture across internal APIs, public or versioned APIs, Copilot, and other trusted tool adapters. Use when adding a protected endpoint, tool command, or CRUD method; removing route- or tool-local authorization and business logic; consolidating resource reads or writes behind semantic operation policies; or adding another surface to an existing application operation while preserving contracts, identity, errors, rate limits, audit, analytics, and compatibility behavior. Treat v1, uploads, streams, large bodies, bulk recursion, and polymorphic tools as explicitly scoped special cases. +--- + +# Create Or Migrate Application Operation + +Create or migrate one bounded semantic operation at a time. Share authorization and business behavior without forcing internal APIs, public APIs, Copilot, and other tools to share authentication, input schemas, or response shapes. + +## Enforce the application boundary + +Apply this invariant: + +> Every real operation on persisted or protected data enters through an authorized application use case. + +This includes mutations, content and metadata reads, canonical resource lookup, and reference-to-resource resolution when the lookup is authorization-sensitive. + +Surface helpers may: + +- Normalize an already-authenticated surface context into a `Principal`. +- Translate aliases or wire arguments into application input. +- Select a code-defined operation and application use case. +- Call the application use case. +- Translate typed results and errors into the surface contract. + +Surface helpers must not: + +- Query databases or storage. +- Decide workspace or resource authorization. +- Implement business transactions. +- Record semantic audit or shared domain notifications. +- Infer authoritative identity, workspace, audience, or scope from untrusted arguments. +- Substitute billing attribution for identity. + +A helper that resolves a path is valid only when the actual protected lookup runs through an authorized application resolver. If a helper begins doing real data work, move that work into an application use case. + +## Read the foundation first + +Read these files completely before editing: + +- `packages/auth/src/principal.ts` +- `apps/sim/lib/core/application/operation.ts` +- `apps/sim/lib/core/application/workspace-operation.ts` +- `apps/sim/lib/core/application/workspace-authorization.ts` +- `apps/sim/lib/core/application/authorized-workspace-use-case.ts` +- `apps/sim/lib/api/server/routes/definition.ts` +- `apps/sim/lib/api/server/routes/internal-json-route.ts` +- `apps/sim/lib/api/server/routes/v2-json-route.ts` +- `apps/sim/lib/auth/internal-delegation.ts` +- `apps/sim/lib/copilot/application/application-adapter.ts` +- `apps/sim/lib/copilot/auth/application-delegation.ts` + +Use the file domain only as a representative golden slice: + +- `apps/sim/lib/workspace-files/application/operations.ts` +- `apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts` +- `apps/sim/lib/workspace-files/application/rename-workspace-file.ts` +- `apps/sim/lib/copilot/application/execute-file-use-case.ts` +- `apps/sim/lib/copilot/auth/file-delegation.ts` + +Then read the target domain's operation registry, application code, repositories, contracts, adapters, aliases, resume paths, and focused tests. Fail immediately if the shared foundation is absent. Do not recreate it inside the domain. + +## Bound the migration + +Inventory every entry point for the behavior before editing: + +- Internal HTTP routes and contracts. +- Public or versioned API routes and contracts. +- Copilot tools, aliases, resume paths, and polymorphic branches. +- Other tool servers, workflow executors, jobs, or service callers. +- Current authentication, authorization, workspace assertions, and concealment. +- Manager or orchestration call chains. +- Audit, notification, analytics, and billing side effects. +- Error/status/result behavior. +- Rate-limit identity, rollout gates, quota, and concurrency admission. + +Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent operations merely because they share a module. Do not modify v1 unless the request explicitly includes it. + +Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. + +## Keep the layers distinct + +Use these responsibilities: + +1. Authentication adapter: verify the surface credential or trusted execution context and construct a `Principal`. +2. Route or tool adapter: select rate policy, parse its contract, translate input, call the application use case, and render its own result. +3. Application use case: load canonical context, compare asserted scope, authorize the semantic operation, execute business behavior, project semantic audit, and trigger shared domain effects. +4. Manager or repository: perform database and storage reads or writes using canonical identifiers and scope. Never accept credentials or principals. +5. Presenter: return only the surface success body or typed binary descriptor. Never construct auth, rate, or error behavior. + +For ordinary public JSON routes, preserve this order: + +```text +IP abuse limit + -> authenticate + -> build Principal + -> operation rate limit + -> parse surface contract + -> application use case + -> canonical load + -> asserted-scope concealment + -> current authorization + -> manager read or mutation + -> semantic audit + -> shared domain effects + -> surface presenter +``` + +Internal routes may omit the IP bucket or operation limit only through an explicit policy with a reason. Usage billing, storage quota, cost admission, and concurrency are separate from request-rate limiting. + +Never query API keys or sessions from the application layer. Never add fallback identity or authorization behavior. Propagate infrastructure failures instead of turning them into not-found or forbidden results. + +## Define the semantic operation once + +Add one stable entry to the target domain's operation registry: + +```ts +rename: defineWorkspaceOperation({ + id: 'widgets.rename', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ['session', 'personal_api_key', 'workspace_api_key', 'delegated'], + delegatedServices: ['copilot'], +}) +``` + +Do not create internal-, public-, or Copilot-specific versions of the same semantic operation. If two callers have materially different business or transactional semantics, define separate semantic operations and use cases and explain the distinction. + +Choose principal kinds from actual behavior. Do not accept every principal merely because the use case is shared. Workspace API keys have a write ceiling and cannot satisfy admin operations. The operation definition must fail fast when its role, workspace-key policy, and principal kinds disagree. + +Route declarations, tool adapters, and use cases must use the same literal operation. Runtime operation selection is permitted only from a trusted, code-defined registry. Never accept an operation ID or permission tag from an HTTP body, model argument, or other untrusted input. + +## Implement the application use case + +Use `defineAuthorizedWorkspaceUseCase` directly or a thin domain binding that supplies domain-specific authorization options: + +```ts +export const renameWidget = defineAuthorizedWorkspaceUseCase({ + operation: widgetOperations.rename, + resolveContext: ({ input }: { input: RenameWidgetInput }) => + loadCanonicalWidgetContext(input.id, input.assertedWorkspaceId), + authorizationOptions: { delegation: widgetDelegationPolicy }, + execute: async ({ input, context }) => renameWidgetRecord({ + workspaceId: context.workspaceId, + widgetId: context.resourceId, + name: input.name, + }), + projectAudit: ({ result }) => ({ + action: AuditAction.WIDGET_UPDATED, + resourceType: AuditResourceType.WIDGET, + resourceId: result.id, + resourceName: result.name, + }), + afterSuccess: ({ context }) => notifyWidgetsChanged(context.workspaceId), +}) +``` + +Adapt the example to the domain's real authorization options; do not copy invented field names. + +The wrapper must own this lifecycle: + +1. Reject disallowed principal kinds before protected loading. +2. Load canonical context and conceal asserted-scope mismatches as required. +3. Authorize the operation using current policy state. +4. Execute the manager or repository primitive. +5. Project audit from authoritative results. +6. Await shared post-success effects. + +Do not call shared authorization, principal audit attribution, or `recordAudit` manually from an ordinary migrated use-case body. Use `projectAudit` only when the operation has semantic audit. Return no audit entries for authoritative no-ops. Keep product analytics such as `captureServerEvent` surface-specific through the adapter's success hook. + +Inspect legacy orchestration before reusing it. If it already authorizes, audits, notifies, or captures analytics, call a lower-level primitive or remove duplicate responsibility for migrated callers. + +Application code must remain surface-neutral. It must not import `app/api/**`, `next/server`, internal/v1/v2 contracts or presenters, or Copilot tool handlers. Return domain values and let each surface presenter project its own wire result. + +## Adapt internal APIs + +Use `defineInternalJsonRoute` for ordinary JSON routes. Explicitly declare the contract, authentication policy, semantic operation, rate policy, error policy, input mapping, use case, and presenter when the wire result differs. + +Use `internalSessionAuth` for session-only routes. Use `createInternalSessionOrExecutorAuth` only when the endpoint genuinely supports signed executor delegation; the semantic operation must then allow `delegated` principals from the `executor` service. Never turn an actorless legacy JWT into a fake session, owner, or user principal. + +The internal adapter owns authentication and internal response envelopes. It must not implement workspace authorization. Preserve internal-only analytics through `onSuccess` after application success. + +Keep the route module declarative. If several internal routes repeat authentication, parsing, error rendering, or response construction, improve the shared internal route builder instead of adding a domain-specific route wrapper. + +## Adapt public or versioned APIs + +Use the appropriate public/versioned route builder, such as `defineV2JsonRoute`, with API-key authentication, explicit semantic operation and rate policy, external error projection, input mapping, application use case, and an external presenter. V2 rollout admission is centralized by the builder; do not invent a route-local rollout policy. + +Authentication and HTTP formatting may differ from internal APIs; authorization and business behavior must not. Rate-limit using the credential or principal subject, never a billed owner. Resolve billing attribution only for billing, quota, or legacy required-user fields. + +Keep surface contracts separate when their wire shapes differ. Reuse shared primitive schemas and domain validators for invariants such as IDs, names, bounds, and formats. Do not maintain duplicate internal and external schemas merely because the routes are separate; import the same schema when the wire shape is genuinely identical. Never cast one surface response into another. + +Keep v1 middleware and routes unchanged unless explicitly included. + +## Adapt Copilot + +Copilot is a surface adapter, not a separate application layer. If an HTTP or other surface already uses an application use case, Copilot must call that exact use case rather than reimplementing protected business behavior under `lib/copilot`. + +Create one domain-level Copilot application adapter with `createCopilotApplicationAdapter` instead of constructing delegated principals in every tool: + +```ts +executeCopilotWidgetUseCase(context, renameWidget, input, { resourceId }) +``` + +That adapter must: + +- Require a trusted server-authored Copilot execution marker. +- Require the authenticated subject, canonical workspace, tool-call or execution identity, and required audience or lifecycle scope. +- Construct the shared delegated `Principal` in one place. +- Optionally bind the canonical resource scope after trusted resolution. +- Verify that the use case exposes a registered code-defined operation. +- Use the domain's exact immutable operation registry so operation-object membership and identity are checked centrally. +- Call the application use case directly. + +Never construct authoritative delegation from model-provided workspace IDs, user IDs, operation IDs, resource scope, or permission tags. Model arguments are requested targets only and must be checked against trusted execution context and canonical data. + +Tool handlers own argument aliases, resumable legacy names, abort checks, tool-call reporting, and tool-specific presentation. They must not query managers directly for protected operations, manually authorize, or implement protected business behavior. If a Copilot-only compound action expresses real domain behavior, define a surface-neutral domain operation and application use case for it. + +A Copilot reference helper may translate a path to a resource only by calling an authorized application resolver under the intended semantic operation. Passing a code-defined operation object is acceptable; passing a model-provided operation string is not. An immediate same-request resolver followed by the operation may reuse one trusted principal, though the application operation still performs its own canonical authorization. Fresh authentication and authorization are required across lifecycle boundaries such as resumed tool calls, executor callbacks, queued or background work, upload control legs and finalization, durable completion, and long-running provider operations. When resolution and execution form one business operation, need a consistent snapshot, or appear repeatedly together, prefer a top-level application use case such as `renameWidgetByReference`. + +Surface adapters must not compose protected mutations. An atomic compound action requires one top-level semantic domain operation and application use case that owns the transaction and authoritative result. An explicitly best-effort application command may coordinate multiple operations only when it defines hard input and expansion caps, cancellation checkpoints, partial-result semantics, audit behavior, and rate/quota policy. Keep composition exceptional and explicit; ordinary tools should use the shared execution adapter. + +Map expected typed errors to safe tool results. Unknown errors must become generic system/retryable messages while retaining full causes in server logs. Never return raw database or storage errors to the model. + +## Adapt other internal or external tools + +Treat every tool runtime as a surface adapter: + +- Normalize its already-authenticated execution context into an existing `Principal` through one shared adapter for that runtime or domain. +- Call the same application use case used by HTTP and Copilot surfaces. +- Preserve the tool protocol's input, output, retry, and cancellation semantics. +- Keep authoritative workspace and subject scope server-authored. + +An internal caller is not automatically trusted to bypass authorization. It must supply an explicit principal or use a deliberately designed service/delegation principal. If the current principal model cannot express its authority, stop and extend the identity model intentionally; do not fall back to an owner, uploader, creator, or arbitrary user ID. + +External tool endpoints authenticate at their adapter exactly like public APIs. Do not authenticate again inside the application use case. + +## Preserve identity and attribution + +- Session and personal-key principals authorize through current human workspace permission. +- Personal API keys also respect the workspace's personal-key policy. +- Workspace keys authorize as the workspace under explicit operation policy and the write ceiling, independent of creator membership. +- Delegated principals re-check the current subject and their workspace, audience, expiry, execution, and resource scope. +- Billing owners are attribution for billing or legacy required columns only, never authorization, rate identity, delegated identity, audit actor, or human analytics identity. +- Preserve structured `PrincipalActor` metadata in semantic audit. + +If a required legacy user column cannot represent the real actor, label the compatibility attribution explicitly. Never pretend it is the acting human. + +## Handle special operations explicitly + +Do not force these through an ordinary JSON migration: + +- Upload or multipart lifecycles: bind immutable credential identity, reauthorize control legs and finalization, and make durable completion idempotent. +- Large bodies: authenticate and perform cheap admission before bounded buffering. +- Binary or streaming responses: use binary/stream builders and typed descriptors. +- Bulk or recursive operations: deduplicate and cap inputs and expansion, load all resources canonically, and define atomic versus best-effort behavior. +- Polymorphic tools: select the semantic operation only after trusted target-kind resolution; do not route unrelated branches through one domain registry. +- Multi-resource transactions: keep canonical scope predicates and derive audit from authoritative affected rows. + +Stop and report a missing design rather than weakening identity, authorization, limits, or errors. + +## Test the complete matrix + +Add focused tests for every migrated surface and principal kind allowed by the operation: + +- Application: allowed and disallowed roles, principal-kind rejection before canonical loading, workspace assertion mismatch, delegated scope, not found, conflict, no-op, and infrastructure propagation. +- Operation registry: role/workspace-key/principal-kind/delegated-service consistency and fail-fast rejection of invalid definitions. +- Repository: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. +- Internal API: authentication before parsing, exact contract, typed errors, and surface analytics only after success. +- Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. +- Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. +- Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. + +Run at minimum: + +```bash +bunx vitest run +bunx biome check +bunx turbo run type-check --filter=sim --filter=@sim/auth +bun run check:api-validation:strict +git diff --check +``` + +Do not claim a check passed unless it completed successfully. + +## Work safely in parallel + +- Assign non-overlapping route modules and caller sets. Two methods in one route file are one ownership unit. +- Treat operation registries, contract families, route policies, and shared surface adapters as merge hotspots. +- Keep shared core foundations owned by one task; ordinary domain migrations should consume them without modifying them. +- Preserve unrelated working-tree changes. Never stage proposal docs, lockfile drift, or another agent's edits. +- Do not commit, push, or open a PR unless requested. + +## Hand off + +Report: + +1. Semantic operation, role, workspace-key policy, and principal kinds. +2. Migrated, deferred, and non-goal entry points. +3. Behavior preserved per internal, public, Copilot, and other tool surface. +4. Identity construction and authoritative scope source for each surface. +5. Files changed and shared merge hotspots. +6. Tests and checks run with results. +7. Remaining risks or blockers. Fail fast when an invariant could not be implemented. diff --git a/.agents/skills/migrate-application-operation/agents/openai.yaml b/.agents/skills/migrate-application-operation/agents/openai.yaml new file mode 100644 index 00000000000..2efab38a8af --- /dev/null +++ b/.agents/skills/migrate-application-operation/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Create Or Migrate Application Operation" + short_description: "Share one protected operation across surfaces" + default_prompt: "Use $migrate-application-operation to create or migrate one protected resource operation across internal APIs, public APIs, Copilot, and other tools." diff --git a/.claude/commands/add-block.md b/.claude/commands/add-block.md index 57df2b4dd1b..1f9a8554af8 100644 --- a/.claude/commands/add-block.md +++ b/.claude/commands/add-block.md @@ -51,6 +51,12 @@ export const {ServiceName}Block: BlockConfig = { // Auth mode authMode: AuthMode.OAuth, // or AuthMode.ApiKey + // Card summary sentences — see "Canvas Sentences" below + canvasPresentation: { + defaultTitle: '{Default Operation}', + sentences: { byOperation: { /* one per operation dropdown option id */ } }, + }, + subBlocks: [ // Define all UI fields here ], @@ -944,6 +950,35 @@ Derive templates from the service's real use cases. Each prompt should name a co - **Ground every skill in operations the block actually exposes** — cross-check each skill's steps against `tools.access`. Never describe an action the integration cannot perform. - **Derive skills from real, popular use cases found online — never invent them.** Web-search the service's documented use cases (vendor use-case/solutions pages, official docs describing the workflow, reputable "top automations for X" articles) and only add a skill you can source as something people genuinely do with the service. Do not hallucinate skills. +## Canvas Sentences + +Every block declares a one-line prose summary that replaces its card's field rows: + +``` +Slack ← header (already names the block) +Posts ⟨Ship it 🚀⟩ to ⟨#eng⟩ ← the sentence; ⟨…⟩ are live value chips +``` + +Write one `byOperation` entry per operation dropdown option (or a single `default` +when the block has no operation dropdown). + +**The full authoring contract — voice, structure, and the two mistakes that break +cards silently — is `apps/sim/blocks/AGENTS.md` → "Canvas sentences". Read it +before writing any.** The two failures worth repeating here, because both are +invisible at runtime: + +1. A clause naming only one member of a `canonicalParamId` pair drops the sentence + for every advanced-mode user. List all members: + `field: ['channelSelector', 'manualChannel']`. +2. A clause referencing a subblock whose `condition` excludes that operation can + never render. + +Validate before finishing: + +```bash +bun run apps/sim/scripts/check-canvas-sentences.ts --block={service} +``` + ## Generated artifacts Adding a block on its own needs no **tool metadata** regeneration — a block references existing @@ -962,7 +997,6 @@ bun run integration-catalog:check The catalog check independently derives deployment metadata from the executable block registry and compares it with the committed `apps/sim/lib/integrations/integrations.json`. Review the generated diff and keep only intentional changes. - ## Checklist Before Finishing - [ ] `integrationType` is set to the correct `IntegrationType` enum value @@ -990,6 +1024,7 @@ diff and keep only intentional changes. - [ ] Exported `{Service}BlockMeta` with at least 7 templates - [ ] `url` set on `{Service}BlockMeta` to the external service's verified homepage (omit only for first-party blocks with no external service) - [ ] `skills` added to `{Service}BlockMeta`, each grounded in `tools.access` and sourced from a real online use case (not invented) +- [ ] `canvasPresentation.sentences` covers every operation, and `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` passes with 100% coverage ## Final Validation (Required) diff --git a/.claude/commands/add-integration.md b/.claude/commands/add-integration.md index 864dc9ab9b3..08d8fc92f08 100644 --- a/.claude/commands/add-integration.md +++ b/.claude/commands/add-integration.md @@ -767,9 +767,11 @@ tools: { } ``` -#### 3. Create Internal API Route +#### 3. Create Special Internal Tool Execution Route -Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. +Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. This raw route pattern is only for an integration's provider-execution boundary when it needs special file normalization, large-body handling, or protocol behavior. It is not the pattern for CRUD or other operations on protected Sim resources. For those, use the `migrate-application-operation` skill and an authorized application use case with the ordinary internal/v2 route builders. + +Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. Authenticate and perform cheap admission before parsing or downloading files. ```typescript // apps/sim/lib/api/contracts/tools/{service}.ts diff --git a/.claude/commands/migrate-application-operation.md b/.claude/commands/migrate-application-operation.md new file mode 100644 index 00000000000..bc61333c358 --- /dev/null +++ b/.claude/commands/migrate-application-operation.md @@ -0,0 +1,303 @@ +--- +description: Create or migrate a protected Sim resource operation in the shared Principal and application-use-case architecture across internal APIs, public or versioned APIs, Copilot, and other trusted tool adapters. Use when adding a protected endpoint, tool command, or CRUD method; removing route- or tool-local authorization and business logic; consolidating resource reads or writes behind semantic operation policies; or adding another surface to an existing application operation while preserving contracts, identity, errors, rate limits, audit, analytics, and compatibility behavior. Treat v1, uploads, streams, large bodies, bulk recursion, and polymorphic tools as explicitly scoped special cases. +--- + +# Create Or Migrate Application Operation + +Create or migrate one bounded semantic operation at a time. Share authorization and business behavior without forcing internal APIs, public APIs, Copilot, and other tools to share authentication, input schemas, or response shapes. + +## Enforce the application boundary + +Apply this invariant: + +> Every real operation on persisted or protected data enters through an authorized application use case. + +This includes mutations, content and metadata reads, canonical resource lookup, and reference-to-resource resolution when the lookup is authorization-sensitive. + +Surface helpers may: + +- Normalize an already-authenticated surface context into a `Principal`. +- Translate aliases or wire arguments into application input. +- Select a code-defined operation and application use case. +- Call the application use case. +- Translate typed results and errors into the surface contract. + +Surface helpers must not: + +- Query databases or storage. +- Decide workspace or resource authorization. +- Implement business transactions. +- Record semantic audit or shared domain notifications. +- Infer authoritative identity, workspace, audience, or scope from untrusted arguments. +- Substitute billing attribution for identity. + +A helper that resolves a path is valid only when the actual protected lookup runs through an authorized application resolver. If a helper begins doing real data work, move that work into an application use case. + +## Read the foundation first + +Read these files completely before editing: + +- `packages/auth/src/principal.ts` +- `apps/sim/lib/core/application/operation.ts` +- `apps/sim/lib/core/application/workspace-operation.ts` +- `apps/sim/lib/core/application/workspace-authorization.ts` +- `apps/sim/lib/core/application/authorized-workspace-use-case.ts` +- `apps/sim/lib/api/server/routes/definition.ts` +- `apps/sim/lib/api/server/routes/internal-json-route.ts` +- `apps/sim/lib/api/server/routes/v2-json-route.ts` +- `apps/sim/lib/auth/internal-delegation.ts` +- `apps/sim/lib/copilot/application/application-adapter.ts` +- `apps/sim/lib/copilot/auth/application-delegation.ts` + +Use the file domain only as a representative golden slice: + +- `apps/sim/lib/workspace-files/application/operations.ts` +- `apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts` +- `apps/sim/lib/workspace-files/application/rename-workspace-file.ts` +- `apps/sim/lib/copilot/application/execute-file-use-case.ts` +- `apps/sim/lib/copilot/auth/file-delegation.ts` + +Then read the target domain's operation registry, application code, repositories, contracts, adapters, aliases, resume paths, and focused tests. Fail immediately if the shared foundation is absent. Do not recreate it inside the domain. + +## Bound the migration + +Inventory every entry point for the behavior before editing: + +- Internal HTTP routes and contracts. +- Public or versioned API routes and contracts. +- Copilot tools, aliases, resume paths, and polymorphic branches. +- Other tool servers, workflow executors, jobs, or service callers. +- Current authentication, authorization, workspace assertions, and concealment. +- Manager or orchestration call chains. +- Audit, notification, analytics, and billing side effects. +- Error/status/result behavior. +- Rate-limit identity, rollout gates, quota, and concurrency admission. + +Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent operations merely because they share a module. Do not modify v1 unless the request explicitly includes it. + +Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. + +## Keep the layers distinct + +Use these responsibilities: + +1. Authentication adapter: verify the surface credential or trusted execution context and construct a `Principal`. +2. Route or tool adapter: select rate policy, parse its contract, translate input, call the application use case, and render its own result. +3. Application use case: load canonical context, compare asserted scope, authorize the semantic operation, execute business behavior, project semantic audit, and trigger shared domain effects. +4. Manager or repository: perform database and storage reads or writes using canonical identifiers and scope. Never accept credentials or principals. +5. Presenter: return only the surface success body or typed binary descriptor. Never construct auth, rate, or error behavior. + +For ordinary public JSON routes, preserve this order: + +```text +IP abuse limit + -> authenticate + -> build Principal + -> operation rate limit + -> parse surface contract + -> application use case + -> canonical load + -> asserted-scope concealment + -> current authorization + -> manager read or mutation + -> semantic audit + -> shared domain effects + -> surface presenter +``` + +Internal routes may omit the IP bucket or operation limit only through an explicit policy with a reason. Usage billing, storage quota, cost admission, and concurrency are separate from request-rate limiting. + +Never query API keys or sessions from the application layer. Never add fallback identity or authorization behavior. Propagate infrastructure failures instead of turning them into not-found or forbidden results. + +## Define the semantic operation once + +Add one stable entry to the target domain's operation registry: + +```ts +rename: defineWorkspaceOperation({ + id: 'widgets.rename', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ['session', 'personal_api_key', 'workspace_api_key', 'delegated'], + delegatedServices: ['copilot'], +}) +``` + +Do not create internal-, public-, or Copilot-specific versions of the same semantic operation. If two callers have materially different business or transactional semantics, define separate semantic operations and use cases and explain the distinction. + +Choose principal kinds from actual behavior. Do not accept every principal merely because the use case is shared. Workspace API keys have a write ceiling and cannot satisfy admin operations. The operation definition must fail fast when its role, workspace-key policy, and principal kinds disagree. + +Route declarations, tool adapters, and use cases must use the same literal operation. Runtime operation selection is permitted only from a trusted, code-defined registry. Never accept an operation ID or permission tag from an HTTP body, model argument, or other untrusted input. + +## Implement the application use case + +Use `defineAuthorizedWorkspaceUseCase` directly or a thin domain binding that supplies domain-specific authorization options: + +```ts +export const renameWidget = defineAuthorizedWorkspaceUseCase({ + operation: widgetOperations.rename, + resolveContext: ({ input }: { input: RenameWidgetInput }) => + loadCanonicalWidgetContext(input.id, input.assertedWorkspaceId), + authorizationOptions: { delegation: widgetDelegationPolicy }, + execute: async ({ input, context }) => renameWidgetRecord({ + workspaceId: context.workspaceId, + widgetId: context.resourceId, + name: input.name, + }), + projectAudit: ({ result }) => ({ + action: AuditAction.WIDGET_UPDATED, + resourceType: AuditResourceType.WIDGET, + resourceId: result.id, + resourceName: result.name, + }), + afterSuccess: ({ context }) => notifyWidgetsChanged(context.workspaceId), +}) +``` + +Adapt the example to the domain's real authorization options; do not copy invented field names. + +The wrapper must own this lifecycle: + +1. Reject disallowed principal kinds before protected loading. +2. Load canonical context and conceal asserted-scope mismatches as required. +3. Authorize the operation using current policy state. +4. Execute the manager or repository primitive. +5. Project audit from authoritative results. +6. Await shared post-success effects. + +Do not call shared authorization, principal audit attribution, or `recordAudit` manually from an ordinary migrated use-case body. Use `projectAudit` only when the operation has semantic audit. Return no audit entries for authoritative no-ops. Keep product analytics such as `captureServerEvent` surface-specific through the adapter's success hook. + +Inspect legacy orchestration before reusing it. If it already authorizes, audits, notifies, or captures analytics, call a lower-level primitive or remove duplicate responsibility for migrated callers. + +Application code must remain surface-neutral. It must not import `app/api/**`, `next/server`, internal/v1/v2 contracts or presenters, or Copilot tool handlers. Return domain values and let each surface presenter project its own wire result. + +## Adapt internal APIs + +Use `defineInternalJsonRoute` for ordinary JSON routes. Explicitly declare the contract, authentication policy, semantic operation, rate policy, error policy, input mapping, use case, and presenter when the wire result differs. + +Use `internalSessionAuth` for session-only routes. Use `createInternalSessionOrExecutorAuth` only when the endpoint genuinely supports signed executor delegation; the semantic operation must then allow `delegated` principals from the `executor` service. Never turn an actorless legacy JWT into a fake session, owner, or user principal. + +The internal adapter owns authentication and internal response envelopes. It must not implement workspace authorization. Preserve internal-only analytics through `onSuccess` after application success. + +Keep the route module declarative. If several internal routes repeat authentication, parsing, error rendering, or response construction, improve the shared internal route builder instead of adding a domain-specific route wrapper. + +## Adapt public or versioned APIs + +Use the appropriate public/versioned route builder, such as `defineV2JsonRoute`, with API-key authentication, explicit semantic operation and rate policy, external error projection, input mapping, application use case, and an external presenter. V2 rollout admission is centralized by the builder; do not invent a route-local rollout policy. + +Authentication and HTTP formatting may differ from internal APIs; authorization and business behavior must not. Rate-limit using the credential or principal subject, never a billed owner. Resolve billing attribution only for billing, quota, or legacy required-user fields. + +Keep surface contracts separate when their wire shapes differ. Reuse shared primitive schemas and domain validators for invariants such as IDs, names, bounds, and formats. Do not maintain duplicate internal and external schemas merely because the routes are separate; import the same schema when the wire shape is genuinely identical. Never cast one surface response into another. + +Keep v1 middleware and routes unchanged unless explicitly included. + +## Adapt Copilot + +Copilot is a surface adapter, not a separate application layer. If an HTTP or other surface already uses an application use case, Copilot must call that exact use case rather than reimplementing protected business behavior under `lib/copilot`. + +Create one domain-level Copilot application adapter with `createCopilotApplicationAdapter` instead of constructing delegated principals in every tool: + +```ts +executeCopilotWidgetUseCase(context, renameWidget, input, { resourceId }) +``` + +That adapter must: + +- Require a trusted server-authored Copilot execution marker. +- Require the authenticated subject, canonical workspace, tool-call or execution identity, and required audience or lifecycle scope. +- Construct the shared delegated `Principal` in one place. +- Optionally bind the canonical resource scope after trusted resolution. +- Verify that the use case exposes a registered code-defined operation. +- Use the domain's exact immutable operation registry so operation-object membership and identity are checked centrally. +- Call the application use case directly. + +Never construct authoritative delegation from model-provided workspace IDs, user IDs, operation IDs, resource scope, or permission tags. Model arguments are requested targets only and must be checked against trusted execution context and canonical data. + +Tool handlers own argument aliases, resumable legacy names, abort checks, tool-call reporting, and tool-specific presentation. They must not query managers directly for protected operations, manually authorize, or implement protected business behavior. If a Copilot-only compound action expresses real domain behavior, define a surface-neutral domain operation and application use case for it. + +A Copilot reference helper may translate a path to a resource only by calling an authorized application resolver under the intended semantic operation. Passing a code-defined operation object is acceptable; passing a model-provided operation string is not. An immediate same-request resolver followed by the operation may reuse one trusted principal, though the application operation still performs its own canonical authorization. Fresh authentication and authorization are required across lifecycle boundaries such as resumed tool calls, executor callbacks, queued or background work, upload control legs and finalization, durable completion, and long-running provider operations. When resolution and execution form one business operation, need a consistent snapshot, or appear repeatedly together, prefer a top-level application use case such as `renameWidgetByReference`. + +Surface adapters must not compose protected mutations. An atomic compound action requires one top-level semantic domain operation and application use case that owns the transaction and authoritative result. An explicitly best-effort application command may coordinate multiple operations only when it defines hard input and expansion caps, cancellation checkpoints, partial-result semantics, audit behavior, and rate/quota policy. Keep composition exceptional and explicit; ordinary tools should use the shared execution adapter. + +Map expected typed errors to safe tool results. Unknown errors must become generic system/retryable messages while retaining full causes in server logs. Never return raw database or storage errors to the model. + +## Adapt other internal or external tools + +Treat every tool runtime as a surface adapter: + +- Normalize its already-authenticated execution context into an existing `Principal` through one shared adapter for that runtime or domain. +- Call the same application use case used by HTTP and Copilot surfaces. +- Preserve the tool protocol's input, output, retry, and cancellation semantics. +- Keep authoritative workspace and subject scope server-authored. + +An internal caller is not automatically trusted to bypass authorization. It must supply an explicit principal or use a deliberately designed service/delegation principal. If the current principal model cannot express its authority, stop and extend the identity model intentionally; do not fall back to an owner, uploader, creator, or arbitrary user ID. + +External tool endpoints authenticate at their adapter exactly like public APIs. Do not authenticate again inside the application use case. + +## Preserve identity and attribution + +- Session and personal-key principals authorize through current human workspace permission. +- Personal API keys also respect the workspace's personal-key policy. +- Workspace keys authorize as the workspace under explicit operation policy and the write ceiling, independent of creator membership. +- Delegated principals re-check the current subject and their workspace, audience, expiry, execution, and resource scope. +- Billing owners are attribution for billing or legacy required columns only, never authorization, rate identity, delegated identity, audit actor, or human analytics identity. +- Preserve structured `PrincipalActor` metadata in semantic audit. + +If a required legacy user column cannot represent the real actor, label the compatibility attribution explicitly. Never pretend it is the acting human. + +## Handle special operations explicitly + +Do not force these through an ordinary JSON migration: + +- Upload or multipart lifecycles: bind immutable credential identity, reauthorize control legs and finalization, and make durable completion idempotent. +- Large bodies: authenticate and perform cheap admission before bounded buffering. +- Binary or streaming responses: use binary/stream builders and typed descriptors. +- Bulk or recursive operations: deduplicate and cap inputs and expansion, load all resources canonically, and define atomic versus best-effort behavior. +- Polymorphic tools: select the semantic operation only after trusted target-kind resolution; do not route unrelated branches through one domain registry. +- Multi-resource transactions: keep canonical scope predicates and derive audit from authoritative affected rows. + +Stop and report a missing design rather than weakening identity, authorization, limits, or errors. + +## Test the complete matrix + +Add focused tests for every migrated surface and principal kind allowed by the operation: + +- Application: allowed and disallowed roles, principal-kind rejection before canonical loading, workspace assertion mismatch, delegated scope, not found, conflict, no-op, and infrastructure propagation. +- Operation registry: role/workspace-key/principal-kind/delegated-service consistency and fail-fast rejection of invalid definitions. +- Repository: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. +- Internal API: authentication before parsing, exact contract, typed errors, and surface analytics only after success. +- Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. +- Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. +- Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. + +Run at minimum: + +```bash +bunx vitest run +bunx biome check +bunx turbo run type-check --filter=sim --filter=@sim/auth +bun run check:api-validation:strict +git diff --check +``` + +Do not claim a check passed unless it completed successfully. + +## Work safely in parallel + +- Assign non-overlapping route modules and caller sets. Two methods in one route file are one ownership unit. +- Treat operation registries, contract families, route policies, and shared surface adapters as merge hotspots. +- Keep shared core foundations owned by one task; ordinary domain migrations should consume them without modifying them. +- Preserve unrelated working-tree changes. Never stage proposal docs, lockfile drift, or another agent's edits. +- Do not commit, push, or open a PR unless requested. + +## Hand off + +Report: + +1. Semantic operation, role, workspace-key policy, and principal kinds. +2. Migrated, deferred, and non-goal entry points. +3. Behavior preserved per internal, public, Copilot, and other tool surface. +4. Identity construction and authoritative scope source for each surface. +5. Files changed and shared merge hotspots. +6. Tests and checks run with results. +7. Remaining risks or blockers. Fail fast when an invariant could not be implemented. diff --git a/.claude/rules/emcn-components.md b/.claude/rules/emcn-components.md index 5ca3d2a8dcb..c5548d327df 100644 --- a/.claude/rules/emcn-components.md +++ b/.claude/rules/emcn-components.md @@ -11,7 +11,7 @@ Import components, `cn`, and tokens from the `@sim/emcn` barrel; icons come from Never hand-roll the chip pill from raw class strings (they go stale). Compose from the canonical sources: -- **Surface, typography + content tokens:** `chip/chip-chrome.ts` — `chipFilledSurfaceTokens`, `chipFieldSurfaceClass`, `chipFieldTextClass` (text fields and the dropdown search box build on these), plus the chip-content chrome `chipContentGap`, `chipGeometryClass`, `chipContentIconClass`, `chipContentLabelClass`, and `cellIconNodeClass` (non-chip surfaces that must visually match chip content, e.g. resource table cells). All are re-exported from the `@sim/emcn` barrel — no subpath import needed. +- **Surface, typography + content tokens:** `chip/chip-chrome.ts` — `chipFilledSurfaceTokens`, `chipFieldSurfaceClass`, `chipFieldTextClass` (text fields and the dropdown search box build on these), plus the chip-content chrome `chipContentGap`, `chipGeometryClass`, `chipContentIconClass`, `chipContentLabelClass`, `cellIconNodeClass` (non-chip surfaces that must visually match chip content, e.g. resource table cells), and the row-state pair `chipHoverSurfaceClass` / `chipActiveSurfaceClass` (hover vs. selected — mutually exclusive, so a selected row holds its surface through hover; every hand-rolled row imports these rather than restating the literals). All are re-exported from the `@sim/emcn` barrel — no subpath import needed. - **Pill geometry:** `chip/chip.tsx` — `chipVariants` (30px tall, `rounded-lg`, `px-2`, icon↔text `gap-1.5`). Every pill-shaped trigger (`ChipDropdown`, `ChipSelect`, `ChipSwitch`) reuses it for visual parity. Canonical look: normal font-weight (never `font-medium`/`font-semibold`), value text `--text-body`, icons `--text-icon` at `size-[14px]`, placeholder `--text-muted`, `transition-colors`, **no focus ring** (the caret marks focus). Filled surface is `--surface-5` light / `--surface-4` dark with a `--border-1` border. diff --git a/.claude/rules/global.md b/.claude/rules/global.md index 86b2ee3be27..afd2290e37d 100644 --- a/.claude/rules/global.md +++ b/.claude/rules/global.md @@ -4,7 +4,12 @@ Import `createLogger` from `@sim/logger`. Use `logger.info`, `logger.warn`, `logger.error` instead of `console.log`. Inside API routes wrapped with `withRouteHandler`, loggers automatically include the request ID. ## API Route Handlers -All API route handlers must be wrapped with `withRouteHandler` from `@/lib/core/utils/with-route-handler`. Never export a bare `async function GET/POST/...` — always use `export const METHOD = withRouteHandler(...)`. +All API route handlers must run inside `withRouteHandler`. Ordinary internal and v2 JSON/binary handlers use the shared route builders, which already apply it; never double-wrap them. Use raw `withRouteHandler` only for documented protocol or lifecycle exceptions. Never export a bare `async function GET/POST/...`. + +## Application Operation Boundary +Every protected read, write, canonical lookup, or authorization-sensitive reference resolution must enter through an authorized application use case. Surfaces authenticate and build a `Principal`, rate-limit, parse, map input, call the use case, and present their own result. They must not query protected data, decide resource authorization, implement business transactions, or record semantic audit. + +Define one stable semantic operation with its role, workspace-key policy, principal kinds, and delegated services. Internal, v2, Copilot, and trusted-tool adapters call the same use case when domain behavior is the same. Copilot uses `createCopilotApplicationAdapter`; it is not a separate protected business layer. Protected compound mutations require one top-level semantic application operation. Never substitute billing attribution, an uploader, creator, or key owner for the acting principal. Use the `migrate-application-operation` skill for new or migrated protected operations. ## Comments Use TSDoc for documentation. No `====` separators. No non-TSDoc comments. diff --git a/.claude/rules/sim-architecture.md b/.claude/rules/sim-architecture.md index a0cfbfcd050..6886710a61c 100644 --- a/.claude/rules/sim-architecture.md +++ b/.claude/rules/sim-architecture.md @@ -38,6 +38,21 @@ packages/ # @sim/* — audit, auth, db, logger, realtime-protocol - `apps/* → packages/*` only. Packages never import from `apps/*`. - `apps/realtime` avoids Next.js, React, the block/tool registry, provider SDKs, and the executor; never add `@/lib/webhooks/providers/*`, `@/executor/*`, `@/blocks/*`, or `@/tools/*` imports to any package it consumes. CI enforces this via `scripts/check-monorepo-boundaries.ts` and `scripts/check-realtime-prune-graph.ts`. +## Protected Application Operations + +Every real operation on protected or persisted data crosses one authorized application boundary: + +1. The surface authenticates its credential or trusted context and constructs a `Principal`. +2. A fixed, code-defined semantic operation declares minimum role, workspace-key policy, allowed principal kinds, and delegated services. +3. The application use case loads canonical context, checks asserted scope, authorizes current access, executes the manager/repository, projects semantic audit, and runs shared domain effects. +4. The surface presents its own internal, v2, Copilot, or tool result. + +Routes and tools must not query protected data, authorize resources, implement business transactions, or record semantic audit. Application modules must not import `app/api/**`, `next/server`, route contracts/presenters, or Copilot handlers. Copilot must call the same domain use case through `createCopilotApplicationAdapter`; do not create a second Copilot business implementation. Atomic compound mutations need one top-level semantic application operation rather than sequential surface calls. + +Ordinary internal and v2 routes use the shared JSON/binary route builders. Those builders already apply `withRouteHandler`; do not double-wrap them. Use raw `withRouteHandler` only for explicit protocol, streaming, large-body, multipart, or lifecycle exceptions, while keeping protected business work inside application use cases. + +Use the `migrate-application-operation` skill before creating or migrating a protected endpoint, tool command, or resource method. + ## The `'use client'` server boundary Every export of a `'use client'` module becomes a *client reference* on the server — server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can only *render* it as a component or pass it as a prop, never *call* it (doing so throws at runtime, e.g. `tableKeys.list is not a function`; `next build` does not catch it). Keep server-importable query primitives (key factories, fetchers, mappers, constants) in non-`'use client'` modules — see `.claude/rules/sim-queries.md`. Enforced by `scripts/check-client-boundary-imports.ts`. diff --git a/.cursor/commands/add-block.md b/.cursor/commands/add-block.md index 1ac378de897..d73e9efbac7 100644 --- a/.cursor/commands/add-block.md +++ b/.cursor/commands/add-block.md @@ -46,6 +46,12 @@ export const {ServiceName}Block: BlockConfig = { // Auth mode authMode: AuthMode.OAuth, // or AuthMode.ApiKey + // Card summary sentences — see "Canvas Sentences" below + canvasPresentation: { + defaultTitle: '{Default Operation}', + sentences: { byOperation: { /* one per operation dropdown option id */ } }, + }, + subBlocks: [ // Define all UI fields here ], @@ -939,6 +945,35 @@ Derive templates from the service's real use cases. Each prompt should name a co - **Ground every skill in operations the block actually exposes** — cross-check each skill's steps against `tools.access`. Never describe an action the integration cannot perform. - **Derive skills from real, popular use cases found online — never invent them.** Web-search the service's documented use cases (vendor use-case/solutions pages, official docs describing the workflow, reputable "top automations for X" articles) and only add a skill you can source as something people genuinely do with the service. Do not hallucinate skills. +## Canvas Sentences + +Every block declares a one-line prose summary that replaces its card's field rows: + +``` +Slack ← header (already names the block) +Posts ⟨Ship it 🚀⟩ to ⟨#eng⟩ ← the sentence; ⟨…⟩ are live value chips +``` + +Write one `byOperation` entry per operation dropdown option (or a single `default` +when the block has no operation dropdown). + +**The full authoring contract — voice, structure, and the two mistakes that break +cards silently — is `apps/sim/blocks/AGENTS.md` → "Canvas sentences". Read it +before writing any.** The two failures worth repeating here, because both are +invisible at runtime: + +1. A clause naming only one member of a `canonicalParamId` pair drops the sentence + for every advanced-mode user. List all members: + `field: ['channelSelector', 'manualChannel']`. +2. A clause referencing a subblock whose `condition` excludes that operation can + never render. + +Validate before finishing: + +```bash +bun run apps/sim/scripts/check-canvas-sentences.ts --block={service} +``` + ## Generated artifacts Adding a block on its own needs no **tool metadata** regeneration — a block references existing @@ -957,7 +992,6 @@ bun run integration-catalog:check The catalog check independently derives deployment metadata from the executable block registry and compares it with the committed `apps/sim/lib/integrations/integrations.json`. Review the generated diff and keep only intentional changes. - ## Checklist Before Finishing - [ ] `integrationType` is set to the correct `IntegrationType` enum value @@ -985,6 +1019,7 @@ diff and keep only intentional changes. - [ ] Exported `{Service}BlockMeta` with at least 7 templates - [ ] `url` set on `{Service}BlockMeta` to the external service's verified homepage (omit only for first-party blocks with no external service) - [ ] `skills` added to `{Service}BlockMeta`, each grounded in `tools.access` and sourced from a real online use case (not invented) +- [ ] `canvasPresentation.sentences` covers every operation, and `bun run apps/sim/scripts/check-canvas-sentences.ts --block={service}` passes with 100% coverage ## Final Validation (Required) diff --git a/.cursor/commands/add-integration.md b/.cursor/commands/add-integration.md index 40cc28d8b8f..193707613f7 100644 --- a/.cursor/commands/add-integration.md +++ b/.cursor/commands/add-integration.md @@ -762,9 +762,11 @@ tools: { } ``` -#### 3. Create Internal API Route +#### 3. Create Special Internal Tool Execution Route -Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. +Create `apps/sim/app/api/tools/{service}/{action}/route.ts`. This raw route pattern is only for an integration's provider-execution boundary when it needs special file normalization, large-body handling, or protocol behavior. It is not the pattern for CRUD or other operations on protected Sim resources. For those, use the `migrate-application-operation` skill and an authorized application use case with the ordinary internal/v2 route builders. + +Internal tool routes are HTTP boundaries and follow the same contract policy as public routes — define the request/response shape in `apps/sim/lib/api/contracts/tools/{service}.ts` (or an existing aggregate) and validate with canonical helpers from `@/lib/api/server`. Never write a route-local Zod schema. Authenticate and perform cheap admission before parsing or downloading files. ```typescript // apps/sim/lib/api/contracts/tools/{service}.ts diff --git a/.cursor/commands/migrate-application-operation.md b/.cursor/commands/migrate-application-operation.md new file mode 100644 index 00000000000..9fac674ca6f --- /dev/null +++ b/.cursor/commands/migrate-application-operation.md @@ -0,0 +1,299 @@ +# Create Or Migrate Application Operation + +Create or migrate one bounded semantic operation at a time. Share authorization and business behavior without forcing internal APIs, public APIs, Copilot, and other tools to share authentication, input schemas, or response shapes. + +## Enforce the application boundary + +Apply this invariant: + +> Every real operation on persisted or protected data enters through an authorized application use case. + +This includes mutations, content and metadata reads, canonical resource lookup, and reference-to-resource resolution when the lookup is authorization-sensitive. + +Surface helpers may: + +- Normalize an already-authenticated surface context into a `Principal`. +- Translate aliases or wire arguments into application input. +- Select a code-defined operation and application use case. +- Call the application use case. +- Translate typed results and errors into the surface contract. + +Surface helpers must not: + +- Query databases or storage. +- Decide workspace or resource authorization. +- Implement business transactions. +- Record semantic audit or shared domain notifications. +- Infer authoritative identity, workspace, audience, or scope from untrusted arguments. +- Substitute billing attribution for identity. + +A helper that resolves a path is valid only when the actual protected lookup runs through an authorized application resolver. If a helper begins doing real data work, move that work into an application use case. + +## Read the foundation first + +Read these files completely before editing: + +- `packages/auth/src/principal.ts` +- `apps/sim/lib/core/application/operation.ts` +- `apps/sim/lib/core/application/workspace-operation.ts` +- `apps/sim/lib/core/application/workspace-authorization.ts` +- `apps/sim/lib/core/application/authorized-workspace-use-case.ts` +- `apps/sim/lib/api/server/routes/definition.ts` +- `apps/sim/lib/api/server/routes/internal-json-route.ts` +- `apps/sim/lib/api/server/routes/v2-json-route.ts` +- `apps/sim/lib/auth/internal-delegation.ts` +- `apps/sim/lib/copilot/application/application-adapter.ts` +- `apps/sim/lib/copilot/auth/application-delegation.ts` + +Use the file domain only as a representative golden slice: + +- `apps/sim/lib/workspace-files/application/operations.ts` +- `apps/sim/lib/workspace-files/application/authorized-workspace-file-use-case.ts` +- `apps/sim/lib/workspace-files/application/rename-workspace-file.ts` +- `apps/sim/lib/copilot/application/execute-file-use-case.ts` +- `apps/sim/lib/copilot/auth/file-delegation.ts` + +Then read the target domain's operation registry, application code, repositories, contracts, adapters, aliases, resume paths, and focused tests. Fail immediately if the shared foundation is absent. Do not recreate it inside the domain. + +## Bound the migration + +Inventory every entry point for the behavior before editing: + +- Internal HTTP routes and contracts. +- Public or versioned API routes and contracts. +- Copilot tools, aliases, resume paths, and polymorphic branches. +- Other tool servers, workflow executors, jobs, or service callers. +- Current authentication, authorization, workspace assertions, and concealment. +- Manager or orchestration call chains. +- Audit, notification, analytics, and billing side effects. +- Error/status/result behavior. +- Rate-limit identity, rollout gates, quota, and concurrency admission. + +Classify each as `migrate`, `defer`, or `non-goal`. Do not migrate adjacent operations merely because they share a module. Do not modify v1 unless the request explicitly includes it. + +Preserve behavior unless the task explicitly changes it. Stop and report a decision when surfaces currently disagree on security or compatibility behavior; do not silently choose one. + +## Keep the layers distinct + +Use these responsibilities: + +1. Authentication adapter: verify the surface credential or trusted execution context and construct a `Principal`. +2. Route or tool adapter: select rate policy, parse its contract, translate input, call the application use case, and render its own result. +3. Application use case: load canonical context, compare asserted scope, authorize the semantic operation, execute business behavior, project semantic audit, and trigger shared domain effects. +4. Manager or repository: perform database and storage reads or writes using canonical identifiers and scope. Never accept credentials or principals. +5. Presenter: return only the surface success body or typed binary descriptor. Never construct auth, rate, or error behavior. + +For ordinary public JSON routes, preserve this order: + +```text +IP abuse limit + -> authenticate + -> build Principal + -> operation rate limit + -> parse surface contract + -> application use case + -> canonical load + -> asserted-scope concealment + -> current authorization + -> manager read or mutation + -> semantic audit + -> shared domain effects + -> surface presenter +``` + +Internal routes may omit the IP bucket or operation limit only through an explicit policy with a reason. Usage billing, storage quota, cost admission, and concurrency are separate from request-rate limiting. + +Never query API keys or sessions from the application layer. Never add fallback identity or authorization behavior. Propagate infrastructure failures instead of turning them into not-found or forbidden results. + +## Define the semantic operation once + +Add one stable entry to the target domain's operation registry: + +```ts +rename: defineWorkspaceOperation({ + id: 'widgets.rename', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ['session', 'personal_api_key', 'workspace_api_key', 'delegated'], + delegatedServices: ['copilot'], +}) +``` + +Do not create internal-, public-, or Copilot-specific versions of the same semantic operation. If two callers have materially different business or transactional semantics, define separate semantic operations and use cases and explain the distinction. + +Choose principal kinds from actual behavior. Do not accept every principal merely because the use case is shared. Workspace API keys have a write ceiling and cannot satisfy admin operations. The operation definition must fail fast when its role, workspace-key policy, and principal kinds disagree. + +Route declarations, tool adapters, and use cases must use the same literal operation. Runtime operation selection is permitted only from a trusted, code-defined registry. Never accept an operation ID or permission tag from an HTTP body, model argument, or other untrusted input. + +## Implement the application use case + +Use `defineAuthorizedWorkspaceUseCase` directly or a thin domain binding that supplies domain-specific authorization options: + +```ts +export const renameWidget = defineAuthorizedWorkspaceUseCase({ + operation: widgetOperations.rename, + resolveContext: ({ input }: { input: RenameWidgetInput }) => + loadCanonicalWidgetContext(input.id, input.assertedWorkspaceId), + authorizationOptions: { delegation: widgetDelegationPolicy }, + execute: async ({ input, context }) => renameWidgetRecord({ + workspaceId: context.workspaceId, + widgetId: context.resourceId, + name: input.name, + }), + projectAudit: ({ result }) => ({ + action: AuditAction.WIDGET_UPDATED, + resourceType: AuditResourceType.WIDGET, + resourceId: result.id, + resourceName: result.name, + }), + afterSuccess: ({ context }) => notifyWidgetsChanged(context.workspaceId), +}) +``` + +Adapt the example to the domain's real authorization options; do not copy invented field names. + +The wrapper must own this lifecycle: + +1. Reject disallowed principal kinds before protected loading. +2. Load canonical context and conceal asserted-scope mismatches as required. +3. Authorize the operation using current policy state. +4. Execute the manager or repository primitive. +5. Project audit from authoritative results. +6. Await shared post-success effects. + +Do not call shared authorization, principal audit attribution, or `recordAudit` manually from an ordinary migrated use-case body. Use `projectAudit` only when the operation has semantic audit. Return no audit entries for authoritative no-ops. Keep product analytics such as `captureServerEvent` surface-specific through the adapter's success hook. + +Inspect legacy orchestration before reusing it. If it already authorizes, audits, notifies, or captures analytics, call a lower-level primitive or remove duplicate responsibility for migrated callers. + +Application code must remain surface-neutral. It must not import `app/api/**`, `next/server`, internal/v1/v2 contracts or presenters, or Copilot tool handlers. Return domain values and let each surface presenter project its own wire result. + +## Adapt internal APIs + +Use `defineInternalJsonRoute` for ordinary JSON routes. Explicitly declare the contract, authentication policy, semantic operation, rate policy, error policy, input mapping, use case, and presenter when the wire result differs. + +Use `internalSessionAuth` for session-only routes. Use `createInternalSessionOrExecutorAuth` only when the endpoint genuinely supports signed executor delegation; the semantic operation must then allow `delegated` principals from the `executor` service. Never turn an actorless legacy JWT into a fake session, owner, or user principal. + +The internal adapter owns authentication and internal response envelopes. It must not implement workspace authorization. Preserve internal-only analytics through `onSuccess` after application success. + +Keep the route module declarative. If several internal routes repeat authentication, parsing, error rendering, or response construction, improve the shared internal route builder instead of adding a domain-specific route wrapper. + +## Adapt public or versioned APIs + +Use the appropriate public/versioned route builder, such as `defineV2JsonRoute`, with API-key authentication, explicit semantic operation and rate policy, external error projection, input mapping, application use case, and an external presenter. V2 rollout admission is centralized by the builder; do not invent a route-local rollout policy. + +Authentication and HTTP formatting may differ from internal APIs; authorization and business behavior must not. Rate-limit using the credential or principal subject, never a billed owner. Resolve billing attribution only for billing, quota, or legacy required-user fields. + +Keep surface contracts separate when their wire shapes differ. Reuse shared primitive schemas and domain validators for invariants such as IDs, names, bounds, and formats. Do not maintain duplicate internal and external schemas merely because the routes are separate; import the same schema when the wire shape is genuinely identical. Never cast one surface response into another. + +Keep v1 middleware and routes unchanged unless explicitly included. + +## Adapt Copilot + +Copilot is a surface adapter, not a separate application layer. If an HTTP or other surface already uses an application use case, Copilot must call that exact use case rather than reimplementing protected business behavior under `lib/copilot`. + +Create one domain-level Copilot application adapter with `createCopilotApplicationAdapter` instead of constructing delegated principals in every tool: + +```ts +executeCopilotWidgetUseCase(context, renameWidget, input, { resourceId }) +``` + +That adapter must: + +- Require a trusted server-authored Copilot execution marker. +- Require the authenticated subject, canonical workspace, tool-call or execution identity, and required audience or lifecycle scope. +- Construct the shared delegated `Principal` in one place. +- Optionally bind the canonical resource scope after trusted resolution. +- Verify that the use case exposes a registered code-defined operation. +- Use the domain's exact immutable operation registry so operation-object membership and identity are checked centrally. +- Call the application use case directly. + +Never construct authoritative delegation from model-provided workspace IDs, user IDs, operation IDs, resource scope, or permission tags. Model arguments are requested targets only and must be checked against trusted execution context and canonical data. + +Tool handlers own argument aliases, resumable legacy names, abort checks, tool-call reporting, and tool-specific presentation. They must not query managers directly for protected operations, manually authorize, or implement protected business behavior. If a Copilot-only compound action expresses real domain behavior, define a surface-neutral domain operation and application use case for it. + +A Copilot reference helper may translate a path to a resource only by calling an authorized application resolver under the intended semantic operation. Passing a code-defined operation object is acceptable; passing a model-provided operation string is not. An immediate same-request resolver followed by the operation may reuse one trusted principal, though the application operation still performs its own canonical authorization. Fresh authentication and authorization are required across lifecycle boundaries such as resumed tool calls, executor callbacks, queued or background work, upload control legs and finalization, durable completion, and long-running provider operations. When resolution and execution form one business operation, need a consistent snapshot, or appear repeatedly together, prefer a top-level application use case such as `renameWidgetByReference`. + +Surface adapters must not compose protected mutations. An atomic compound action requires one top-level semantic domain operation and application use case that owns the transaction and authoritative result. An explicitly best-effort application command may coordinate multiple operations only when it defines hard input and expansion caps, cancellation checkpoints, partial-result semantics, audit behavior, and rate/quota policy. Keep composition exceptional and explicit; ordinary tools should use the shared execution adapter. + +Map expected typed errors to safe tool results. Unknown errors must become generic system/retryable messages while retaining full causes in server logs. Never return raw database or storage errors to the model. + +## Adapt other internal or external tools + +Treat every tool runtime as a surface adapter: + +- Normalize its already-authenticated execution context into an existing `Principal` through one shared adapter for that runtime or domain. +- Call the same application use case used by HTTP and Copilot surfaces. +- Preserve the tool protocol's input, output, retry, and cancellation semantics. +- Keep authoritative workspace and subject scope server-authored. + +An internal caller is not automatically trusted to bypass authorization. It must supply an explicit principal or use a deliberately designed service/delegation principal. If the current principal model cannot express its authority, stop and extend the identity model intentionally; do not fall back to an owner, uploader, creator, or arbitrary user ID. + +External tool endpoints authenticate at their adapter exactly like public APIs. Do not authenticate again inside the application use case. + +## Preserve identity and attribution + +- Session and personal-key principals authorize through current human workspace permission. +- Personal API keys also respect the workspace's personal-key policy. +- Workspace keys authorize as the workspace under explicit operation policy and the write ceiling, independent of creator membership. +- Delegated principals re-check the current subject and their workspace, audience, expiry, execution, and resource scope. +- Billing owners are attribution for billing or legacy required columns only, never authorization, rate identity, delegated identity, audit actor, or human analytics identity. +- Preserve structured `PrincipalActor` metadata in semantic audit. + +If a required legacy user column cannot represent the real actor, label the compatibility attribution explicitly. Never pretend it is the acting human. + +## Handle special operations explicitly + +Do not force these through an ordinary JSON migration: + +- Upload or multipart lifecycles: bind immutable credential identity, reauthorize control legs and finalization, and make durable completion idempotent. +- Large bodies: authenticate and perform cheap admission before bounded buffering. +- Binary or streaming responses: use binary/stream builders and typed descriptors. +- Bulk or recursive operations: deduplicate and cap inputs and expansion, load all resources canonically, and define atomic versus best-effort behavior. +- Polymorphic tools: select the semantic operation only after trusted target-kind resolution; do not route unrelated branches through one domain registry. +- Multi-resource transactions: keep canonical scope predicates and derive audit from authoritative affected rows. + +Stop and report a missing design rather than weakening identity, authorization, limits, or errors. + +## Test the complete matrix + +Add focused tests for every migrated surface and principal kind allowed by the operation: + +- Application: allowed and disallowed roles, principal-kind rejection before canonical loading, workspace assertion mismatch, delegated scope, not found, conflict, no-op, and infrastructure propagation. +- Operation registry: role/workspace-key/principal-kind/delegated-service consistency and fail-fast rejection of invalid definitions. +- Repository: canonical active lookup, workspace-predicated writes, archived resources, authoritative affected rows, and database error propagation. +- Internal API: authentication before parsing, exact contract, typed errors, and surface analytics only after success. +- Public API: personal and workspace keys, rate and rollout behavior, concealment, exact external envelope, and rate headers. +- Copilot or tools: trusted context, exact registered operation membership, rejected forged scope, aliases and resume paths, permission re-check, safe errors, and unchanged tool result shapes. +- Side effects: audit derives from authoritative results; shared notifications follow audit; neither occurs for rejection or no-op. + +Run at minimum: + +```bash +bunx vitest run +bunx biome check +bunx turbo run type-check --filter=sim --filter=@sim/auth +bun run check:api-validation:strict +git diff --check +``` + +Do not claim a check passed unless it completed successfully. + +## Work safely in parallel + +- Assign non-overlapping route modules and caller sets. Two methods in one route file are one ownership unit. +- Treat operation registries, contract families, route policies, and shared surface adapters as merge hotspots. +- Keep shared core foundations owned by one task; ordinary domain migrations should consume them without modifying them. +- Preserve unrelated working-tree changes. Never stage proposal docs, lockfile drift, or another agent's edits. +- Do not commit, push, or open a PR unless requested. + +## Hand off + +Report: + +1. Semantic operation, role, workspace-key policy, and principal kinds. +2. Migrated, deferred, and non-goal entry points. +3. Behavior preserved per internal, public, Copilot, and other tool surface. +4. Identity construction and authoritative scope source for each surface. +5. Files changed and shared merge hotspots. +6. Tests and checks run with results. +7. Remaining risks or blockers. Fail fast when an invariant could not be implemented. diff --git a/.cursor/rules/global.mdc b/.cursor/rules/global.mdc index bb8b8ed6dff..5535b598224 100644 --- a/.cursor/rules/global.mdc +++ b/.cursor/rules/global.mdc @@ -8,7 +8,12 @@ alwaysApply: true Import `createLogger` from `@sim/logger`. Use `logger.info`, `logger.warn`, `logger.error` instead of `console.log`. Inside API routes wrapped with `withRouteHandler`, loggers automatically include the request ID. ## API Route Handlers -All API route handlers must be wrapped with `withRouteHandler` from `@/lib/core/utils/with-route-handler`. Never export a bare `async function GET/POST/...` — always use `export const METHOD = withRouteHandler(...)`. +All API route handlers must run inside `withRouteHandler`. Ordinary internal and v2 JSON/binary handlers use the shared route builders, which already apply it; never double-wrap them. Use raw `withRouteHandler` only for documented protocol or lifecycle exceptions. Never export a bare `async function GET/POST/...`. + +## Application Operation Boundary +Every protected read, write, canonical lookup, or authorization-sensitive reference resolution must enter through an authorized application use case. Surfaces authenticate and build a `Principal`, rate-limit, parse, map input, call the use case, and present their own result. They must not query protected data, decide resource authorization, implement business transactions, or record semantic audit. + +Define one stable semantic operation with its role, workspace-key policy, principal kinds, and delegated services. Internal, v2, Copilot, and trusted-tool adapters call the same use case when domain behavior is the same. Copilot uses `createCopilotApplicationAdapter`; it is not a separate protected business layer. Protected compound mutations require one top-level semantic application operation. Never substitute billing attribution, an uploader, creator, or key owner for the acting principal. Use the `migrate-application-operation` skill for new or migrated protected operations. ## Comments Use TSDoc for documentation. No `====` separators. No non-TSDoc comments. diff --git a/.cursor/rules/sim-architecture.mdc b/.cursor/rules/sim-architecture.mdc index 90bac74294d..af712e4fa6a 100644 --- a/.cursor/rules/sim-architecture.mdc +++ b/.cursor/rules/sim-architecture.mdc @@ -37,6 +37,21 @@ packages/ # @sim/* — audit, auth, db, logger, realtime-protocol - `apps/* → packages/*` only. Packages never import from `apps/*`. - `apps/realtime` avoids Next.js, React, the block/tool registry, provider SDKs, and the executor; never add `@/lib/webhooks/providers/*`, `@/executor/*`, `@/blocks/*`, or `@/tools/*` imports to any package it consumes. CI enforces this via `scripts/check-monorepo-boundaries.ts` and `scripts/check-realtime-prune-graph.ts`. +## Protected Application Operations + +Every real operation on protected or persisted data crosses one authorized application boundary: + +1. The surface authenticates its credential or trusted context and constructs a `Principal`. +2. A fixed, code-defined semantic operation declares minimum role, workspace-key policy, allowed principal kinds, and delegated services. +3. The application use case loads canonical context, checks asserted scope, authorizes current access, executes the manager/repository, projects semantic audit, and runs shared domain effects. +4. The surface presents its own internal, v2, Copilot, or tool result. + +Routes and tools must not query protected data, authorize resources, implement business transactions, or record semantic audit. Application modules must not import `app/api/**`, `next/server`, route contracts/presenters, or Copilot handlers. Copilot must call the same domain use case through `createCopilotApplicationAdapter`; do not create a second Copilot business implementation. Atomic compound mutations need one top-level semantic application operation rather than sequential surface calls. + +Ordinary internal and v2 routes use the shared JSON/binary route builders. Those builders already apply `withRouteHandler`; do not double-wrap them. Use raw `withRouteHandler` only for explicit protocol, streaming, large-body, multipart, or lifecycle exceptions, while keeping protected business work inside application use cases. + +Use the `migrate-application-operation` skill before creating or migrating a protected endpoint, tool command, or resource method. + ## Feature Organization Features live under `app/workspace/[workspaceId]/`: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4940a8d6b34..cf2f27d7836 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,7 +72,7 @@ jobs: # Detect shell-code changes on dev/staging pushes. Web-only changes never # need a desktop build (installed shells load the web app live); changes to # the Electron app or the bridge packages trigger a per-env prerelease build - # (dev → alpha channel, staging → beta) that the env's update feed + # (dev → dev stream, staging → staging) that the env's update feed # (/api/desktop/update) starts offering automatically. detect-desktop-changes: name: Detect Desktop Changes @@ -707,10 +707,10 @@ jobs: secrets: inherit # Per-env desktop prereleases: a dev/staging push that touches shell code - # publishes a channel-tagged GitHub prerelease (vX.Y.Z-alpha.N from dev, - # vX.Y.Z-beta.N from staging). Each environment's /api/desktop/update feed - # offers only its channel, so dev-pointed shells pick up alpha builds, - # staging-pointed shells beta builds, and prod-pointed shells stable + # publishes an environment-tagged GitHub prerelease (vX.Y.Z-dev.N from dev, + # vX.Y.Z-staging.N from staging). Each environment's /api/desktop/update feed + # offers only its stream, so dev-pointed shells pick up dev builds, + # staging-pointed shells staging builds, and prod-pointed shells stable # releases — independently. Unlike stable releases, prereleases build even # before the Apple signing secrets exist — unsigned, so the update pipeline # is testable end to end; installed shells detect the missing Developer ID @@ -738,7 +738,7 @@ jobs: GH_REPO: ${{ github.repository }} SIGNED: ${{ needs.check-desktop-signing.outputs.configured }} run: | - if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=alpha; APP_NAME="Sim Dev"; else CHANNEL=beta; APP_NAME="Sim Staging"; fi + if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=dev; APP_NAME="Sim Dev"; else CHANNEL=staging; APP_NAME="Sim Staging"; fi # Prerelease core = next patch after the latest stable release, so # channel builds always outrank the stable they are built on top of # and are always superseded by the next stable. The run-attempt @@ -825,9 +825,9 @@ jobs: steps: - name: Delete stale prereleases run: | - if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=alpha; else CHANNEL=beta; fi + if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNELS='(dev|alpha)'; else CHANNELS='(staging|beta)'; fi gh release list --limit 100 --json tagName,isPrerelease,isDraft,createdAt \ - --jq "[.[] | select(.isPrerelease and (.isDraft | not) and (.tagName | test(\"-${CHANNEL}\\\\.\")))] | sort_by(.createdAt) | reverse | .[5:] | .[].tagName" | + --jq "[.[] | select(.isPrerelease and (.isDraft | not) and (.tagName | test(\"-${CHANNELS}\\\\.\")))] | sort_by(.createdAt) | reverse | .[5:] | .[].tagName" | while read -r TAG; do [ -n "$TAG" ] || continue echo "Deleting stale prerelease $TAG" @@ -836,11 +836,11 @@ jobs: - name: Delete leftover draft prereleases run: | - if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=alpha; else CHANNEL=beta; fi + if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNELS='(dev|alpha)'; else CHANNELS='(staging|beta)'; fi # Drafts have no tag ref, so delete by release id via the API # (gh release delete resolves by tag, which is ambiguous for drafts). gh api "repos/${GH_REPO}/releases?per_page=100" \ - --jq ".[] | select(.draft and (.tag_name | test(\"-${CHANNEL}\\\\.\"))) | .id" | + --jq ".[] | select(.draft and (.tag_name | test(\"-${CHANNELS}\\\\.\"))) | .id" | while read -r ID; do [ -n "$ID" ] || continue echo "Deleting leftover draft release $ID" diff --git a/.github/workflows/desktop-e2e.yml b/.github/workflows/desktop-e2e.yml index 4feb7b91318..7632b0bea66 100644 --- a/.github/workflows/desktop-e2e.yml +++ b/.github/workflows/desktop-e2e.yml @@ -17,7 +17,7 @@ concurrency: jobs: e2e: name: E2E (${{ matrix.electron }}) - runs-on: macos-14 + runs-on: macos-26 strategy: fail-fast: false matrix: @@ -58,7 +58,7 @@ jobs: package-smoke: name: Unsigned package smoke - runs-on: macos-14 + runs-on: macos-26 steps: - name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index d8a9e650d89..a160d22c37f 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -49,7 +49,7 @@ permissions: jobs: build-sign-notarize: name: Build, Sign, Notarize - runs-on: macos-14 + runs-on: macos-26 steps: - name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 @@ -91,8 +91,9 @@ jobs: exit 1 fi - # Prerelease versions carry their environment in the tag: -alpha.N is a - # dev build, -beta.N a staging build. The channel decides the app's + # Prerelease versions carry their environment in the tag: -dev.N is a + # dev build, -staging.N a staging build. Legacy -alpha/-beta tags remain + # accepted while already-published builds age out. The channel decides the app's # identity (name/bundle id — a separate app per environment, installable # side by side) and the default origin baked into the bundle, which in # turn selects the update feed the installed app polls. @@ -102,9 +103,9 @@ jobs: VERSION: ${{ inputs.version }} run: | case "$VERSION" in - *-alpha.*) + *-dev.*|*-alpha.*) NAME='Sim Dev'; APP_ID=ai.sim.desktop.dev; ORIGIN=https://www.dev.sim.ai ;; - *-beta.*) + *-staging.*|*-beta.*) NAME='Sim Staging'; APP_ID=ai.sim.desktop.staging; ORIGIN=https://www.staging.sim.ai ;; *) NAME='Sim'; APP_ID=ai.sim.desktop; ORIGIN='' ;; @@ -167,8 +168,8 @@ jobs: if: ${{ inputs.sign }} run: | DMG="$(ls apps/desktop/release/*.dmg | head -1)" - xcrun stapler validate "$DMG" hdiutil attach "$DMG" -mountpoint /tmp/sim-dmg -nobrowse -quiet + xcrun stapler validate /tmp/sim-dmg/*.app spctl --assess --type execute --verbose /tmp/sim-dmg/*.app codesign --verify --deep --strict /tmp/sim-dmg/*.app hdiutil detach /tmp/sim-dmg -quiet diff --git a/AGENTS.md b/AGENTS.md index f36d633df61..ae4d76e0de0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ You are a professional software engineer. All code must follow best practices: a - **Linting / Audit**: `bun run check:api-validation` must pass on PRs. Do not introduce route-local boundary Zod schemas, direct route Zod imports, or ad-hoc client wire types — see "API Contracts" and "API Route Pattern" below - **Logging**: Import `createLogger` from `@sim/logger`. Use `logger.info`, `logger.warn`, `logger.error` instead of `console.log`. Inside API routes wrapped with `withRouteHandler`, loggers automatically include the request ID — no manual `withMetadata({ requestId })` needed -- **API Route Handlers**: All API route handlers (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`) must be wrapped with `withRouteHandler` from `@/lib/core/utils/with-route-handler`. This provides request ID tracking, automatic error logging for 4xx/5xx responses, and unhandled error catching. See "API Route Pattern" section below +- **API Route Handlers**: All API route handlers (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`) must run inside `withRouteHandler`. Ordinary internal and v2 handlers use the shared JSON/binary route builders, which already apply it; never double-wrap a builder. Use raw `withRouteHandler` only for documented protocol or lifecycle exceptions. See "API Route Pattern" below - **Comments**: Use TSDoc for documentation. No `====` separators. No non-TSDoc comments - **Styling**: Never update global styles. Keep all styling local to components - **ID Generation**: Never use `crypto.randomUUID()`, `nanoid`, or `uuid` package. Use `generateId()` (UUID v4) or `generateShortId()` (compact) from `@sim/utils/id` @@ -29,6 +29,17 @@ You are a professional software engineer. All code must follow best practices: a 3. Type Safety First: TypeScript interfaces for all props, state, return types 4. Predictable State: Zustand for global state, useState for UI-only concerns +### Application Operation Boundary + +- Every protected read, write, canonical resource lookup, or authorization-sensitive reference resolution enters through an authorized application use case. +- Define one stable semantic operation with its minimum role, workspace-key policy, allowed principal kinds, and delegated services. Internal APIs, v2 APIs, Copilot, and trusted tools call the same use case when the domain behavior is the same. +- Surface adapters authenticate and construct a `Principal`, apply request-rate policy, parse contracts, map input, and present results. They never query protected data, decide resource authorization, implement business transactions, or record semantic audit. +- Application use cases load canonical context, compare asserted scope, authorize current access, execute managers/repositories, project semantic audit, and trigger shared domain effects. Managers accept canonical IDs and scope, never credentials or principals. +- Copilot is a surface adapter. Use `createCopilotApplicationAdapter` and the domain's registered operation object; do not create Copilot-only authorization or business implementations. +- Protected compound mutations belong in one top-level semantic application operation. Do not sequence independently committing mutations in a route or tool adapter. +- Never substitute a billing owner, uploader, creator, or API-key owner for the acting principal. Fail fast when the identity model or operation policy cannot express the caller. +- Use the `migrate-application-operation` skill whenever creating or migrating a protected endpoint, tool command, or resource method. + ### Root Structure ``` @@ -166,56 +177,49 @@ const provider = config as unknown as LegacyProvider ## API Route Pattern -Every API route handler must be wrapped with `withRouteHandler`. This sets up `AsyncLocalStorage`-based request context so all loggers in the request lifecycle automatically include the request ID. +Every route method must run inside `withRouteHandler`. Ordinary internal and v2 JSON/binary routes use `defineInternalJsonRoute`, `defineV2JsonRoute`, or the matching binary/stream builder. These builders already apply `withRouteHandler`; never wrap them again. Use raw `withRouteHandler` only for explicit protocol or lifecycle exceptions such as streaming, multipart control, large-body admission, OAuth, or public execution. -Routes never `import { z } from 'zod'` and never define route-local boundary schemas. They consume the contract from `@/lib/api/contracts/**` and validate with canonical helpers from `@/lib/api/server`: +Routes never `import { z } from 'zod'` and never define route-local boundary schemas. Declarative builders consume contracts and own authentication, admission, parsing, use-case execution, response validation, and error projection. A raw special route consumes the same contracts and validates with canonical helpers from `@/lib/api/server`, after authentication and cheap admission: - `parseRequest(contract, request, context, options?)` — fully contract-bound routes; parses params, query, body, and headers in one call. Pass `{}` for `context` on routes without route params, or the route's `context` argument when route params exist. Returns a discriminated union; check `parsed.success` and return `parsed.response` on failure - `validationErrorResponse(error)` and `getValidationErrorMessage(error, fallback)` — produce 400 responses from a `ZodError` - `validationErrorResponseFromError(error)` — when handling unknown caught errors that may or may not be a `ZodError` - `isZodError(error)` — type guard. Routes never use `instanceof z.ZodError` -### Fully contract-bound route (`parseRequest`) +### Ordinary authorized JSON route ```typescript -import { createLogger } from '@sim/logger' -import type { NextRequest } from 'next/server' -import { NextResponse } from 'next/server' -import { createFolderContract } from '@/lib/api/contracts/folders' -import { parseRequest } from '@/lib/api/server' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' - -const logger = createLogger('FoldersAPI') - -export const POST = withRouteHandler(async (request: NextRequest) => { - const parsed = await parseRequest(createFolderContract, request, {}) - if (!parsed.success) return parsed.response - const { body } = parsed.data - logger.info('Creating folder', { workspaceId: body.workspaceId }) - return NextResponse.json({ ok: true }) +export const PATCH = defineInternalJsonRoute({ + contract: renameWidgetContract, + auth: internalSessionAuth, + operation: widgetOperations.rename, + rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal behavior' }), + errorPolicy: internalWidgetErrorPolicy, + mapInput: ({ params, body }) => ({ + widgetId: params.widgetId, + assertedWorkspaceId: params.workspaceId, + name: body.name, + }), + useCase: renameWidget, + present: ({ widget }) => ({ success: true, widget }), }) ``` -### Composing with other middleware - -```typescript -export const POST = withRouteHandler(withAdminAuth(async (request) => { - return NextResponse.json({ ok: true }) -})) -``` +The contract, operation, and use case must agree at definition time. Authentication and request-rate admission happen before parsing; canonical loading and authorization happen in the application use case. The presenter returns only the surface success body. Routes under `apps/sim/app/api/v1/**` use the shared middleware in `apps/sim/app/api/v1/middleware.ts` for auth, rate-limit, and workspace access. Compose contract validation inside that middleware — never reimplement auth/rate-limit per-route. -Never export a bare `async function GET/POST/...` — always use `export const METHOD = withRouteHandler(...)`. +Never export a bare `async function GET/POST/...`. Export the result of a shared builder or, for a documented special route, `withRouteHandler(...)`. ### Adding a new boundary feature end-to-end When adding a new route + client surface, follow this order. Each step has one place it lives. 1. **Author the contract first** in `apps/sim/lib/api/contracts/.ts` (or a subdirectory for large domains: `knowledge/`, `selectors/`, `tools/`). Define one schema per request slice (`params`, `query`, `body`, `headers`) and one for the response, then wrap with `defineRouteContract`. Export named type aliases (`z.input` for inputs, `z.output` for outputs). -2. **Implement the route** in `apps/sim/app/api//route.ts`. Auth always runs **before** `parseRequest` — never validate untrusted input before authenticating the caller. The route returns exactly the shape declared in `contract.response.schema`. -3. **Add the React Query hook** in `apps/sim/hooks/queries/.ts`. Use `requestJson(contract, input)` for the call. Build a hierarchical query-key factory (`all` → `lists()` → `list(workspaceId)` → `details()` → `detail(id)`) so invalidations can target prefixes. -4. **Use the hook in the component**. The mutation's `data` and `error` are fully typed from the contract; surface `error.message` (already extracted from the response body's `error` or `message` field by `requestJson`). +2. **Define the semantic operation and application use case** under `apps/sim/lib//application/`. The use case owns canonical loading, asserted-scope checks, current authorization, business behavior, semantic audit, and shared domain effects. +3. **Implement the route adapter** in `apps/sim/app/api//route.ts` with the appropriate shared builder. Declare auth, operation, rate policy, error policy, input mapping, use case, and presenter. Auth always runs **before** parsing. Use raw `withRouteHandler` only for an explicit special route, and keep protected work in application use cases. +4. **Add the React Query hook** in `apps/sim/hooks/queries/.ts`. Use `requestJson(contract, input)` for the call. Build a hierarchical query-key factory (`all` → `lists()` → `list(workspaceId)` → `details()` → `detail(id)`) so invalidations can target prefixes. +5. **Use the hook in the component**. The mutation's `data` and `error` are fully typed from the contract; surface `error.message` (already extracted from the response body's `error` or `message` field by `requestJson`). ### Schema review checklist (read the contract diff like a DB migration) @@ -473,4 +477,3 @@ For the full authoring instructions — SubBlock property tables, `condition`/`d Table column types are registry entries in `apps/sim/lib/table/column-types/` — one file per type owning its label, icon, storage cast, coercion, validation, conversion compatibility, formatting, and editor. `Record` on `registry.ts` and `registry.server.ts` is a compile-time completeness gate: adding a type to the union errors until both entries exist. Never add a `case 'sometype':` outside `column-types/` — a missing arm fails silently (a wrong `jsonbCast` breaks every filter on the column). If a consumer needs per-type knowledge, add a registry field. Use `/add-column-type` for the full procedure. - diff --git a/CLAUDE.md b/CLAUDE.md index fc63380d153..9bd7d678400 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ You are a professional software engineer. All code must follow best practices: a - **Linting / Audit**: `bun run check:api-validation` must pass on PRs. Do not introduce route-local boundary Zod schemas, direct route Zod imports, or ad-hoc client wire types — see "API Contracts" and "API Route Pattern" below - **Logging**: Import `createLogger` from `@sim/logger`. Use `logger.info`, `logger.warn`, `logger.error` instead of `console.log`. Inside API routes wrapped with `withRouteHandler`, loggers automatically include the request ID — no manual `withMetadata({ requestId })` needed -- **API Route Handlers**: All API route handlers (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`) must be wrapped with `withRouteHandler` from `@/lib/core/utils/with-route-handler`. This provides request ID tracking, automatic error logging for 4xx/5xx responses, and unhandled error catching. See "API Route Pattern" section below +- **API Route Handlers**: All API route handlers (`GET`, `POST`, `PUT`, `DELETE`, `PATCH`) must run inside `withRouteHandler`. Ordinary internal and v2 handlers use the shared JSON/binary route builders, which already apply it; never double-wrap a builder. Use raw `withRouteHandler` only for documented protocol or lifecycle exceptions. See "API Route Pattern" below - **Comments**: Use TSDoc for documentation. No `====` separators. No non-TSDoc comments - **Styling**: Never update global styles. Keep all styling local to components - **ID Generation**: Never use `crypto.randomUUID()`, `nanoid`, or `uuid` package. Use `generateId()` (UUID v4) or `generateShortId()` (compact) from `@sim/utils/id` @@ -30,6 +30,17 @@ You are a professional software engineer. All code must follow best practices: a 3. Type Safety First: TypeScript interfaces for all props, state, return types 4. Predictable State: Zustand for global state, useState for UI-only concerns +### Application Operation Boundary + +- Every protected read, write, canonical resource lookup, or authorization-sensitive reference resolution enters through an authorized application use case. +- Define one stable semantic operation with its minimum role, workspace-key policy, allowed principal kinds, and delegated services. Internal APIs, v2 APIs, Copilot, and trusted tools call the same use case when the domain behavior is the same. +- Surface adapters authenticate and construct a `Principal`, apply request-rate policy, parse contracts, map input, and present results. They never query protected data, decide resource authorization, implement business transactions, or record semantic audit. +- Application use cases load canonical context, compare asserted scope, authorize current access, execute managers/repositories, project semantic audit, and trigger shared domain effects. Managers accept canonical IDs and scope, never credentials or principals. +- Copilot is a surface adapter. Use `createCopilotApplicationAdapter` and the domain's registered operation object; do not create Copilot-only authorization or business implementations. +- Protected compound mutations belong in one top-level semantic application operation. Do not sequence independently committing mutations in a route or tool adapter. +- Never substitute a billing owner, uploader, creator, or API-key owner for the acting principal. Fail fast when the identity model or operation policy cannot express the caller. +- Use the `migrate-application-operation` skill whenever creating or migrating a protected endpoint, tool command, or resource method. + ### Root Structure ``` @@ -169,56 +180,49 @@ const provider = config as unknown as LegacyProvider ## API Route Pattern -Every API route handler must be wrapped with `withRouteHandler`. This sets up `AsyncLocalStorage`-based request context so all loggers in the request lifecycle automatically include the request ID. +Every route method must run inside `withRouteHandler`. Ordinary internal and v2 JSON/binary routes use `defineInternalJsonRoute`, `defineV2JsonRoute`, or the matching binary/stream builder. These builders already apply `withRouteHandler`; never wrap them again. Use raw `withRouteHandler` only for explicit protocol or lifecycle exceptions such as streaming, multipart control, large-body admission, OAuth, or public execution. -Routes never `import { z } from 'zod'` and never define route-local boundary schemas. They consume the contract from `@/lib/api/contracts/**` and validate with canonical helpers from `@/lib/api/server`: +Routes never `import { z } from 'zod'` and never define route-local boundary schemas. Declarative builders consume contracts and own authentication, admission, parsing, use-case execution, response validation, and error projection. A raw special route consumes the same contracts and validates with canonical helpers from `@/lib/api/server`, after authentication and cheap admission: - `parseRequest(contract, request, context, options?)` — fully contract-bound routes; parses params, query, body, and headers in one call. Pass `{}` for `context` on routes without route params, or the route's `context` argument when route params exist. Returns a discriminated union; check `parsed.success` and return `parsed.response` on failure - `validationErrorResponse(error)` and `getValidationErrorMessage(error, fallback)` — produce 400 responses from a `ZodError` - `validationErrorResponseFromError(error)` — when handling unknown caught errors that may or may not be a `ZodError` - `isZodError(error)` — type guard. Routes never use `instanceof z.ZodError` -### Fully contract-bound route (`parseRequest`) +### Ordinary authorized JSON route ```typescript -import { createLogger } from '@sim/logger' -import type { NextRequest } from 'next/server' -import { NextResponse } from 'next/server' -import { createFolderContract } from '@/lib/api/contracts/folders' -import { parseRequest } from '@/lib/api/server' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' - -const logger = createLogger('FoldersAPI') - -export const POST = withRouteHandler(async (request: NextRequest) => { - const parsed = await parseRequest(createFolderContract, request, {}) - if (!parsed.success) return parsed.response - const { body } = parsed.data - logger.info('Creating folder', { workspaceId: body.workspaceId }) - return NextResponse.json({ ok: true }) +export const PATCH = defineInternalJsonRoute({ + contract: renameWidgetContract, + auth: internalSessionAuth, + operation: widgetOperations.rename, + rateLimit: internalRateLimits.none({ reason: 'Preserve existing internal behavior' }), + errorPolicy: internalWidgetErrorPolicy, + mapInput: ({ params, body }) => ({ + widgetId: params.widgetId, + assertedWorkspaceId: params.workspaceId, + name: body.name, + }), + useCase: renameWidget, + present: ({ widget }) => ({ success: true, widget }), }) ``` -### Composing with other middleware - -```typescript -export const POST = withRouteHandler(withAdminAuth(async (request) => { - return NextResponse.json({ ok: true }) -})) -``` +The contract, operation, and use case must agree at definition time. Authentication and request-rate admission happen before parsing; canonical loading and authorization happen in the application use case. The presenter returns only the surface success body. Routes under `apps/sim/app/api/v1/**` use the shared middleware in `apps/sim/app/api/v1/middleware.ts` for auth, rate-limit, and workspace access. Compose contract validation inside that middleware — never reimplement auth/rate-limit per-route. -Never export a bare `async function GET/POST/...` — always use `export const METHOD = withRouteHandler(...)`. +Never export a bare `async function GET/POST/...`. Export the result of a shared builder or, for a documented special route, `withRouteHandler(...)`. ### Adding a new boundary feature end-to-end When adding a new route + client surface, follow this order. Each step has one place it lives. 1. **Author the contract first** in `apps/sim/lib/api/contracts/.ts` (or a subdirectory for large domains: `knowledge/`, `selectors/`, `tools/`). Define one schema per request slice (`params`, `query`, `body`, `headers`) and one for the response, then wrap with `defineRouteContract`. Export named type aliases (`z.input` for inputs, `z.output` for outputs). -2. **Implement the route** in `apps/sim/app/api//route.ts`. Auth always runs **before** `parseRequest` — never validate untrusted input before authenticating the caller. The route returns exactly the shape declared in `contract.response.schema`. -3. **Add the React Query hook** in `apps/sim/hooks/queries/.ts`. Use `requestJson(contract, input)` for the call. Build a hierarchical query-key factory (`all` → `lists()` → `list(workspaceId)` → `details()` → `detail(id)`) so invalidations can target prefixes. -4. **Use the hook in the component**. The mutation's `data` and `error` are fully typed from the contract; surface `error.message` (already extracted from the response body's `error` or `message` field by `requestJson`). +2. **Define the semantic operation and application use case** under `apps/sim/lib//application/`. The use case owns canonical loading, asserted-scope checks, current authorization, business behavior, semantic audit, and shared domain effects. +3. **Implement the route adapter** in `apps/sim/app/api//route.ts` with the appropriate shared builder. Declare auth, operation, rate policy, error policy, input mapping, use case, and presenter. Auth always runs **before** parsing. Use raw `withRouteHandler` only for an explicit special route, and keep protected work in application use cases. +4. **Add the React Query hook** in `apps/sim/hooks/queries/.ts`. Use `requestJson(contract, input)` for the call. Build a hierarchical query-key factory (`all` → `lists()` → `list(workspaceId)` → `details()` → `detail(id)`) so invalidations can target prefixes. +5. **Use the hook in the component**. The mutation's `data` and `error` are fully typed from the contract; surface `error.message` (already extracted from the response body's `error` or `message` field by `requestJson`). ### Schema review checklist (read the contract diff like a DB migration) @@ -490,4 +494,3 @@ For the full authoring instructions — SubBlock property tables, `condition`/`d Table column types are registry entries in `apps/sim/lib/table/column-types/` — one file per type owning its label, icon, storage cast, coercion, validation, conversion compatibility, formatting, and editor. `Record` on `registry.ts` and `registry.server.ts` is a compile-time completeness gate: adding a type to the union errors until both entries exist. Never add a `case 'sometype':` outside `column-types/` — a missing arm fails silently (a wrong `jsonbCast` breaks every filter on the column). If a consumer needs per-type knowledge, add a registry field. Use `/add-column-type` for the full procedure. - diff --git a/README.md b/README.md index 03fc399473e..5967bfa7c12 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ Manage your install with `bun run sim`: ```bash bun run sim start | stop | restart # bring your install up / down / cycle +bun run sim update # pull/rebuild and apply Compose images bun run sim status # what's installed and healthy bun run sim logs # follow logs bun run sim doctor # diagnose configuration problems diff --git a/apps/desktop/README.md b/apps/desktop/README.md index ee0d772730a..eab15594e1f 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -35,6 +35,7 @@ src/main/ # main process (bundled to dist/main.cjs) browser-sites/ # imported site directory, safeStorage at rest browser-import/ # one-shot import of profiles, cookies and passwords src/preload/ # contextBridge IPC bridge (bundled to dist/preload.cjs) +native/ # Node-API/AppKit bridge for native macOS Help docs search static/ # bundled local pages (offline.html) e2e/ # Playwright _electron smoke suite ``` @@ -168,7 +169,7 @@ Raw local file bytes are never exposed through the preload bridge and cannot be ## Auto-update, channels, rollout, rollback - `electron-updater` reads the GitHub Releases feed (`publish` is pinned to `simstudioai/sim`); deltas via `.zip.blockmap`. Install is prompt-based (Restart Now / Later; Later installs on quit) — never forced mid-session. -- Channels: stable builds (`X.Y.Z`) follow `latest`; `-beta.N` builds follow `beta` (never attach a beta `latest-mac.yml` to a stable tag). +- Streams: production follows stable `X.Y.Z` releases, dev follows `-dev.N`, and staging follows `-staging.N`. The feed still recognizes legacy `-alpha.N`/`-beta.N` releases during migration. - Staged rollout: after publishing, edit `stagingPercentage: 10` into the release's `latest-mac.yml`, then raise as crash metrics stay clean. - Rollback: a pulled release must be superseded by a **higher** version — users on the broken build will not reinstall an equal one. (A blocked-versions kill-switch was removed as unwired dead code; reintroduce it in `updater.ts` if a remote config source ever exists to feed it.) - Ship the DMG and tell users to install to `/Applications` — App Translocation breaks Squirrel.Mac updates from quarantined paths. diff --git a/apps/desktop/build/dmg-background.png b/apps/desktop/build/dmg-background.png new file mode 100644 index 00000000000..a79f4414535 Binary files /dev/null and b/apps/desktop/build/dmg-background.png differ diff --git a/apps/desktop/build/dmg-background@2x.png b/apps/desktop/build/dmg-background@2x.png new file mode 100644 index 00000000000..2b0f9170246 Binary files /dev/null and b/apps/desktop/build/dmg-background@2x.png differ diff --git a/apps/desktop/build/entitlements.mac.plist b/apps/desktop/build/entitlements.mac.plist index 446fe171da8..152179c4392 100644 --- a/apps/desktop/build/entitlements.mac.plist +++ b/apps/desktop/build/entitlements.mac.plist @@ -4,5 +4,10 @@ com.apple.security.cs.allow-jit + + com.apple.security.device.audio-input + diff --git a/apps/desktop/electron-builder.yml b/apps/desktop/electron-builder.yml index c13ceaaa30c..f7e226725e1 100644 --- a/apps/desktop/electron-builder.yml +++ b/apps/desktop/electron-builder.yml @@ -13,11 +13,12 @@ files: asar: true -# Native modules cannot be dlopen'd from inside an asar. -# `scripts/ensure-pty-prebuilds.ts` guarantees both arch prebuilds are present -# before packaging; see mac.x64ArchFiles for how the universal merge treats them. +# Native modules cannot be dlopen'd from inside an asar. The Help-search addon +# is already universal; `scripts/ensure-pty-prebuilds.ts` guarantees both pty +# prebuilds are present before packaging. asarUnpack: - "**/node_modules/@lydell/node-pty-*/prebuilds/**" + - "**/dist/native/*.node" # Space-free regardless of productName ("Sim Dev" etc.): GitHub rewrites # asset names containing spaces, which would desync the electron-updater @@ -54,12 +55,32 @@ mac: mergeASARs: false hardenedRuntime: true gatekeeperAssess: false + # macOS refuses to show the microphone prompt at all — it kills the process — + # unless the bundle declares why it wants the device. + extendInfo: + NSMicrophoneUsageDescription: Sim uses your microphone for voice input in Chat. entitlements: build/entitlements.mac.plist entitlementsInherit: build/entitlements.mac.plist notarize: true dmg: sign: false + title: ${productName} + # Finder uses the image dimensions as the installer window dimensions. The + # @2x companion is detected automatically and keeps the mountain artwork and + # install arrow sharp on Retina displays. + background: build/dmg-background.png + iconSize: 96 + iconTextSize: 13 + # Keep both icon centers inside the compact 660x420 Finder canvas. + contents: + - x: 165 + y: 210 + type: file + - x: 495 + y: 210 + type: link + path: /Applications # node-pty ships pure N-API prebuilds, which are ABI-stable across Node and # Electron versions, so there is nothing to rebuild against Electron's ABI. diff --git a/apps/desktop/native/help-search.mm b/apps/desktop/native/help-search.mm new file mode 100644 index 00000000000..b82e044e12c --- /dev/null +++ b/apps/desktop/native/help-search.mm @@ -0,0 +1,439 @@ +#import +#import + +#include + +#include +#include +#include +#include +#include + +constexpr NSInteger kMaximumResultCount = 20; +constexpr NSUInteger kMaximumQueryLength = 256; +constexpr NSUInteger kMaximumResponseLength = 512 * 1024; +constexpr NSUInteger kMaximumDisplayTextLength = 240; +constexpr int64_t kSearchDebounceNanoseconds = 200 * NSEC_PER_MSEC; + +/** + * This native trust anchor intentionally duplicates the JS-configured origin: + * caller input must not widen where Help can fetch or open results. + */ +static NSString* const kDocumentationBaseURL = @"https://docs.sim.ai/"; +static NSString* const kDocumentationHost = @"docs.sim.ai"; +static NSString* const kDocumentationSearchPath = @"/api/search"; + +static bool IsTrustedDocumentationURL(NSURL* URL) { + NSURLComponents* components = + URL ? [NSURLComponents componentsWithURL:URL resolvingAgainstBaseURL:NO] : nil; + return components && [components.scheme.lowercaseString isEqualToString:@"https"] && + [components.host.lowercaseString isEqualToString:kDocumentationHost] && + components.user == nil && components.password == nil && components.port == nil; +} + +static bool IsTrustedSearchEndpoint(NSURL* URL, bool allowsQuery) { + if (!IsTrustedDocumentationURL(URL)) { + return false; + } + NSURLComponents* components = + [NSURLComponents componentsWithURL:URL resolvingAgainstBaseURL:NO]; + return [components.path isEqualToString:kDocumentationSearchPath] && + components.fragment == nil && (allowsQuery || components.query == nil); +} + +static NSString* SanitizeDisplayText(id rawText) { + if (![rawText isKindOfClass:[NSString class]]) { + return nil; + } + NSString* text = [[(NSString*)rawText + componentsSeparatedByCharactersInSet:[NSCharacterSet controlCharacterSet]] + componentsJoinedByString:@" "]; + text = [text + stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + if (text.length > kMaximumDisplayTextLength) { + NSRange safeRange = [text + rangeOfComposedCharacterSequencesForRange:NSMakeRange(0, kMaximumDisplayTextLength)]; + text = [text substringWithRange:safeRange]; + } + return text.length > 0 ? text : nil; +} + +@interface SIMDocumentationHelpItem : NSObject + +@property(nonatomic, copy) NSArray* localizedTitles; +@property(nonatomic, strong) NSURL* URL; + +- (instancetype)initWithLocalizedTitles:(NSArray*)localizedTitles URL:(NSURL*)URL; + +@end + +@implementation SIMDocumentationHelpItem + +- (instancetype)initWithLocalizedTitles:(NSArray*)localizedTitles URL:(NSURL*)URL { + self = [super init]; + if (self) { + _localizedTitles = [localizedTitles copy]; + _URL = URL; + } + return self; +} + +@end + +@interface SIMDocumentationHelpSearchProvider + : NSObject + +- (instancetype)initWithEndpoint:(NSURL*)endpoint; +- (void)invalidate; + +@end + + +@interface SIMDocumentationHelpSearchProvider () + +@property(nonatomic, strong) NSURL* endpoint; +@property(nonatomic, strong, nullable) NSURLSession* session; +@property(nonatomic, strong, nullable) NSURLSessionDataTask* currentTask; +@property(nonatomic) NSUInteger searchGeneration; + +@end + + +@implementation SIMDocumentationHelpSearchProvider + +- (instancetype)initWithEndpoint:(NSURL*)endpoint { + self = [super init]; + if (self) { + _endpoint = endpoint; + + NSURLSessionConfiguration* configuration = + [NSURLSessionConfiguration ephemeralSessionConfiguration]; + configuration.requestCachePolicy = NSURLRequestReloadIgnoringLocalCacheData; + configuration.timeoutIntervalForRequest = 8; + configuration.timeoutIntervalForResource = 10; + configuration.HTTPMaximumConnectionsPerHost = 1; + configuration.URLCache = nil; + configuration.HTTPCookieStorage = nil; + configuration.URLCredentialStorage = nil; + configuration.HTTPShouldSetCookies = NO; + configuration.HTTPAdditionalHeaders = @{ + @"Accept" : @"application/json", + @"User-Agent" : @"Sim Desktop Help Search" + }; + _session = [NSURLSession sessionWithConfiguration:configuration + delegate:self + delegateQueue:nil]; + } + return self; +} + +- (void)invalidate { + NSURLSession* session = nil; + @synchronized(self) { + self.searchGeneration += 1; + [self.currentTask cancel]; + self.currentTask = nil; + session = self.session; + self.session = nil; + } + [session invalidateAndCancel]; +} + +- (void)URLSession:(NSURLSession*)session + task:(NSURLSessionTask*)task + willPerformHTTPRedirection:(NSHTTPURLResponse*)response + newRequest:(NSURLRequest*)request + completionHandler:(void (^)(NSURLRequest* _Nullable))completionHandler { + (void)session; + (void)task; + (void)response; + (void)request; + completionHandler(nil); +} + +- (void)searchForItemsWithSearchString:(NSString*)searchString + resultLimit:(NSInteger)resultLimit + matchedItemHandler: + (void (^)(NSArray* items))handleMatchedItems { + void (^handleNoMatches)(void) = ^{ + handleMatchedItems(@[]); + }; + NSString* query = [searchString + stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + if (query.length == 0) { + handleNoMatches(); + return; + } + if (resultLimit <= 0) { + handleNoMatches(); + return; + } + if (query.length > kMaximumQueryLength) { + NSRange safeRange = [query + rangeOfComposedCharacterSequencesForRange:NSMakeRange(0, kMaximumQueryLength)]; + query = [query substringWithRange:safeRange]; + } + + NSInteger boundedLimit = + std::clamp(resultLimit, static_cast(1), kMaximumResultCount); + __block NSUInteger generation; + @synchronized(self) { + self.searchGeneration += 1; + generation = self.searchGeneration; + [self.currentTask cancel]; + self.currentTask = nil; + } + + dispatch_after( + dispatch_time(DISPATCH_TIME_NOW, kSearchDebounceNanoseconds), + dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{ + @synchronized(self) { + if (generation != self.searchGeneration) { + return; + } + } + + NSURLComponents* components = + [NSURLComponents componentsWithURL:self.endpoint resolvingAgainstBaseURL:NO]; + components.queryItems = @[ + [NSURLQueryItem queryItemWithName:@"query" value:query], + [NSURLQueryItem queryItemWithName:@"locale" value:@"en"], + [NSURLQueryItem queryItemWithName:@"limit" + value:[NSString stringWithFormat:@"%ld", + boundedLimit]], + ]; + NSURL* requestURL = components.URL; + if (!requestURL) { + handleNoMatches(); + return; + } + + __block __weak NSURLSessionDataTask* task = nil; + void (^completionHandler)(NSData*, NSURLResponse*, NSError*) = + ^(NSData* data, NSURLResponse* response, NSError* error) { + @synchronized(self) { + if (generation != self.searchGeneration || self.currentTask != task) { + return; + } + self.currentTask = nil; + } + + if (error || !data || data.length > kMaximumResponseLength) { + handleNoMatches(); + return; + } + + NSHTTPURLResponse* httpResponse = + [response isKindOfClass:[NSHTTPURLResponse class]] + ? (NSHTTPURLResponse*)response + : nil; + if (!httpResponse || httpResponse.statusCode < 200 || + httpResponse.statusCode >= 300 || + !IsTrustedSearchEndpoint(httpResponse.URL, true)) { + handleNoMatches(); + return; + } + + NSError* jsonError = nil; + id payload = [NSJSONSerialization JSONObjectWithData:data options:0 error:&jsonError]; + if (jsonError || ![payload isKindOfClass:[NSArray class]]) { + handleNoMatches(); + return; + } + + NSMutableArray* items = [NSMutableArray array]; + for (id rawResult in (NSArray*)payload) { + if (items.count >= static_cast(boundedLimit)) { + break; + } + if (![rawResult isKindOfClass:[NSDictionary class]]) { + continue; + } + + NSDictionary* result = (NSDictionary*)rawResult; + id rawURL = result[@"url"]; + if (![rawURL isKindOfClass:[NSString class]]) { + continue; + } + + NSString* title = SanitizeDisplayText(result[@"content"]); + if (!title) { + continue; + } + + NSURL* resultURL = [NSURL URLWithString:(NSString*)rawURL + relativeToURL:[NSURL URLWithString:kDocumentationBaseURL]]; + resultURL = resultURL.absoluteURL; + if (!IsTrustedDocumentationURL(resultURL)) { + continue; + } + + NSMutableArray* localizedTitles = + [NSMutableArray arrayWithObject:@"Sim Documentation"]; + id rawBreadcrumbs = result[@"breadcrumbs"]; + if ([rawBreadcrumbs isKindOfClass:[NSArray class]]) { + for (id rawBreadcrumb in (NSArray*)rawBreadcrumbs) { + if (localizedTitles.count >= 5) { + break; + } + NSString* breadcrumb = SanitizeDisplayText(rawBreadcrumb); + if (breadcrumb && ![breadcrumb isEqualToString:title] && + ![localizedTitles containsObject:breadcrumb]) { + [localizedTitles addObject:breadcrumb]; + } + } + } + [localizedTitles addObject:title]; + + [items addObject:[[SIMDocumentationHelpItem alloc] + initWithLocalizedTitles:localizedTitles + URL:resultURL]]; + } + + handleMatchedItems(items); + }; + + @synchronized(self) { + if (generation != self.searchGeneration || !self.session) { + return; + } + task = [self.session dataTaskWithURL:requestURL + completionHandler:completionHandler]; + self.currentTask = task; + } + [task resume]; + }); +} + +- (NSArray*)localizedTitlesForItem:(id)item { + if (![item isKindOfClass:[SIMDocumentationHelpItem class]]) { + return @[]; + } + SIMDocumentationHelpItem* result = (SIMDocumentationHelpItem*)item; + return result.localizedTitles; +} + +- (void)performActionForItem:(id)item { + if (![item isKindOfClass:[SIMDocumentationHelpItem class]]) { + return; + } + SIMDocumentationHelpItem* result = (SIMDocumentationHelpItem*)item; + [[NSWorkspace sharedWorkspace] openURL:result.URL]; +} + +@end + +static __strong SIMDocumentationHelpSearchProvider* gProvider = nil; +static std::atomic gOwnerEnvironment{nullptr}; + +static bool ReadStringArgument(napi_env env, + napi_callback_info info, + std::string* value) { + size_t argument_count = 1; + napi_value arguments[1]; + if (napi_get_cb_info(env, info, &argument_count, arguments, nullptr, nullptr) != napi_ok || + argument_count != 1) { + napi_throw_type_error(env, nullptr, "install expects one endpoint URL"); + return false; + } + + napi_valuetype type; + if (napi_typeof(env, arguments[0], &type) != napi_ok || type != napi_string) { + napi_throw_type_error(env, nullptr, "endpoint URL must be a string"); + return false; + } + + size_t length = 0; + if (napi_get_value_string_utf8(env, arguments[0], nullptr, 0, &length) != napi_ok) { + napi_throw_type_error(env, nullptr, "could not read endpoint URL"); + return false; + } + std::vector buffer(length + 1); + if (napi_get_value_string_utf8(env, arguments[0], buffer.data(), buffer.size(), &length) != + napi_ok) { + napi_throw_type_error(env, nullptr, "could not read endpoint URL"); + return false; + } + value->assign(buffer.data(), length); + return true; +} + +static void UnregisterProvider() { + @autoreleasepool { + if (!gProvider) { + gOwnerEnvironment.store(nullptr); + return; + } + [gProvider invalidate]; + [NSApp unregisterUserInterfaceItemSearchHandler:gProvider]; + gProvider = nil; + gOwnerEnvironment.store(nullptr); + } +} + +static void UnregisterProviderOnMainThread() { + if ([NSThread isMainThread]) { + UnregisterProvider(); + return; + } + dispatch_sync(dispatch_get_main_queue(), ^{ + UnregisterProvider(); + }); +} + +static napi_value BooleanResult(napi_env env, bool value) { + napi_value result; + napi_get_boolean(env, value, &result); + return result; +} + +static napi_value Install(napi_env env, napi_callback_info info) { + @autoreleasepool { + std::string endpoint_string; + if (!ReadStringArgument(env, info, &endpoint_string)) { + return nullptr; + } + + NSString* endpoint_text = + [[NSString alloc] initWithBytes:endpoint_string.data() + length:endpoint_string.size() + encoding:NSUTF8StringEncoding]; + NSURL* endpoint = endpoint_text ? [NSURL URLWithString:endpoint_text] : nil; + if (!IsTrustedSearchEndpoint(endpoint, false) || NSApp == nil || + ![NSThread isMainThread]) { + return BooleanResult(env, false); + } + + UnregisterProvider(); + gProvider = [[SIMDocumentationHelpSearchProvider alloc] initWithEndpoint:endpoint]; + gOwnerEnvironment.store(env); + [NSApp registerUserInterfaceItemSearchHandler:gProvider]; + return BooleanResult(env, true); + } +} + +static napi_value Uninstall(napi_env env, napi_callback_info info) { + (void)info; + if (env == gOwnerEnvironment.load()) { + UnregisterProviderOnMainThread(); + } + napi_value undefined; + napi_get_undefined(env, &undefined); + return undefined; +} + +static void Cleanup(void* data) { + if (data == gOwnerEnvironment.load()) { + UnregisterProviderOnMainThread(); + } +} + +NAPI_MODULE_INIT() { + napi_property_descriptor properties[] = { + {"install", nullptr, Install, nullptr, nullptr, nullptr, napi_default, nullptr}, + {"uninstall", nullptr, Uninstall, nullptr, nullptr, nullptr, napi_default, nullptr}, + }; + napi_define_properties(env, exports, std::size(properties), properties); + napi_add_env_cleanup_hook(env, Cleanup, env); + return exports; +} diff --git a/apps/desktop/scripts/build.ts b/apps/desktop/scripts/build.ts index f60371058c9..f2b553661d2 100644 --- a/apps/desktop/scripts/build.ts +++ b/apps/desktop/scripts/build.ts @@ -1,4 +1,6 @@ -import { cpSync, rmSync } from 'node:fs' +import { execFileSync } from 'node:child_process' +import { cpSync, existsSync, mkdirSync, rmSync } from 'node:fs' +import { dirname, join } from 'node:path' import { build } from 'esbuild' import { identityForOrigin } from './channels' @@ -30,6 +32,52 @@ rmSync(generatedIcon, { force: true, recursive: true }) cpSync(appIcon, generatedIcon, { recursive: true }) console.log(`• Selecting desktop icon: ${appIcon}`) +function compileNativeHelpSearch(): void { + const outputDirectory = 'dist/native' + rmSync(outputDirectory, { force: true, recursive: true }) + if (process.platform !== 'darwin') return + + const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { + encoding: 'utf8', + }).trim() + const nodeIncludeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node') + const nodeApiHeader = join(nodeIncludeDirectory, 'node_api.h') + if (!existsSync(nodeApiHeader)) { + throw new Error(`Could not find Node-API headers at ${nodeApiHeader}`) + } + + mkdirSync(outputDirectory, { recursive: true }) + execFileSync( + 'xcrun', + [ + 'clang++', + '-std=c++17', + '-DNAPI_VERSION=8', + '-fobjc-arc', + '-fblocks', + '-bundle', + '-undefined', + 'dynamic_lookup', + '-mmacosx-version-min=12.0', + '-arch', + 'arm64', + '-arch', + 'x86_64', + '-I', + nodeIncludeDirectory, + '-framework', + 'AppKit', + '-framework', + 'Foundation', + '-o', + join(outputDirectory, 'help-search.node'), + 'native/help-search.mm', + ], + { stdio: 'inherit' } + ) + console.log('• Compiled native macOS documentation Help search') +} + const common = { bundle: true, platform: 'node' as const, @@ -47,6 +95,7 @@ const common = { } async function run(): Promise { + compileNativeHelpSearch() if (watch) { const { context } = await import('esbuild') const mainCtx = await context({ diff --git a/apps/desktop/src/main/browser-agent/cdp.ts b/apps/desktop/src/main/browser-agent/cdp.ts index 02f4fe944d0..4a7088e1b24 100644 --- a/apps/desktop/src/main/browser-agent/cdp.ts +++ b/apps/desktop/src/main/browser-agent/cdp.ts @@ -36,6 +36,8 @@ export interface CdpCallbacks { const callbacksByContents = new WeakMap() /** Contents already instrumented (attach survives for the tab's lifetime). */ const instrumented = new WeakSet() +/** Tracks trusted input currently being dispatched by the agent itself. */ +const agentInputDepthByContents = new WeakMap() /** Flattened CDP child-target sessions keyed by their protocol frame/target id. */ const childSessionsByContents = new WeakMap>() const FRAME_WORLD_NAME = 'sim-browser-agent' @@ -62,6 +64,7 @@ async function sendInput( method: string, params: Record ): Promise { + agentInputDepthByContents.set(contents, (agentInputDepthByContents.get(contents) ?? 0) + 1) let timer: NodeJS.Timeout | undefined const timeout = new Promise((_resolve, reject) => { timer = setTimeout( @@ -73,9 +76,17 @@ async function sendInput( await Promise.race([send(contents, method, params), timeout]) } finally { clearTimeout(timer) + const nextDepth = (agentInputDepthByContents.get(contents) ?? 1) - 1 + if (nextDepth > 0) agentInputDepthByContents.set(contents, nextDepth) + else agentInputDepthByContents.delete(contents) } } +/** Distinguishes user input from CDP input when Electron mirrors it as an event. */ +export function isDispatchingAgentInput(contents: WebContents): boolean { + return (agentInputDepthByContents.get(contents) ?? 0) > 0 +} + /** Idempotently instruments a tab's WebContents. */ export async function ensureInstrumented(contents: WebContents, cb: CdpCallbacks): Promise { callbacksByContents.set(contents, cb) diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index 33875151bf8..f98083e1add 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -144,6 +144,234 @@ describe('executeTool', () => { expect(second.result).toMatchObject({ tabs: [] }) }) + it('keeps a takeover pending when the clock advances beyond twelve hours', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + const now = vi.spyOn(Date, 'now').mockReturnValue(0) + try { + let settled = false + const takeover = driver + .executeTool('chat-test', 'browser_request_takeover', { + reason: 'Please finish in the browser', + }) + .then((result) => { + settled = true + return result + }) + + await vi.advanceTimersByTimeAsync(0) + now.mockReturnValue(13 * 60 * 60 * 1000) + await vi.advanceTimersByTimeAsync(1_500) + expect(settled).toBe(false) + + await driver.handlePanelAction('chat-test', { action: 'takeover-done' }) + await vi.advanceTimersByTimeAsync(1_500) + await expect(takeover).resolves.toMatchObject({ + ok: true, + result: { completed: true }, + }) + } finally { + now.mockRestore() + vi.useRealTimers() + } + }) + + it('cancels the exact takeover and clears its attention state immediately', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const takeover = driver.executeTool( + 'chat-test', + 'browser_request_takeover', + { reason: 'Please finish in the browser' }, + 'tool-takeover' + ) + await vi.advanceTimersByTimeAsync(0) + expect(session.getTabsState().automationNeedsAttention).toBe(true) + + expect(driver.cancelTool('chat-test', 'tool-takeover')).toBe(true) + expect(session.getTabsState().automationNeedsAttention).toBe(false) + await vi.advanceTimersByTimeAsync(1_500) + + await expect(takeover).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled'), + }) + await expect( + driver.executeTool('chat-test', 'browser_list_tabs', {}, 'tool-takeover') + ).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled before it started'), + }) + } finally { + vi.useRealTimers() + } + }) + + it('does not let cancelled takeover cleanup clear a newer takeover', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const firstTakeover = driver.executeTool( + 'chat-test', + 'browser_request_takeover', + { reason: 'First handoff' }, + 'tool-takeover-first' + ) + await vi.advanceTimersByTimeAsync(0) + + expect(driver.cancelTool('chat-test', 'tool-takeover-first')).toBe(true) + await expect(firstTakeover).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled'), + }) + + const secondTakeover = driver.executeTool( + 'chat-test', + 'browser_request_takeover', + { reason: 'Second handoff' }, + 'tool-takeover-second' + ) + await vi.advanceTimersByTimeAsync(0) + expect(session.getTabsState().automationNeedsAttention).toBe(true) + + // Let the detached first takeover observe cancellation and run finally. + await vi.advanceTimersByTimeAsync(1_500) + expect(session.getTabsState().automationNeedsAttention).toBe(true) + + await driver.handlePanelAction('chat-test', { action: 'takeover-done' }) + await vi.advanceTimersByTimeAsync(1_500) + await expect(secondTakeover).resolves.toMatchObject({ + ok: true, + result: { completed: true }, + }) + } finally { + vi.useRealTimers() + } + }) + + it('honors cancellation that arrives before the authorized tool invocation', async () => { + expect(driver.cancelTool('chat-test', 'tool-before-authorization')).toBe(true) + + await expect( + driver.executeTool('chat-test', 'browser_list_tabs', {}, 'tool-before-authorization') + ).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled before it started'), + }) + }) + + it('settles native automation activity immediately when an active tool is cancelled', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const waiting = driver.executeTool( + 'chat-test', + 'browser_wait_for', + { timeoutMs: 120_000 }, + 'tool-waiting' + ) + await vi.advanceTimersByTimeAsync(0) + expect(session.getTabsState().automationActive).toBe(true) + + expect(driver.cancelActiveTool('chat-test')).toBe(true) + await vi.advanceTimersByTimeAsync(0) + expect(session.getTabsState().automationActive).toBe(false) + await expect(waiting).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled'), + }) + } finally { + vi.useRealTimers() + } + }) + + it('cancels queued pre-boundary tools while allowing later browser work', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const waiting = driver.executeTool( + 'chat-test', + 'browser_wait_for', + { timeoutMs: 120_000 }, + 'tool-active' + ) + await vi.advanceTimersByTimeAsync(0) + const queuedOpen = driver.executeTool('chat-test', 'browser_open_tab', {}, 'tool-queued') + + expect(driver.cancelActiveTool('chat-test')).toBe(true) + + await expect(waiting).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled'), + }) + await expect(queuedOpen).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled before it started'), + }) + expect(session.getTabsState().tabs).toHaveLength(1) + + await expect( + driver.executeTool('chat-test', 'browser_open_tab', {}, 'tool-after-boundary') + ).resolves.toMatchObject({ ok: true }) + expect(session.getTabsState().tabs).toHaveLength(2) + } finally { + vi.useRealTimers() + } + }) + + it('releases an abandoned takeover when a newer browser action arrives', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const takeover = driver.executeTool('chat-test', 'browser_request_takeover', { + reason: 'Please finish in the browser', + }) + await vi.advanceTimersByTimeAsync(0) + + const listTabs = driver.executeTool('chat-test', 'browser_list_tabs', {}) + await vi.advanceTimersByTimeAsync(1_500) + + await expect(takeover).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('superseded by a newer browser action'), + }) + await expect(listTabs).resolves.toMatchObject({ + ok: true, + result: { tabs: expect.any(Array) }, + }) + } finally { + vi.useRealTimers() + } + }) + + it('returns a free-text takeover instruction to the browser agent', async () => { + await driver.executeTool('chat-test', 'browser_open_tab', {}) + vi.useFakeTimers() + try { + const takeover = driver.executeTool('chat-test', 'browser_request_takeover', { + reason: 'Please pick a match in the draw', + }) + await vi.advanceTimersByTimeAsync(0) + + await driver.handlePanelAction('chat-test', { + action: 'takeover-done', + takeoverResponse: 'Open the second match', + }) + await vi.advanceTimersByTimeAsync(1_500) + + await expect(takeover).resolves.toMatchObject({ + ok: true, + result: { + completed: true, + userInstruction: 'Open the second match', + }, + }) + } finally { + vi.useRealTimers() + } + }) + it('publishes a settled tab when the main frame finishes before subresources', async () => { const onPageState = vi.fn() const onTabsState = vi.fn() @@ -200,6 +428,74 @@ describe('executeTool', () => { expect(refreshAvailability).toHaveBeenCalledWith(true) }) + it('keeps target-blank initiation user-owned while automation is active', async () => { + driver = freshDriver() + await driver.executeTool('chat-test', 'browser_open_tab', {}) + const source = session.requireTab().view.webContents + session.setAutomationActive(true) + const beforeMouse = (source.on as unknown as ReturnType).mock.calls.find( + ([eventName]) => eventName === 'before-mouse-event' + )?.[1] as (event: unknown, mouse: { type: string }) => void + beforeMouse({}, { type: 'mouseDown' }) + const openWindow = vi.mocked(source.setWindowOpenHandler).mock.calls[0]?.[0] as (details: { + url: string + }) => { action: string } + + expect(openWindow({ url: 'https://user-popup.example/' })).toEqual({ action: 'deny' }) + const popup = session.activeTab()?.view.webContents + if (!popup) throw new Error('Expected user popup tab') + expect(session.automationTab()?.view.webContents).toBe(source) + expect(popup.loadURL).toHaveBeenCalledWith('https://user-popup.example/') + }) + + it('keeps agent-opened target-blank tabs agent-owned after dispatch ends', async () => { + driver = freshDriver() + await driver.executeTool('chat-test', 'browser_open_tab', {}) + const source = session.requireTab().view.webContents + session.setAutomationActive(true) + const openWindow = vi.mocked(source.setWindowOpenHandler).mock.calls[0]?.[0] as (details: { + url: string + }) => { action: string } + + expect(openWindow({ url: 'https://agent-popup.example/' })).toEqual({ action: 'deny' }) + const popup = session.requireAutomationTab().view.webContents + expect(session.activeTab()?.view.webContents).toBe(source) + session.setAutomationActive(false) + expect(session.automationTab()?.view.webContents).toBe(popup) + expect(popup.loadURL).toHaveBeenCalledWith('https://agent-popup.example/') + }) + + it('keeps context-menu new tabs user-owned while automation is active', async () => { + driver = freshDriver() + await driver.executeTool('chat-test', 'browser_open_tab', {}) + const source = session.requireTab().view.webContents + session.setAutomationActive(true) + vi.mocked(Menu.buildFromTemplate).mockClear() + const contextMenu = (source.on as unknown as ReturnType).mock.calls.find( + ([eventName]) => eventName === 'context-menu' + )?.[1] as (event: unknown, params: unknown) => void + contextMenu( + {}, + { + selectionText: '', + linkURL: 'https://context-link.example/', + isEditable: false, + editFlags: { canPaste: false }, + } + ) + const template = vi.mocked(Menu.buildFromTemplate).mock.calls.at(-1)?.[0] as + | MenuItemConstructorOptions[] + | undefined + template + ?.find((item) => item.label === 'Open Link in New Tab') + ?.click?.({} as never, undefined as never, {} as never) + + const popup = session.activeTab()?.view.webContents + if (!popup) throw new Error('Expected context-menu tab') + expect(session.automationTab()?.view.webContents).toBe(source) + expect(popup.loadURL).toHaveBeenCalledWith('https://context-link.example/') + }) + it('builds the native toolbar menu and routes renderer-owned actions back to its chat', async () => { await driver.executeTool('chat-test', 'browser_open_tab', {}) const win = new BrowserWindow() diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 33b1584572a..8d3674cbd73 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -72,7 +72,6 @@ const NAVIGATION_SETTLE_MS = 400 const DEFAULT_WAIT_FOR_TIMEOUT_MS = 10_000 const MAX_WAIT_FOR_TIMEOUT_MS = 120_000 const TAKEOVER_POLL_MS = 1_500 -const TAKEOVER_MAX_MS = 12 * 60 * 60 * 1000 /** * Hard ceiling on any single tool execution (takeover excepted): whatever * goes wrong, the Sim side always gets a response. Sits above the longest @@ -114,6 +113,17 @@ interface DriverScopeState { pendingNotices: string[] takeoverActive: boolean takeoverDone: boolean + takeoverResponse: string | null + /** Invocation that currently owns the shared takeover response state. */ + takeoverInvocationEpoch: number | null + /** Monotonic browser-tool invocation id used to supersede an abandoned takeover. */ + toolInvocationEpoch: number + /** Exact client tool currently at the head of this scope's serialized queue. */ + activeToolCallId: string | null + /** Rejects the active queue entry while its underlying Chromium work winds down. */ + activeToolCancel: (() => void) | null + /** Invalidates every invocation already queued when a scope-level cancel establishes a boundary. */ + toolQueueCancellationEpoch: number lastTabsStateFingerprint: string | null toolQueue: Promise /** True while activation is the only operation that has touched this scope. */ @@ -130,11 +140,23 @@ interface DriverScopeState { toolExecutionEpoch: number } +/** Captures the native queue boundary before an async authorization round trip. */ +export interface BrowserToolQueueBoundary { + scopeId: string + cancellationEpoch: number +} + function createDriverScopeState(): DriverScopeState { return { pendingNotices: [], takeoverActive: false, takeoverDone: false, + takeoverResponse: null, + takeoverInvocationEpoch: null, + toolInvocationEpoch: 0, + activeToolCallId: null, + activeToolCancel: null, + toolQueueCancellationEpoch: 0, lastTabsStateFingerprint: null, toolQueue: Promise.resolve(), activationOnly: true, @@ -154,6 +176,22 @@ function invalidateSnapshot(state = driverScopeState()): void { const driverScopeStates = new Map() const driverScopeAliases = new Map() +const CANCELLED_TOOL_TTL_MS = 5 * 60_000 +const MAX_CANCELLED_TOOL_TOMBSTONES = 256 +const cancelledToolCallIds = new Map() + +function pruneCancelledToolCallIds(now = Date.now()): void { + for (const [toolCallId, expiresAt] of cancelledToolCallIds) { + if (expiresAt > now && cancelledToolCallIds.size <= MAX_CANCELLED_TOOL_TOMBSTONES) break + cancelledToolCallIds.delete(toolCallId) + } +} + +function isToolCallCancelled(toolCallId: string | undefined): boolean { + if (!toolCallId) return false + pruneCancelledToolCallIds() + return cancelledToolCallIds.has(toolCallId) +} function resolveDriverScopeId(scopeId: string): string { let resolved = scopeId @@ -175,6 +213,19 @@ function driverScopeState(scopeId = session.getBrowserScopeId()): DriverScopeSta return state } +export function captureBrowserToolQueueBoundary(scopeId: string): BrowserToolQueueBoundary { + const resolvedScopeId = resolveDriverScopeId(scopeId) + return { + scopeId: resolvedScopeId, + cancellationEpoch: driverScopeState(resolvedScopeId).toolQueueCancellationEpoch, + } +} + +function isBrowserToolQueueBoundaryCurrent(boundary: BrowserToolQueueBoundary): boolean { + const state = driverScopeStates.get(resolveDriverScopeId(boundary.scopeId)) + return state?.toolQueueCancellationEpoch === boundary.cancellationEpoch +} + function recordNotice(notice: string): void { const state = driverScopeState() state.activationOnly = false @@ -182,8 +233,8 @@ function recordNotice(notice: string): void { } /** - * True while browser_request_takeover waits on the user. The Done chip on the - * chat's takeover tool row completes it via the `takeover-done` panel action; + * True while browser_request_takeover waits on the user. The question card on + * the chat's takeover tool row completes it via the `takeover-done` panel action; * the state lives here (session-level, not in the page) so it survives * navigations and tab switches. */ @@ -261,7 +312,7 @@ function instrumentTab(contents: WebContents): void { contents.on( 'did-navigate', inScope(() => { - if (session.activeTab()?.view.webContents === contents) { + if (session.automationTab()?.view.webContents === contents) { invalidateSnapshot() } knownSessions?.noteTopLevelNavigation(contents.getURL()) @@ -281,7 +332,7 @@ function instrumentTab(contents: WebContents): void { inScope(() => { if ( event === 'did-navigate-in-page' && - session.activeTab()?.view.webContents === contents + session.automationTab()?.view.webContents === contents ) { invalidateSnapshot() } @@ -309,6 +360,7 @@ export function initDriver( // first tab push as a duplicate. driverScopeStates.clear() driverScopeAliases.clear() + cancelledToolCallIds.clear() // The serialization chain, too. A takeover from the previous session can sit // unresolved indefinitely, and its `takeoverDone` flag is reset above — so // leaving the old chain head in place would queue the new session's first @@ -667,7 +719,7 @@ async function execInPage( )) as Result } if ('frameTreeNodeId' in target && typeof target.frameTreeNodeId === 'number') { - const contents = session.activeTab()?.view.webContents + const contents = session.automationTab()?.view.webContents const frame = target as WebFrameMain if ( !contents || @@ -967,7 +1019,7 @@ async function activeElementState(target: PageExecutionTarget): Promise { const state = driverScopeState() - const tab = session.requireTab() + const tab = session.requireAutomationTab() if (tab.view.webContents !== contents) { throw new ToolError('The active tab changed before the snapshot started. Try again.') } @@ -1473,7 +1526,7 @@ async function captureSnapshot(contents: WebContents, notAfter?: number): Promis const targets = new Map() const targetLineIndexes = new Map() const stillCurrent = (): boolean => { - const active = session.activeTab() + const active = session.automationTab() return ( state.snapshotCaptureEpoch === captureEpoch && active?.id === capturedTabId && @@ -1627,41 +1680,58 @@ async function captureSnapshot(contents: WebContents, notAfter?: number): Promis /** * Hands control to the user: the page is already natively interactive in the - * panel, and the chat's takeover tool row shows the reason with a Done chip. - * The tool resolves when that chip sends the `takeover-done` panel action. + * panel, and the chat's takeover tool row shows the reason as a question. + * The tool resolves when that question sends the `takeover-done` panel action. * Nothing is injected into the page, so nothing covers page content and the * pending state survives navigations. */ -async function runTakeover(purpose: string | undefined): Promise { - const tab = session.ensureTab() +async function runTakeover(purpose: string | undefined, invocationEpoch: number): Promise { + const tab = session.ensureAutomationTab() const contents = tab.view.webContents const state = driverScopeState() state.takeoverActive = true state.takeoverDone = false + state.takeoverResponse = null + state.takeoverInvocationEpoch = invocationEpoch + session.setAutomationNeedsAttention(true) const startedAt = Date.now() try { - while (Date.now() - startedAt < TAKEOVER_MAX_MS) { + for (;;) { await sleep(TAKEOVER_POLL_MS) if (!session.hasSession() || contents.isDestroyed()) { throw new ToolError( 'The browser session was closed during takeover. Ask the user what happened, then reopen with browser_navigate.' ) } + if (state.toolInvocationEpoch !== invocationEpoch) { + throw new ToolError('The browser takeover was superseded by a newer browser action.') + } if (state.takeoverDone) { if (purpose === 'sign_in') { - const activeContents = session.activeTab()?.view.webContents + const activeContents = session.automationTab()?.view.webContents if (activeContents && !activeContents.isDestroyed()) { knownSessions?.noteSignInCompleted(activeContents.getURL()) } } - return { completed: true, elapsedMs: Date.now() - startedAt } + return { + completed: true, + elapsedMs: Date.now() - startedAt, + ...(state.takeoverResponse ? { userInstruction: state.takeoverResponse } : {}), + } } } - throw new ToolError('Takeover timed out after 12 hours without the user finishing.') } finally { - state.takeoverActive = false - state.takeoverDone = false + // Cancellation releases the serialized tool queue before this polling + // loop observes its superseding epoch. Never let that delayed cleanup + // erase a newer takeover that has already claimed the same scope state. + if (state.takeoverInvocationEpoch === invocationEpoch) { + session.setAutomationNeedsAttention(false) + state.takeoverActive = false + state.takeoverDone = false + state.takeoverResponse = null + state.takeoverInvocationEpoch = null + } } } @@ -1669,7 +1739,8 @@ async function executeToolInner( tool: BrowserToolName, params: Record, assertCurrentExecution: () => void, - executionDeadline?: number + executionDeadline: number | undefined, + invocationEpoch: number ): Promise { switch (tool) { case 'browser_navigate': { @@ -1685,7 +1756,7 @@ async function executeToolInner( throw new ToolError(guard.error ?? 'That address was blocked.') } assertCurrentExecution() - const tab = session.ensureTab() + const tab = session.ensureAutomationTab() const contents = tab.view.webContents assertCurrentExecution() return await loadUrlAndGetResult(contents, url) @@ -1702,7 +1773,7 @@ async function executeToolInner( throw new ToolError(guard.error ?? 'That address was blocked.') } assertCurrentExecution() - const tab = session.ensureTab() + const tab = session.ensureAutomationTab() const contents = tab.view.webContents assertCurrentExecution() const nav = await loadUrlAndGetResult(contents, url) @@ -1718,7 +1789,7 @@ async function executeToolInner( case 'browser_go_back': case 'browser_go_forward': { invalidateSnapshot() - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const history = contents.navigationHistory assertCurrentExecution() let completion: Promise @@ -1746,7 +1817,7 @@ async function executeToolInner( } } assertCurrentExecution() - const tab = session.addTab() + const tab = session.addAutomationTab() const contents = tab.view.webContents if (url) { assertCurrentExecution() @@ -1758,7 +1829,7 @@ async function executeToolInner( case 'browser_switch_tab': { invalidateSnapshot() - const tab = session.switchTab(requireStr(params, 'tabId')) + const tab = session.switchAutomationTab(requireStr(params, 'tabId')) const contents = tab.view.webContents return { tabId: tab.id, url: contents.getURL(), title: contents.getTitle() } } @@ -1766,13 +1837,13 @@ async function executeToolInner( case 'browser_close_tab': { invalidateSnapshot() const tabId = requireStr(params, 'tabId') - session.closeTab(tabId) + session.closeAutomationTab(tabId) return { closed: tabId } } case 'browser_list_tabs': { session.restoreBrowserSession() - return session.getTabsState() + return session.getAutomationTabsState() } case 'browser_list_sessions': { @@ -1790,10 +1861,10 @@ async function executeToolInner( await sleep(timeoutMs) return { waitedMs: timeoutMs } } - const waitedTab = session.requireTab() + const waitedTab = session.requireAutomationTab() const contents = waitedTab.view.webContents while (Date.now() - startedAt < timeoutMs) { - const active = session.activeTab() + const active = session.automationTab() if (active?.id !== waitedTab.id || active.view.webContents !== contents) { throw new ToolError( 'The active tab changed while waiting. Start browser_wait_for again on the tab you want to inspect.' @@ -1832,13 +1903,13 @@ async function executeToolInner( } case 'browser_snapshot': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents assertCurrentExecution() return await captureSnapshot(contents, executionDeadline) } case 'browser_read_text': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const elementId = num(params, 'elementId') if (elementId === undefined) return await readWholePageText(contents, executionDeadline) const target = pageTargetForElement(contents, elementId) @@ -1848,7 +1919,7 @@ async function executeToolInner( } case 'browser_screenshot': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const dataUrl = await cdp.captureScreenshot(contents).catch(() => null) if (dataUrl === null) { throw new ToolError( @@ -1866,13 +1937,13 @@ async function executeToolInner( case 'browser_extract': { const instruction = requireStr(params, 'instruction') - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const page = await readWholePageText(contents, executionDeadline) return { instruction, page } } case 'browser_click': { - const clickedTab = session.requireTab() + const clickedTab = session.requireAutomationTab() const contents = clickedTab.view.webContents const elementId = requireNum(params, 'elementId') const target = pageTargetForElement(contents, elementId) @@ -2136,7 +2207,7 @@ async function executeToolInner( const topObservation = observeTopPage ? pageEffect(beforeTopPage, afterTopPage, beforeTopElement, afterTopElement) : observation - const activeTab = session.activeTab() + const activeTab = session.automationTab() const tabChanged = activeTab?.id !== clickedTab.id const effect = { ...observation.effect, @@ -2207,7 +2278,7 @@ async function executeToolInner( const elementId = requireNum(params, 'elementId') const text = requireStr(params, 'text') const submit = params.submit === true - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const target = pageTargetForElement(contents, elementId) const targetFrame = frameExecutionTarget(target, contents) @@ -2519,7 +2590,7 @@ async function executeToolInner( case 'browser_press_key': { const requestedKey = requireStr(params, 'key') const combo = parseKeyCombo(requestedKey) - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const pressedPageUrl = contents.getURL() let target: PageExecutionTarget = focusedPageTarget(contents) let pressedFrameUrl = target === contents ? undefined : (target as WebFrameMain).url @@ -2692,7 +2763,7 @@ async function executeToolInner( } case 'browser_scroll': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const elementId = num(params, 'elementId') const target = elementId !== undefined @@ -2728,7 +2799,7 @@ async function executeToolInner( } case 'browser_select_option': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const elementId = requireNum(params, 'elementId') const target = pageTargetForElement(contents, elementId) const targetFrame = frameExecutionTarget(target, contents) @@ -2793,7 +2864,7 @@ async function executeToolInner( } case 'browser_hover': { - const contents = session.requireTab().view.webContents + const contents = session.requireAutomationTab().view.webContents const elementId = requireNum(params, 'elementId') const target = pageTargetForElement(contents, elementId) const targetFrame = frameExecutionTarget(target, contents) @@ -2942,7 +3013,7 @@ async function executeToolInner( // The reason renders in the chat's tool row, not here — but require it // so the model always tells the user why control was handed over. requireStr(params, 'reason') - return await runTakeover(str(params, 'purpose')) + return await runTakeover(str(params, 'purpose'), invocationEpoch) } default: { @@ -2971,7 +3042,9 @@ function withNotices(result: unknown): unknown { export async function executeTool( scopeId: string, tool: BrowserToolName, - params: Record + params: Record, + toolCallId?: string, + authorizationBoundary?: BrowserToolQueueBoundary ): Promise<{ ok: boolean; result?: unknown; error?: string }> { const resolvedScopeId = resolveDriverScopeId(scopeId) if (session.isBrowserScopeSuspended(resolvedScopeId)) { @@ -2982,7 +3055,22 @@ export async function executeTool( } const state = driverScopeState(resolvedScopeId) state.activationOnly = false + const invocationEpoch = ++state.toolInvocationEpoch + const queueCancellationEpoch = state.toolQueueCancellationEpoch const run = async () => { + if ( + (authorizationBoundary && !isBrowserToolQueueBoundaryCurrent(authorizationBoundary)) || + queueCancellationEpoch !== state.toolQueueCancellationEpoch || + isToolCallCancelled(toolCallId) + ) { + throw new ToolError('This browser action was cancelled before it started.') + } + state.activeToolCallId = toolCallId ?? null + let cancelActiveExecution: () => void = () => {} + const cancellation = new Promise((_resolve, reject) => { + cancelActiveExecution = () => reject(new ToolError('This browser action was cancelled.')) + }) + state.activeToolCancel = cancelActiveExecution return await session.withBrowserScope(resolvedScopeId, async () => { logger.info('Executing browser tool', { tool, scopeId: resolvedScopeId }) const keepHiddenPageActive = tool !== 'browser_request_takeover' @@ -2998,21 +3086,31 @@ export async function executeTool( throw new ToolError('This browser action expired before it could dispatch input.') } } - const execution = executeToolInner(tool, params, assertCurrentExecution, executionDeadline) - return withNotices( - await (watchdogMs === null + const execution = executeToolInner( + tool, + params, + assertCurrentExecution, + executionDeadline, + invocationEpoch + ) + const guardedExecution = + watchdogMs === null ? execution : raceAgainstWatchdog(execution, watchdogMs, () => { if (state.toolExecutionEpoch === executionEpoch) state.toolExecutionEpoch++ if (tool === 'browser_snapshot' || tool === 'browser_open_url') { invalidateSnapshot(state) } - })) - ) + }) + return withNotices(await Promise.race([guardedExecution, cancellation])) } finally { if (keepHiddenPageActive) { session.setAutomationActive(false) } + if (state.activeToolCancel === cancelActiveExecution) { + state.activeToolCallId = null + state.activeToolCancel = null + } } }) } @@ -3034,6 +3132,40 @@ export async function executeTool( } } +/** + * Cancels one exact browser tool, including a cancellation that arrives while + * its authorization IPC is still in flight. The bounded tombstone lets that + * later invocation observe the stop without retaining call ids indefinitely. + */ +export function cancelTool(scopeId: string, toolCallId: string): boolean { + pruneCancelledToolCallIds() + cancelledToolCallIds.set(toolCallId, Date.now() + CANCELLED_TOOL_TTL_MS) + pruneCancelledToolCallIds() + + const resolvedScopeId = resolveDriverScopeId(scopeId) + const state = driverScopeStates.get(resolvedScopeId) + if (!state || state.activeToolCallId !== toolCallId) return true + + state.toolInvocationEpoch++ + state.toolExecutionEpoch++ + state.activeToolCancel?.() + void session.withBrowserScope(resolvedScopeId, () => { + session.setAutomationActive(false) + session.setAutomationNeedsAttention(false) + }) + return true +} + +/** Cancels the active tool and every older invocation already queued for this scope. */ +export function cancelActiveTool(scopeId: string): boolean { + const resolvedScopeId = resolveDriverScopeId(scopeId) + const state = driverScopeStates.get(resolvedScopeId) + if (!state) return false + state.toolQueueCancellationEpoch++ + const toolCallId = state.activeToolCallId + return toolCallId ? cancelTool(resolvedScopeId, toolCallId) : false +} + /** Browser-chrome commands from the panel header; fire-and-forget. */ export async function handlePanelAction( scopeId: string, @@ -3042,11 +3174,18 @@ export async function handlePanelAction( const resolvedScopeId = resolveDriverScopeId(scopeId) if (session.isBrowserScopeSuspended(resolvedScopeId)) return return await session.withBrowserScope(resolvedScopeId, async () => { - // The Done chip on the chat's takeover tool row: hands control back to the + // The question card on the chat's takeover tool row hands control back to the // agent. Meaningful only while a takeover is actually waiting. if (action.action === 'takeover-done') { const state = driverScopeState() - if (state.takeoverActive) state.takeoverDone = true + if (state.takeoverActive) { + session.returnAutomationTabToAgent() + state.takeoverResponse = + typeof action.takeoverResponse === 'string' && action.takeoverResponse.trim() + ? action.takeoverResponse.trim() + : null + state.takeoverDone = true + } return } // Navigate bootstraps the session: the user can open the panel manually @@ -3054,6 +3193,7 @@ export async function handlePanelAction( // bar. The other chrome actions need an existing page. if (action.action === 'navigate') { if (typeof action.url === 'string' && /^https?:\/\//i.test(action.url)) { + session.claimActiveTabForUser() const contents = session.ensureTab().view.webContents void contents.loadURL(action.url).catch(() => {}) } @@ -3081,6 +3221,7 @@ export async function handlePanelAction( } return } + session.claimActiveTabForUser() const tab = session.activeTab() if (!tab) return const contents = tab.view.webContents diff --git a/apps/desktop/src/main/browser-agent/panel.test.ts b/apps/desktop/src/main/browser-agent/panel.test.ts index 2ddb6720e85..4af126c5269 100644 --- a/apps/desktop/src/main/browser-agent/panel.test.ts +++ b/apps/desktop/src/main/browser-agent/panel.test.ts @@ -74,6 +74,7 @@ describe('panel chat scope', () => { const scopeId = panel.getActivePanelScopeId() vi.mocked(view.setVisible).mockClear() vi.mocked(view.setBounds).mockClear() + vi.mocked(view.webContents.invalidate).mockClear() await expect(panel.capturePanelSnapshot(win, scopeId)).resolves.toEqual({ dataUrl: 'data:image/png;base64,c2lt', @@ -97,9 +98,27 @@ describe('panel chat scope', () => { expect(panel.setPanelOccluded(false, win, scopeId)).toBe(true) expect(view.setVisible).toHaveBeenLastCalledWith(true) + expect(view.webContents.invalidate).toHaveBeenCalledOnce() expect(view.setBounds).not.toHaveBeenCalled() }) + it('requests a fresh compositor frame when a browser view is attached or revealed', () => { + const { win, view } = showPanel(panel) + + expect(view.webContents.invalidate).toHaveBeenCalledOnce() + + panel.setPanelBounds(null, win) + expect(view.setVisible).toHaveBeenLastCalledWith(false) + expect(view.webContents.invalidate).toHaveBeenCalledOnce() + + panel.setPanelBounds(PANEL_RECT, win) + expect(view.setVisible).toHaveBeenLastCalledWith(true) + expect(view.webContents.invalidate).toHaveBeenCalledTimes(2) + + panel.setPanelBounds(PANEL_RECT, win) + expect(view.webContents.invalidate).toHaveBeenCalledTimes(2) + }) + it('reports the exact applied native rectangle in renderer viewport coordinates', async () => { const { win, view } = showPanel(panel) const scopeId = panel.getActivePanelScopeId() diff --git a/apps/desktop/src/main/browser-agent/panel.ts b/apps/desktop/src/main/browser-agent/panel.ts index 905c13554c3..992f2f2b669 100644 --- a/apps/desktop/src/main/browser-agent/panel.ts +++ b/apps/desktop/src/main/browser-agent/panel.ts @@ -411,6 +411,9 @@ export function layout(): void { if (lastAppliedVisibility !== visible) { lastAppliedVisibility = visible active.view.setVisible(visible) + if (visible && !active.view.webContents.isDestroyed()) { + active.view.webContents.invalidate() + } } } diff --git a/apps/desktop/src/main/browser-agent/session.test.ts b/apps/desktop/src/main/browser-agent/session.test.ts index 4147f0cdc29..5e96931f06f 100644 --- a/apps/desktop/src/main/browser-agent/session.test.ts +++ b/apps/desktop/src/main/browser-agent/session.test.ts @@ -28,6 +28,7 @@ interface MockView { getTitle: ReturnType close: ReturnType focus: ReturnType + invalidate: ReturnType isFocused: ReturnType isDestroyed: ReturnType isLoading: ReturnType @@ -929,11 +930,12 @@ describe('browser-agent session', () => { expect(session.listTabs()[0].tabId).not.toBe(blankTabId) session.setPanelFocused(false) + panel.setPanelBounds(null) expect(session.handleFocusedShortcut('close-tab')).toBe(false) expect(session.listTabs()).toHaveLength(1) }) - it('treats renderer browser chrome as browser focus', () => { + it('keeps close-tab routed to a visible browser through a transient focus loss', () => { panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) const first = session.requireTab() const second = session.addTab() @@ -945,17 +947,38 @@ describe('browser-agent session', () => { expect(session.listTabs()[0].tabId).not.toBe(second.id) session.setPanelFocused(false) + expect(session.handleFocusedShortcut('close-tab')).toBe(true) + expect(session.listTabs()).toHaveLength(1) + + panel.setPanelBounds(null) expect(session.handleFocusedShortcut('close-tab')).toBe(false) }) - it('opens a tab from the shared resource shortcut while browser chrome owns focus', () => { + it('keeps browser tab shortcuts routed while the visible panel has no DOM focus', () => { panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) session.requireTab() - session.setPanelFocused(true) + session.setPanelFocused(false) const before = session.listTabs().length expect(session.handleFocusedShortcut('new-tab')).toBe(true) expect(session.listTabs()).toHaveLength(before + 1) + expect(session.handleFocusedShortcut('close-tab')).toBe(true) + expect(session.listTabs()).toHaveLength(before) + expect(session.handleFocusedShortcut('reopen-closed-tab')).toBe(true) + expect(session.listTabs()).toHaveLength(before + 1) + + vi.mocked(win.webContents.send).mockClear() + expect(session.handleFocusedShortcut('focus-omnibox')).toBe(true) + expect(win.webContents.send).toHaveBeenCalledWith( + 'browser-agent:focus-omnibox', + 'select', + session.getBrowserScopeId() + ) + + panel.setPanelBounds(null) + expect(session.handleFocusedShortcut('new-tab')).toBe(false) + expect(session.handleFocusedShortcut('reopen-closed-tab')).toBe(false) + expect(session.handleFocusedShortcut('focus-omnibox')).toBe(false) }) it('reloads only the focused browser tab', () => { @@ -1037,7 +1060,7 @@ describe('browser-agent session', () => { expect(activeContents.setBackgroundThrottling).toHaveBeenLastCalledWith(true) }) - it('moves the automation exemption to whichever tab becomes active', () => { + it('moves the automation exemption with the agent cursor, not visible selection', () => { const first = session.ensureTab() const second = session.addTab() session.switchTab(first.id) @@ -1048,7 +1071,7 @@ describe('browser-agent session', () => { firstContents.setBackgroundThrottling.mockClear() secondContents.setBackgroundThrottling.mockClear() - session.switchTab(second.id) + session.switchAutomationTab(second.id) // The old active tab is re-throttled, the new one exempted — otherwise a // mid-tool switch would strand the wake on a tab the agent left behind. @@ -1056,6 +1079,99 @@ describe('browser-agent session', () => { expect(secondContents.setBackgroundThrottling).toHaveBeenLastCalledWith(false) }) + it('keeps the user-visible tab selected while the agent opens and switches background tabs', () => { + const first = session.ensureTab() + const visible = session.addTab() + + session.switchAutomationTab(first.id) + const background = session.addAutomationTab() + + expect(session.getTabsState().activeTabId).toBe(visible.id) + expect(session.getTabsState().automationTabId).toBe(background.id) + expect(session.getTabsState().tabs.find((tab) => tab.active)?.tabId).toBe(visible.id) + }) + + it('refuses to let automation close a visible tab claimed by the user', () => { + const visible = session.ensureTab() + session.switchTab(visible.id) + + expect(() => session.closeAutomationTab(visible.id)).toThrow('currently being used by the user') + expect(session.getTabsState().activeTabId).toBe(visible.id) + }) + + it('keeps agent actions on a tab after the user selects it', () => { + const visible = session.ensureTab() + session.switchTab(visible.id) + + const agent = session.ensureAutomationTab() + + expect(agent.id).toBe(visible.id) + expect(session.getTabsState().activeTabId).toBe(visible.id) + expect(session.getTabsState().automationTabId).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + }) + + it('keeps agent actions on a tab after a toolbar action claims it', () => { + const visible = session.ensureTab() + + expect(session.claimActiveTabForUser()?.id).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + + const agent = session.ensureAutomationTab() + expect(agent.id).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + }) + + it('does not treat a passive native focus event as user takeover', () => { + const visible = session.ensureTab() + const contents = (visible.view as unknown as MockView).webContents + const focusListener = contents.on.mock.calls.find( + ([eventName]) => eventName === 'focus' + )?.[1] as (() => void) | undefined + + focusListener?.() + + expect(session.ensureAutomationTab().id).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + }) + + it('keeps automation on the same tab after real page interaction', () => { + const visible = session.ensureTab() + const contents = (visible.view as unknown as MockView).webContents + const beforeMouse = contents.on.mock.calls.find( + ([eventName]) => eventName === 'before-mouse-event' + )?.[1] as ((event: unknown, input: { type: string }) => void) | undefined + + beforeMouse?.({}, { type: 'mouseDown' }) + + expect(session.ensureAutomationTab().id).toBe(visible.id) + expect(session.getTabsState().activeTabId).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + }) + + it('clears automation indicators instead of moving them when their tab closes', () => { + const target = session.ensureAutomationTab() + session.setAutomationActive(true) + session.setAutomationNeedsAttention(true) + + session.closeTab(target.id) + + expect(session.getTabsState()).toMatchObject({ + automationActive: false, + automationNeedsAttention: false, + }) + }) + + it('resumes takeover in the same tab after the user explicitly hands it back', () => { + const visible = session.ensureTab() + session.switchTab(visible.id) + + session.returnAutomationTabToAgent() + + expect(session.ensureAutomationTab().id).toBe(visible.id) + expect(session.listTabs()).toHaveLength(1) + }) + it('updates the native backdrop when Sim changes browser theme', () => { const tab = session.ensureTab() const view = tab.view as unknown as MockView @@ -1149,6 +1265,7 @@ describe('browser-agent session', () => { expect(contents?.focus).toHaveBeenCalled() session.setPanelFocused(false) + panel.setPanelBounds(null) expect(session.handleFocusedShortcut('reopen-closed-tab')).toBe(false) }) @@ -1305,14 +1422,17 @@ describe('browser-agent session', () => { win as unknown as { contentView: { removeChildView: ReturnType } } ).contentView.removeChildView view.setVisible.mockClear() + view.webContents.invalidate.mockClear() panel.setPanelBounds(null) expect(view.setVisible).toHaveBeenCalledWith(false) + expect(view.webContents.invalidate).not.toHaveBeenCalled() expect(removeChildView).not.toHaveBeenCalled() // Showing it again reuses the attached view rather than re-adding it. content.addChildView.mockClear() panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) expect(view.setVisible).toHaveBeenLastCalledWith(true) + expect(view.webContents.invalidate).toHaveBeenCalledOnce() expect(content.addChildView).not.toHaveBeenCalled() }) @@ -1336,6 +1456,7 @@ describe('browser-agent session', () => { // two native views stacked in the window would composite over each other. expect(content.removeChildView).toHaveBeenCalledWith(first.view) expect(content.addChildView).toHaveBeenCalledWith(second.view) + expect(second.view.webContents.invalidate).toHaveBeenCalledOnce() }) // The measured report is the sole writer of bounds. A main-process @@ -1581,6 +1702,48 @@ describe('browser-agent session', () => { expect(contents.loadURL).not.toHaveBeenCalled() }) + it('keeps agent popups in the background and context-menu links user-owned', () => { + const onTabCreated = vi.fn() + session = freshSession(win, { onTabCreated }) + const sourceTab = session.ensureTab() + const source = (sourceTab.view as unknown as MockView).webContents + onTabCreated.mockClear() + session.setAutomationActive(true) + + const openWindow = source.setWindowOpenHandler.mock.calls[0]?.[0] as (details: { + url: string + }) => { action: string } + openWindow({ url: 'https://agent-popup.example/' }) + const agentPopup = session.automationTab() + expect(agentPopup).not.toBeNull() + expect(session.activeTab()).toBe(sourceTab) + expect(onTabCreated).toHaveBeenLastCalledWith(agentPopup?.view.webContents) + + const contextMenu = source.on.mock.calls.find( + ([eventName]) => eventName === 'context-menu' + )?.[1] as (event: unknown, params: unknown) => void + contextMenu( + {}, + { + selectionText: '', + linkURL: 'https://context-link.example/', + isEditable: false, + editFlags: { canPaste: false }, + } + ) + const template = vi.mocked(Menu.buildFromTemplate).mock.calls.at(-1)?.[0] as + | MenuItemConstructorOptions[] + | undefined + template + ?.find((item) => item.label === 'Open Link in New Tab') + ?.click?.({} as never, undefined as never, {} as never) + + const userTab = session.activeTab() + expect(userTab).not.toBe(sourceTab) + expect(session.automationTab()).toBe(agentPopup) + expect(onTabCreated).toHaveBeenLastCalledWith(userTab?.view.webContents) + }) + it('blocks controlled pages from moving or resizing the desktop window', () => { const tab = session.ensureTab() const contents = (tab.view as unknown as MockView).webContents diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index b8fe5fef5af..3d1b1589e14 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -37,6 +37,7 @@ import { shell, WebContentsView, } from 'electron' +import { isDispatchingAgentInput } from '@/main/browser-agent/cdp' import { attachAgentContextMenu, BASE_ZOOM_FACTOR, @@ -63,7 +64,12 @@ import { } from '@/main/browser-agent/url-guard' import type { BrowserSessionSnapshot } from '@/main/desktop-chat-session-store' import { suggestedFilename, uniqueDownloadPath } from '@/main/downloads' -import { type FocusedResourceShortcut, zoomActionForShortcut } from '@/main/resource-shortcuts' +import { + type FocusedResourceShortcut, + isResourceTabSelectionShortcut, + resourceTabTargetIndex, + zoomActionForShortcut, +} from '@/main/resource-shortcuts' const logger = createLogger('BrowserAgentSession') @@ -170,6 +176,8 @@ interface BrowserScopeState { tabs: AgentTab[] recentlyClosedTabUrls: string[] activeTabId: string | null + automationTabId: string | null + visibleTabUserSelected: boolean nextTabId: number /** True until anything beyond scope activation inspects or materializes this state. */ activationOnly: boolean @@ -179,6 +187,7 @@ interface BrowserScopeState { focusedBrowserTabId: string | null focusedBrowserClearTimer: ReturnType | null automationActive: boolean + automationNeedsAttention: boolean /** * Tab a find is currently running on. Tracked because the find outlives the * call that started it — Chromium keeps the highlights until it is told to @@ -195,6 +204,8 @@ function createBrowserScopeState(): BrowserScopeState { tabs: [], recentlyClosedTabUrls: [], activeTabId: null, + automationTabId: null, + visibleTabUserSelected: false, nextTabId: 1, activationOnly: true, restored: false, @@ -203,6 +214,7 @@ function createBrowserScopeState(): BrowserScopeState { focusedBrowserTabId: null, focusedBrowserClearTimer: null, automationActive: false, + automationNeedsAttention: false, findingTabId: null, findingRequestId: null, } @@ -542,6 +554,7 @@ function isActivationOnlyBrowserScope(scopeId: string): boolean { state.tabs.length === 0 && state.recentlyClosedTabUrls.length === 0 && state.activeTabId === null && + state.automationTabId === null && state.nextTabId === 1 && !state.restored && !state.restoring @@ -1048,10 +1061,10 @@ export function stopFindInActiveTab(focusPage: boolean): void { * inside the browser resource rather than spawn a native window, and both are * reached from an untrusted page, so the scheme is checked here once. */ -function openTabWithUrl(url: string): void { +function openTabWithUrl(url: string, agentOwned: boolean): void { if (!/^https?:\/\//i.test(url)) return try { - const tab = addTab() + const tab = agentOwned ? addAutomationTab() : addTab() void tab.view.webContents.loadURL(url).catch(() => {}) } catch (error) { logger.warn('Could not open a link in a new browser tab', { @@ -1090,7 +1103,7 @@ function createTabView(): WebContentsView { configureAgentPartition(contents.session) attachAgentContextMenu(contents, { addToChat: (text) => withBrowserScope(scopeId, () => addPageSelectionToChat(contents, text)), - openTab: (url) => withBrowserScope(scopeId, () => openTabWithUrl(url)), + openTab: (url) => withBrowserScope(scopeId, () => openTabWithUrl(url, false)), defaultZoomFactor: getBrowserDefaultZoomFactor, }) @@ -1105,6 +1118,19 @@ function createTabView(): WebContentsView { currentScope.focusedBrowserTabId = tab?.id ?? currentScope.activeTabId }) ) + contents.on( + 'before-mouse-event', + bindToBrowserScope(scopeId, (_event, mouse) => { + if ( + isDispatchingAgentInput(contents) || + !['mouseDown', 'contextMenu', 'mouseWheel'].includes(mouse.type) + ) { + return + } + const tab = tabs.find((entry) => entry.view.webContents === contents) + if (tab?.id === currentScope.activeTabId) currentScope.visibleTabUserSelected = true + }) + ) contents.on( 'blur', bindToBrowserScope(scopeId, () => { @@ -1131,7 +1157,7 @@ function createTabView(): WebContentsView { // Keep popups inside the browser resource: http(s) window.open and // target=_blank requests become a new internal tab, never a native window. contents.setWindowOpenHandler((details) => { - withBrowserScope(scopeId, () => openTabWithUrl(details.url)) + withBrowserScope(scopeId, () => openTabWithUrl(details.url, agentOwnsPopupFrom(contents))) return { action: 'deny' } }) @@ -1163,6 +1189,10 @@ function createTabView(): WebContentsView { contents.on( 'before-input-event', bindToBrowserScope(scopeId, (event, input) => { + const tab = tabs.find((entry) => entry.view === view) + if (!isDispatchingAgentInput(contents) && tab?.id === currentScope.activeTabId) { + currentScope.visibleTabUserSelected = true + } const shortcut = browserShortcutForInput(input) if (!shortcut) return @@ -1181,7 +1211,6 @@ function createTabView(): WebContentsView { return } - const tab = tabs.find((entry) => entry.view === view) if (tab) closeTabFromUser(tab.id) }) ) @@ -1284,8 +1313,16 @@ export function hasSession(): boolean { * touches it, and network loading is not throttled anyway. */ export function setAutomationActive(active: boolean): void { + if (currentScope.automationActive === active) return currentScope.automationActive = active applyActiveTabThrottling() + events?.onTabsChanged() +} + +export function setAutomationNeedsAttention(needsAttention: boolean): void { + if (currentScope.automationNeedsAttention === needsAttention) return + currentScope.automationNeedsAttention = needsAttention + events?.onTabsChanged() } /** @@ -1296,11 +1333,18 @@ export function setAutomationActive(active: boolean): void { function applyActiveTabThrottling(): void { for (const tab of tabs) { if (tab.view.webContents.isDestroyed()) continue - const exempt = currentScope.automationActive && tab.id === currentScope.activeTabId + const exempt = currentScope.automationActive && tab.id === currentScope.automationTabId tab.view.webContents.setBackgroundThrottling(!exempt) } } +/** A closed target must not transfer its activity marker to a replacement tab. */ +function clearAutomationIndicatorsForTab(tabId: string): void { + if (currentScope.automationTabId !== tabId) return + currentScope.automationActive = false + currentScope.automationNeedsAttention = false +} + function browserBackgroundColor(): string { const dark = browserTheme === 'dark' || (browserTheme === 'system' && nativeTheme.shouldUseDarkColors) @@ -1430,6 +1474,7 @@ function addTabInternal({ pinned, } insertPinnedAware(tab) + if (currentScope.automationTabId === null) currentScope.automationTabId = tab.id if (activate || currentScope.activeTabId === null) { currentScope.activeTabId = tab.id applyActiveTabThrottling() @@ -1444,6 +1489,21 @@ function addTabInternal({ return tab } +/** Marks the visible page as user-selected without blocking automation on it. */ +export function claimActiveTabForUser(): AgentTab | null { + const tab = activeTab() + if (!tab) return null + currentScope.visibleTabUserSelected = true + return tab +} + +/** Explicit hand-back after takeover lets automation resume in the same page. */ +export function returnAutomationTabToAgent(): void { + if (currentScope.activeTabId === currentScope.automationTabId) { + currentScope.visibleTabUserSelected = false + } +} + export function restoreBrowserSession(): void { if (isBrowserScopeSuspended(getBrowserScopeId())) { throw new SessionError('This task browser is suspended until the task is reopened.') @@ -1481,6 +1541,7 @@ export function restoreBrowserSession(): void { } } currentScope.activeTabId = restoredTabs[snapshot.activeIndex]?.id ?? restoredTabs[0]?.id ?? null + currentScope.automationTabId = currentScope.activeTabId currentScope.lastPersistedSnapshot = JSON.stringify(snapshot) } @@ -1496,15 +1557,53 @@ export function restoreBrowserSession(): void { export function addTab(): AgentTab { restoreBrowserSession() + currentScope.visibleTabUserSelected = true return addTabInternal() } +/** Opens a tab for agent work without replacing the page the user is viewing. */ +export function addAutomationTab(): AgentTab { + restoreBrowserSession() + const tab = addTabInternal({ activate: false, notify: false }) + currentScope.automationTabId = tab.id + applyActiveTabThrottling() + persistBrowserSession() + events?.onTabsChanged() + return tab +} + +/** Agent target, creating or adopting a page without changing visible selection. */ +export function ensureAutomationTab(): AgentTab { + restoreBrowserSession() + let tab = automationTab() + if (tab) return tab + tab = activeTab() + if (tab) { + currentScope.automationTabId = tab.id + applyActiveTabThrottling() + events?.onTabsChanged() + return tab + } + return addAutomationTab() +} + +/** Current agent target without creating one. */ +export function requireAutomationTab(): AgentTab { + restoreBrowserSession() + const tab = automationTab() + if (!tab) { + throw new SessionError('No page is open yet — call browser_navigate or browser_open_tab first.') + } + return tab +} + /** Restores the most recently closed regular tab for the current app session. */ export function reopenClosedTab(): AgentTab | null { restoreBrowserSession() const url = recentlyClosedTabUrls.shift() if (!url) return null + currentScope.visibleTabUserSelected = true const tab = addTabInternal() if (url !== 'about:blank') { // No checkAgentUrl here, unlike the tool-driven navigations: the stored @@ -1528,6 +1627,7 @@ export function duplicateTab(tabId: string): AgentTab | null { if (!source) return null const url = sanitizeRestorableUrl(source.view.webContents.getURL()) + currentScope.visibleTabUserSelected = true const tab = addTabInternal() if (url && url !== 'about:blank') { // Sanitized to http(s) without embedded credentials above, and the @@ -1550,8 +1650,9 @@ export function switchTab(tabId: string): AgentTab { currentScope.focusedBrowserTabId !== null || tabs.some((entry) => entry.view.webContents.isFocused()) currentScope.activeTabId = tab.id - // The automation exemption follows the active tab, so a mid-tool switch - // unthrottles the new one and re-throttles the old. + currentScope.visibleTabUserSelected = true + // Visible selection does not move the automation exemption; the user may + // inspect another page while a tool continues in its background tab. applyActiveTabThrottling() layout() if (transferBrowserFocus) currentScope.focusedBrowserTabId = tab.id @@ -1561,6 +1662,17 @@ export function switchTab(tabId: string): AgentTab { return tab } +/** Moves the agent cursor without moving or focusing the user's visible tab. */ +export function switchAutomationTab(tabId: string): AgentTab { + restoreBrowserSession() + const tab = tabs.find((entry) => entry.id === tabId) + if (!tab) throw new SessionError(`No tab with id ${tabId} — call browser_list_tabs.`) + currentScope.automationTabId = tab.id + applyActiveTabThrottling() + events?.onTabsChanged() + return tab +} + /** * Moves a tab to a final list index while preserving the pinned/regular * boundary. Dragging across that boundary moves to its nearest valid edge. @@ -1601,6 +1713,7 @@ function forgetTab(tab: AgentTab): void { // on a tab that is going away keeps `findingTabId` naming a dead tab and // leaves the bar open counting matches on a page nobody can see. dismissFind(tab.id) + clearAutomationIndicatorsForTab(tab.id) tabs.splice(index, 1) const transferBrowserFocus = currentScope.focusedBrowserTabId === tab.id clearFocusedBrowserTab(tab.id) @@ -1613,6 +1726,10 @@ function forgetTab(tab: AgentTab): void { events?.onActiveTabChanged(active.view.webContents) } } + if (currentScope.automationTabId === tab.id) { + currentScope.automationTabId = (tabs[index] ?? tabs[index - 1])?.id ?? null + applyActiveTabThrottling() + } if (!hasSession() && getBrowserScopeId() === getActiveBrowserScopeId() && isPanelVisible()) { addTab() if (transferBrowserFocus) currentScope.focusedBrowserTabId = currentScope.activeTabId @@ -1635,6 +1752,7 @@ export function closeTab(tabId: string): void { } // Before the splice, while the tab is still resolvable — see forgetTab. dismissFind(tabId) + clearAutomationIndicatorsForTab(tabId) const [tab] = tabs.splice(index, 1) recentlyClosedTabUrls.unshift(sanitizeRestorableUrl(tabUrl(tab)) ?? 'about:blank') if (recentlyClosedTabUrls.length > MAX_RECENTLY_CLOSED_TABS) { @@ -1653,6 +1771,10 @@ export function closeTab(tabId: string): void { events?.onActiveTabChanged(active.view.webContents) } } + if (currentScope.automationTabId === tab.id) { + currentScope.automationTabId = (tabs[index] ?? tabs[index - 1])?.id ?? null + applyActiveTabThrottling() + } // Closing the last tab must not leave a visible browser resource with an // empty strip. Replace it with a fresh New tab, matching normal browser UI. if (!hasSession() && getBrowserScopeId() === getActiveBrowserScopeId() && isPanelVisible()) { @@ -1668,6 +1790,16 @@ export function closeTab(tabId: string): void { } } +/** Closes agent-owned work while protecting a visible tab the user claimed. */ +export function closeAutomationTab(tabId: string): void { + if (tabId === currentScope.activeTabId && currentScope.visibleTabUserSelected) { + throw new SessionError( + 'That tab is currently being used by the user. Switch to another agent tab instead of closing it.' + ) + } + closeTab(tabId) +} + /** * Pins or unpins a live tab. Pinned tabs form a stable group at the far left, * and their latest URLs are persisted locally for the next browser opening. @@ -1712,7 +1844,9 @@ export function showTabContextMenu(tabId: string): void { /** The live page whose browser surface owns a menu accelerator. */ function focusedTabForShortcut(ownerWindow?: BrowserWindow | null): AgentTab | null { - if (!panelUpdateAllowed(ownerWindow ?? undefined, getBrowserScopeId())) return null + if (!isPanelVisible() || !panelUpdateAllowed(ownerWindow ?? undefined, getBrowserScopeId())) { + return null + } return ( tabs.find( (tab) => @@ -1722,6 +1856,14 @@ function focusedTabForShortcut(ownerWindow?: BrowserWindow | null): AgentTab | n ) } +/** The active tab while this window owns an on-screen browser panel. */ +function visibleActiveTab(ownerWindow?: BrowserWindow | null): AgentTab | null { + if (!isPanelVisible() || !panelUpdateAllowed(ownerWindow ?? undefined, getBrowserScopeId())) { + return null + } + return activeTab() +} + /** * Claims a global resource shortcut only while this browser owns interaction. * @@ -1733,8 +1875,32 @@ export function handleFocusedShortcut( shortcut: FocusedResourceShortcut, ownerWindow?: BrowserWindow | null ): boolean { - const focusedTab = focusedTabForShortcut(ownerWindow) - if (!focusedTab) return false + // Tab-management accelerators belong to the visible Browser even after + // focus moves into Sim chrome. Otherwise Cmd-T/L/Shift-T silently stop + // behaving like browser shortcuts, and Cmd-W becomes especially unsafe. + const visibleBrowserShortcut = + shortcut === 'close-tab' || + shortcut === 'new-tab' || + shortcut === 'reopen-closed-tab' || + shortcut === 'focus-omnibox' + const shortcutTab = + focusedTabForShortcut(ownerWindow) ?? + (visibleBrowserShortcut ? visibleActiveTab(ownerWindow) : null) + if (!shortcutTab) return false + + if (isResourceTabSelectionShortcut(shortcut)) { + const targetIndex = resourceTabTargetIndex( + shortcut, + tabs.length, + tabs.findIndex((tab) => tab.id === shortcutTab.id) + ) + const target = targetIndex === null ? null : tabs[targetIndex] + if (target) { + switchTab(target.id) + target.view.webContents.focus() + } + return true + } switch (shortcut) { case 'new-tab': @@ -1747,15 +1913,18 @@ export function handleFocusedShortcut( return true } case 'close-tab': - closeTabFromUser(focusedTab.id) + closeTabFromUser(shortcutTab.id) + return true + case 'focus-omnibox': + focusRendererOmnibox('select') return true case 'reload-or-clear': - focusedTab.view.webContents.reload() + shortcutTab.view.webContents.reload() return true } const zoomAction = zoomActionForShortcut(shortcut) - const contents = focusedTab.view.webContents + const contents = shortcutTab.view.webContents const factor = zoomAction === 'reset' ? getBrowserDefaultZoomFactor() @@ -1781,6 +1950,7 @@ export function setPanelFocused( currentScope.focusedBrowserClearTimer = null } currentScope.focusedBrowserTabId = activeTab()?.id ?? null + currentScope.visibleTabUserSelected = true }) } @@ -1794,7 +1964,10 @@ function clearFocusedBrowserTab(tabId?: string): void { } function closeTabFromUser(tabId: string): void { - if (tabs.find((tab) => tab.id === tabId)?.pinned) return + if (tabs.find((tab) => tab.id === tabId)?.pinned) { + shell.beep() + return + } const closingLastTab = listTabs().length === 1 closeTab(tabId) const active = activeTab() @@ -1816,6 +1989,10 @@ function closeLiveTabs(): void { } recentlyClosedTabUrls.length = 0 currentScope.activeTabId = null + currentScope.automationTabId = null + currentScope.automationActive = false + currentScope.automationNeedsAttention = false + currentScope.visibleTabUserSelected = false clearFocusedBrowserTab() } @@ -1952,6 +2129,22 @@ export function getTabsState(): BrowserTabsState { scopeId: getBrowserScopeId(), tabs: listTabs(), activeTabId: activeTab()?.id ?? null, + automationTabId: automationTab()?.id ?? null, + automationActive: currentScope.automationActive, + automationNeedsAttention: currentScope.automationNeedsAttention, + } +} + +/** Tool-facing tab list whose active marker follows the agent cursor. */ +export function getAutomationTabsState(): BrowserTabsState { + const automationTabId = automationTab()?.id ?? null + return { + scopeId: getBrowserScopeId(), + tabs: listTabs().map((tab) => ({ ...tab, active: tab.tabId === automationTabId })), + activeTabId: automationTabId, + automationTabId, + automationActive: currentScope.automationActive, + automationNeedsAttention: currentScope.automationNeedsAttention, } } @@ -1965,3 +2158,25 @@ export function activeTab(): AgentTab | null { if (!tab || tab.view.webContents.isDestroyed()) return null return tab } + +export function automationTab(): AgentTab | null { + const tab = tabs.find((entry) => entry.id === currentScope.automationTabId) ?? null + if (!tab || tab.view.webContents.isDestroyed()) return null + return tab +} + +export function automationTabClaimedByUser(): boolean { + return ( + currentScope.visibleTabUserSelected && + currentScope.activeTabId !== null && + currentScope.activeTabId === currentScope.automationTabId + ) +} + +/** Keeps page-created tabs with the input owner that opened them. */ +function agentOwnsPopupFrom(contents: WebContents): boolean { + if (isDispatchingAgentInput(contents)) return true + if (automationTab()?.view.webContents !== contents) return false + if (automationTabClaimedByUser()) return false + return currentScope.automationActive +} diff --git a/apps/desktop/src/main/desktop-settings.test.ts b/apps/desktop/src/main/desktop-settings.test.ts index 25c137aa8ca..b7849a57558 100644 --- a/apps/desktop/src/main/desktop-settings.test.ts +++ b/apps/desktop/src/main/desktop-settings.test.ts @@ -1,7 +1,7 @@ import { mkdtempSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { TERMINAL_DARK_THEME } from '@sim/desktop-bridge' +import { TERMINAL_DARK_THEME, TERMINAL_LIGHT_THEME } from '@sim/desktop-bridge' import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) @@ -15,6 +15,14 @@ const IMPORTED_PALETTE = { ...TERMINAL_DARK_THEME, background: '#101010', } +const IMPORTED_LIGHT_PALETTE = { + ...TERMINAL_LIGHT_THEME, + background: '#fafafa', +} +const IMPORTED_DARK_PALETTE = { + ...TERMINAL_DARK_THEME, + background: '#202020', +} function makeService() { const config = createConfigStore( @@ -191,7 +199,7 @@ describe('desktop settings service', () => { expect(preferences?.browserDownloadDirectory).toBe('/tmp/custom-downloads') }) - it('caches and selects a Terminal or iTerm2 profile', () => { + it('persists a Terminal or iTerm2 profile with appearance-specific palettes', () => { const { config, service } = makeService() const preferences = service.selectTerminalProfile({ @@ -199,6 +207,8 @@ describe('desktop settings service', () => { name: 'Ocean', source: 'iterm2', palette: IMPORTED_PALETTE, + lightPalette: IMPORTED_LIGHT_PALETTE, + darkPalette: IMPORTED_DARK_PALETTE, }) expect(config.get('terminalTheme')).toEqual({ @@ -206,6 +216,8 @@ describe('desktop settings service', () => { name: 'Ocean', source: 'iterm2', palette: IMPORTED_PALETTE, + lightPalette: IMPORTED_LIGHT_PALETTE, + darkPalette: IMPORTED_DARK_PALETTE, }) expect(preferences).toMatchObject({ terminalTheme: { id: 'iterm2:ocean', name: 'Ocean' }, diff --git a/apps/desktop/src/main/desktop-settings.ts b/apps/desktop/src/main/desktop-settings.ts index aeaef5a0852..f84b364b40b 100644 --- a/apps/desktop/src/main/desktop-settings.ts +++ b/apps/desktop/src/main/desktop-settings.ts @@ -1,5 +1,6 @@ import { isAbsolute } from 'node:path' import { + cloneTerminalSelectedProfile, type DesktopAppearanceTheme, type DesktopNotificationPayload, type DesktopPreferenceKey, @@ -177,12 +178,7 @@ export function createDesktopSettingsService( return read() }, selectTerminalProfile(profile) { - deps.config.set('terminalTheme', { - id: profile.id, - name: profile.name, - source: profile.source, - palette: { ...profile.palette }, - }) + deps.config.set('terminalTheme', cloneTerminalSelectedProfile(profile)) deps.config.flush() return read() }, diff --git a/apps/desktop/src/main/external-links.ts b/apps/desktop/src/main/external-links.ts new file mode 100644 index 00000000000..4e061888493 --- /dev/null +++ b/apps/desktop/src/main/external-links.ts @@ -0,0 +1,3 @@ +export const DOCS_URL = 'https://docs.sim.ai' +export const DOCS_SEARCH_ENDPOINT = `${DOCS_URL}/api/search` +export const STATUS_URL = 'https://status.sim.ai' diff --git a/apps/desktop/src/main/help-search.test.ts b/apps/desktop/src/main/help-search.test.ts new file mode 100644 index 00000000000..52016cdecd5 --- /dev/null +++ b/apps/desktop/src/main/help-search.test.ts @@ -0,0 +1,60 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + installDocumentationHelpSearch, + uninstallDocumentationHelpSearch, +} from '@/main/help-search' + +describe('documentation Help search', () => { + afterEach(() => { + uninstallDocumentationHelpSearch() + }) + + it('registers the native provider with the canonical docs endpoint', () => { + const bridge = { + install: vi.fn(() => true), + uninstall: vi.fn(), + } + + expect( + installDocumentationHelpSearch({ + isElectron: true, + isMacOS: true, + loadBridge: () => bridge, + }) + ).toBe(true) + expect(bridge.install).toHaveBeenCalledWith('https://docs.sim.ai/api/search') + + uninstallDocumentationHelpSearch() + expect(bridge.uninstall).toHaveBeenCalledOnce() + }) + + it('does not load the bridge outside Electron on macOS', () => { + const loadBridge = vi.fn() + + expect( + installDocumentationHelpSearch({ + isElectron: false, + isMacOS: true, + loadBridge, + }) + ).toBe(false) + expect(loadBridge).not.toHaveBeenCalled() + }) + + it('fails closed for an invalid or refusing bridge', () => { + expect( + installDocumentationHelpSearch({ + isElectron: true, + isMacOS: true, + loadBridge: () => ({ install: () => true }), + }) + ).toBe(false) + expect( + installDocumentationHelpSearch({ + isElectron: true, + isMacOS: true, + loadBridge: () => ({ install: () => false, uninstall: vi.fn() }), + }) + ).toBe(false) + }) +}) diff --git a/apps/desktop/src/main/help-search.ts b/apps/desktop/src/main/help-search.ts new file mode 100644 index 00000000000..8b9eca5810d --- /dev/null +++ b/apps/desktop/src/main/help-search.ts @@ -0,0 +1,75 @@ +import { join } from 'node:path' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import { DOCS_SEARCH_ENDPOINT } from '@/main/external-links' + +const logger = createLogger('DesktopHelpSearch') + +interface NativeHelpSearchBridge { + install: (endpoint: string) => boolean + uninstall: () => void +} + +interface HelpSearchRuntime { + isElectron: boolean + isMacOS: boolean + loadBridge: () => unknown +} + +let activeBridge: NativeHelpSearchBridge | null = null + +function isNativeHelpSearchBridge(value: unknown): value is NativeHelpSearchBridge { + if (!value || typeof value !== 'object') return false + const candidate = value as Partial + return typeof candidate.install === 'function' && typeof candidate.uninstall === 'function' +} + +function loadNativeBridge(): unknown { + return require(join(__dirname, 'native', 'help-search.node')) +} + +const nativeRuntime: HelpSearchRuntime = { + isElectron: Boolean(process.versions.electron), + isMacOS: process.platform === 'darwin', + loadBridge: loadNativeBridge, +} + +/** + * Registers Sim's documentation index with the native macOS Help search field. + * The bridge is optional so non-macOS development and unit tests remain portable. + */ +export function installDocumentationHelpSearch(runtime = nativeRuntime): boolean { + if (!runtime.isMacOS || !runtime.isElectron) return false + + try { + const bridge = runtime.loadBridge() + if (!isNativeHelpSearchBridge(bridge)) { + logger.warn('Native documentation Help search bridge has an invalid interface') + return false + } + if (!bridge.install(DOCS_SEARCH_ENDPOINT)) { + logger.warn('Native documentation Help search bridge refused its endpoint') + return false + } + activeBridge = bridge + return true + } catch (error) { + logger.warn('Could not install native documentation Help search', { + error: getErrorMessage(error), + }) + return false + } +} + +/** Unregisters the native provider before Electron tears AppKit down. */ +export function uninstallDocumentationHelpSearch(): void { + try { + activeBridge?.uninstall() + } catch (error) { + logger.warn('Could not uninstall native documentation Help search', { + error: getErrorMessage(error), + }) + } finally { + activeBridge = null + } +} diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index ad30bdb8b06..cf82af3da67 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -38,6 +38,10 @@ import { DesktopChatSessionStore } from '@/main/desktop-chat-session-store' import { createDesktopSettingsService } from '@/main/desktop-settings' import { attachDownloadHandling } from '@/main/downloads' import { createAuthFlow, createConnectFlow, createHandoffManager } from '@/main/handoff' +import { + installDocumentationHelpSearch, + uninstallDocumentationHelpSearch, +} from '@/main/help-search' import { registerIpcHandlers } from '@/main/ipc' import { attachLoadHealth, type LoadHealthHandle } from '@/main/load-health' import { LocalFilesystemService } from '@/main/local-filesystem' @@ -500,6 +504,7 @@ function main(): void { // set. This prevents a navigation event racing the synchronous quit flush. quiesceBrowserSessions() terminal.dispose() + uninstallDocumentationHelpSearch() flushDesktopChatSessions('before-quit') // Settings writes coalesce, so a change made in the last moments before // quit is still pending here. @@ -664,13 +669,15 @@ function main(): void { newWindow: () => void createAndLoadAppWindow(), newChat: () => void openMainWindowAt(newChatRoute(config.get('lastRoute'))), handleFocusedResourceShortcut: (win, shortcut) => - terminal.handleFocusedShortcut(win, shortcut) || - handleFocusedBrowserShortcut(shortcut, win), + handleFocusedBrowserShortcut(shortcut, win) || + terminal.handleFocusedShortcut(win, shortcut), toggleSidebar: () => getMainWindow()?.webContents.send('desktop:command', 'toggle-sidebar'), + openSearch: () => getMainWindow()?.webContents.send('desktop:command', 'open-search'), signOut: signOutFromMenu, checkForUpdates: () => checkForUpdatesInteractive({ getWindow: getMainWindow, events, handle: updater }), }) + installDocumentationHelpSearch() setTrayEnabled(config.get('trayEnabled') ?? true) updater = initUpdater({ getWindow: getMainWindow, diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 0376da1a065..a23f83b4745 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -669,6 +669,64 @@ describe('registerIpcHandlers', () => { ) }) + it('routes exact browser-tool cancellation without waiting for authorization', async () => { + const { invoke } = collectHandlers() + const cancel = vi.spyOn(browserDriver, 'cancelTool').mockReturnValue(true) + const cancelActive = vi.spyOn(browserDriver, 'cancelActiveTool').mockReturnValue(true) + const handler = invoke.get('browser-agent:cancel-tool') + const activeHandler = invoke.get('browser-agent:cancel-active-tool') + + await expect(handler?.(appEvent, 'tool-1', 'chat-1')).resolves.toBe(true) + expect(cancel).toHaveBeenCalledWith('chat-1', 'tool-1') + await expect(activeHandler?.(appEvent, 'chat-reloaded')).resolves.toBe(true) + expect(cancelActive).toHaveBeenCalledWith('chat-reloaded') + await expect(handler?.(evilEvent, 'tool-2', 'chat-1')).resolves.toBe(false) + await expect(activeHandler?.(evilEvent, 'chat-reloaded')).resolves.toBe(false) + expect(cancel).toHaveBeenCalledTimes(1) + expect(cancelActive).toHaveBeenCalledTimes(1) + cancel.mockRestore() + cancelActive.mockRestore() + }) + + it('rejects a browser tool authorized after its scope cancellation boundary', async () => { + const { invoke } = collectHandlers() + const executeHandler = invoke.get('browser-agent:execute-tool') + const cancelActiveHandler = invoke.get('browser-agent:cancel-active-tool') + let resolveAuthorization: (response: Response) => void = () => {} + const fetchAuthorization = vi.fn( + () => + new Promise((resolve) => { + resolveAuthorization = resolve + }) + ) + const delayedEvent = { + senderFrame: { url: `${APP}/workspace/ws1` }, + sender: { session: { fetch: fetchAuthorization } }, + } + + const execution = executeHandler?.( + delayedEvent, + 'tool-delayed-authorization', + 'browser_open_tab', + {}, + 'chat-delayed-authorization' + ) + await Promise.resolve() + await cancelActiveHandler?.(delayedEvent, 'chat-delayed-authorization') + resolveAuthorization( + Response.json({ + chatId: 'chat-delayed-authorization', + toolName: 'browser_open_tab', + args: {}, + }) + ) + + await expect(execution).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('cancelled before it started'), + }) + }) + it('routes terminal tools by the server-authorized chat, not renderer scope', async () => { const { invoke } = collectHandlers() const executeTool = vi.spyOn(deps.terminal, 'executeTool').mockResolvedValue({ ok: true }) @@ -919,6 +977,42 @@ describe('registerIpcHandlers', () => { restore.mockRestore() }) + it('creates browser tabs through an acknowledged active-scope operation', async () => { + const tabsState = { + scopeId: 'chat-b', + tabs: [ + { + tabId: '2', + url: '', + title: '', + loading: false, + active: true, + pinned: false, + }, + ], + activeTabId: '2', + } + const add = vi.spyOn(browserSession, 'addTab').mockReturnValue({} as never) + const peek = vi.spyOn(browserSession, 'peekTabsState').mockReturnValue(tabsState) + const { invoke } = collectHandlers() + + await invoke.get('browser-agent:activate-scope')?.(appEvent, 'chat-b') + peek.mockClear() + await expect(invoke.get('browser-agent:open-tab')?.(appEvent, 'chat-b')).resolves.toEqual( + tabsState + ) + await expect(invoke.get('browser-agent:open-tab')?.(evilEvent, 'chat-b')).resolves.toEqual({ + scopeId: '', + tabs: [], + activeTabId: null, + }) + + expect(add).toHaveBeenCalledOnce() + expect(peek).toHaveBeenCalledOnce() + add.mockRestore() + peek.mockRestore() + }) + it('routes browser scope events after activation and a valid provisional migration', async () => { const migrate = vi.spyOn(browserDriver, 'migrateBrowserScope').mockReturnValue(true) const { invoke } = collectHandlers() diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 93f95b8d6b3..4b4b48bb7d8 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -26,6 +26,10 @@ import { isRecordLike } from '@sim/utils/object' import type { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent, WebContents } from 'electron' import { clipboard, ipcMain } from 'electron' import { + type BrowserToolQueueBoundary, + cancelActiveTool, + cancelTool, + captureBrowserToolQueueBoundary, clearBrowsingData, disposeBrowserScope, executeTool, @@ -38,6 +42,7 @@ import { } from '@/main/browser-agent/driver' import { isAgentWebContents } from '@/main/browser-agent/registry' import { + addTab, findInActiveTab, getBrowserDownloadsState, peekTabsState, @@ -745,12 +750,53 @@ export function registerIpcHandlers(deps: IpcDeps): void { gate: 'app-origin', requires: 'browser', denied: { ok: false, error: 'Browser automation is not allowed from this page.' }, - handler: (scope, tool, params) => { - if (typeof scope !== 'string' || typeof tool !== 'string' || !isBrowserToolName(tool)) { + handler: (scope, toolCallId, tool, params, authorizationBoundary) => { + if ( + typeof scope !== 'string' || + typeof toolCallId !== 'string' || + typeof tool !== 'string' || + !isBrowserToolName(tool) + ) { return { ok: false, error: `Unknown browser tool: ${String(tool)}` } } const toolParams = isRecordLike(params) ? params : {} - return executeTool(scope, tool, toolParams) + return executeTool( + scope, + tool, + toolParams, + toolCallId, + authorizationBoundary as BrowserToolQueueBoundary | undefined + ) + }, + }, + 'browser-agent:cancel-tool': { + kind: 'invoke', + gate: 'app-origin', + requires: 'browser', + passSender: true, + denied: false, + handler: (sender, toolCallId, rawScope) => { + const scope = rendererScope(browserScopeBySender, sender as WebContents, rawScope) + if ( + !scope || + typeof toolCallId !== 'string' || + toolCallId.length < 1 || + toolCallId.length > 256 + ) { + return false + } + return cancelTool(scope, toolCallId) + }, + }, + 'browser-agent:cancel-active-tool': { + kind: 'invoke', + gate: 'app-origin', + requires: 'browser', + passSender: true, + denied: false, + handler: (sender, rawScope) => { + const scope = rendererScope(browserScopeBySender, sender as WebContents, rawScope) + return scope ? cancelActiveTool(scope) : false }, }, 'browser-agent:get-tabs-state': { @@ -767,6 +813,22 @@ export function registerIpcHandlers(deps: IpcDeps): void { : { tabs: [], activeTabId: null } }, }, + 'browser-agent:open-tab': { + kind: 'invoke', + gate: 'app-origin', + requires: 'browser', + passSender: true, + denied: { scopeId: '', tabs: [], activeTabId: null }, + handler: (sender, rawScope) => { + const contents = sender as WebContents + const scope = activeRendererScope(browserScopeBySender, contents, rawScope) + if (!scope) return { scopeId: '', tabs: [], activeTabId: null } + return withBrowserScope(scope, () => { + addTab() + return peekTabsState() + }) + }, + }, 'browser-agent:activate-scope': { kind: 'invoke', gate: 'app-origin', @@ -1391,6 +1453,16 @@ export function registerIpcHandlers(deps: IpcDeps): void { if (scope) deps.terminal.setPanelFocused(scope, focused === true, sender as WebContents) }, }, + 'terminal:visible': { + kind: 'send', + gate: 'app-origin', + requires: 'terminal', + passSender: true, + handler: (sender, visible, rawScope) => { + const scope = rendererScope(terminalScopeBySender, sender as WebContents, rawScope) + if (scope) deps.terminal.setPanelVisible(scope, visible === true, sender as WebContents) + }, + }, 'terminal:paste': { kind: 'invoke', gate: 'app-origin', @@ -1554,6 +1626,24 @@ export function registerIpcHandlers(deps: IpcDeps): void { return { ...tabs, scopeId: scope } }, }, + 'terminal:reorder': { + kind: 'invoke', + gate: 'app-origin', + requires: 'terminal', + passSender: true, + denied: { tabs: [], activeTerminalId: null }, + handler: (sender, terminalId, targetIndex, rawScope) => { + const scope = rendererScope(terminalScopeBySender, sender as WebContents, rawScope) + if (!scope) return { tabs: [], activeTerminalId: null } + const tabs = + typeof terminalId === 'string' && + typeof targetIndex === 'number' && + Number.isFinite(targetIndex) + ? deps.terminal.reorderTerminal(scope, terminalId, targetIndex) + : deps.terminal.getTabs(scope) + return { ...tabs, scopeId: scope } + }, + }, 'terminal:close': { kind: 'invoke', gate: 'app-origin', @@ -1646,6 +1736,10 @@ export function registerIpcHandlers(deps: IpcDeps): void { let handlerArgs = args if (channel === 'browser-agent:execute-tool') { const requestedTool = args[1] + const requestedScope = parseDesktopScope(args[3]) + const authorizationBoundary = requestedScope + ? captureBrowserToolQueueBoundary(requestedScope) + : undefined const authorization = await fetchDesktopToolAuthorization(event, deps, args[0]) if ( !authorization || @@ -1658,7 +1752,13 @@ export function registerIpcHandlers(deps: IpcDeps): void { error: 'This browser action is not an authorized pending Copilot tool call.', } } - handlerArgs = [authorization.chatId, authorization.toolName, authorization.args] + handlerArgs = [ + authorization.chatId, + args[0], + authorization.toolName, + authorization.args, + authorizationBoundary, + ] } if (channel === 'terminal:execute-tool') { const requestedTool = args[1] diff --git a/apps/desktop/src/main/menu.test.ts b/apps/desktop/src/main/menu.test.ts index 12bb128655d..faf1648b413 100644 --- a/apps/desktop/src/main/menu.test.ts +++ b/apps/desktop/src/main/menu.test.ts @@ -22,6 +22,7 @@ function makeDeps(): MenuDeps { newChat: vi.fn(), handleFocusedResourceShortcut: vi.fn(() => false), toggleSidebar: vi.fn(), + openSearch: vi.fn(), signOut: vi.fn(), checkForUpdates: vi.fn(), } @@ -67,9 +68,17 @@ describe('buildMenuTemplate', () => { 'New Chat', 'separator', 'Reopen Closed Tab', + 'Focus Address Bar', + 'separator', + 'Next Tab', + 'Previous Tab', + 'Select Tab', + 'separator', + 'Close Tab', 'Close Window', ]) expect(submenu(template, 'View').map((item) => item.label ?? item.role ?? item.type)).toEqual([ + 'Search', 'Toggle Sidebar', 'separator', 'Back', @@ -83,9 +92,9 @@ describe('buildMenuTemplate', () => { ]) }) - it('keeps Help limited to documentation and system status', () => { + it('keeps Help limited to documentation and Sim status', () => { const help = submenu(buildMenuTemplate(makeDeps()), 'Help') - expect(help.map((item) => item.label)).toEqual(['Sim Documentation', 'System Status']) + expect(help.map((item) => item.label)).toEqual(['Sim Documentation', 'Sim Status']) }) it('never exposes developer tools in the application menu', () => { @@ -93,7 +102,7 @@ describe('buildMenuTemplate', () => { expect(view.some((item) => item.role === 'toggleDevTools')).toBe(false) }) - it('routes the close accelerator through the focused resource before closing a window', () => { + it('reserves the close-tab accelerator for resources and never closes the window', () => { const handleFocusedResourceShortcut = vi.fn(() => true) const deps = Object.assign(makeDeps(), { handleFocusedResourceShortcut }) const closeItem = submenu(buildMenuTemplate(deps), 'File').find( @@ -101,7 +110,7 @@ describe('buildMenuTemplate', () => { ) const focusedWindow = new BrowserWindow() - expect(closeItem).toMatchObject({ label: 'Close Window', accelerator: 'CmdOrCtrl+W' }) + expect(closeItem).toMatchObject({ label: 'Close Tab', accelerator: 'CmdOrCtrl+W' }) expect(closeItem?.role).toBeUndefined() const click = closeItem?.click as unknown as ( @@ -115,10 +124,52 @@ describe('buildMenuTemplate', () => { handleFocusedResourceShortcut.mockReturnValue(false) click({}, focusedWindow) + expect(focusedWindow.close).not.toHaveBeenCalled() + }) + + it('always offers a separate close-window accelerator', () => { + const deps = makeDeps() + const closeWindow = submenu(buildMenuTemplate(deps), 'File').find( + (item) => item.accelerator === 'CmdOrCtrl+Shift+W' + ) + const focusedWindow = new BrowserWindow() + + ;(closeWindow?.click as unknown as (menuItem: unknown, browserWindow: BrowserWindow) => void)( + {}, + focusedWindow + ) + + expect(closeWindow?.label).toBe('Close Window') expect(focusedWindow.close).toHaveBeenCalledOnce() + expect(deps.handleFocusedResourceShortcut).not.toHaveBeenCalled() + }) + + it('routes next, previous, and numbered tab accelerators to the focused resource', () => { + const handleFocusedResourceShortcut = vi.fn(() => true) + const template = buildMenuTemplate( + Object.assign(makeDeps(), { + handleFocusedResourceShortcut, + }) + ) + const file = submenu(template, 'File') + const selectTabs = submenu(file, 'Select Tab') + const focusedWindow = new BrowserWindow() + const invoke = (item: MenuItemConstructorOptions | undefined) => + (item?.click as unknown as (menuItem: unknown, browserWindow: BrowserWindow) => void)( + {}, + focusedWindow + ) + + invoke(file.find((item) => item.accelerator === 'Ctrl+Tab')) + invoke(file.find((item) => item.accelerator === 'Ctrl+Shift+Tab')) + invoke(selectTabs.find((item) => item.accelerator === 'CmdOrCtrl+9')) + + expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith(1, focusedWindow, 'next-tab') + expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith(2, focusedWindow, 'previous-tab') + expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith(3, focusedWindow, 'select-tab-9') }) - it('routes new and reopen accelerators through the focused resource', () => { + it('routes new, reopen, and address-bar accelerators through the active resource', () => { const handleFocusedResourceShortcut = vi.fn(() => true) const template = buildMenuTemplate( Object.assign(makeDeps(), { @@ -129,6 +180,7 @@ describe('buildMenuTemplate', () => { const reopenItem = submenu(template, 'File').find( (item) => item.accelerator === 'CmdOrCtrl+Shift+T' ) + const addressItem = submenu(template, 'File').find((item) => item.accelerator === 'CmdOrCtrl+L') expect(reopenItem).toMatchObject({ label: 'Reopen Closed Tab', @@ -143,12 +195,17 @@ describe('buildMenuTemplate', () => { {}, focusedWindow ) + ;(addressItem?.click as unknown as (menuItem: unknown, browserWindow: BrowserWindow) => void)( + {}, + focusedWindow + ) expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith(1, focusedWindow, 'new-tab') expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith( 2, focusedWindow, 'reopen-closed-tab' ) + expect(handleFocusedResourceShortcut).toHaveBeenNthCalledWith(3, focusedWindow, 'focus-omnibox') }) it('routes reload through the focused resource before falling back to the Sim renderer', () => { @@ -210,6 +267,18 @@ describe('buildMenuTemplate', () => { expect(deps.config.set).toHaveBeenCalledWith('zoomLevel', 0) }) + it('opens the search palette from View with the platform Mod+K accelerator', () => { + const deps = makeDeps() + const item = submenu(buildMenuTemplate(deps), 'View').find( + (entry) => entry.accelerator === 'CmdOrCtrl+K' + ) + + expect(item).toMatchObject({ label: 'Search' }) + ;(item?.click as unknown as () => void)() + expect(deps.openSearch).toHaveBeenCalledOnce() + expect(deps.handleFocusedResourceShortcut).not.toHaveBeenCalled() + }) + it('offers the standard new-window command', () => { const deps = makeDeps() const item = submenu(buildMenuTemplate(deps), 'File').find( diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index 5261f241048..25d4a4c6025 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -1,11 +1,13 @@ import type { MenuItemConstructorOptions } from 'electron' import { app, BrowserWindow, Menu } from 'electron' import type { ConfigStore } from '@/main/config' +import { DOCS_URL, STATUS_URL } from '@/main/external-links' import { openExternalSafe } from '@/main/navigation' -import type { FocusedResourceShortcut } from '@/main/resource-shortcuts' +import type { + FocusedResourceShortcut, + ResourceTabSelectionShortcut, +} from '@/main/resource-shortcuts' -const DOCS_URL = 'https://docs.sim.ai' -const STATUS_URL = 'https://status.sim.ai' const ZOOM_STEP = 0.5 export interface MenuDeps { @@ -25,6 +27,7 @@ export interface MenuDeps { shortcut: FocusedResourceShortcut ) => boolean toggleSidebar: () => void + openSearch: () => void signOut: () => void checkForUpdates: () => void } @@ -46,6 +49,24 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] const focusedOrMain = (focusedWindow: unknown): BrowserWindow | null => focusedWindow instanceof BrowserWindow ? focusedWindow : deps.getMainWindow() + const resourceShortcut = ( + shortcut: FocusedResourceShortcut + ): NonNullable => { + return (_item, focusedWindow) => { + deps.handleFocusedResourceShortcut(focusedOrMain(focusedWindow), shortcut) + } + } + + const numberedTabItems: MenuItemConstructorOptions[] = Array.from({ length: 9 }, (_, index) => { + const number = index + 1 + const shortcut = `select-tab-${number}` as ResourceTabSelectionShortcut + return { + label: number === 9 ? 'Last Tab' : `Tab ${number}`, + accelerator: `CmdOrCtrl+${number}`, + click: resourceShortcut(shortcut), + } + }) + const setZoom = ( action: 'in' | 'out' | 'reset' ): NonNullable => { @@ -62,6 +83,16 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] } const viewSubmenu: MenuItemConstructorOptions[] = [ + /** + * The command palette is the web app's own `Mod+K` command; claiming the + * accelerator here means the menu, not the renderer, resolves it — so the + * click must drive the same palette the page would have opened. + */ + { + label: 'Search', + accelerator: 'CmdOrCtrl+K', + click: deps.openSearch, + }, { label: 'Toggle Sidebar', accelerator: 'CmdOrCtrl+B', @@ -144,11 +175,36 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] }, }, { - label: 'Close Window', + label: 'Focus Address Bar', + accelerator: 'CmdOrCtrl+L', + click: resourceShortcut('focus-omnibox'), + }, + { type: 'separator' }, + { + label: 'Next Tab', + accelerator: 'Ctrl+Tab', + click: resourceShortcut('next-tab'), + }, + { + label: 'Previous Tab', + accelerator: 'Ctrl+Shift+Tab', + click: resourceShortcut('previous-tab'), + }, + { label: 'Select Tab', submenu: numberedTabItems }, + { type: 'separator' }, + { + label: 'Close Tab', accelerator: 'CmdOrCtrl+W', click: (_item, focusedWindow) => { const win = focusedOrMain(focusedWindow) - if (deps.handleFocusedResourceShortcut(win, 'close-tab')) return + deps.handleFocusedResourceShortcut(win, 'close-tab') + }, + }, + { + label: 'Close Window', + accelerator: 'CmdOrCtrl+Shift+W', + click: (_item, focusedWindow) => { + const win = focusedOrMain(focusedWindow) if (win && !win.isDestroyed()) win.close() }, }, @@ -165,7 +221,7 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] click: () => void openExternalSafe(DOCS_URL, deps.allowHttpLocalhost()), }, { - label: 'System Status', + label: 'Sim Status', click: () => void openExternalSafe(STATUS_URL, deps.allowHttpLocalhost()), }, ], diff --git a/apps/desktop/src/main/resource-shortcuts.test.ts b/apps/desktop/src/main/resource-shortcuts.test.ts new file mode 100644 index 00000000000..b59abaf36c5 --- /dev/null +++ b/apps/desktop/src/main/resource-shortcuts.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest' +import { resourceTabTargetIndex } from '@/main/resource-shortcuts' + +describe('resourceTabTargetIndex', () => { + it('cycles through tabs in both directions', () => { + expect(resourceTabTargetIndex('next-tab', 3, 2)).toBe(0) + expect(resourceTabTargetIndex('previous-tab', 3, 0)).toBe(2) + expect(resourceTabTargetIndex('next-tab', 3, -1)).toBe(0) + }) + + it('selects numbered tabs and reserves nine for the last tab', () => { + expect(resourceTabTargetIndex('select-tab-1', 5, 2)).toBe(0) + expect(resourceTabTargetIndex('select-tab-4', 5, 2)).toBe(3) + expect(resourceTabTargetIndex('select-tab-9', 5, 2)).toBe(4) + expect(resourceTabTargetIndex('select-tab-6', 5, 2)).toBeNull() + }) +}) diff --git a/apps/desktop/src/main/resource-shortcuts.ts b/apps/desktop/src/main/resource-shortcuts.ts index 3305fc89c72..2e5d8dab62f 100644 --- a/apps/desktop/src/main/resource-shortcuts.ts +++ b/apps/desktop/src/main/resource-shortcuts.ts @@ -9,9 +9,40 @@ export type FocusedResourceShortcut = | 'new-tab' | 'reopen-closed-tab' | 'close-tab' + | 'focus-omnibox' + | ResourceTabSelectionShortcut | 'reload-or-clear' | `zoom-${DesktopZoomAction}` +export type ResourceTabSelectionShortcut = + | 'next-tab' + | 'previous-tab' + | `select-tab-${1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9}` + +export function isResourceTabSelectionShortcut( + shortcut: FocusedResourceShortcut +): shortcut is ResourceTabSelectionShortcut { + return ( + shortcut === 'next-tab' || shortcut === 'previous-tab' || shortcut.startsWith('select-tab-') + ) +} + +/** Resolves browser-style next/previous and numbered tab shortcuts. */ +export function resourceTabTargetIndex( + shortcut: ResourceTabSelectionShortcut, + tabCount: number, + activeIndex: number +): number | null { + if (tabCount <= 0) return null + if (shortcut === 'next-tab') return activeIndex < 0 ? 0 : (activeIndex + 1) % tabCount + if (shortcut === 'previous-tab') { + return activeIndex < 0 ? tabCount - 1 : (activeIndex - 1 + tabCount) % tabCount + } + const requested = Number(shortcut.slice('select-tab-'.length)) + const target = requested === 9 ? tabCount - 1 : requested - 1 + return target >= 0 && target < tabCount ? target : null +} + export function zoomActionForShortcut(shortcut: `zoom-${DesktopZoomAction}`): DesktopZoomAction { switch (shortcut) { case 'zoom-in': diff --git a/apps/desktop/src/main/terminal-themes.test.ts b/apps/desktop/src/main/terminal-themes.test.ts index 55538455b0c..4887ea43342 100644 --- a/apps/desktop/src/main/terminal-themes.test.ts +++ b/apps/desktop/src/main/terminal-themes.test.ts @@ -1,4 +1,4 @@ -import { TERMINAL_DARK_THEME } from '@sim/desktop-bridge' +import { TERMINAL_DARK_THEME, TERMINAL_LIGHT_THEME } from '@sim/desktop-bridge' import { describe, expect, it } from 'vitest' import { parseTerminalThemeProfiles } from '@/main/terminal-themes' @@ -6,6 +6,10 @@ const PALETTE = { ...TERMINAL_DARK_THEME, background: '#101010', } +const LIGHT_PALETTE = { + ...TERMINAL_LIGHT_THEME, + background: '#fafafa', +} function profile(id: string, overrides: Record = {}) { return { @@ -22,10 +26,22 @@ describe('parseTerminalThemeProfiles', () => { expect(parseTerminalThemeProfiles([profile('iterm2:ocean')])).toEqual([profile('iterm2:ocean')]) }) + it('preserves separate iTerm2 light and dark palettes', () => { + const separateProfile = profile('iterm2:ocean', { + lightPalette: LIGHT_PALETTE, + darkPalette: PALETTE, + }) + + expect(parseTerminalThemeProfiles([separateProfile])).toEqual([separateProfile]) + }) + it('drops malformed colors and unsupported applications', () => { expect( parseTerminalThemeProfiles([ profile('bad-color', { palette: { ...PALETTE, background: 'rgb(0, 0, 0)' } }), + profile('bad-mode-color', { + lightPalette: { ...LIGHT_PALETTE, foreground: 'white' }, + }), profile('bad-source', { source: 'warp' }), ]) ).toEqual([]) diff --git a/apps/desktop/src/main/terminal-themes.ts b/apps/desktop/src/main/terminal-themes.ts index eecbe2e631c..dfd26cb60f5 100644 --- a/apps/desktop/src/main/terminal-themes.ts +++ b/apps/desktop/src/main/terminal-themes.ts @@ -1,6 +1,7 @@ import { execFile } from 'node:child_process' import { promisify } from 'node:util' import { + cloneTerminalSelectedProfile, isTerminalSelectedProfile, TERMINAL_DARK_THEME, TERMINAL_LIGHT_THEME, @@ -86,21 +87,25 @@ function terminalPalette(profile) { return palette } -function itermPalette(profile) { - const background = dictionaryColor(profile['Background Color'], LIGHT_THEME.background) +function itermColor(profile, key, suffix, fallback) { + return dictionaryColor(profile[key + suffix], dictionaryColor(profile[key], fallback)) +} + +function itermPalette(profile, suffix) { + const background = itermColor(profile, 'Background Color', suffix, LIGHT_THEME.background) const dark = isDark(background) const fallback = dark ? DARK_THEME : LIGHT_THEME const palette = { background: background, - foreground: dictionaryColor(profile['Foreground Color'], fallback.foreground), - cursor: dictionaryColor(profile['Cursor Color'], fallback.cursor), - cursorAccent: dictionaryColor(profile['Cursor Text Color'], background), - selectionBackground: dictionaryColor(profile['Selection Color'], fallback.selectionBackground), - selectionForeground: dictionaryColor(profile['Selected Text Color'], fallback.foreground) + foreground: itermColor(profile, 'Foreground Color', suffix, fallback.foreground), + cursor: itermColor(profile, 'Cursor Color', suffix, fallback.cursor), + cursorAccent: itermColor(profile, 'Cursor Text Color', suffix, background), + selectionBackground: itermColor(profile, 'Selection Color', suffix, fallback.selectionBackground), + selectionForeground: itermColor(profile, 'Selected Text Color', suffix, fallback.foreground) } for (let index = 0; index < PALETTE_KEYS.length; index += 1) { const key = PALETTE_KEYS[index] - palette[key] = dictionaryColor(profile['Ansi ' + index + ' Color'], fallback[key]) + palette[key] = itermColor(profile, 'Ansi ' + index + ' Color', suffix, fallback[key]) } return palette } @@ -131,12 +136,18 @@ try { const guid = String(profile.Guid || '') const name = String(profile.Name || '') if (!guid || !name) continue - profiles.push({ + const result = { id: 'iterm2:' + encodeURIComponent(guid), name: name, source: 'iterm2', - palette: itermPalette(profile) - }) + palette: itermPalette(profile, '') + } + const separateColors = profile['Use Separate Colors for Light and Dark Mode'] + if (separateColors === true || separateColors === 1) { + result.lightPalette = itermPalette(profile, ' (Light)') + result.darkPalette = itermPalette(profile, ' (Dark)') + } + profiles.push(result) } } catch (_) {} @@ -151,12 +162,7 @@ export function parseTerminalThemeProfiles(value: unknown): TerminalThemeProfile for (const candidate of value) { if (!isTerminalSelectedProfile(candidate) || seen.has(candidate.id)) continue seen.add(candidate.id) - profiles.push({ - id: candidate.id, - name: candidate.name, - source: candidate.source, - palette: { ...candidate.palette }, - }) + profiles.push(cloneTerminalSelectedProfile(candidate)) } return profiles.sort( (left, right) => left.source.localeCompare(right.source) || left.name.localeCompare(right.name) @@ -180,9 +186,8 @@ async function readTerminalThemeProfiles(): Promise { let cachedProfiles: TerminalThemeProfile[] | null = null let profileLoad: Promise | null = null -/** Reads Terminal.app and iTerm2 profiles once per desktop process. */ +/** Reads current Terminal.app and iTerm2 profiles, coalescing concurrent requests. */ export async function listTerminalThemeProfiles(): Promise { - if (cachedProfiles) return cachedProfiles profileLoad ??= readTerminalThemeProfiles() .then((profiles) => { cachedProfiles = profiles diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 074cdc0776c..3ac6530f1ed 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -34,7 +34,11 @@ import { import { sleep } from '@sim/utils/helpers' import { isRecordLike } from '@sim/utils/object' import type { BrowserWindow, WebContents } from 'electron' -import type { FocusedResourceShortcut } from '@/main/resource-shortcuts' +import { + type FocusedResourceShortcut, + isResourceTabSelectionShortcut, + resourceTabTargetIndex, +} from '@/main/resource-shortcuts' import { elide, TerminalSession } from '@/main/terminal/session' import { activePane, @@ -158,6 +162,8 @@ export class TerminalService { /** Insertion-ordered, which is also the tab order the user sees. */ private readonly sessions = new Map() private activeId: string | null = null + private agentActiveId: string | null = null + private activeTerminalUserSelected = false /** True while tearing every shell down, so an exit does not respawn one. */ private disposing = false private nextId = 1 @@ -175,6 +181,9 @@ export class TerminalService { */ private focusOwner: WebContents | null = null private releaseFocusListeners: (() => void) | null = null + /** Renderer currently displaying this terminal panel, independent of DOM focus. */ + private visibleOwner: WebContents | null = null + private releaseVisibleListeners: (() => void) | null = null /** Directories of recently closed terminals, newest first, for reopening. */ private readonly recentlyClosedCwds: string[] = [] /** Terminals handed to the user; the value is whether they have handed back. */ @@ -232,13 +241,30 @@ export class TerminalService { session.tabState(session.terminalId === this.activeId) ), activeTerminalId: this.activeId, + agentActiveTerminalId: this.agentActiveId, + } + } + + /** Tool-facing tab list whose active marker follows the agent cursor. */ + private getAgentTabs(): TerminalTabsState { + const state = this.getTabs() + return { + ...state, + tabs: state.tabs.map((tab) => ({ + ...tab, + active: tab.terminalId === this.agentActiveId, + })), + activeTerminalId: this.agentActiveId, } } /** Opens the first terminal, or adopts what is already running. */ start(options: TerminalStartOptions): TerminalTabsState { if (this.sessions.size === 0) { - this.spawn(this.startingCwd(), options.cols, options.rows) + this.spawn(this.startingCwd(), options.cols, options.rows, { + activateVisible: true, + activateAgent: true, + }) } return this.getTabs() } @@ -273,20 +299,90 @@ export class TerminalService { const size = active ? { cols: active.cols, rows: active.rows } : { cols: 80, rows: 24 } // A new terminal opens where the current one is: the user is almost always // continuing the same piece of work in a second shell. - this.spawn(cwd ?? active?.currentCwd ?? this.startingCwd(), size.cols, size.rows) + this.activeTerminalUserSelected = true + this.spawn(cwd ?? active?.currentCwd ?? this.startingCwd(), size.cols, size.rows, { + activateVisible: true, + activateAgent: this.agentActiveId === null, + }) + return this.getTabs() + } + + /** Recreates a persisted tab without treating restoration as user interaction. */ + restoreTerminal(cwd?: string): TerminalTabsState { + const active = this.activeId ? this.sessions.get(this.activeId) : null + const size = active ? { cols: active.cols, rows: active.rows } : { cols: 80, rows: 24 } + this.spawn(cwd ?? active?.currentCwd ?? this.startingCwd(), size.cols, size.rows, { + activateVisible: true, + activateAgent: true, + }) + this.activeTerminalUserSelected = false + return this.getTabs() + } + + /** Restores the persisted visible/agent cursor without claiming user ownership. */ + restoreActiveTerminal(terminalId: string): TerminalTabsState { + if (!this.sessions.has(terminalId)) { + throw new TerminalError('NO_SUCH_TERMINAL', unknownTerminal(terminalId)) + } + this.activeId = terminalId + this.agentActiveId = terminalId + this.activeTerminalUserSelected = false + this.emitTabs() return this.getTabs() } + /** Opens a shell for agent work without changing the terminal the user sees. */ + private openAgentTerminal(cwd?: string): TerminalTabsState { + const agent = this.agentActiveId ? this.sessions.get(this.agentActiveId) : null + const visible = this.activeId ? this.sessions.get(this.activeId) : null + const source = agent ?? visible + const size = source ? { cols: source.cols, rows: source.rows } : { cols: 80, rows: 24 } + this.spawn(cwd ?? source?.currentCwd ?? this.startingCwd(), size.cols, size.rows, { + activateVisible: this.activeId === null, + activateAgent: true, + }) + return this.getAgentTabs() + } + switchTerminal(terminalId: string): TerminalTabsState { if (!this.sessions.has(terminalId)) { throw new TerminalError('NO_SUCH_TERMINAL', unknownTerminal(terminalId)) } this.activeId = terminalId + this.activeTerminalUserSelected = true this.emitTabs() void this.sessions.get(terminalId)?.refreshCwd() return this.getTabs() } + /** Moves the agent cursor without changing the visible terminal. */ + private switchAgentTerminal(terminalId: string): TerminalTabsState { + if (!this.sessions.has(terminalId)) { + throw new TerminalError('NO_SUCH_TERMINAL', unknownTerminal(terminalId)) + } + this.agentActiveId = terminalId + this.emitTabs() + return this.getAgentTabs() + } + + /** Moves one terminal to a final list index without changing the active shell. */ + reorderTerminal(terminalId: string, targetIndex: number): TerminalTabsState { + if (!this.sessions.has(terminalId)) { + throw new TerminalError('NO_SUCH_TERMINAL', unknownTerminal(terminalId)) + } + if (!Number.isFinite(targetIndex)) return this.getTabs() + const entries = [...this.sessions.entries()] + const currentIndex = entries.findIndex(([id]) => id === terminalId) + const nextIndex = Math.max(0, Math.min(entries.length - 1, Math.trunc(targetIndex))) + if (currentIndex === nextIndex) return this.getTabs() + const [entry] = entries.splice(currentIndex, 1) + entries.splice(nextIndex, 0, entry) + this.sessions.clear() + for (const [id, session] of entries) this.sessions.set(id, session) + this.emitTabs() + return this.getTabs() + } + /** * Closes a terminal, or resets it when it is the only one left. * @@ -309,6 +405,17 @@ export class TerminalService { return this.retire(terminalId) } + /** Closes agent-owned work without destroying the shell the user claimed. */ + private closeAgentTerminal(terminalId: string): TerminalTabsState { + if (terminalId === this.activeId && this.activeTerminalUserSelected) { + throw new TerminalError( + 'INVALID_REQUEST', + 'That terminal is currently being used by the user. Switch to another agent terminal instead of closing it.' + ) + } + return this.closeTerminal(terminalId) + } + /** * Removes the temp directories of tracked runs that have since finished. * @@ -359,7 +466,10 @@ export class TerminalService { this.releasePendingRuns(terminalId) if (this.sessions.size === 0) { - this.spawn(this.resolveCwd(closedCwd), cols, rows) + this.spawn(this.resolveCwd(closedCwd), cols, rows, { + activateVisible: true, + activateAgent: true, + }) return this.getTabs() } @@ -367,6 +477,9 @@ export class TerminalService { if (this.activeId === terminalId) { this.activeId = order[index + 1] ?? order[index - 1] ?? null } + if (this.agentActiveId === terminalId) { + this.agentActiveId = order[index + 1] ?? order[index - 1] ?? null + } this.emitTabs() return this.getTabs() } @@ -383,9 +496,27 @@ export class TerminalService { handleFocusedShortcut( shortcut: FocusedResourceShortcut, ownerWindow: BrowserWindow | null, - emitRendererCommand: (command: TerminalShortcutCommand, terminalId: string) => void + emitRendererCommand: (command: TerminalShortcutCommand, terminalId: string) => void, + confirmCloseRunning?: (running: string) => boolean ): boolean { - if (!this.ownsInteraction(ownerWindow)) return false + if (shortcut === 'focus-omnibox') return false + const visibleTabShortcut = + shortcut === 'new-tab' || shortcut === 'reopen-closed-tab' || shortcut === 'close-tab' + const ownsVisibleTabs = + visibleTabShortcut && this.sessions.size > 0 && this.ownsVisiblePanel(ownerWindow) + if (!this.ownsInteraction(ownerWindow) && !ownsVisibleTabs) return false + + if (isResourceTabSelectionShortcut(shortcut)) { + const ids = [...this.sessions.keys()] + const targetIndex = resourceTabTargetIndex( + shortcut, + ids.length, + this.activeId ? ids.indexOf(this.activeId) : -1 + ) + const targetId = targetIndex === null ? null : ids[targetIndex] + if (targetId) this.switchTerminal(targetId) + return true + } switch (shortcut) { case 'new-tab': @@ -397,7 +528,12 @@ export class TerminalService { return true } case 'close-tab': - if (this.activeId) this.closeTerminal(this.activeId) + if (this.activeId) { + const active = this.sessions.get(this.activeId) + const running = active?.isBusy ? (active.foreground ?? 'A process') : null + if (running && confirmCloseRunning && !confirmCloseRunning(running)) return true + this.closeTerminal(this.activeId) + } return true case 'reload-or-clear': if (this.activeId) { @@ -438,6 +574,7 @@ export class TerminalService { // renderer behind it there is nothing that could ever release it. if (!owner || owner.isDestroyed()) return this.focusOwner = owner + this.activeTerminalUserSelected = true const release = () => this.setPanelFocused(false, owner) const onNavigate = (details: { isMainFrame: boolean; isSameDocument: boolean }) => { // A same-document route change keeps the React tree that made the claim. @@ -452,6 +589,29 @@ export class TerminalService { } } + /** Records which app window is currently displaying this terminal resource. */ + setPanelVisible(visible: boolean, owner?: WebContents | null): void { + if (!visible) { + if (owner && this.visibleOwner && owner !== this.visibleOwner) return + this.releaseVisibleOwner() + return + } + this.releaseVisibleOwner() + if (!owner || owner.isDestroyed()) return + this.visibleOwner = owner + const release = () => this.setPanelVisible(false, owner) + const onNavigate = (details: { isMainFrame: boolean; isSameDocument: boolean }) => { + if (details.isMainFrame && !details.isSameDocument) release() + } + owner.once('destroyed', release) + owner.on('did-start-navigation', onNavigate) + this.releaseVisibleListeners = () => { + if (owner.isDestroyed()) return + owner.removeListener('destroyed', release) + owner.removeListener('did-start-navigation', onNavigate) + } + } + /** Drops the claim and unsubscribes from the owner's lifecycle. */ private releaseFocusOwner(): void { this.releaseFocusListeners?.() @@ -459,6 +619,12 @@ export class TerminalService { this.focusOwner = null } + private releaseVisibleOwner(): void { + this.releaseVisibleListeners?.() + this.releaseVisibleListeners = null + this.visibleOwner = null + } + /** * Whether a global accelerator fired in the window that actually holds the * focused terminal panel. Without the window check a claim made in one window @@ -477,6 +643,15 @@ export class TerminalService { return ownerWindow.webContents === this.focusOwner } + private ownsVisiblePanel(ownerWindow: BrowserWindow | null): boolean { + return Boolean( + ownerWindow && + this.visibleOwner && + !this.visibleOwner.isDestroyed() && + ownerWindow.webContents === this.visibleOwner + ) + } + private rememberClosed(cwd: string | null): void { this.recentlyClosedCwds.unshift(cwd ?? '') if (this.recentlyClosedCwds.length > MAX_RECENTLY_CLOSED_TERMINALS) { @@ -521,8 +696,11 @@ export class TerminalService { } this.pendingRuns.clear() this.activeId = null + this.agentActiveId = null + this.activeTerminalUserSelected = false // A stale claim here is what let Cmd-W close a shell that no longer exists. this.setPanelFocused(false) + this.setPanelVisible(false) this.disposing = false } @@ -552,16 +730,16 @@ export class TerminalService { ): Promise { switch (operation) { case 'list': - return this.getTabs() + return this.getAgentTabs() case 'new': - return this.openTerminal(typeof args.cwd === 'string' ? args.cwd : undefined) + return this.openAgentTerminal(typeof args.cwd === 'string' ? args.cwd : undefined) case 'switch': - return this.switchTerminal(this.requireId(args)) + return this.switchAgentTerminal(this.requireId(args)) case 'close': // A named pane is a tmux thing and needs the session resolved below; // without one, close means the Sim terminal. if (typeof args.pane !== 'string' || !args.pane.trim()) { - return this.closeTerminal(this.requireId(args)) + return this.closeAgentTerminal(this.requireId(args)) } break default: @@ -718,7 +896,11 @@ export class TerminalService { /** The user pressing the hand-back button on a waiting handoff. */ finishHandoff(terminalId: string): void { - if (this.handoffs.has(terminalId)) this.handoffs.set(terminalId, true) + if (!this.handoffs.has(terminalId)) return + this.handoffs.set(terminalId, true) + if (terminalId === this.activeId && terminalId === this.agentActiveId) { + this.activeTerminalUserSelected = false + } } /** @@ -904,7 +1086,12 @@ export class TerminalService { return session.runCommand(command, toolCallId, resolveWaitMs(args.waitSeconds)) } - private spawn(cwd: string, cols: number, rows: number): TerminalSession { + private spawn( + cwd: string, + cols: number, + rows: number, + options: { activateVisible: boolean; activateAgent: boolean } + ): TerminalSession { const terminalId = String(this.nextId++) try { const session = TerminalSession.create({ @@ -927,7 +1114,8 @@ export class TerminalService { }, }) this.sessions.set(terminalId, session) - this.activeId = terminalId + if (options.activateVisible || this.activeId === null) this.activeId = terminalId + if (options.activateAgent || this.agentActiveId === null) this.agentActiveId = terminalId this.emitTabs() return session } catch (error) { @@ -952,10 +1140,13 @@ export class TerminalService { return session } - const active = this.activeId ? this.sessions.get(this.activeId) : null + const active = this.agentActiveId ? this.sessions.get(this.agentActiveId) : null if (active?.alive) return active - const spawned = this.spawn(this.startingCwd(), 80, 24) + const spawned = this.spawn(this.startingCwd(), 80, 24, { + activateVisible: this.activeId === null, + activateAgent: true, + }) if (!spawned.alive) { throw new TerminalError('SPAWN_FAILED', 'Could not open a terminal on this machine.') } diff --git a/apps/desktop/src/main/terminal/registry.ts b/apps/desktop/src/main/terminal/registry.ts index c9f6a9dcd65..673b29beb99 100644 --- a/apps/desktop/src/main/terminal/registry.ts +++ b/apps/desktop/src/main/terminal/registry.ts @@ -1,13 +1,14 @@ import { statSync } from 'node:fs' -import type { - TerminalCommandEvent, - TerminalOperation, - TerminalStartOptions, - TerminalTabsState, - TerminalToolArgs, - TerminalToolResponse, +import { + describeRunningCommand, + type TerminalCommandEvent, + type TerminalOperation, + type TerminalStartOptions, + type TerminalTabsState, + type TerminalToolArgs, + type TerminalToolResponse, } from '@sim/terminal-protocol' -import type { BrowserWindow, WebContents } from 'electron' +import { type BrowserWindow, dialog, type WebContents } from 'electron' import type { TerminalSessionSnapshot } from '@/main/desktop-chat-session-store' import type { FocusedResourceShortcut } from '@/main/resource-shortcuts' import { TerminalService, type TerminalServiceOptions, type TerminalSink } from '@/main/terminal' @@ -123,6 +124,10 @@ export class TerminalRegistry { return this.serviceFor(scope).switchTerminal(terminalId) } + reorderTerminal(scope: string, terminalId: string, targetIndex: number): TerminalTabsState { + return this.serviceFor(scope).reorderTerminal(terminalId, targetIndex) + } + closeTerminal(scope: string, terminalId: string): TerminalTabsState { return this.serviceFor(scope).closeTerminal(terminalId) } @@ -198,6 +203,18 @@ export class TerminalRegistry { this.serviceFor(scope).setPanelFocused(focused, owner) } + /** Records the visible terminal scope without treating visibility as keyboard focus. */ + setPanelVisible(scope: string, visible: boolean, owner?: WebContents | null): void { + if (this.suspendedScopes.has(scope)) return + if (!visible && !this.entries.has(scope)) return + if (visible && owner) { + for (const entry of this.entries.values()) { + if (entry.scope !== scope) entry.service.setPanelVisible(false, owner) + } + } + this.serviceFor(scope).setPanelVisible(visible, owner) + } + /** Handles a menu accelerator, which has a window but no renderer chat scope. */ handleFocusedShortcut( ownerWindow: BrowserWindow | null, @@ -205,15 +222,34 @@ export class TerminalRegistry { ): boolean { for (const entry of this.entries.values()) { if ( - entry.service.handleFocusedShortcut(shortcut, ownerWindow, (command, terminalId) => { - if (!ownerWindow || ownerWindow.webContents.isDestroyed()) return - ownerWindow.webContents.send( - 'terminal:shortcut-command', - command, - entry.scope, - terminalId - ) - }) + entry.service.handleFocusedShortcut( + shortcut, + ownerWindow, + (command, terminalId) => { + if (!ownerWindow || ownerWindow.webContents.isDestroyed()) return + ownerWindow.webContents.send( + 'terminal:shortcut-command', + command, + entry.scope, + terminalId + ) + }, + (running) => { + if (!ownerWindow || ownerWindow.isDestroyed()) return false + return ( + dialog.showMessageBoxSync(ownerWindow, { + type: 'warning', + title: 'Close Running Terminal?', + message: `${describeRunningCommand(running)} is still running.`, + detail: 'Closing this terminal will stop the process.', + buttons: ['Close Terminal', 'Cancel'], + defaultId: 1, + cancelId: 1, + noLink: true, + }) === 0 + ) + } + ) ) { return true } @@ -320,11 +356,11 @@ export class TerminalRegistry { const persisted = entry.persisted if (persisted) { for (const tab of persisted.tabs.slice(1)) { - entry.service.openTerminal(restorableCwd(tab.cwd)) + entry.service.restoreTerminal(restorableCwd(tab.cwd)) } const restored = entry.service.getTabs() const active = restored.tabs[persisted.activeIndex] - if (active) entry.service.switchTerminal(active.terminalId) + if (active) entry.service.restoreActiveTerminal(active.terminalId) } tabs = entry.service.getTabs() } finally { diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 662ba9f27cf..cef327baa5e 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -129,6 +129,32 @@ describe('closing terminals', () => { }) }) +describe('reordering terminals', () => { + it('moves a terminal without changing the active shell', () => { + const terminal = service() + const first = terminal.start({ cols: 80, rows: 24 }).activeTerminalId as string + const second = terminal.openTerminal().activeTerminalId as string + const third = terminal.openTerminal().activeTerminalId as string + + const after = terminal.reorderTerminal(first, 2) + + expect(after.tabs.map((tab) => tab.terminalId)).toEqual([second, third, first]) + expect(after.activeTerminalId).toBe(third) + }) + + it('clamps the destination and rejects an unknown terminal', () => { + const terminal = service() + const first = terminal.start({ cols: 80, rows: 24 }).activeTerminalId as string + const second = terminal.openTerminal().activeTerminalId as string + + expect(terminal.reorderTerminal(second, -100).tabs.map((tab) => tab.terminalId)).toEqual([ + second, + first, + ]) + expect(() => terminal.reorderTerminal('no-such-terminal', 0)).toThrow() + }) +}) + type OwnerWindow = Parameters[1] function runShortcut( @@ -188,6 +214,58 @@ describe('focus-gated shortcuts', () => { expect(terminal.getTabs().tabs).toHaveLength(1) }) + it('keeps tab shortcuts routed while the terminal panel is visible without focus', () => { + const terminal = service() + terminal.start({ cols: 80, rows: 24 }) + const renderer = rendererStub() + terminal.setPanelVisible(true, renderer.contents) + + expect(runShortcut(terminal, 'new-tab', renderer.window)).toBe(true) + expect(terminal.getTabs().tabs).toHaveLength(2) + expect(runShortcut(terminal, 'close-tab', renderer.window)).toBe(true) + expect(terminal.getTabs().tabs).toHaveLength(1) + expect(runShortcut(terminal, 'reopen-closed-tab', renderer.window)).toBe(true) + expect(terminal.getTabs().tabs).toHaveLength(2) + expect(runShortcut(terminal, 'focus-omnibox', renderer.window)).toBe(false) + + terminal.setPanelVisible(false, renderer.contents) + expect(runShortcut(terminal, 'new-tab', renderer.window)).toBe(false) + }) + + it('keeps agent work in the visible terminal after the user focuses it', async () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const visibleId = started.activeTerminalId as string + const renderer = rendererStub() + terminal.setPanelFocused(true, renderer.contents) + + const response = await terminal.executeTool('call-cwd', 'cwd', { terminalId: visibleId }) + const result = response.result as { terminalId: string } | undefined + + expect(response.ok).toBe(true) + expect(result?.terminalId).toBe(visibleId) + expect(terminal.getTabs().tabs).toHaveLength(1) + expect(terminal.getTabs().activeTerminalId).toBe(visibleId) + expect(terminal.getTabs().agentActiveTerminalId).toBe(visibleId) + }) + + it('keeps a running terminal open when close confirmation is declined', () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const activeId = started.activeTerminalId as string + stubSessions.get(activeId)?.setBusy(true) + const renderer = rendererStub() + const confirmClose = vi.fn(() => false) + terminal.setPanelFocused(true, renderer.contents) + + expect( + terminal.handleFocusedShortcut('close-tab', renderer.window, vi.fn(), confirmClose) + ).toBe(true) + + expect(confirmClose).toHaveBeenCalledWith('sleep 1') + expect(terminal.getTabs().activeTerminalId).toBe(activeId) + }) + it('opens tabs in main and sends canvas commands to the focused renderer', () => { const terminal = service() terminal.start({ cols: 80, rows: 24 }) @@ -212,6 +290,24 @@ describe('focus-gated shortcuts', () => { expect(emit).toHaveBeenLastCalledWith('zoom-reset', terminal.getTabs().activeTerminalId) }) + it('switches tabs with cycle and numbered shortcuts', () => { + const terminal = service() + const first = terminal.start({ cols: 80, rows: 24 }).activeTerminalId as string + const second = terminal.openTerminal().activeTerminalId as string + const third = terminal.openTerminal().activeTerminalId as string + const renderer = rendererStub() + terminal.setPanelFocused(true, renderer.contents) + + expect(runShortcut(terminal, 'next-tab', renderer.window)).toBe(true) + expect(terminal.getTabs().activeTerminalId).toBe(first) + expect(runShortcut(terminal, 'previous-tab', renderer.window)).toBe(true) + expect(terminal.getTabs().activeTerminalId).toBe(third) + expect(runShortcut(terminal, 'select-tab-2', renderer.window)).toBe(true) + expect(terminal.getTabs().activeTerminalId).toBe(second) + expect(runShortcut(terminal, 'select-tab-9', renderer.window)).toBe(true) + expect(terminal.getTabs().activeTerminalId).toBe(third) + }) + it('claims reopen even with no history, then reopens the latest closed terminal', () => { const terminal = service() terminal.start({ cols: 80, rows: 24 }) @@ -379,6 +475,29 @@ describe('handing the terminal to the user', () => { expect(() => terminal.finishHandoff(started.activeTerminalId as string)).not.toThrow() }) + it('resumes in the same terminal after the user explicitly hands it back', async () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const id = started.activeTerminalId as string + stubSessions.get(id)?.setBusy(true) + const renderer = rendererStub() + + const handoff = terminal.executeTool('call-handoff', 'handoff', { + terminalId: id, + reason: 'Sign in', + }) + terminal.setPanelFocused(true, renderer.contents) + setTimeout(() => { + terminal.finishHandoff(id) + stubSessions.get(id)?.setBusy(false) + }, 20) + await handoff + + const resumed = await terminal.executeTool('call-cwd', 'cwd', { terminalId: id }) + expect((resumed.result as { terminalId: string }).terminalId).toBe(id) + expect(terminal.getTabs().tabs).toHaveLength(1) + }) + it('fails the handoff if the terminal is closed while it waits', async () => { const terminal = service() const started = terminal.start({ cols: 80, rows: 24 }) @@ -395,7 +514,7 @@ describe('handing the terminal to the user', () => { }) describe('closing', () => { - it('closes the Sim terminal when no pane is named', async () => { + it('does not let the agent close the visible terminal the user selected', async () => { const terminal = service() terminal.start({ cols: 80, rows: 24 }) const second = terminal.openTerminal() @@ -404,8 +523,31 @@ describe('closing', () => { terminalId: second.activeTerminalId as string, }) - expect(response.ok).toBe(true) - expect(terminal.getTabs().tabs).toHaveLength(1) + expect(response.ok).toBe(false) + expect(response.code).toBe('INVALID_REQUEST') + expect(terminal.getTabs().tabs).toHaveLength(2) + }) + + it('opens and closes an agent terminal without changing visible selection', async () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const visible = terminal.openTerminal().activeTerminalId as string + + const opened = await terminal.executeTool('call-new', 'new', {}) + const agentTerminalId = (opened.result as { activeTerminalId: string | null } | undefined) + ?.activeTerminalId + + expect(opened.ok).toBe(true) + expect(agentTerminalId).toBeTruthy() + expect(terminal.getTabs().activeTerminalId).toBe(visible) + expect(terminal.getTabs().agentActiveTerminalId).toBe(agentTerminalId) + expect(terminal.getTabs().activeTerminalId).not.toBe(started.activeTerminalId) + + const closed = await terminal.executeTool('call-close', 'close', { + terminalId: agentTerminalId as string, + }) + expect(closed.ok).toBe(true) + expect(terminal.getTabs().activeTerminalId).toBe(visible) }) it('refuses to close a pane in a terminal that has no tmux', async () => { diff --git a/apps/desktop/src/main/terminal/session.test.ts b/apps/desktop/src/main/terminal/session.test.ts index d11886d3071..0224a2f1e0f 100644 --- a/apps/desktop/src/main/terminal/session.test.ts +++ b/apps/desktop/src/main/terminal/session.test.ts @@ -1,5 +1,46 @@ -import { describe, expect, it } from 'vitest' -import { stripAnsi, stripTerminalQueries, toInputChunks } from '@/main/terminal/session' +import type { TerminalCommandEvent } from '@sim/terminal-protocol' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + stripAnsi, + stripTerminalQueries, + TerminalSession, + toInputChunks, +} from '@/main/terminal/session' + +const ptyStub = vi.hoisted(() => ({ + dataHandler: null as ((data: string) => void) | null, + exitHandler: null as (() => void) | null, + writes: [] as string[], +})) + +vi.mock('@lydell/node-pty', () => ({ + spawn: () => ({ + pid: 1234, + onData: (handler: (data: string) => void) => { + ptyStub.dataHandler = handler + }, + onExit: (handler: () => void) => { + ptyStub.exitHandler = handler + }, + write: (data: string) => ptyStub.writes.push(data), + resize: vi.fn(), + kill: vi.fn(), + pause: vi.fn(), + resume: vi.fn(), + }), +})) + +vi.mock('@/main/terminal/shell-integration', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, createNonce: () => 'test-nonce' } +}) + +afterEach(() => { + vi.useRealTimers() + ptyStub.dataHandler = null + ptyStub.exitHandler = null + ptyStub.writes.length = 0 +}) describe('toInputChunks', () => { it('separates Enter from the text so the text is actually submitted', () => { @@ -93,3 +134,91 @@ describe('stripTerminalQueries', () => { expect(stripTerminalQueries('plain output\n')).toBe('plain output\n') }) }) + +describe('TerminalSession command lifecycle', () => { + it('keeps normal long-command activity until the foreground process exits', async () => { + vi.useFakeTimers() + const originalShell = process.env.SHELL + process.env.SHELL = '/bin/zsh' + const commandEvents: TerminalCommandEvent[] = [] + const session = TerminalSession.create({ + terminalId: 'terminal-1', + cwd: '/tmp', + cols: 80, + rows: 24, + callbacks: { + onData: () => {}, + onState: () => {}, + onCommand: (event) => commandEvents.push(event), + onExit: () => {}, + }, + }) + + try { + const resultPromise = session.runCommand('sleep 10', 'tool-call-long', 100) + ptyStub.dataHandler?.('\u001b]633;C;test-nonce\u0007working\n') + await vi.advanceTimersByTimeAsync(100) + expect(await resultPromise).toMatchObject({ status: 'running' }) + expect(commandEvents.filter((event) => event.phase === 'end')).toEqual([]) + + ptyStub.dataHandler?.('\u001b]633;D;0;test-nonce\u0007') + expect(commandEvents.at(-1)).toMatchObject({ + terminalId: 'terminal-1', + phase: 'end', + command: 'sleep 10', + toolCallId: 'tool-call-long', + exitCode: 0, + }) + } finally { + session.dispose() + if (originalShell === undefined) process.env.SHELL = undefined + else process.env.SHELL = originalShell + } + }) + + it('ends tool activity when an interactive command detaches, not when its process exits', async () => { + vi.useFakeTimers() + const originalShell = process.env.SHELL + process.env.SHELL = '/bin/zsh' + const commandEvents: TerminalCommandEvent[] = [] + const session = TerminalSession.create({ + terminalId: 'terminal-1', + cwd: '/tmp', + cols: 80, + rows: 24, + callbacks: { + onData: () => {}, + onState: () => {}, + onCommand: (event) => commandEvents.push(event), + onExit: () => {}, + }, + }) + + try { + const resultPromise = session.runCommand('vim', 'tool-call-1', 10_000) + ptyStub.dataHandler?.('\u001b]633;C;test-nonce\u0007\u001b[?1049h') + expect(await resultPromise).toMatchObject({ status: 'interactive' }) + + expect(commandEvents.at(-1)).toMatchObject({ + terminalId: 'terminal-1', + phase: 'end', + command: 'vim', + toolCallId: 'tool-call-1', + }) + + ptyStub.dataHandler?.('\u001b]633;D;0;test-nonce\u0007') + + expect(commandEvents.at(-1)).toMatchObject({ + terminalId: 'terminal-1', + phase: 'end', + command: 'vim', + exitCode: 0, + }) + expect(commandEvents.at(-1)?.toolCallId).toBeUndefined() + } finally { + session.dispose() + if (originalShell === undefined) process.env.SHELL = undefined + else process.env.SHELL = originalShell + } + }) +}) diff --git a/apps/desktop/src/main/terminal/session.ts b/apps/desktop/src/main/terminal/session.ts index 9a5a3810b24..350c1aa070d 100644 --- a/apps/desktop/src/main/terminal/session.ts +++ b/apps/desktop/src/main/terminal/session.ts @@ -315,6 +315,7 @@ export class TerminalSession { private shellIntegration = false private altScreen = false private foregroundCommand: string | null = null + private foregroundToolCallId: string | null = null private pendingCommand: PendingCommand | null = null /** Command line reported by the shell but not yet bracketed by output-start. */ private announcedCommand: string | null = null @@ -595,6 +596,7 @@ export class TerminalSession { resolve, } this.foregroundCommand = command + this.foregroundToolCallId = toolCallId this.emitState() this.callbacks.onCommand({ terminalId: this.terminalId, phase: 'start', command, toolCallId }) @@ -845,25 +847,22 @@ export class TerminalSession { * returned, since they are frames rather than output. */ private resolveInteractiveCommand(): void { - this.detachStillRunning((pending) => ({ - command: pending.command, - output: - 'This opened a full-screen interactive program, which now holds the terminal until it exits. terminal_read renders its current screen, so you can watch it: if it is doing work the user is waiting on, keep polling with wait + terminal_read until it finishes, exactly as you would a long command. Type into it with terminal_input and stop it with terminal_kill. The user can also drive it in the panel. terminal_run reports BUSY until it exits.', - status: 'interactive', - exitCode: null, - durationMs: Date.now() - pending.startedAt, - cwd: this.cwd, - terminalId: this.terminalId, - truncated: false, - })) + this.detachStillRunning( + (pending) => ({ + command: pending.command, + output: + 'This opened a full-screen interactive program, which now holds the terminal until it exits. terminal_read renders its current screen, so you can watch it: if it is doing work the user is waiting on, keep polling with wait + terminal_read until it finishes, exactly as you would a long command. Type into it with terminal_input and stop it with terminal_kill. The user can also drive it in the panel. terminal_run reports BUSY until it exits.', + status: 'interactive', + exitCode: null, + durationMs: Date.now() - pending.startedAt, + cwd: this.cwd, + terminalId: this.terminalId, + truncated: false, + }), + true + ) } - /** - * Hands back a command that is still going when the wait window elapses, - * with whatever it has printed so far. Not a failure: the agent polls from - * here with wait + terminal_read, which keeps the user seeing progress and - * lets the agent notice a prompt or an error as it appears. - */ /** * Hands a command back early when it has stopped mid-line and gone quiet — * the shape of something sitting on a prompt. Output that ends with a @@ -896,7 +895,7 @@ export class TerminalSession { truncated, ...(awaitingInput ? { awaitingInput: true } : {}), } - }) + }, false) } /** @@ -905,13 +904,27 @@ export class TerminalSession { * slot would let the next terminal_run interleave with it instead of * correctly reporting BUSY. */ - private detachStillRunning(build: (pending: PendingCommand) => TerminalRunResult): void { + private detachStillRunning( + build: (pending: PendingCommand) => TerminalRunResult, + endActivity: boolean + ): void { const pending = this.pendingCommand if (!pending) return clearTimeout(pending.timer) clearInterval(pending.promptWatchdog) this.pendingCommand = null - pending.resolve(build(pending)) + const result = build(pending) + if (endActivity) { + this.callbacks.onCommand({ + terminalId: this.terminalId, + phase: 'end', + command: pending.command, + toolCallId: pending.toolCallId, + durationMs: result.durationMs, + }) + this.foregroundToolCallId = null + } + pending.resolve(result) } private capturedText(pending: PendingCommand): string { @@ -956,11 +969,13 @@ export class TerminalSession { terminalId: this.terminalId, phase: 'end', command, + ...(this.foregroundToolCallId ? { toolCallId: this.foregroundToolCallId } : {}), ...(exitCode === null ? {} : { exitCode }), }) } this.foregroundCommand = null + this.foregroundToolCallId = null this.announcedCommand = null this.altScreen = false this.emitState() diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index 0ba00f9fdca..6feece68361 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -8,6 +8,7 @@ const autoUpdaterMock = { allowDowngrade: false, autoDownload: true, autoInstallOnAppQuit: false, + autoRunAppAfterInstall: true, logger: null as unknown, on: vi.fn(), setFeedURL: vi.fn(), @@ -25,6 +26,8 @@ import { isNewerVersion, parseSemver, resolveUpdateChannel, + type UpdaterHandle, + updateCheckIntervalMs, } from '@/main/updater' describe('resolveUpdateChannel', () => { @@ -34,8 +37,24 @@ describe('resolveUpdateChannel', () => { }) it('maps prerelease versions to their channel', () => { - expect(resolveUpdateChannel('1.2.3-beta.1')).toBe('beta') - expect(resolveUpdateChannel('1.2.3-alpha.2')).toBe('alpha') + expect(resolveUpdateChannel('1.2.3-dev.2')).toBe('dev') + expect(resolveUpdateChannel('1.2.3-staging.1')).toBe('staging') + }) + + it('keeps legacy alpha and beta builds on their environment streams', () => { + expect(resolveUpdateChannel('1.2.3-alpha.2')).toBe('dev') + expect(resolveUpdateChannel('1.2.3-beta.1')).toBe('staging') + }) +}) + +describe('updateCheckIntervalMs', () => { + it('checks dev and staging builds every five minutes', () => { + expect(updateCheckIntervalMs('1.2.3-dev.2')).toBe(5 * 60 * 1000) + expect(updateCheckIntervalMs('1.2.3-staging.1')).toBe(5 * 60 * 1000) + }) + + it('checks production builds every thirty minutes', () => { + expect(updateCheckIntervalMs('1.2.3')).toBe(30 * 60 * 1000) }) }) @@ -49,6 +68,7 @@ describe('parseSemver', () => { it('returns null for garbage', () => { expect(parseSemver('latest')).toBeNull() expect(parseSemver('1.2')).toBeNull() + expect(parseSemver('1.2.3garbage')).toBeNull() expect(parseSemver('')).toBeNull() }) }) @@ -125,7 +145,11 @@ describe('initUpdater state machine', () => { } } - async function createUpdater(options?: { autoDownload?: boolean; feedAvailable?: boolean }) { + async function createUpdater(options?: { + autoDownload?: boolean + feedAvailable?: boolean | 'no-release' + probeOriginFeed?: (feedUrl: string) => Promise + }) { const states: DesktopUpdateState[] = [] const handle = initUpdater({ getWindow: () => null, @@ -135,7 +159,7 @@ describe('initUpdater state machine', () => { onStateChange: (state) => states.push(state), loadAutoUpdater: () => autoUpdaterMock as unknown as typeof import('electron-updater')['autoUpdater'], - probeOriginFeed: async () => options?.feedAvailable ?? false, + probeOriginFeed: options?.probeOriginFeed ?? (async () => options?.feedAvailable ?? false), canSelfUpdate: async () => true, }) // Engine selection (signature detection) resolves asynchronously. @@ -150,7 +174,7 @@ describe('initUpdater state machine', () => { autoUpdaterMock.checkForUpdates.mockClear() autoUpdaterMock.downloadUpdate.mockClear() autoUpdaterMock.quitAndInstall.mockClear() - // Keep the update-downloaded dialog from resolving into quitAndInstall. + autoUpdaterMock.autoRunAppAfterInstall = false vi.mocked(dialog.showMessageBox).mockResolvedValue({ response: 1, checkboxChecked: false }) }) @@ -176,12 +200,14 @@ describe('initUpdater state machine', () => { { status: 'downloading', version: '2.0.0', percent: 42 }, { status: 'ready', version: '2.0.0' }, ]) + expect(dialog.showMessageBox).not.toHaveBeenCalled() + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() handle.install() expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) - it('stops at available and downloads on demand when auto-download is off', async () => { + it('downloads, installs, and relaunches from one Update action', async () => { autoUpdaterMock.autoDownload = false const { handle } = await createUpdater({ autoDownload: false }) @@ -191,6 +217,11 @@ describe('initUpdater state machine', () => { handle.check() expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1) expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + + emit('update-downloaded', { version: '2.0.0' }) + expect(dialog.showMessageBox).not.toHaveBeenCalled() + expect(autoUpdaterMock.autoRunAppAfterInstall).toBe(true) + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) it('checks from idle and ignores re-entrant checks while busy', async () => { @@ -204,6 +235,31 @@ describe('initUpdater state machine', () => { expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) }) + it('does not lose an interactive check while updater capability is initializing', async () => { + let resolveCapability: ((capable: boolean) => void) | undefined + const capability = new Promise((resolve) => { + resolveCapability = resolve + }) + const states: DesktopUpdateState[] = [] + const handle = initUpdater({ + getWindow: () => null, + events, + appOrigin: () => 'https://www.dev.sim.ai', + onStateChange: (state) => states.push(state), + loadAutoUpdater: () => + autoUpdaterMock as unknown as typeof import('electron-updater')['autoUpdater'], + probeOriginFeed: async () => true, + canSelfUpdate: () => capability, + }) + + handle.check() + expect(states).toEqual([{ status: 'checking' }]) + + resolveCapability?.(true) + await vi.advanceTimersByTimeAsync(0) + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + it('resets to idle when a downloaded update is a blocked downgrade', async () => { const { handle } = await createUpdater() emit('update-downloaded', { version: '0.0.1' }) @@ -212,6 +268,17 @@ describe('initUpdater state machine', () => { expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() }) + it('never exposes an equal, older, malformed, or cross-stream candidate as an update', async () => { + const { handle } = await createUpdater() + + for (const version of ['1.0.0', '0.9.9', 'nightly', '2.0.0-dev.1']) { + emit('update-available', { version }) + expect(handle.getState()).toEqual({ status: 'idle' }) + } + + expect(autoUpdaterMock.downloadUpdate).not.toHaveBeenCalled() + }) + it('surfaces updater errors and recovers via update-not-available', async () => { const { handle } = await createUpdater() emit('error', new Error('feed unreachable')) @@ -221,7 +288,8 @@ describe('initUpdater state machine', () => { }) it('switches to the per-env origin feed when the origin serves one', async () => { - await createUpdater({ feedAvailable: true }) + const { handle } = await createUpdater({ feedAvailable: true }) + handle.check() await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.setFeedURL).toHaveBeenCalledWith({ provider: 'generic', @@ -232,27 +300,70 @@ describe('initUpdater state machine', () => { }) it('keeps the packaged GitHub feed when the origin has no feed', async () => { - await createUpdater({ feedAvailable: false }) + const { handle } = await createUpdater({ feedAvailable: false }) + handle.check() await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.setFeedURL).not.toHaveBeenCalled() }) - it('skips checks on prerelease builds when the origin feed is down', async () => { + it('completes an interactive check immediately when the environment has no release', async () => { + const { handle, states } = await createUpdater({ feedAvailable: 'no-release' }) + + handle.check() + await vi.advanceTimersByTimeAsync(0) + + expect(states).toEqual([{ status: 'checking' }, { status: 'idle' }]) + expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + }) + + it('re-probes a no-release environment so a newly published update appears without restart', async () => { + const probeOriginFeed = vi + .fn<(feedUrl: string) => Promise>() + .mockResolvedValueOnce('no-release') + .mockResolvedValueOnce(true) + const { handle } = await createUpdater({ probeOriginFeed }) + + handle.check() + await vi.advanceTimersByTimeAsync(0) + expect(handle.getState()).toEqual({ status: 'idle' }) + expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + + handle.check() + await vi.advanceTimersByTimeAsync(0) + expect(probeOriginFeed).toHaveBeenCalledTimes(2) + expect(autoUpdaterMock.setFeedURL).toHaveBeenCalledTimes(1) + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + + it('recovers an interactive check when the feed probe times out', async () => { + const probeOriginFeed = vi.fn(() => new Promise(() => {})) + const { handle } = await createUpdater({ probeOriginFeed }) + + handle.check() + expect(handle.getState()).toEqual({ status: 'checking' }) + await vi.advanceTimersByTimeAsync(10_000) + + expect(handle.getState()).toEqual({ status: 'error' }) + expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + }) + + it('fails interactive checks promptly on prerelease builds when the origin feed is down', async () => { // The GitHub fallback is stable-only: a Sim Dev shell can never apply a // prod-identity artifact, so it must not check against it. - vi.mocked(app.getVersion).mockReturnValue('1.0.1-alpha.7') + vi.mocked(app.getVersion).mockReturnValue('1.0.1-dev.7') try { const { handle } = await createUpdater({ feedAvailable: false }) handle.check() await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() + expect(handle.getState()).toEqual({ status: 'error' }) } finally { vi.mocked(app.getVersion).mockReturnValue('1.0.0') } }) it('checks prerelease builds normally through the origin feed', async () => { - vi.mocked(app.getVersion).mockReturnValue('1.0.1-alpha.7') + vi.mocked(app.getVersion).mockReturnValue('1.0.1-dev.7') try { const { handle } = await createUpdater({ feedAvailable: true }) handle.check() @@ -262,6 +373,22 @@ describe('initUpdater state machine', () => { vi.mocked(app.getVersion).mockReturnValue('1.0.0') } }) + + it.each([ + ['1.0.1-dev.7', 5 * 60 * 1000], + ['1.0.1-staging.7', 5 * 60 * 1000], + ['1.0.1', 30 * 60 * 1000], + ])('schedules %s update polling every %i milliseconds', async (version, interval) => { + vi.mocked(app.getVersion).mockReturnValue(version) + const intervalSpy = vi.spyOn(globalThis, 'setInterval') + try { + await createUpdater({ feedAvailable: true }) + expect(intervalSpy).toHaveBeenCalledWith(expect.any(Function), interval) + } finally { + intervalSpy.mockRestore() + vi.mocked(app.getVersion).mockReturnValue('1.0.0') + } + }) }) function manifest(version: string): string { @@ -477,11 +604,34 @@ describe('checkForUpdatesInteractive', () => { await vi.advanceTimersByTimeAsync(0) expect(dialog.showMessageBox).toHaveBeenCalledWith( - expect.objectContaining({ message: 'Sim is up to date' }) + expect.objectContaining({ + type: 'info', + buttons: ['OK'], + defaultId: 0, + message: 'You’re up to date!', + detail: `Sim ${app.getVersion()} is currently the newest version available.`, + }) ) expect(shell.openExternal).not.toHaveBeenCalled() }) + it('fails a hung interactive check after twelve seconds instead of waiting thirty', async () => { + const handle: UpdaterHandle = { + setAutoDownload: () => {}, + getState: () => ({ status: 'checking' }), + check: vi.fn(), + install: vi.fn(), + onState: () => () => {}, + } + + checkForUpdatesInteractive({ getWindow: () => null, events, handle }) + await vi.advanceTimersByTimeAsync(12_000) + + expect(dialog.showMessageBox).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Could not check for updates' }) + ) + }) + it('only explains packaged-build updates when unpackaged', async () => { ;(app as unknown as { isPackaged: boolean }).isPackaged = false checkForUpdatesInteractive({ getWindow: () => null, events, handle: null }) diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index 711875cfdf6..31333eb1403 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -10,15 +10,19 @@ import type { EventRecorder } from '@/main/observability' const logger = createLogger('DesktopUpdater') const INITIAL_CHECK_DELAY_MS = 10_000 -const CHECK_INTERVAL_MS = 4 * 60 * 60 * 1000 +const PRERELEASE_CHECK_INTERVAL_MS = 5 * 60 * 1000 +const STABLE_CHECK_INTERVAL_MS = 30 * 60 * 1000 +const UPDATE_CHECK_TIMEOUT_MS = 10_000 +const INTERACTIVE_FEEDBACK_TIMEOUT_MS = 12_000 +const FEED_STATUS_HEADER = 'x-sim-desktop-update-feed' -export type UpdateChannel = 'latest' | 'beta' | 'alpha' +export type UpdateChannel = 'latest' | 'staging' | 'dev' /** * The per-environment update feed served by the Sim deployment this shell is * pointed at (`/api/desktop/update/latest-mac.yml`). Each environment pins - * which shell build its clients are offered — dev serves alpha builds, - * staging beta, prod stable — so the environment, not the client, is the + * which shell build its clients are offered — dev serves dev builds, + * staging serves staging builds, prod stable — so the environment, not the client, is the * channel. Returns null for origins that can't host a feed. */ export function feedUrlForOrigin(origin: string): string | null { @@ -58,20 +62,27 @@ function isReleaseAssetUrl(rawUrl: string): boolean { /** * Maps the running version to its update channel: prerelease builds follow * their prerelease channel, stable builds only ever see stable releases. - * Channels are strictly isolated — alpha/beta builds carry their own app + * Channels are strictly isolated — dev/staging builds carry their own app * identity (Sim Dev / Sim Staging) and update only via their origin feed; * the packaged GitHub fallback feed is stable-only. */ export function resolveUpdateChannel(version: string): UpdateChannel { - if (version.includes('-alpha')) { - return 'alpha' + if (/-dev(?:\.|$)/.test(version) || /-alpha(?:\.|$)/.test(version)) { + return 'dev' } - if (version.includes('-beta')) { - return 'beta' + if (/-staging(?:\.|$)/.test(version) || /-beta(?:\.|$)/.test(version)) { + return 'staging' } return 'latest' } +/** Dev/staging shells poll rapidly; production shells use a quieter cadence. */ +export function updateCheckIntervalMs(version: string): number { + return resolveUpdateChannel(version) === 'latest' + ? STABLE_CHECK_INTERVAL_MS + : PRERELEASE_CHECK_INTERVAL_MS +} + interface ParsedSemver { major: number minor: number @@ -85,7 +96,7 @@ interface ParsedSemver { * misconfigured feed). */ export function parseSemver(version: string): ParsedSemver | null { - const match = /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?/.exec(version.trim()) + const match = /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z.-]+))?$/.exec(version.trim()) if (!match) { return null } @@ -175,7 +186,7 @@ export interface UpdaterDeps { /** Test seam: overrides the lazy electron-updater load. */ loadAutoUpdater?: () => typeof import('electron-updater')['autoUpdater'] /** Test seam: overrides the origin feed availability probe. */ - probeOriginFeed?: (feedUrl: string) => Promise + probeOriginFeed?: (feedUrl: string) => Promise /** * Test seam: overrides Squirrel self-update capability detection (whether * the running bundle carries a real Developer ID signature). @@ -219,6 +230,14 @@ export function isNewerVersion(candidateVersion: string, currentVersion: string) return isDowngrade(candidateVersion, currentVersion) } +/** A signed shell may only install a strictly newer build from its own environment stream. */ +function isValidAutomaticUpdate(candidateVersion: string, currentVersion: string): boolean { + return ( + resolveUpdateChannel(candidateVersion) === resolveUpdateChannel(currentVersion) && + isNewerVersion(candidateVersion, currentVersion) + ) +} + /** * Whether Squirrel.Mac can swap this bundle in place. It validates a * downloaded update against the running app's code signature, so only builds @@ -255,7 +274,8 @@ async function detectSelfUpdateCapability(): Promise { * the download in the browser), `install` performs the `ready` action. */ interface UpdateEngine { - check(): void + /** `interactive` requests must always publish a terminal state for their waiting UI. */ + check(interactive?: boolean): void advance(): void install(): void setAutoDownload(enabled: boolean): void @@ -263,11 +283,12 @@ interface UpdateEngine { /** * Keeps installed shells current against the per-environment update feed: - * checks on launch and every four hours, and mirrors pipeline state to the + * checks on launch, then every five minutes for dev/staging builds or every + * thirty minutes for production builds, and mirrors pipeline state to the * renderer for the settings update UI and the minimum-shell-version gate. * * Developer-ID-signed builds use electron-updater (background download, - * install on user confirmation — never mid-session without consent). Builds + * then install and relaunch from an explicit Update action). Builds * that can't self-update (ad-hoc signed: local installs, pre-signing CI * prereleases) still poll the same feed but surface `available` as a manual * download link, so the whole pipeline is testable before signing exists. @@ -279,6 +300,7 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { const currentVersion = app.getVersion() let state: DesktopUpdateState = { status: 'idle' } + let installAfterDownload = false const listeners = new Set<(state: DesktopUpdateState) => void>() const setState = (next: DesktopUpdateState) => { state = next @@ -302,6 +324,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.channel = resolveUpdateChannel(currentVersion) autoUpdater.allowDowngrade = false autoUpdater.autoDownload = deps.autoDownload?.() ?? true + // Explicit Update actions must reopen Sim after Squirrel swaps the bundle. + autoUpdater.autoRunAppAfterInstall = true // Never install without vetting the downloaded version first. Enabled per // download in the update-downloaded handler, but only for accepted updates // — so a blocked/downgrade build that was already downloaded is never @@ -309,15 +333,39 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.autoInstallOnAppQuit = false autoUpdater.logger = null + let activeCheckId: number | null = null + let nextCheckId = 0 + let checkTimeout: ReturnType | null = null + const finishCheck = () => { + activeCheckId = null + if (checkTimeout !== null) { + clearTimeout(checkTimeout) + checkTimeout = null + } + } + autoUpdater.on('checking-for-update', () => { setState({ status: 'checking' }) }) autoUpdater.on('update-not-available', () => { + finishCheck() + installAfterDownload = false setState({ status: 'idle' }) }) autoUpdater.on('update-available', (info) => { + finishCheck() + if (!isValidAutomaticUpdate(info.version, currentVersion)) { + installAfterDownload = false + autoUpdater.autoInstallOnAppQuit = false + deps.events.record('update_blocked_version', { + version: info.version, + reason: 'not-newer', + }) + setState({ status: 'idle' }) + return + } deps.events.record('update_check', { available: info.version }) // With auto-download on, download-progress events follow immediately; // `available` is the terminal state only when downloads are manual. @@ -336,7 +384,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { }) autoUpdater.on('update-downloaded', (info) => { - if (isDowngrade(currentVersion, info.version)) { + if (!isValidAutomaticUpdate(info.version, currentVersion)) { + installAfterDownload = false autoUpdater.autoInstallOnAppQuit = false deps.events.record('update_blocked_version', { version: info.version }) setState({ status: 'idle' }) @@ -345,24 +394,15 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.autoInstallOnAppQuit = true deps.events.record('update_downloaded', { version: info.version }) setState({ status: 'ready', version: info.version }) - const win = deps.getWindow() - const options = { - type: 'info' as const, - buttons: ['Restart Now', 'Later'], - defaultId: 0, - cancelId: 1, - message: `Sim ${info.version} is ready to install`, - detail: 'Restart to finish updating. If you choose Later, the update installs on quit.', + if (installAfterDownload) { + installAfterDownload = false + autoUpdater.quitAndInstall() } - const prompt = win ? dialog.showMessageBox(win, options) : dialog.showMessageBox(options) - void prompt.then(({ response }) => { - if (response === 0) { - autoUpdater.quitAndInstall() - } - }) }) autoUpdater.on('error', (error) => { + finishCheck() + installAfterDownload = false deps.events.record('update_error', { message: getErrorMessage(error, 'unknown') }) setState({ status: 'error', version: state.version }) }) @@ -382,23 +422,37 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { const probeOriginFeed = deps.probeOriginFeed ?? (async (feedUrl: string) => { - const response = await net.fetch(`${feedUrl}/latest-mac.yml`) - return response.ok + const response = await net.fetch(`${feedUrl}/latest-mac.yml`, { + signal: AbortSignal.timeout(UPDATE_CHECK_TIMEOUT_MS), + }) + if (response.ok) return true + return response.status === 404 && response.headers.get(FEED_STATUS_HEADER) === 'no-release' + ? 'no-release' + : false }) - const feedConfigured: Promise = (async () => { + type FeedResolution = 'origin' | 'fallback' | 'no-release' | 'skip' + let originFeedConfigured = false + let feedProbeInFlight: Promise | null = null + const resolveFeedForCheck = async (): Promise => { + if (originFeedConfigured) return 'origin' const feedUrl = feedUrlForOrigin(deps.appOrigin()) const stableBuild = resolveUpdateChannel(currentVersion) === 'latest' if (!feedUrl) { - return stableBuild + return stableBuild ? 'fallback' : 'skip' } try { - if (!(await probeOriginFeed(feedUrl))) { + const availability = await probeOriginFeed(feedUrl) + if (availability === 'no-release') { + return 'no-release' + } + if (!availability) { throw new Error('feed responded non-OK') } autoUpdater.setFeedURL({ provider: 'generic', url: feedUrl, channel: 'latest' }) autoUpdater.channel = 'latest' + originFeedConfigured = true deps.events.record('update_feed', { url: feedUrl }) - return true + return 'origin' } catch (error) { logger.warn( stableBuild @@ -406,23 +460,69 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { : 'Origin update feed unavailable; prerelease build skips update checks', { feedUrl, message: getErrorMessage(error, 'unknown') } ) - return stableBuild + return stableBuild ? 'fallback' : 'skip' } - })() + } + const feedForCheck = (): Promise => { + if (originFeedConfigured) return Promise.resolve('origin') + if (feedProbeInFlight) return feedProbeInFlight + feedProbeInFlight = resolveFeedForCheck().finally(() => { + feedProbeInFlight = null + }) + return feedProbeInFlight + } return { - check() { - void feedConfigured.then((mayCheck) => { - if (!mayCheck) { + check(interactive = false) { + if ( + activeCheckId !== null || + state.status === 'available' || + state.status === 'downloading' || + state.status === 'ready' + ) { + return + } + const checkId = ++nextCheckId + activeCheckId = checkId + if (interactive) { + setState({ status: 'checking' }) + } + checkTimeout = setTimeout(() => { + if (activeCheckId !== checkId) return + finishCheck() + deps.events.record('update_error', { message: 'Update check timed out' }) + if (state.status === 'checking') setState({ status: 'error' }) + }, UPDATE_CHECK_TIMEOUT_MS) + void feedForCheck().then((feed) => { + if (activeCheckId !== checkId) return + if (feed === 'no-release') { + finishCheck() + if (interactive && state.status === 'checking') setState({ status: 'idle' }) return } - autoUpdater.checkForUpdates().catch((error) => { - logger.warn('Update check failed', { message: getErrorMessage(error, 'unknown') }) - }) + if (feed === 'skip') { + finishCheck() + if (interactive && state.status === 'checking') setState({ status: 'error' }) + return + } + autoUpdater + .checkForUpdates() + .then(() => { + if (activeCheckId !== checkId) return + finishCheck() + if (interactive && state.status === 'checking') setState({ status: 'error' }) + }) + .catch((error) => { + if (activeCheckId !== checkId) return + finishCheck() + logger.warn('Update check failed', { message: getErrorMessage(error, 'unknown') }) + if (state.status === 'checking') setState({ status: 'error' }) + }) }) }, advance() { autoUpdater.downloadUpdate().catch((error) => { + installAfterDownload = false logger.warn('Update download failed', { message: getErrorMessage(error, 'unknown') }) setState({ status: 'error', version: state.version }) }) @@ -440,12 +540,17 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { const fetchManifest = deps.fetchManifest ?? (async (url: string) => { - const response = await net.fetch(url) + const response = await net.fetch(url, { + signal: AbortSignal.timeout(UPDATE_CHECK_TIMEOUT_MS), + }) return response.ok ? await response.text() : null }) let downloadUrl: string | null = null + let checkInFlight = false const doCheck = async () => { + if (checkInFlight || state.status === 'available') return + checkInFlight = true setState({ status: 'checking', manual: true }) try { const feedUrl = feedUrlForOrigin(deps.appOrigin()) @@ -488,6 +593,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { } catch (error) { logger.warn('Manual update check failed', { message: getErrorMessage(error, 'unknown') }) setState({ status: 'error', version: state.version, manual: true }) + } finally { + checkInFlight = false } } @@ -511,6 +618,7 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { let engine: UpdateEngine | null = null let pendingAutoDownload: boolean | null = null + let pendingInteractiveCheck = false const canSelfUpdate = deps.canSelfUpdate ?? detectSelfUpdateCapability void canSelfUpdate() @@ -518,6 +626,10 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { .then((capable) => { engine = capable ? buildAutoEngine() : buildManualEngine() if (!engine) { + if (pendingInteractiveCheck) { + pendingInteractiveCheck = false + setState({ status: 'error' }) + } return } if (pendingAutoDownload !== null) { @@ -526,9 +638,13 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { if (!capable) { deps.events.record('update_manual_mode', {}) } + if (pendingInteractiveCheck) { + pendingInteractiveCheck = false + engine.check(true) + } const check = () => engine?.check() setTimeout(check, INITIAL_CHECK_DELAY_MS) - setInterval(check, CHECK_INTERVAL_MS) + setInterval(check, updateCheckIntervalMs(currentVersion)) }) return { @@ -541,14 +657,20 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { }, getState: () => state, check() { - if (!engine || state.status === 'checking' || state.status === 'downloading') { + if (state.status === 'checking' || state.status === 'downloading') { + return + } + if (!engine) { + pendingInteractiveCheck = true + setState({ status: 'checking' }) return } if (state.status === 'available') { + installAfterDownload = !state.manual engine.advance() return } - engine.check() + engine.check(true) }, install() { if (!engine) { @@ -571,8 +693,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { } } -const INTERACTIVE_CHECK_TIMEOUT_MS = 30_000 - /** * Menu-triggered manual check with user-visible feedback. A thin dialog layer * over the updater handle — it drives whichever pipeline initUpdater selected @@ -643,7 +763,13 @@ export function checkForUpdatesInteractive( }) return default: - void showDialog({ type: 'info', message: 'Sim is up to date' }) + void showDialog({ + type: 'info', + buttons: ['OK'], + defaultId: 0, + message: 'You’re up to date!', + detail: `Sim ${app.getVersion()} is currently the newest version available.`, + }) } } @@ -671,6 +797,6 @@ export function checkForUpdatesInteractive( } finish(state) }) - const timeout = setTimeout(() => finish(handle.getState()), INTERACTIVE_CHECK_TIMEOUT_MS) + const timeout = setTimeout(() => finish({ status: 'error' }), INTERACTIVE_FEEDBACK_TIMEOUT_MS) handle.check() } diff --git a/apps/desktop/src/main/window.test.ts b/apps/desktop/src/main/window.test.ts index 97096cb5c77..85644923119 100644 --- a/apps/desktop/src/main/window.test.ts +++ b/apps/desktop/src/main/window.test.ts @@ -1,16 +1,18 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { BrowserWindow } from 'electron' +import { BrowserWindow, systemPreferences } from 'electron' import type { ConfigStore } from '@/main/config' import type { EventRecorder } from '@/main/observability' import { backgroundColorFor, createMainWindow, createSecureWebPreferences, + ensureMicrophoneAccess, resolvePermission, sanitizeBounds, + setupPermissionHandlers, } from '@/main/window' const APP = 'https://sim.ai' @@ -28,22 +30,161 @@ describe('resolvePermission', () => { expect(resolvePermission('clipboard-read', '', APP)).toBe(false) }) - it('default-denies everything else, including media and unknown future permissions', () => { + it('allows audio-only media from the trusted origin, so voice input works', () => { + expect(resolvePermission('media', APP, APP, ['audio'])).toBe(true) + expect(resolvePermission('media', 'https://evil.example', APP, ['audio'])).toBe(false) + expect(resolvePermission('media', '', APP, ['audio'])).toBe(false) + }) + + it('denies media that is not narrowed to audio', () => { + expect(resolvePermission('media', APP, APP, ['video'])).toBe(false) + expect(resolvePermission('media', APP, APP, ['audio', 'video'])).toBe(false) + expect(resolvePermission('media', APP, APP, ['unknown'])).toBe(false) + expect(resolvePermission('media', APP, APP, [])).toBe(false) + expect(resolvePermission('media', APP, APP)).toBe(false) + }) + + it('default-denies everything else, including unknown future permissions', () => { for (const permission of [ - 'media', 'geolocation', 'notifications', 'camera', + 'display-capture', 'midi', 'pointerLock', 'openExternal', 'some-future-permission', ]) { expect(resolvePermission(permission, APP, APP)).toBe(false) + expect(resolvePermission(permission, APP, APP, ['audio'])).toBe(false) } }) }) +describe('ensureMicrophoneAccess', () => { + const realPlatform = process.platform + + function setPlatform(platform: NodeJS.Platform) { + Object.defineProperty(process, 'platform', { value: platform, configurable: true }) + } + + beforeEach(() => { + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('granted') + vi.mocked(systemPreferences.askForMediaAccess).mockResolvedValue(true) + }) + + afterEach(() => { + setPlatform(realPlatform) + vi.clearAllMocks() + }) + + it('skips the OS check off macOS, where there is no TCC gate', async () => { + setPlatform('win32') + await expect(ensureMicrophoneAccess()).resolves.toBe(true) + expect(systemPreferences.getMediaAccessStatus).not.toHaveBeenCalled() + }) + + it('raises the macOS prompt when access has never been decided', async () => { + setPlatform('darwin') + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('not-determined') + await expect(ensureMicrophoneAccess()).resolves.toBe(true) + expect(systemPreferences.askForMediaAccess).toHaveBeenCalledWith('microphone') + }) + + it('does not re-prompt once macOS already granted access', async () => { + setPlatform('darwin') + await expect(ensureMicrophoneAccess()).resolves.toBe(true) + expect(systemPreferences.askForMediaAccess).not.toHaveBeenCalled() + }) + + it('reports a blocked microphone without prompting, since macOS would not show one', async () => { + setPlatform('darwin') + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('denied') + await expect(ensureMicrophoneAccess()).resolves.toBe(false) + expect(systemPreferences.askForMediaAccess).not.toHaveBeenCalled() + }) + + it('denies when the OS request itself fails', async () => { + setPlatform('darwin') + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('not-determined') + vi.mocked(systemPreferences.askForMediaAccess).mockRejectedValue(new Error('boom')) + await expect(ensureMicrophoneAccess()).resolves.toBe(false) + }) +}) + +describe('setupPermissionHandlers', () => { + const realPlatform = process.platform + + function createSession() { + const session = { + setPermissionRequestHandler: vi.fn(), + setPermissionCheckHandler: vi.fn(), + } + setupPermissionHandlers(session as never, () => APP) + return { + request: session.setPermissionRequestHandler.mock.calls[0][0] as ( + contents: unknown, + permission: string, + callback: (granted: boolean) => void, + details: Record + ) => void, + check: session.setPermissionCheckHandler.mock.calls[0][0] as ( + contents: unknown, + permission: string, + requestingOrigin: string, + details: Record + ) => boolean, + } + } + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: realPlatform, configurable: true }) + vi.clearAllMocks() + }) + + it('grants a microphone request only after the OS agrees', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('denied') + const { request } = createSession() + const callback = vi.fn() + + request(null, 'media', callback, { requestingUrl: `${APP}/workspace`, mediaTypes: ['audio'] }) + await vi.waitFor(() => expect(callback).toHaveBeenCalledWith(false)) + + vi.mocked(systemPreferences.getMediaAccessStatus).mockReturnValue('granted') + request(null, 'media', callback, { requestingUrl: `${APP}/workspace`, mediaTypes: ['audio'] }) + await vi.waitFor(() => expect(callback).toHaveBeenLastCalledWith(true)) + }) + + it('rejects a camera request without touching the OS', () => { + const { request } = createSession() + const callback = vi.fn() + + request(null, 'media', callback, { requestingUrl: `${APP}/workspace`, mediaTypes: ['video'] }) + + expect(callback).toHaveBeenCalledWith(false) + expect(systemPreferences.getMediaAccessStatus).not.toHaveBeenCalled() + }) + + it('answers a clipboard request synchronously', () => { + const { request } = createSession() + const callback = vi.fn() + + request(null, 'clipboard-read', callback, { requestingUrl: `${APP}/workspace` }) + + expect(callback).toHaveBeenCalledWith(true) + }) + + it('reports microphone as permitted on the check path', () => { + const { check } = createSession() + + expect(check(null, 'media', APP, { mediaType: 'audio' })).toBe(true) + expect(check(null, 'media', APP, { mediaType: 'video' })).toBe(false) + expect(check(null, 'media', APP, {})).toBe(false) + expect(check(null, 'media', 'https://evil.example', { mediaType: 'audio' })).toBe(false) + }) +}) + describe('backgroundColorFor', () => { it('matches the persisted web-app theme', () => { expect(backgroundColorFor('dark', false)).toBe('#0c0c0c') diff --git a/apps/desktop/src/main/window.ts b/apps/desktop/src/main/window.ts index e5bdfb979b7..5ec4c02b7f8 100644 --- a/apps/desktop/src/main/window.ts +++ b/apps/desktop/src/main/window.ts @@ -1,6 +1,7 @@ import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import type { Session, WebPreferences } from 'electron' -import { app, BrowserWindow, dialog, nativeTheme } from 'electron' +import { app, BrowserWindow, dialog, nativeTheme, systemPreferences } from 'electron' import { type ConfigStore, isSafeInternalPath, type WindowBounds } from '@/main/config' import { isAppOrigin, isAuthSurfacePath } from '@/main/navigation' import type { EventRecorder } from '@/main/observability' @@ -51,27 +52,73 @@ export function createSecureWebPreferences( } /** - * The permission matrix: clipboard access for the trusted app origin, - * default-deny for everything else including unknown future permissions - * (media/camera/microphone stay denied). + * The permission matrix: clipboard and microphone access for the trusted app + * origin, default-deny for everything else including unknown future + * permissions (camera and screen capture stay denied). * * Clipboard reads are what the terminal's Paste action runs on — xterm has no * native paste target to fall back to, so a denied read is a Paste that fails. - * The grant is scoped to the app's own origin, which already reaches far more - * sensitive surfaces through the preload bridge, so it widens nothing that a - * compromise of that origin would not already own. + * `media` is what the composer's voice input runs on, and is narrowed to + * audio-only requests so a `getUserMedia({ video: true })` still gets nothing. + * Both grants are scoped to the app's own origin, which already reaches far + * more sensitive surfaces through the preload bridge, so they widen nothing + * that a compromise of that origin would not already own. + * + * `mediaTypes` is the capture kind Chromium asked for. It is absent on + * non-media permissions and, on the check path, may arrive as `unknown` — an + * un-narrowable request is treated as a camera request and denied. */ export function resolvePermission( permission: string, requestingOrigin: string, - appOrigin: string + appOrigin: string, + mediaTypes?: readonly string[] ): boolean { if (!requestingOrigin || requestingOrigin !== appOrigin) { return false } + if (permission === 'media') { + return ( + mediaTypes !== undefined && + mediaTypes.length > 0 && + mediaTypes.every((type) => type === 'audio') + ) + } return permission === 'clipboard-sanitized-write' || permission === 'clipboard-read' } +/** + * macOS gates microphone capture behind TCC on top of Chromium's own + * permission, and Chromium does not raise that system prompt for an Electron + * app — an un-granted app just gets a hard `NotAllowedError`. So the shell + * asks for OS access itself and only then answers the page's request. + * + * A `denied`/`restricted` status is not re-askable: macOS shows no second + * prompt, so this resolves false and the renderer surfaces the "blocked" + * message rather than the click doing nothing at all. + */ +export async function ensureMicrophoneAccess(): Promise { + if (process.platform !== 'darwin') { + return true + } + const status = systemPreferences.getMediaAccessStatus('microphone') + if (status === 'granted') { + return true + } + if (status === 'denied' || status === 'restricted') { + logger.warn('Microphone access is blocked by macOS privacy settings', { status }) + return false + } + try { + const granted = await systemPreferences.askForMediaAccess('microphone') + logger.info('Requested macOS microphone access', { granted }) + return granted + } catch (error) { + logger.error('Could not request macOS microphone access', { error: getErrorMessage(error) }) + return false + } +} + function originOf(raw: string): string { try { return new URL(raw).origin @@ -87,11 +134,21 @@ function originOf(raw: string): string { export function setupPermissionHandlers(session: Session, getAppOrigin: () => string): void { session.setPermissionRequestHandler((webContents, permission, callback, details) => { const requestingUrl = details.requestingUrl || webContents?.getURL() || '' - callback(resolvePermission(permission, originOf(requestingUrl), getAppOrigin())) + const mediaTypes = 'mediaTypes' in details ? details.mediaTypes : undefined + if (!resolvePermission(permission, originOf(requestingUrl), getAppOrigin(), mediaTypes)) { + callback(false) + return + } + if (permission === 'media') { + void ensureMicrophoneAccess().then(callback) + return + } + callback(true) }) - session.setPermissionCheckHandler((_webContents, permission, requestingOrigin) => { - return resolvePermission(permission, originOf(requestingOrigin), getAppOrigin()) + session.setPermissionCheckHandler((_webContents, permission, requestingOrigin, details) => { + const mediaTypes = details.mediaType ? [details.mediaType] : undefined + return resolvePermission(permission, originOf(requestingOrigin), getAppOrigin(), mediaTypes) }) } diff --git a/apps/desktop/src/preload/index.test.ts b/apps/desktop/src/preload/index.test.ts index 698bac6e381..dea8ac89579 100644 --- a/apps/desktop/src/preload/index.test.ts +++ b/apps/desktop/src/preload/index.test.ts @@ -27,11 +27,15 @@ describe('desktop preload bridge', () => { if (!exposed) throw new Error('Expected the desktop preload API to be exposed') expect(exposed.browserAgent.supportsAtomicPanelOcclusion).toBe(true) + await exposed.browserAgent.cancelTool?.('tool-1', 'chat-default') + await exposed.browserAgent.cancelActiveTool?.('chat-reloaded') await exposed.browserAgent.setPanelOccluded(true, 'chat-default') await exposed.browserAgent.setPanelOccluded(false, 'chat-explicit-false', false) await exposed.browserAgent.setPanelOccluded(true, 'chat-force', true) expect(invoke.mock.calls).toEqual([ + ['browser-agent:cancel-tool', 'tool-1', 'chat-default'], + ['browser-agent:cancel-active-tool', 'chat-reloaded'], ['browser-agent:set-panel-occluded', true, 'chat-default', false], ['browser-agent:set-panel-occluded', false, 'chat-explicit-false', false], ['browser-agent:set-panel-occluded', true, 'chat-force', true], diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index c9a5f404cfb..ffb5314df73 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -185,9 +185,15 @@ const api: SimDesktopApi = { scopeId: string ): Promise => ipcRenderer.invoke('browser-agent:execute-tool', toolCallId, tool, params, scopeId), + cancelTool: (toolCallId: string, scopeId: string): Promise => + ipcRenderer.invoke('browser-agent:cancel-tool', toolCallId, scopeId), + cancelActiveTool: (scopeId: string): Promise => + ipcRenderer.invoke('browser-agent:cancel-active-tool', scopeId), panelAction: (action: BrowserPanelAction, scopeId: string): void => { ipcRenderer.send('browser-agent:panel-action', action, scopeId) }, + openTab: (scopeId: string): Promise => + ipcRenderer.invoke('browser-agent:open-tab', scopeId), activateScope: (scopeId: string): Promise => ipcRenderer.invoke('browser-agent:activate-scope', scopeId), restoreScope: (scopeId: string): Promise => @@ -438,6 +444,12 @@ const api: SimDesktopApi = { ipcRenderer.invoke('terminal:open', cwd, scopeId), switchTerminal: (terminalId: string, scopeId: string): Promise => ipcRenderer.invoke('terminal:switch', terminalId, scopeId), + reorderTerminal: ( + terminalId: string, + targetIndex: number, + scopeId: string + ): Promise => + ipcRenderer.invoke('terminal:reorder', terminalId, targetIndex, scopeId), closeTerminal: (terminalId: string, scopeId: string): Promise => ipcRenderer.invoke('terminal:close', terminalId, scopeId), getTabs: (scopeId: string): Promise => @@ -470,6 +482,9 @@ const api: SimDesktopApi = { setFocused: (focused: boolean, scopeId: string): void => { ipcRenderer.send('terminal:focused', focused, scopeId) }, + setVisible: (visible: boolean, scopeId: string): void => { + ipcRenderer.send('terminal:visible', visible, scopeId) + }, finishHandoff: (terminalId: string, scopeId: string): void => { ipcRenderer.send('terminal:handoff-done', terminalId, scopeId) }, diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts index 8d3db2a1b30..fafa34ee4ac 100644 --- a/apps/desktop/src/test/electron-mock.ts +++ b/apps/desktop/src/test/electron-mock.ts @@ -35,6 +35,7 @@ export const crashReporter = { } export const shell = { + beep: vi.fn(), openExternal: vi.fn(() => Promise.resolve()), openPath: vi.fn(() => Promise.resolve('')), showItemInFolder: vi.fn(), @@ -57,6 +58,13 @@ export const clipboard = { readText: vi.fn(() => ''), } +export const systemPreferences = { + getMediaAccessStatus: vi.fn(() => 'granted'), + askForMediaAccess: vi.fn(() => Promise.resolve(true)), + canPromptTouchID: vi.fn(() => false), + promptTouchID: vi.fn(() => Promise.resolve()), +} + export const nativeTheme = { shouldUseDarkColors: false, on: vi.fn(), @@ -135,6 +143,7 @@ function createWebContentsMock() { reload: vi.fn(), print: vi.fn(), focus: vi.fn(), + invalidate: vi.fn(), isFocused: vi.fn(() => false), close: vi.fn(), isDestroyed: vi.fn(() => false), diff --git a/apps/docs/app/[lang]/[[...slug]]/page.tsx b/apps/docs/app/[lang]/[[...slug]]/page.tsx index a5702cc93c9..36c31389949 100644 --- a/apps/docs/app/[lang]/[[...slug]]/page.tsx +++ b/apps/docs/app/[lang]/[[...slug]]/page.tsx @@ -11,6 +11,7 @@ import { PageFooter } from '@/components/docs-layout/page-footer' import { PageNavigationArrows } from '@/components/docs-layout/page-navigation-arrows' import { LLMCopyButton } from '@/components/page-actions' import { StructuredData } from '@/components/structured-data' +import { APIExampleSelector } from '@/components/ui/api-example-selector' import { CodeBlock } from '@/components/ui/code-block' import { Heading } from '@/components/ui/heading' import { ResponseSection } from '@/components/ui/response-section' @@ -70,12 +71,16 @@ function stripLocalePrefix(url: string, lang: string): string { const APIPage = createAPIPage(openapi, { playground: { enabled: false }, + client: { + operation: { APIExampleSelector }, + }, content: { - renderOperationLayout: async (slots) => { + renderOperationLayout: (slots) => { return (
{slots.header} + {slots.description} {slots.apiPlayground} {slots.authSchemes &&
{slots.authSchemes}
} {slots.parameters} diff --git a/apps/docs/app/[lang]/layout.tsx b/apps/docs/app/[lang]/layout.tsx index f27d3187e69..aedb87c341d 100644 --- a/apps/docs/app/[lang]/layout.tsx +++ b/apps/docs/app/[lang]/layout.tsx @@ -3,6 +3,8 @@ import { defineI18nUI } from 'fumadocs-ui/i18n' import { DocsLayout } from 'fumadocs-ui/layouts/docs' import { RootProvider } from 'fumadocs-ui/provider/next' import { Geist_Mono, Inter } from 'next/font/google' +import Script from 'next/script' +import { ThemeProvider } from 'next-themes' import { SidebarFolder, SidebarItem, @@ -90,40 +92,49 @@ export default async function Layout({ children, params }: LayoutProps) { suppressHydrationWarning > -