Skip to content

AGENTS.md

Latest commit

 

History

History
233 lines (136 loc) · 57.7 KB

AGENTS.md

File metadata and controls

233 lines (136 loc) · 57.7 KB
Raw

AGENTS.md

This file provides guidance to agents when working with code in this repository. The root CLAUDE.md is a symlink to this file. Rules cite symbols and files by name rather than line number (line numbers drift); the @shofer/core carve-out means most non-host logic lives under packages/core/src/**, while host-only code (webview, ContextProxy, activation, task/skill managers) stays under src/**.

This file holds the rules that cross package boundaries (most do — the webview↔host bridge and the core↔plugin seams are where the invariants live). Rules scoped to a single directory live in that directory's own AGENTS.md (each with a CLAUDE.md symlink): webview-ui/ (SettingsView save-gating, ChatView send/draft rules, task-state rendering, Radix primitives) and plugins/rag-indexing/ (ignore-filter oracle, sole-indexer/search-only, index identity, embedder concurrency lanes, submodule-aware scanning).

Settings & configuration

  • SettingsView editing rules (bind to cachedState, save-gating for every control, the default-vs-current-profile distinction) live in webview-ui/AGENTS.md; the storage/merge architecture is docs/settings_overlay.md.

  • Typed Settings Rule: All extension settings and secrets MUST go through ContextProxy, not vscode.workspace.getConfiguration("shofer.*") or context.secrets.get/store directly. Add new state keys to globalSettingsSchema in packages/types/src/global-settings.ts and new secret keys to GLOBAL_SECRET_KEYS in the same file; access via ContextProxy.getInstance(context).getValue(key) for state and getSecret(key) for secrets. The proxy auto-routes by key and gives one cache + one event stream for the whole extension. Bypassing it splits the source-of-truth and breaks settings-view round-tripping.

  • No Ad-Hoc VS Code Config Reads Rule: vscode.workspace.getConfiguration(...) and context.globalState.get/update MUST NOT appear in feature code. Only three kinds of call sites may bypass ContextProxy: (1) ContextProxy itself; (2) the registration boundary reading non-shofer settings declared in ALLOWED_VSCODE_SETTINGS (e.g. terminal.integrated.inheritEnv in webviewMessageHandler.ts); (3) extension-point discovery whose schema lives in package.json (e.g. arkware.privateToolProviders in build-tools.ts). Note the runtime VS Code config namespace is arkware.*, not shofer.*. Everywhere else, go through ContextProxy.

  • Typed Command Rule: Commands MUST be registered through the typed CommandId plumbing — add the id to the commandIds const in packages/types/src/vscode.ts and wire the handler in src/activate/registerCommands.ts. Do NOT call vscode.commands.registerCommand(...) ad-hoc from extension.ts or service constructors. Use getCommand(id) from packages/core/src/utils/commands.ts anywhere a prefixed command name is needed so the prefix is centralized.

Task state, lifecycle & the agent loop

  • Task State Model: Task state is a two-axis structure TaskState = { lifecycle: TaskLifecycle, rating?: CompletionRating } defined in packages/types/src/history.ts. Never conflate the two axes into a single string enum (the old TaskExecutionState mistake — values like "completed_well"). When adding a new lifecycle, update TaskLifecycle, LIFECYCLE_VISUAL in TaskSelector.tsx, sanitizeRestoredState in TaskManager.ts if the lifecycle is transient, isTerminalLifecycle() in history.ts if it's terminal, and docs/task_states.md. When adding a rating value, update CompletionRating, RATING_VISUAL in TaskSelector.tsx, and completionRatingSchema in history.ts.

  • Single-Writer Persistence Rule: TaskManager.setState(taskId, state) is the only writer of taskState to both in-memory ManagedTask.state and persisted HistoryItem.taskState. Do NOT write taskState directly via provider.updateTaskHistory({ ..., taskState }) from tools, event handlers, or ShoferProvider. Pass an initialState to provider.createTask() for new tasks; route all subsequent transitions through setState. One choke point keeps in-memory and persisted views consistent and centralizes invariants/telemetry.

  • How the webview renders task state (the runtime?.state ?? item.taskState fallback chain, resolveStateVisual, the LIFECYCLE_VISUAL/RATING_VISUAL tables) is governed by webview-ui/AGENTS.md — the Task State Model rule above still names the tables a new lifecycle/rating must update.

  • Restore-Ordering Rule: Methods on TaskManager that depend on the managed-task map being authoritative (e.g. registerBackgroundTask) must call assertRestored() at the top to prevent order-of-initialization bugs. restoreManagedTasks sets the restored flag exactly once after rehydrating from history and running sanitizeRestoredState to downgrade transient lifecycles to idle.

  • Preload-Before-Publish Rule: A Task rehydrated from a HistoryItem MUST have its shoferMessages and apiConversationHistory populated from disk BEFORE it is pushed onto ShoferProvider.shoferStack (i.e. before it becomes getCurrentTask()). Construct with startTask: false, await task.preloadShoferMessages() (idempotent, sets historyPreloaded), THEN addShoferToStack(task) / in-place swap, THEN task.startFromHistory(). Awaiting a messagesReady promise in the SAME caller after publishing does NOT close the race — a concurrent postStateToWebview() sees the new task with shoferMessages: [] and broadcasts an empty snapshot, which ChatView renders as the home screen (the "task-switch home-screen flash"). showTaskWithId MUST await this.postStateToWebview() between createTaskWithHistoryItem and the chatButtonClicked dispatch. See Task.preloadShoferMessages and ShoferProvider.createTaskWithHistoryItem.

  • Self-Contained Events Rule: Lifecycle events (TaskCompleted, TaskAborted in packages/types/src/events.ts) must carry the data needed to interpret them — TaskCompleted carries { rating, isSubtask }, TaskAborted carries { reason }. Consumers must not call back into the task instance or ShoferProvider to figure out what happened. New lifecycle events include all decision-relevant fields in the payload type.

  • Self-Declared Terminal State Rule: A tool that represents the agent's own terminal state (currently only attempt_completion) MUST NOT call task.ask(...) to gate its lifecycle event. The completion result/rating/feedback are produced by the agent itself — no user approval, no follow-up prompt, no waiting on yesButtonClicked. Render via task.say("completion_result", …), persist artefacts, then synchronously call emitTaskCompleted and set task.abort = true. Using an isIdleAsk here is a bug: the statusMutationTimeout in Task.ask() fires TaskIdle → setState(idle) and the never-arriving response prevents emitTaskCompleted, so the rating overlay never lands. See AttemptCompletionTool.execute and docs/task_states.md.

  • Terminal-State Queue-Drain Rule: Because a self-declared terminal-state tool no longer calls task.ask(…), it loses Task.ask()'s free queue-drain. The tool MUST explicitly check task.messageQueueService.isEmpty() BEFORE the persist + emitTaskCompleted + task.abort = true block. If non-empty, dequeue the head (FIFO), render via task.say("user_feedback", …), pushToolResult(formatResponse.toolResult(<user_message>…</user_message>, images)), and return — letting the loop continue. The complementary case (a NEW message arriving via queueMessage AFTER abort) is handled in the host queueMessage handler (webviewMessageHandler.ts): if currentTask.abort === true at enqueue time it auto-triggers cancelAndProcessQueuedMessages(). See docs/message_queue.md.

  • Waiting Lifecycle Rule: A task parked inside wait (or any blocking-on-mail primitive) MUST transition to the waiting lifecycle for the block and back on resume. waiting is distinct from idle (not awaiting user input — it has live work elsewhere) and from running (its own loop isn't advancing). It is transient and MUST be downgraded to idle by sanitizeRestoredState in TaskManager.ts on restart. Per the Task State Model rule, also add it to TaskLifecycle + LIFECYCLE_VISUAL.

  • Subtask Question Routing Rule: When a child task (every task new_task creates) calls ask_followup_question, the question MUST route to the parent, NOT the user. It is delivered as a request envelope into the parent's mailbox — the parent sees it in its environment_details digest and answers with reply, which resolves the child's parked ask — while the child ALSO raises the ask in its own chat, so a human present there may answer instead. First answer wins and withdraws the other channel (docs/task_messaging.md). Never escalate a child's question to the webview ask UI of a DIFFERENT conversation — the user there has no context; the parent is the decision-maker. cancel_tasks is the parent-side escape hatch. The three control-plane subtask tools — new_task, attempt_completion (finishTask), cancel_tasks — share the single alwaysAllowSubtasks toggle in checkAutoApproval; keep them grouped so the toggle's meaning matches its label. The mailbox tools (send_message, reply, wait) sit OUTSIDE that toggle and are unconditionally approved: none of them blocks anything but the caller's own loop.

  • Decide-Before-Publish Rule (auto-approval vs. the ask's first emission): When Task.ask() publishes a COMPLETE ask, the auto-approval decision MUST already be on the message — autoApproved (and isAnswered, for an approved tool ask) set before the persist and before addToShoferMessages / updateShoferMessage. There are three such paths and all three must stay in sync: the single new complete message, the partial: undefined message, and the partial→final transition every streamed native tool call takes. Publishing first and stamping afterwards is not a cosmetic bug: locally it flickers Accept/Reject until a follow-up messageUpdated retracts them, and over the ShoferApi transport it hands a controller a finalized ask with no decision on it, which a controller that records asks durably turns into a pending approval nobody will ever be asked about. The decision is taken at most once per ask (decideAutoApproval's memo in Task.ask()) — a second checkAutoApproval can read provider state that disagrees with the answer already on the wire — and because every complete path returns on approve/deny, no post-publication fallback exists to stamp the flag late.

  • Emit-A-Snapshot Rule: Task mutates ShoferMessage objects in place (a streamed message across its chunks, an ask when it finalizes, handleWebviewAskResponse marking one answered). Every publication of one — emit(ShoferEventName.Message, …), postMessageToWebview, appendTaskMessage — therefore ships { ...message }, taken at the moment the call is made, never the live reference. Both addToShoferMessages and updateShoferMessage are reached after at least one await, and their consumers serialize later still (the SSE writer in http-server.ts, a task store that queues writes), so a reference publishes whatever state the object has drifted to by then. Nothing may correlate events by reference identity — use ts or askId.

  • Ordered-Publication Rule: every ShoferEventName.Message publication for a task — updated AND created events — rides the per-task FIFO in Task.ts (enqueueMessagePublication), because each publication awaits variable-latency persistence (a Postgres-backed task store under load) before emitting, and un-serialized emissions REORDER: a consumer reconstructing a stream from event order (a controller's SSE relay, its delta view) then re-emits whole prefixes, and — the worst case — a finalized completion_result still parked behind its store append is overtaken by TaskCompleted, so a controller that correctly stops reading there loses the tail of the reply. Do NOT add an emit path that bypasses the chain; a finalize path (attempt_completion's final say, an ask's partial→final transition) additionally AWAITS its publication so its event is on the wire before the caller proceeds to the turn's lifecycle events. The H29 partial-append throttle decision stays at ENQUEUE time — taken inside the queued publication it would measure store latency instead of write intent and disable itself under load.

Asks & auto-approval

  • Ask Categorization Rule: ShoferAsk is partitioned across exactly four state categorizers in packages/types/src/message.ts — isIdleAsk, isInteractiveAsk, isResumableAsk, isAgentRunningAsk — describing how the loop behaves while the ask is outstanding. Every ask MUST belong to exactly one; the partition test in message.test.ts enforces this. The auto-approval policy (isAutoApprovableAsk) is a separate, orthogonal predicate and must NEVER be collapsed onto the state membership lists (the old nonBlockingAsks conflation caused the attempt_completion regression that silently dropped queued messages and typed feedback). See docs/auto_approval.md.

  • Auto-Approval Fast-Path Rule: An ask that ends a turn (anything in idleAsks such as completion_result, api_req_failed, mistake_limit_reached, …) MUST NOT be in autoApprovableAsks. The fast-path in Task.ask() returns synchronously without entering pWaitFor, bypassing queue-drain and messageResponse handling — so auto-approving an idle ask (a) strands queued user messages forever and (b) lands typed feedback on an aborted task where the no-ask guard drops it. The invariant isAutoApprovableAsk(ask) ⇒ isAgentRunningAsk(ask) is asserted by message.test.ts.

  • Tool Name Form Dual-Representation Rule: Auto-approval operates on two tool-name conventions: ShoferSayTool.tool (camelCase, e.g. "runSlashCommand") in the checkAutoApproval path, and ToolName (snake_case, e.g. "run_slash_command") in TOOL_GROUPS. The mapping lives in SAY_TOOL_TO_NATIVE_NAME in auto-approval/tools.ts. When adding or reclassifying a tool, verify ALL THREE: (1) TOOL_GROUPS (the group determines the alwaysAllow* toggle); (2) SAY_TOOL_TO_NATIVE_NAME (missing entry → getToolGroupForSayTool() falls through to prefix inference → likely "uncategorized"); (3) the camelCase blocks in checkAutoApproval (auto-approval/index.ts).

  • No-Ask Handler Pattern Rule: Tool handlers in packages/core/src/tools/ MUST NOT call task.ask(...) directly. Use BaseTool.askApproval("tool", completeMessage) from the ToolCallbacks bag, which renders the ChatRow and gates on checkAutoApproval(). Calling task.ask(...) from a handler bypasses the auto-approval infrastructure and creates a second, uncontrolled path. See the Step 4 example in docs/adding-new-tools.md.

Tools (native / MCP / external)

  • Native Tool Implementation Rule: Adding a native tool is a coordinated multi-file change spanning schema, types, handler, router, parser, and (when it renders UI) ShoferSayTool + ChatRow + i18n. The authoritative checklist is docs/adding-new-tools.md — follow it rather than copying existing tools (which may predate conventions). Non-negotiable invariants: handlers extend BaseTool<"name"> in packages/core/src/tools/<Name>Tool.ts; approval goes through BaseTool.askToolApproval() so even auto-approved invocations render; the tool's ToolGroup in TOOL_GROUPS (packages/types/src/tool.ts) is the single source of truth for mode filtering AND auto-approval; tools MUST NOT branch on mode inside execute() (filtering happens upstream in filterNativeToolsForMode).

  • Native Tool Parser Cases Rule: A new native tool MUST add cases in both switches inside NativeToolCallParser — createPartialToolUse() (partial args) AND parseToolCall() (complete args) — to construct typed nativeArgs. Registering the tool in toolNames, TOOL_GROUPS, the schema, the handler, and the router in presentAssistantMessage.ts is NOT enough: with no parser case, nativeArgs is undefined, the dispatcher guard rejects the call with "missing nativeArgs", and execute() is never reached — surfacing as a generic "Provider Error / API Request Failed" with zero handler logs. Keep the two switches in lock-step; use coerceOptionalNumber / coerceOptionalBoolean / Array.isArray(...) for optional fields and validate required strings before assigning.

  • Advisory Parameter Defaults Rule: Native tool parameters described to the model as "soft"/"advisory" (e.g. softResultLength, softTimeoutSec on new_task) MUST be schema-optional (absent from JSON-schema required in packages/core/src/prompts/tools/native-tools/) AND have host-side defaults applied silently in execute() — never error via sayAndCreateMissingParamError. The prompt and handler must stay coherent: erroring on a field the prompt calls optional causes a retry/churn loop. If the host genuinely needs the value, remove the advisory framing and add it to required. See NewTaskTool.execute for the default + clamp-to-cap pattern.

  • Streaming Path-Stabilization Rule: Tools that update chat UI inside handlePartial() based on a path (or any string from partial-json streaming) MUST gate the update behind hasPathStabilized(block.params.path) from BaseTool, and MUST call resetPartialState() at the end of execute() on both success and error paths. Skipping stabilization flickers truncated paths; skipping the reset short-circuits the next invocation on a stale path.

  • Mode-Filtered Tool Exposure Rule: A tool's availability in a mode is controlled exclusively through (a) its ToolGroup in packages/types/src/tool.ts / mode.ts and (b) the upstream filterNativeToolsForMode / filterMcpToolsForMode / filterPrivateToolsForMode in build-tools.ts. Tools MUST NOT inspect the current mode inside execute() — by then the filter has already authorized the call, and a second gate is a second source of truth that drifts. See docs/tool_access.md.

  • Tool Group Count Coherence Rule: The tool-category vocabulary is open, over a closed set of 8 BUILTINS. toolGroups in tool.ts has exactly 8 members (read, write, execute, mcp, mode, subtasks, questions, uncategorized) — the BuiltinToolGroup type that the two exhaustive records are keyed by (TOOL_GROUPS, and GROUP_GATE in auto-approval/group-gates.ts). Any other name a tool declares is a dynamic category: ToolGroup = BuiltinToolGroup | (string & {}) is the open type every declaration site takes, and toolGroupNameSchema (a lowercase hyphen slug, ≤64 chars) is what validates it.

    • Adding a CATEGORY requires no coordinated change. It is minted at registration by whatever declared it — an MCP server's _meta, an mcp.json toolGroups entry, a private-tool provider's group, a plugin's custom tool, a name typed into the MCP group dropdown — and gated by its entry in the alwaysAllowGroups record. No enum edit, no new alwaysAllow* key, no hand-written UI row. If you find yourself editing five files to add a category, you are adding a builtin.
    • Adding a 9th BUILTIN is the rare coordinated change and needs a stated justification. It touches toolGroups, TOOL_GROUPS, GROUP_GATE, a new flat key in globalSettingsSchema (global-settings.ts), its own UI row, and the docs. A builtin earns its place only by carrying NATIVE tools or approval semantics the record must encode — a name that merely wants a toggle is a dynamic category. Consumer checklist: docs/terminology.md §10 and docs/tool-categories.md.
    • Never index TOOL_GROUPS by a name that came from config. A mode's tools array, an agent's declared groups and a server's _meta are all slug-validated but open, so a dynamic name — or a typo — reaches the lookup and a bare TOOL_GROUPS[name].tools throws. Use getToolGroupConfig(name), which returns undefined for anything that is not a builtin.

  • Tool-Group Dual-Resolution Rule: A tool that is not native has its group resolved by two independent paths that MUST agree — filterPrivateToolsForMode in build-tools.ts for mode VISIBILITY, and getToolGroupForSayTool in auto-approval/tools.ts for AUTO-APPROVAL. What makes them agree is the registry, not a convention: whatever accepts a declared group calls toolGroupRegistry.registerToolMapping(toolName, group) (tool-groups/category-registry.ts), and getToolGroupForSayTool consults that mapping — after the custom-tool registry, BEFORE prefix inference. Prefix inference is the LAST RESORT and guesses only two things: a browser-prefixed name into the dynamic browser category (registering it, so a toggle exists from the next call onward), and ide_ into execute. So a browser_-named tool a server declared as something else is gated by what it DECLARED. Register the mapping wherever you accept a group; without it the two paths disagree silently — the tool is visible as salesforce and gated as uncategorized, its toggle on and the call still asking.

  • Private-Tool Discovery Error Visibility Rule: Private-tool provider discovery in getPrivateLmToolMeta() (build-tools.ts) MUST NOT silently discard provider-level failures. A provider whose getDefinitionsCommand throws (extension not installed/activated/crashed) is currently skipped with no log and no user indication, so a provider configured in arkware.privateToolProviders that produces no tools is undiagnosable. At minimum log to the output channel; preferably surface a warning row in chat. The invokeToolCommand path in presentAssistantMessage.ts has the same problem.

  • MCP Per-Call Header Rule: A header that belongs to the RUN rather than to the host reaches an MCP server through exactly one path — the "resolve-mcp-call-headers" plugin broadcast in callTool, carried to the transport's fetch by the AsyncLocalStorage in call-headers.ts. Do NOT add a second one. In particular: do not mutate a transport's requestInit.headers per call (one hub-scoped connection serves every task, so that races), do not thread a headers argument down the runMcpToolCall → callTool chain into the SDK (the SDK's send() accepts no headers — StreamableHTTPClientTransport reads only resumptionToken/onresumptiontoken from request options), and do not put a credential in _meta instead (it is request metadata the server logs and forwards, not an authorization header). The resolver lives in a plugin because WHICH servers deserve a credential is deployment knowledge — the Core Self-Sufficiency Rule — and its answer is always optional: absent headers must leave the call byte-for-byte what it was.

  • Model Per-Request Header Rule: The same rule, one seam over. A header that belongs to the RUN rather than to the host reaches a MODEL API through exactly one path — the "resolve-model-call-headers" plugin broadcast, resolved once per request in buildApiHandler and carried to each provider client's custom fetch by the AsyncLocalStorage in api/call-headers.ts. Do NOT add a second one, and in particular do NOT resolve it inside a provider: buildApiHandler is the one layer every provider and every caller (the agent loop, the condenser, prompt enhancement) passes through, so a per-provider copy is both duplication and a silent gap for the next provider added. A new provider's only obligation is to pass fetchWithModelCallHeaders (or nodeFetchWithModelCallHeaders, for an SDK still typed against node-fetch) to its client; a client that cannot take one carries no per-request headers, which is the pre-plugin behaviour, not a bug to work around by mutating defaultHeaders (one handler serves every task, so that races). An answered header may never authorize: the merge refuses the credential and transport names, and the shared fetch sets only a header the request does not already carry, so the provider's own credential always wins. Keep both of those — they are why the question can safely name no endpoint.

  • Plugin-Contributed MCP Servers Re-Sync Rule: McpHub reads getContributedMcpServers() in its constructor, but the shared plugin manager is installed LAZILY by ShoferProvider.getPluginManager() — usually afterwards — so that read is not what makes a plugin's contributes.mcpServers child spawn. refreshProjectMcpServers() is, and it MUST be called wherever the plugin set becomes known or changes: the Plugins panel path (resyncAfterPluginChange) and the manager-install path (buildPluginManager). A host with no Plugins panel — shofer serve, a headless worker — reaches neither .shofer/mcp.json edits nor workspace-folder changes, so a missing re-sync there fails as a permanent, silent absence: the plugin loads, its declared server never exists, and nothing logs. The manager-install re-sync waits for activateCodePlugins(), and that ordering is load-bearing, not tidiness: activation awaits each plugin's initialize AND its services' start, which is where a plugin publishes the process env its own server's ${env:…} is declared against — connect first and the child is handed the literal placeholder, silently losing whatever the variable carried. Do not "optimize" the re-sync back to firing as soon as the manager exists. (A server declared in a config FILE against a plugin's runtime env has no such guarantee and cannot be given one — config is read before any plugin runs; the manifest route is the answer.) New discovery-dependent subsystems (skills, modes) have the same shape; wire them at the same seam.

  • MCP Per-Server Config Reads The CONNECTION Rule: a server's effective definition — toolGroups, disabledTools, transport, url — is connection.server.config, whatever scope produced it. Do NOT re-read the writable mcp.json for it. Four scopes feed the hub (org-global $SHOFER_GLOBAL_DIR/mcp.json, user ~/.shofer/mcp.json, project <workspace>/.shofer/mcp.json, and plugin contributes.mcpServers), and only two of them are files a lookup can open; fetchToolsList reading the user file alone dropped the groups of every server that reaches a WORKER, so their tools resolved uncategorized and each call raised an approval ask no headless host can answer. The one legitimate file read is the USER-OVERRIDE layer on top — updateServerToolList / setToolGroup write there and immediately re-list, so that layer must still win.

  • MCP Call-Site Indirection Rule: The canonical mcpHub.callTool() / mcpHub.readResource() call site (threading task.abortSignal) is the shared helper runMcpToolCall in use-mcp-shared.ts, NOT the tool classes (UseMcpToolTool.ts, accessMcpResourceTool.ts) that delegate to it. Tool files adding their own MCP call sites MUST accept signal?: AbortSignal and fall back to task.abortSignal.

Cancellation

  • Dual Cancellation-Path Rule: Task has two cancellation paths that MUST NOT be confused: (a) abortTask() — destructive tear-down (user Stop, budget "kill", stream failure): sets this.abort = true, aborts _taskAbortController, disposes. (b) cancelAndProcessQueuedMessages() — soft-cancel for Send Now: sets _softCancelForQueuedMessage = true, aborts the old controller, waits for the loop to exit, then replaces the controller and restarts with the dequeued message. New cancellation code MUST pick a variant and follow its protocol. A third path that aborts _taskAbortController without setting _softCancelForQueuedMessage = true makes the stream catch block call abortTask() → dispose(), destroying a task that should restart. See Task.ts.

  • Abort-Ordering Invariant: In abortTask(), this.abort = true MUST be set BEFORE _taskAbortController.abort() so synchronous observers see the boolean first. The stream catch block checks task.abort to decide whether to call abortTask(); reversing the order opens a window where the signal is aborted but this.abort is still false, so the catch block skips abortTask() and leaves a zombie task. See Task.ts.

  • Cooperative Cancellation Rule: Long-running async work (LLM calls, queue draining, watcher loops) MUST be cancellable via an AbortSignal threaded through the call stack — producer creates the controller, intermediate layers forward it, the bottom layer either checks signal.aborted between iterations/chunks and throws AbortError, or registers an abort listener that rejects its in-flight promise. Do NOT use polled boolean flags or rely on GC of dropped promises. See docs/cancellation.md.

Persistence & schemas

  • No Backward Compatibility Unless Asked: Persisted-state schema changes do not need migrations or compatibility shims unless the user explicitly requests them. Prefer clean, simple design over compatibility code. Bump the minor version (Y in X.Y.Z) when persisted state shape changes.

  • Schema-First Persistence Rule: Any new persisted shape (file on disk, globalState value, secrets entry, IPC payload) MUST be defined as a Zod schema first — in @shofer/types for cross-boundary shapes or a co-located *.schema.ts for service-local ones — and reads MUST go through schema.safeParse(raw) so corrupt/partial/stale data fails closed (returns undefined/empty, overwritten on next write) rather than throwing or propagating any. Pair with the Versioned Snapshot Rule for top-level containers.

  • Versioned Snapshot Rule: Persisted JSON snapshots MUST include an integer version field. On load, a mismatched version discards the snapshot and returns an empty value (NOT a migration), per "No Backward Compatibility Unless Asked". Cached file contents inside a snapshot MUST be hash-validated (SHA-256 of current on-disk bytes vs stored contentHash) and dropped on mismatch or ENOENT.

  • Module Boundaries Rule: @shofer/types is the single source of truth for cross-boundary schemas (asks, lifecycle events, telemetry events, settings keys, command ids, provider settings). Anything serialized over the webview ↔ host bridge, persisted to globalState/secrets, or sent to telemetry MUST have its shape declared as a Zod schema in packages/types/src/ and consumed via inferred z.infer<> types — never a hand-written interface duplicated per consumer.

  • Core Self-Sufficiency Rule: Shofer is a self-sufficient FOSS agent and MUST stay runnable standalone (VS Code, shofer serve, CLI) with no external service. @shofer/core and the extension host MUST NOT read/write a database, call a SaaS control plane, or otherwise couple to deployment-specific infrastructure — in particular nothing from the arkware.ai SaaS (its user-console, Postgres, buckets, saas.md) may be imported or assumed by core. Core communicates only over its own seams: the ShoferApi transport, events, ContextProxy state/secrets, and the filesystem. A controller/host persists whatever it needs on its side of the ShoferApi — e.g. user-console records brokered asks in its own user_approvals/user_questions tables; core just brokers the ask over the wire and never learns those tables exist. Any glue a specific integration needs (SaaS audit persistence, a hosted telemetry sink, org-IdP identity, remote memory) MUST be a plugin — a packages/core/src/plugins/* lifecycle hook (beforeAsk, beforeToolCall, afterToolCall, system-prompt transforms) packaged like plugins/live-memory/ — never baked into core. See PLUGINS.md and docs/plugin_system.md.

  • Linked-Profile Re-Init Rule: Any service that resolves and caches values from a linked API Configuration profile (e.g. a model's contextWindow) MUST be re-initialized on any profile mutation — saveApiConfiguration, upsertApiConfiguration, renameApiConfiguration, loadApiConfiguration, loadApiConfigurationById, deleteApiConfiguration — not only when the service's own settings change. Route this through a single re-init helper at the top of webviewMessageHandler.ts, called from both the updateSettings branch and every profile-mutator case. Resolution helpers MUST throw on failure (handler build error, missing info.contextWindow) so the service goes to Error state — NOT silently substitute a default constant — and SHOULD capture resolved-source provenance and surface it to the user so cache-vs-source mismatches are debuggable.

Webview & UI

Webview-internal rules (ChatView send-path and draft snapshots, Radix trigger primitives) live in webview-ui/AGENTS.md. The rules below stay here because they bind BOTH sides of the webview↔host bridge.

  • Webview Message Routing Rule: All webview → host messages MUST be a typed WebviewMessage variant (in @shofer/types) dispatched from the central webviewMessageHandler. Do NOT add ad-hoc-shaped vscode.postMessage(...) from components, and do NOT branch on message.type outside the handler. When a case group grows large or pulls a heavy dependency, extract a sibling *MessageHandler.ts module (cf. skillsMessageHandler.ts) and call into it. A plugin's UI never gets IPC message types at all — it talks to its extension half over the plugin UI channel (api.request), which is why the worktree, checkpoint and changed-file messages are gone. Symmetrically, host → webview messages MUST be a typed ExtensionMessage variant.

  • Exhaustive Switch Rule: Switches over discriminated unions in @shofer/types (WebviewMessage["type"], ShoferAsk, ShoferSay, ToolName, TaskLifecycle, TelemetryEventName) MUST end with a default: branch assigning the discriminant to a never-typed local (or assertNever(x)). This turns "forgot to handle the new variant" into a compile error. Do NOT use if (type === "a" || type === "b") … else { /* assume third */ } for unions of more than two variants.

  • Shared Module Isolation Rule: Anything under src/shared/ is imported by BOTH the extension host AND the webview-ui/ bundle (via the @shofer/shared/* alias → ../src/shared). These modules MUST NOT statically import host-only APIs — vscode, Node built-ins (fs, path, child_process, …), or host services (ContextProxy, TaskManager, output channels). The webview bundler can't resolve vscode; the module load throws silently and kills the React tree at startup (symptom: resolveWebviewView succeeds, HTML is assigned, but WelcomeView/ChatView/SettingsView never mount, host logs look normal). If a helper needs vscode, put it in a host-only dir and import from there — cf. syncExperimentContextKeys in src/activate/experimentContextKeys.ts.

  • i18n String Rule: User-facing strings MUST come from locale files. In the webview, const { t } = useAppTranslation() and t("namespace.key"); add the key first to webview-ui/src/i18n/locales/en/<namespace>.json (English is the source of truth; other locales sync from it). In extension-host code, import { t } from "./i18n" and add to src/i18n/locales/<lang>/. Hard-coded English in JSX or vscode.window.showXxxMessage(...) is a bug.

Logging & telemetry

  • Output Channel Logging Rule: Long-form/debug logging from extension code MUST go through the shared output-channel logger (outputChannelLogger.ts), obtained via getOutputChannel() in src/extension.ts and wrapped with createOutputChannelLogger (or createDualLogger). Do NOT use bare console.log in extension-host code — the user can't see it in VS Code and log scrapers can't attribute it. Webview code MUST NOT use console.* either — route diagnostics back to the host via vscode.postMessage so they hit the shared channel.

  • Circular-Import Lazy Logger Rule: A service file that cannot statically import getOutputChannel() from src/extension.ts without a require-cycle (because extension.ts transitively imports it) MUST use a lazy dynamic import: void import("…/extension").then(({ getOutputChannel }) => { const ch = getOutputChannel(); if (!ch) return; ch.appendLine(…) }).catch(() => {}). Do NOT substitute console.log — that violates the Output Channel Logging Rule. See the log() helper in git-watcher.ts.

  • Telemetry Capture Rule: All telemetry MUST go through TelemetryService.instance.captureXxx(...) typed methods in packages/telemetry/src/TelemetryService.ts. New event kinds go in TelemetryEventName in packages/types/src/telemetry.ts and get a typed captureXxx wrapper — do NOT pass raw event-name strings to captureEvent or instantiate posthog/posthog-node directly. Telemetry is opt-in behind BOTH the TELEMETRY_ENABLED build flag and the user's TelemetrySetting; do not bypass either. See docs/telemetry.md.

  • Telemetry Toggle Ordering Rule: When telemetrySetting changes, fire captureTelemetrySettingsChanged(prev, next) BEFORE updateTelemetryState(false) on toggle-OFF, and AFTER updateTelemetryState(true) on toggle-ON, so the change event is captured under the still-enabled side. Enforced by telemetrySettingsTracking.spec.ts.

File-change tracking, code-index & git

  • File Change Tracking Pattern: Any new native tool that modifies workspace files MUST call task.fileContextTracker.captureOriginal(relPath, content) before mutation and task.fileContextTracker.trackFileContext(relPath, "shofer_edited") after. Neither stores anything in core — they publish the file edit to the beforeFileEdit / afterFileEdit plugin hooks, which is how the bundled basics plugin's file-changes feature gets the baseline that makes diff and revert possible. Without both, the file is missing from the panel (capture only) or shows with diffing disabled (track only). Tools using DiffViewProvider get this automatically; tools doing direct filesystem writes (insert_edit, sed, rename_symbol, file for rm/mv, generate_image) must do it manually. See plugins/basics/docs/file-changes.md.

  • Tree-Sitter Language Registration Rule: When adding a file extension to CODEBASE_INDEX_FILE_EXTENSIONS in packages/types/src/codebase-index.ts, add a matching case in loadRequiredLanguageParsers in languageParser.ts. Without it, parseContent throws "Unsupported language: <ext>" — silently dropping files from the RAG index (the bundled rag-indexing plugin) and crashing list_code_definition_names. Extensions in fallbackExtensions (supported-extensions.ts) early-return before the parser switch, making any case for them dead code. Language queries in packages/core/src/services/tree-sitter/queries/*.ts using #match?/#eq? MUST be validated against real code, not only fixtures.

  • Indexer Policy Single-Source-of-Truth Rule: The indexer's static policy lists — CODEBASE_INDEX_FILE_EXTENSIONS and CODEBASE_INDEX_IGNORED_DIRS in codebase-index.ts — are the sole definitions, and they stay in @shofer/types even though the indexer itself is a plugin: the glob service and tree-sitter (glob/constants.ts, tree-sitter/index.ts) re-export rather than redeclare them. New entries belong in @shofer/types; never inline a literal list at a call site.

  • Indexer-internal rules (the IIgnoreFilter gitignore oracle, the sole-indexer/search-only contract, index identity via _resolveIndexKeyPath, per-provider embedder concurrency lanes, submodule-aware git scanning) live in plugins/rag-indexing/AGENTS.md.

  • Headless Hosts Run The Extension Rule: Do NOT assume a headless host skips extension activation. shofer serve loads the same extension.js bundle via ExtensionHost.activate(), so a served host DOES register the code-index/git-index factories and construct a CodeIndexManager. Features are gated by explicit config (e.g. the rag-indexing plugin's searchOnly / enabled settings), never by "the factory is unset on headless". Comments or docs claiming a feature "degrades to off on headless hosts" are wrong and have been corrected once already.

Skills

  • Skill Name Validation Rule: Skill names MUST pass validateSkillName() from @shofer/types (regex /^[a-z0-9]+(-[a-z0-9]+)*$/, max 64 chars), enforced in both SkillsManager.loadSkillMetadata() (discovery-time) and SkillsManager.createSkill() (creation-time) in SkillsManager.ts. Use the shared function; do NOT write a second regex or inline check.

  • SkillsManager Public API Rule: SkillsManager exposes four method categories that MUST NOT be confused: (a) discovery — discoverSkills, getSkillsMetadata, getSkillsForMode, getSkillContent; (b) lifecycle — createSkill, deleteSkill, moveSkill; (c) file watching — setupFileWatchers; (d) internal — scanSkillsDirectory, loadSkillMetadata, validateSkillName, getSkillNameErrorMessage. A new public method needs a matching docs/skills.md entry; a private helper does not.

  • Skills File-Name Collision Warning: Three files are named skills.ts — packages/types/src/skills.ts, packages/core/src/prompts/sections/skills.ts, and packages/core/src/prompts/tools/native-tools/skills.ts. Disambiguate in comments/docs ("skills (types)", "skills (prompt)", "skills (tool)"). Do not add a fourth — choose a distinct name.

Task placement & export

  • Task Placement Rule: Core does NOT decide where a task runs — neither the directory nor the machine. Two sibling broadcasts ask, in that order, and both take the first concrete answer: resolveTaskCwd (resolveTaskCwd.ts) broadcasts "resolve-task-cwd" (the bundled basics plugin's worktrees feature, plugins/basics/src/worktrees/, answers with a per-task git worktree), and resolveTaskPlacement (src/core/webview/resolveTaskPlacement.ts) broadcasts "resolve-task-placement", which a dispatcher plugin may CLAIM by answering { dispatched: { taskId, address?, token? } } — the task was created on another host, so core creates none and attaches to the reference instead (TaskAttachmentManager). For BOTH, a plugin that recognised the question and FAILED returns { error } (a throw means "not my question", which would silently fall back to running here — the outcome placement exists to prevent), and the caller aborts task creation; nobody answering leaves the in-process path byte-for-byte unchanged. Do not reintroduce a worktree- or worker-shaped field on newTask: the choice is plugin state, read at creation time.

  • Singleton Re-export Rule: When a service class has no per-instance state beyond method args, co-locate a pre-constructed instance export at the definition site (e.g. export const worktreeService = new WorktreeService() in plugins/basics/src/worktrees/worktree-service.ts) and do not instantiate the class externally.

  • Export Schema Co-Evolution Rule: The JSON export subsystem (export-json.ts) reads per-call metadata from api_req_started ShoferMessages whose payload (ShoferApiReqInfo) is written in Task.ts and parsed as UiApiReqStartedPayload. The three representations — ShoferApiReqInfo (write), UiApiReqStartedPayload (read), JsonExportCall (output) — MUST stay in lock-step; adding a write-side field without matching read + output keys silently drops it from exports. See docs/task-export.md.

  • Wire-Capture Anchor Rule: snapshotWireRequest() and snapshotApiReqError() in Task.ts use findLastIndex to locate the most recent api_req_started ShoferMessage for in-place mutation. Ordering invariant: (1) emit api_req_started, (2) optionally snapshotWireRequest()/snapshotApiReqError(), (3) this.api.createMessage(). Inserting new ShoferMessages between steps 1 and 2 redirects the wire request/error merge into the wrong entry, corrupting exports.

  • Export Resilient-Read Rule: Code reading persisted task JSON (ui_messages.json, api_conversation_history.json, history_item.json) MUST guard missing files and parse failures: fs.stat existence check, try/catch around JSON.parse, fallback to an empty/zeroed default. Failing closed preserves partial export; throwing loses the whole export. See ShoferProvider.exportTaskWithIdJson and the Schema-First Persistence Rule.

Protected files

  • Protected Files Rule: Sensitive workspace config (anything in .shofer/, .vscode/, AGENTS.md, *.code-workspace) is governed by PROTECTED_PATTERNS in ShoferProtectedController.ts. New sensitive paths go there, and tool write-paths gate on isWriteProtected(relPath) rather than special-casing filenames. SHIELD_SYMBOL is the single UI marker for a protected target — no per-tool icons. See docs/shofer_special_files.md. (Note: the root CLAUDE.md is now a symlink to AGENTS.md; consider adding CLAUDE.md to PROTECTED_PATTERNS.)

Service structure

  • Lazy Service Singleton Rule: Services with non-trivial initialization (CodeIndexManager, TaskManager, TelemetryService, …) MUST expose static getInstance(context, cwd?) and be obtained through it — never new XxxManager(...) ad-hoc from a tool, command handler, or message handler. Modules needing them in hot paths that would otherwise cycle at activation MUST use dynamic await import(...) at the call site (see the imports inside buildNativeToolsArrayWithRestrictions in build-tools.ts). Top-level static imports of those managers from build-tools.ts reintroduce the activation-order bug.

  • Service Module-Split Rule: A service file that grows past ~500–700 lines and accretes orthogonal concerns (persistence, queueing, transport, accounting, watching) MUST be split into focused single-responsibility modules with a thin orchestrator owning composition + lifecycle and re-exporting the public API. Each module should be importable/testable without vscode mocks where possible.

  • Shared ApiHandler Rule: Any service that talks to an LLM MUST go through buildApiHandler(providerSettings, { taskId }) in api/index.ts and consume the resulting ApiHandler interface — never instantiate provider SDKs (@anthropic-ai/sdk, openai, @google/genai, ollama, …) directly and never write bespoke per-provider fetch(). The shared handler gives streaming, abort handling, the full provider catalog, model info/pricing, telemetry, and retries. Depend only on ApiHandler + ApiStream chunk types so a new provider becomes available everywhere by editing the provider switch in one place. See docs/cost-calculation-and-limits.md.

  • Provider Reasoning-Preservation Rule: A provider whose models set preserveReasoning: true in their ModelInfo (Moonshot/Kimi K2.x/K3, DeepSeek-reasoner, Z.ai GLM-4.7+, Anthropic extended-thinking) MUST use a message converter that preserves the provider's native reasoning field (reasoning_content for OpenAI-compatible, thinking blocks for Anthropic) on every assistant message sent back to the API. The generic convertToAiSdkMessages() in ai-sdk.ts does NOT handle {type: "reasoning"} content blocks — it silently drops them, which forces the model to re-derive all prior reasoning from scratch on every tool-call round-trip, burning quota and invalidating the server-side prefix cache. Providers with preserveReasoning MUST either (a) use BaseOpenAiCompatibleProvider (base-openai-compatible-provider.ts) with a dedicated convertTo<Provider>Format() that extracts reasoning from content blocks into the native field (cf. convertToMoonshotFormat, convertToZAiFormat, convertToR1Format), or (b) use a raw-SDK handler that round-trips reasoning natively. The Vercel AI SDK path (OpenAICompatibleHandler) is only safe for providers that do NOT preserve reasoning. When adding a new model to an existing provider, verify the model's preserveReasoning flag and confirm the handler's message converter handles it — a model that starts preserving reasoning under a converter that drops it is a silent quota-burn bug.

TypeScript toolchain

  • Split-Compiler Rule: This workspace runs two TypeScript compilers on purpose. Type-checking (every check-types script) uses the TypeScript 7 native compiler — tsgo, from the @typescript/native-preview devDep — for the ~5× speedup. The library / API / emit path stays on TypeScript 6 (typescript@6.0.3, .bin/tsc): typescript-eslint, tsup's .d.ts generator, zod-to-ts, and @shofer/build's tsc emit all need the classic compiler API, which TS 7 does not ship. So: a check-types script invokes tsgo; a build script that emits invokes tsc (TS 6). Do not "upgrade" typescript to 7 or rename tsgo→tsc in check-types — typescript@6.0.3 being pinned is correct, not stale (see the toolchain note in TODO.md). Two tsup packages (@shofer/types, @shofer/cli) carry ignoreDeprecations: "6.0" because tsup injects a baseUrl we don't own; that is scoped to them deliberately — app tsconfigs dropped baseUrl outright.

Testing

  • Test Layout Rule: Tests live in __tests__/ folders adjacent to the source they cover. Vitest is configured with globals: true everywhere, so do NOT import describe/it/expect/vi from vitest. Naming: extension-host and CLI use *.test.ts(x) (Node env); the webview uses *.spec.ts(x) (jsdom env, with the shared webview-ui/vitest.setup.ts and the vscode mock at webview-ui/src/__mocks__/vscode.ts). Test verbosity is centralized via vitest-verbosity.ts — silent-by-default with --no-silent; never wire ad-hoc console.log filtering per package. See .shofer/commands/test.md for the per-package run recipe (never pnpm test / turbo test).

Pushing

  • Do not push to upstream (remote) git repo (in github). The user will do that. One reason for this is that git push is slow due to many test cases that it runs.

Documentation

  • No Cross-Repo Link Rule: This repository is FOSS with a public remote, and it is consumed as a submodule of a private integrator repo. A link that climbs out of this repo (](../../../…), ](../../../../…)) resolves only in that private checkout and is broken for everyone who clones Shofer — so it MUST NOT appear in any doc here. This is not a path to correct; it is a reference to remove. Rewrite the sentence so it stands on its own, name the external thing in prose without linking it, or link an in-repo doc that covers the same ground. The two directories are the giveaway: plugins/ in this repo holds PUBLIC plugins; an integrator's private plugins live in its own tree (for arkware.ai, the parent's shofer-plugins/, which holds temporal-worker, shofer-mesh, arkware-basics). A doc here referencing ../plugins/temporal-worker/DESIGN.md was therefore not a typo for some correct path — no path from this repo reaches it, and the reference itself was the defect. Referencing in the other direction is fine: a private doc may link freely into this public repo. Naming a private component as an example is acceptable; making a doc here depend on one — a link, or an explanation that only parses if the reader has it — is the Core Self-Sufficiency Rule violation in documentation form. The same boundary governs secrets: a credential must never be committed here (see the root CLAUDE.md § Secrets in the parent).

  • Documentation-Code Coherence Rule: Docs describe the current state only (no changelogs/history) and every named entity a doc references — file paths, function/class/interface/enum/type/constant names, config keys, VS Code command IDs, function signatures, interface field order, enum/union membership, quoted file-content literals, error-message text, item counts, and configuration defaults — MUST match the actual source at the time of writing. Verify with rg / file listing before publishing; prefer stable symbol names over line numbers (line numbers drift). Never fabricate error messages or "example output", never document dead code (a class with zero imports) as active, never state an architecture the code doesn't have, and when listing a subset of an enum qualify it ("core states", "including"). A broken path, phantom symbol, wrong default, wrong namespace (shofer.* vs the actual arkware.*), stale count, or invented behavior is a bug that wastes developer and agent time. (This single rule subsumes the many per-review doc-coherence findings that used to be listed separately.)

  • Documentation Lock-Step Rule: When you change a subsystem, update its companion doc in the same change. The pairings:

    Change Update in lock-step
    native tool added/renamed/reparametrized/regrouped docs/native_tools.md, toolDescription() in presentAssistantMessage.ts, docs/terminology.md §9
    TOOL_GROUPS / ALWAYS_AVAILABLE_TOOLS in tool.ts, or the built-in modes in plugins/builtin-config/plugin.json docs/tool-categories.md, plugins/builtin-config/docs/modes.md
    unconditional/toggle-gated tool in checkAutoApproval docs/auto_approval.md
    MessageQueueService public method / QueueEvents variant docs/message_queue.md + QueuedMessages.tsx if it emits a webview event
    new TaskLifecycle / @shofer/types type docs/terminology.md §7/§12
    skills subsystem (SkillsManager, SkillsTool, skillsMessageHandler, …) docs/skills.md
    new native-tool plumbing touch-point discovered the checklist AND "Gaps" section of docs/adding-new-tools.md
    Doc-only changes do NOT require a version bump; parameter-shape changes do (bump minor Y).

  • Dead Config/Code Rule: A contributes.configuration.properties entry in src/package.json with zero references across the TypeScript source is dead — it shows in the settings UI but does nothing. Either wire it up or remove it; likewise remove or accurately label ("defined but not yet wired") any class/file with zero imports rather than documenting it as active.