From 693d8a5c7b08a96a4c119f013ac971e6561ed52c Mon Sep 17 00:00:00 2001 From: christso <3115311+christso@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:32:08 +1100 Subject: [PATCH 1/2] refactor(plugin): move the catalogue out of the CLI repository --- .claude-plugin/marketplace.json | 30 +- .github/plugin/marketplace.json | 37 +- AGENTS.md | 3 +- README.md | 4 +- docs/src/content/docs/docs/guides/plugins.mdx | 34 + .../content/docs/docs/guides/workspaces.mdx | 1 + .../finance/.allagents/workspace.yaml | 9 + examples/workspaces/finance/README.md | 12 + .../mcp-proxy/.allagents/workspace.yaml | 2 +- plugins/allagents/.claude-plugin/plugin.json | 10 - plugins/allagents/skills/allagents/SKILL.md | 51 - plugins/deepwiki/.claude-plugin/plugin.json | 10 - plugins/deepwiki/.mcp.json | 8 - plugins/deepwiki/skills/deepwiki/SKILL.md | 29 - .../engineering/.claude-plugin/plugin.json | 9 - .../skills/pr-interactive-review/SKILL.md | 187 -- .../scripts/review-site.ts | 2137 ----------------- .../scripts/tsconfig.json | 14 - plugins/engineering/skills/worktree/SKILL.md | 41 - .../skills/worktree/scripts/worktree.sh | 128 - plugins/finance/.claude-plugin/plugin.json | 10 - .../skills/financial-modeling/SKILL.md | 54 - .../references/basic-calculation-blocks.xlsx | Bin 10748 -> 0 bytes .../references/calculation-blocks.md | 68 - .../financial-modeling/references/sources.md | 29 - .../tradingview/.claude-plugin/plugin.json | 10 - plugins/tradingview/.mcp.json | 8 - src/cli/skill-update.ts | 39 +- src/core/marketplace.ts | 51 +- src/core/workspace-modify.ts | 43 +- src/models/marketplace-manifest.ts | 69 +- src/utils/marketplace-manifest-parser.ts | 28 +- tests/unit/cli/skill-update-command.test.ts | 64 +- .../github-skill-source-promotion.test.ts | 14 + .../core/marketplace-external-subdir.test.ts | 174 ++ .../plugins/pr-interactive-review.test.ts | 820 ------- 36 files changed, 502 insertions(+), 3735 deletions(-) create mode 100644 examples/workspaces/finance/.allagents/workspace.yaml create mode 100644 examples/workspaces/finance/README.md delete mode 100644 plugins/allagents/.claude-plugin/plugin.json delete mode 100644 plugins/allagents/skills/allagents/SKILL.md delete mode 100644 plugins/deepwiki/.claude-plugin/plugin.json delete mode 100644 plugins/deepwiki/.mcp.json delete mode 100644 plugins/deepwiki/skills/deepwiki/SKILL.md delete mode 100644 plugins/engineering/.claude-plugin/plugin.json delete mode 100644 plugins/engineering/skills/pr-interactive-review/SKILL.md delete mode 100644 plugins/engineering/skills/pr-interactive-review/scripts/review-site.ts delete mode 100644 plugins/engineering/skills/pr-interactive-review/scripts/tsconfig.json delete mode 100644 plugins/engineering/skills/worktree/SKILL.md delete mode 100755 plugins/engineering/skills/worktree/scripts/worktree.sh delete mode 100644 plugins/finance/.claude-plugin/plugin.json delete mode 100644 plugins/finance/skills/financial-modeling/SKILL.md delete mode 100644 plugins/finance/skills/financial-modeling/references/basic-calculation-blocks.xlsx delete mode 100644 plugins/finance/skills/financial-modeling/references/calculation-blocks.md delete mode 100644 plugins/finance/skills/financial-modeling/references/sources.md delete mode 100644 plugins/tradingview/.claude-plugin/plugin.json delete mode 100644 plugins/tradingview/.mcp.json create mode 100644 tests/unit/core/marketplace-external-subdir.test.ts delete mode 100644 tests/unit/plugins/pr-interactive-review.test.ts diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 161afbe4..6913452c 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,27 +10,47 @@ { "name": "allagents", "description": "End-user guidance for managing AllAgents through its CLI", - "source": "./plugins/allagents" + "source": { + "source": "git-subdir", + "url": "https://github.com/allagentsdev/agent-plugins.git", + "path": "plugins/allagents" + } }, { "name": "deepwiki", "description": "AI-generated documentation for GitHub repositories", - "source": "./plugins/deepwiki" + "source": { + "source": "git-subdir", + "url": "https://github.com/allagentsdev/agent-plugins.git", + "path": "plugins/deepwiki" + } }, { "name": "engineering", "description": "Reusable engineering workflow skills", - "source": "./plugins/engineering" + "source": { + "source": "git-subdir", + "url": "https://github.com/allagentsdev/agent-plugins.git", + "path": "plugins/engineering" + } }, { "name": "tradingview", "description": "Official TradingView market data and analytics", - "source": "./plugins/tradingview" + "source": { + "source": "git-subdir", + "url": "https://github.com/allagentsdev/agent-plugins.git", + "path": "plugins/tradingview" + } }, { "name": "finance", "description": "Transparent spreadsheet financial modeling with reusable calculation blocks and validated outputs", - "source": "./plugins/finance", + "source": { + "source": "git-subdir", + "url": "https://github.com/allagentsdev/agent-plugins.git", + "path": "plugins/finance" + }, "category": "finance" } ] diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 5b4e6806..9052571c 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -10,17 +10,48 @@ { "name": "allagents", "description": "End-user guidance for managing AllAgents through its CLI", - "source": "./plugins/allagents" + "source": { + "source": "github", + "repo": "allagentsdev/agent-plugins", + "path": "plugins/allagents" + } }, { "name": "deepwiki", "description": "AI-generated documentation for GitHub repositories", - "source": "./plugins/deepwiki" + "source": { + "source": "github", + "repo": "allagentsdev/agent-plugins", + "path": "plugins/deepwiki" + } + }, + { + "name": "engineering", + "description": "Reusable engineering workflow skills", + "source": { + "source": "github", + "repo": "allagentsdev/agent-plugins", + "path": "plugins/engineering" + } }, { "name": "tradingview", "description": "Official TradingView market data and analytics", - "source": "./plugins/tradingview" + "source": { + "source": "github", + "repo": "allagentsdev/agent-plugins", + "path": "plugins/tradingview" + } + }, + { + "name": "finance", + "description": "Transparent spreadsheet financial modeling with reusable calculation blocks and validated outputs", + "source": { + "source": "github", + "repo": "allagentsdev/agent-plugins", + "path": "plugins/finance" + }, + "category": "finance" } ] } diff --git a/AGENTS.md b/AGENTS.md index 869f0abd..225e02b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,8 @@ AllAgents is a CLI for managing AI coding-assistant plugins, workspaces, and cli - `tests/unit/`: focused behavioral contracts. - `tests/e2e/`: built CLI and filesystem workflows. - `docs/src/content/docs/`: public documentation. -- `plugins/`: first-party plugin content. +- `examples/workspaces/`: workspace templates that compose plugin capabilities. +- First-party plugin content lives in `allagentsdev/agent-plugins`; this repository retains forwarding marketplace manifests for existing installations. - `.agents/`: task-specific repository guidance reached through the routing table below. ## Routing diff --git a/README.md b/README.md index 4f3bdd3e..d7a90a53 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Install the end-user AllAgents skill when you want your coding agent to operate the CLI using its current machine-readable command contracts: ```bash -npx allagents plugin install allagents@allagentsdev/allagents --scope user +npx allagents plugin install allagents@allagentsdev/agent-plugins --scope user ``` The Finance plugin provides a `financial-modeling` skill for forecasts, budgets, @@ -37,7 +37,7 @@ consistent timelines and traceable outputs, with guidance for preserving missing inputs and validating the delivered workbook. ```bash -npx allagents plugin install finance@allagentsdev/allagents --scope user +npx allagents plugin install finance@allagentsdev/agent-plugins --scope user ``` ## How It Works diff --git a/docs/src/content/docs/docs/guides/plugins.mdx b/docs/src/content/docs/docs/guides/plugins.mdx index 369b5f70..36488976 100644 --- a/docs/src/content/docs/docs/guides/plugins.mdx +++ b/docs/src/content/docs/docs/guides/plugins.mdx @@ -260,3 +260,37 @@ Plugins use the `plugin@marketplace` format: | Well-known | `code-review@claude-plugins-official` | | owner/repo | `my-plugin@owner/repo` | | GitHub marketplace URL | `my-plugin@https://github.com/owner/repo` | + +## First-party Plugin Catalogue + +Reusable AllAgents plugins live in [allagentsdev/agent-plugins](https://github.com/allagentsdev/agent-plugins). +Workspace templates remain in [allagentsdev/allagents/examples/workspaces](https://github.com/allagentsdev/allagents/tree/main/examples/workspaces) +and select capabilities from that catalogue: + +```yaml +plugins: + - finance@allagentsdev/agent-plugins +``` + +Existing marketplace declarations such as `finance@allagentsdev/allagents` +continue to use the `allagents` marketplace, whose entries forward to the new +catalogue. Upgrade AllAgents before refreshing that marketplace: external +plugin paths require the updated resolver. Native Claude and Copilot clients +use their own marketplace source formats; update those clients if their +installed versions do not support external plugin subdirectories. + +Direct old paths such as `allagentsdev/allagents/plugins/deepwiki` must be +changed to `deepwiki@allagentsdev/agent-plugins`. Plugin directories are no +longer stored in the CLI repository. Git refs that predate the move retain +their original contents. + +External marketplace entries accept `github`, `url` and `git-subdir` source +objects. AllAgents preserves their `ref` and `path`, resolves only the selected +subdirectory for online and offline file installations, and rejects paths +that escape the checkout. File installations currently reject explicit `sha` +pins; use a branch or tag in `ref` instead. This limitation does not alter +what native clients support themselves. + +For contributors, merge the catalogue extraction into `agent-plugins/main` +before merging the CLI relocation: the forwarding entries follow the new +repository's default branch. diff --git a/docs/src/content/docs/docs/guides/workspaces.mdx b/docs/src/content/docs/docs/guides/workspaces.mdx index 570eeb18..7460b572 100644 --- a/docs/src/content/docs/docs/guides/workspaces.mdx +++ b/docs/src/content/docs/docs/guides/workspaces.mdx @@ -50,6 +50,7 @@ allagents workspace init my-workspace --from myorg/templates Choose a template that matches the installation boundary: +- [Finance](https://github.com/allagentsdev/allagents/tree/main/examples/workspaces/finance): spreadsheet financial modeling from the reusable `agent-plugins` catalogue. - [Engineering](https://github.com/allagentsdev/allagents/tree/main/examples/workspaces/engineering): portable engineering skills for the `universal` client. - [Pi Engineering](https://github.com/allagentsdev/allagents/tree/main/examples/workspaces/pi-engineering): an opt-in Pi-native stack with web access, Compound Engineering, subagents, and interactive questions. Packages use reviewed version pins and Pi's own installer rather than file-copy sync. Save project trust in Pi before initialization; see the template README for prerequisites and user-owned credential configuration. diff --git a/examples/workspaces/finance/.allagents/workspace.yaml b/examples/workspaces/finance/.allagents/workspace.yaml new file mode 100644 index 00000000..583f1ba6 --- /dev/null +++ b/examples/workspaces/finance/.allagents/workspace.yaml @@ -0,0 +1,9 @@ +version: 2 + +repositories: [] + +plugins: + - finance@allagentsdev/agent-plugins + +clients: + - universal diff --git a/examples/workspaces/finance/README.md b/examples/workspaces/finance/README.md new file mode 100644 index 00000000..69339619 --- /dev/null +++ b/examples/workspaces/finance/README.md @@ -0,0 +1,12 @@ +# Finance workspace + +A portable workspace for financial modeling, budgets, forecasts and workpapers. +It installs the reusable Finance plugin from `allagentsdev/agent-plugins`. + +```sh +allagents workspace init ./finance --from allagentsdev/allagents/examples/workspaces/finance +cd finance +allagents update +``` + +Review model inputs, formulas and checks before using financial outputs. diff --git a/examples/workspaces/mcp-proxy/.allagents/workspace.yaml b/examples/workspaces/mcp-proxy/.allagents/workspace.yaml index a5620c91..f261ac74 100644 --- a/examples/workspaces/mcp-proxy/.allagents/workspace.yaml +++ b/examples/workspaces/mcp-proxy/.allagents/workspace.yaml @@ -34,7 +34,7 @@ plugins: # Real HTTP MCP server from the official AllAgents marketplace. # The deepwiki plugin ships a `.mcp.json` that declares: # { "deepwiki": { "type": "http", "url": "https://mcp.deepwiki.com/mcp" } } - - allagentsdev/allagents/plugins/deepwiki + - deepwiki@allagentsdev/agent-plugins clients: - claude diff --git a/plugins/allagents/.claude-plugin/plugin.json b/plugins/allagents/.claude-plugin/plugin.json deleted file mode 100644 index 2c47f39e..00000000 --- a/plugins/allagents/.claude-plugin/plugin.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "allagents", - "description": "End-user guidance for managing workspaces, plugins, skills, profiles, clients, and MCP servers with the AllAgents CLI.", - "author": { - "name": "AllAgents" - }, - "version": "1.0.0", - "category": "development", - "homepage": "https://github.com/allagentsdev/allagents/tree/main/plugins/allagents" -} diff --git a/plugins/allagents/skills/allagents/SKILL.md b/plugins/allagents/skills/allagents/SKILL.md deleted file mode 100644 index 6d40e09d..00000000 --- a/plugins/allagents/skills/allagents/SKILL.md +++ /dev/null @@ -1,51 +0,0 @@ ---- -name: allagents -description: Manage AllAgents workspaces, plugins, skills, client targets, global profiles, and MCP servers through the allagents CLI. Use when a user asks to initialize, inspect, update, or configure AllAgents; install or remove plugins or skills; manage profiles; or add, inspect, authenticate, update, or remove MCP servers. Resolve current command syntax from machine-readable help instead of relying on remembered flags. ---- - -# AllAgents CLI - -The installed AllAgents CLI owns the current command surface and operational contract. This skill only resolves the runner, loads that contract, and applies it. Do not maintain a second command reference here. - -## Resolve the CLI runner - -Choose one runner at the start of the task and use it for discovery, execution, and verification: - -1. If `allagents` is on `PATH`, use `allagents`. -2. Otherwise, if `npx` is available, use `npx --yes allagents`. -3. If neither is available, stop and ask the user to install AllAgents or Node.js with npm. - -Examples and returned `help_command` values use the canonical `allagents` token. When `npx --yes allagents` is the selected runner, replace only that leading token before executing every discovery, execution, and verification command. Do not mix runners within one operation. - -## Load the current contract - -1. Confirm the selected runner with `allagents --version`. -2. Discover the concise top-level index with `allagents --help --json`. -3. Choose an entry, rewrite its leading runner token when required, and execute its `help_command`; group responses reveal only their immediate children. -4. Continue through nested groups until a leaf command returns its full contract. -5. Follow the leaf's `when_to_use`, positionals, options, examples, interaction requirement, expected output, output schema, and JSON field allowlist. - -The structured response is authoritative. Do not rely on remembered flags, copied examples, aliases, destination behavior, or mutation semantics. If the installed CLI does not advertise an operation, do not invent it. - -## Execute through the discovered surface - -- Run `allagents` without arguments in an interactive terminal only when the user wants to browse and choose in the TUI. -- Use the discovered direct command when the operation is already known. -- For automation, use `--json` plus only the explicit selectors and non-interactive options advertised by the leaf help. -- Use `--json=` only with fields in the leaf's JSON allowlist. Use `--jq` only with JSON output. -- Prefer an advertised CLI command over hand-editing an AllAgents declaration or generated client file. -- Preserve the exact scope, destination, profile, clients, and other ownership selectors requested by the user. If the request is insufficient and the CLI requires a choice, ask rather than guessing. -- Never echo credentials. Supply sensitive values only through mechanisms advertised by the current leaf help. - -Treat the process exit code and structured result as authoritative. After a mutation, rediscover and run the relevant read-only list, get, or status command against the same selectors. Do not claim success from a config write alone when the CLI reports a partial update or failed client reconciliation. - -## Recover from errors - -- Read the structured error before retrying. -- If command syntax is rejected, reload root, group, and leaf help; do not fall back to stale syntax. -- If a mutation may have partially applied, inspect current state before any retry. -- Follow only recovery operations exposed by the installed CLI. - -## Completion - -Report the selected runner, command path, explicit selectors, structured result, and verification command. Completion requires the declared state and corresponding live or generated state to agree, or an explicit partial-update result with the CLI-advertised recovery path. diff --git a/plugins/deepwiki/.claude-plugin/plugin.json b/plugins/deepwiki/.claude-plugin/plugin.json deleted file mode 100644 index 4ba5e2f9..00000000 --- a/plugins/deepwiki/.claude-plugin/plugin.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "deepwiki", - "description": "AI-generated documentation for GitHub repositories. Use when exploring a repo's architecture, comparing repos, or asking how a feature is implemented in an unfamiliar public codebase.", - "author": { - "name": "AllAgents" - }, - "version": "1.0.0", - "category": "documentation", - "homepage": "https://github.com/allagentsdev/allagents/tree/main/plugins/deepwiki" -} diff --git a/plugins/deepwiki/.mcp.json b/plugins/deepwiki/.mcp.json deleted file mode 100644 index d5579f4c..00000000 --- a/plugins/deepwiki/.mcp.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "mcpServers": { - "deepwiki": { - "type": "http", - "url": "https://mcp.deepwiki.com/mcp" - } - } -} diff --git a/plugins/deepwiki/skills/deepwiki/SKILL.md b/plugins/deepwiki/skills/deepwiki/SKILL.md deleted file mode 100644 index 7fb49ccb..00000000 --- a/plugins/deepwiki/skills/deepwiki/SKILL.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -name: deepwiki -description: AI-generated documentation for GitHub repositories ---- - -# DeepWiki MCP Tools - -## When to Use -- Understanding a GitHub repository's architecture -- Cross-repo comparisons and Q&A -- Finding how a specific feature is implemented in a repo -- Getting an overview of unfamiliar codebases - -## Available Tools -- `mcp__deepwiki__read_wiki_structure` — Get documentation topics for a repo -- `mcp__deepwiki__read_wiki_contents` — Read full documentation for a repo -- `mcp__deepwiki__ask_question` — Ask any question about a repo - -## Query Strategies -- Start with `read_wiki_structure` to understand what documentation is available -- Use `ask_question` for specific implementation questions: "How does auth work in owner/repo?" -- For comparisons, ask about each repo separately then synthesize -- Repository names use `owner/repo` format (e.g., "langchain-ai/langchainjs") - -## Limitations -- Free to use, no API key required -- Only works with public GitHub repositories -- Documentation is AI-generated and may miss recent changes -- Best for architecture-level understanding, not line-by-line code review diff --git a/plugins/engineering/.claude-plugin/plugin.json b/plugins/engineering/.claude-plugin/plugin.json deleted file mode 100644 index 5bc8ed2c..00000000 --- a/plugins/engineering/.claude-plugin/plugin.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "name": "engineering", - "description": "Reusable engineering workflow skills for repository maintenance, delivery, and interactive pull request review.", - "author": { - "name": "AllAgents" - }, - "version": "1.0.1", - "homepage": "https://github.com/allagentsdev/allagents/tree/main/plugins/engineering" -} diff --git a/plugins/engineering/skills/pr-interactive-review/SKILL.md b/plugins/engineering/skills/pr-interactive-review/SKILL.md deleted file mode 100644 index 1ed3ccb9..00000000 --- a/plugins/engineering/skills/pr-interactive-review/SKILL.md +++ /dev/null @@ -1,187 +0,0 @@ ---- -name: pr-interactive-review -description: Turn a structured GitHub pull request review into a local interactive site for business context, findings, and reviewer comments. Use when a reviewer needs to triage a PR interactively without posting to GitHub. ---- - -# Interactive Pull Request Review - -Use an existing review skill when one is available, and prefer its structured JSON output. This skill consumes structured findings and presents them locally; it does not select reviewer personas, assign severity, discover review scope, validate findings, or deduplicate findings. - -## Safety boundaries - -- Accept a GitHub PR number or `https://github.com///pull/` URL. -- Prefer the installed review skill's structured or agent output when it supports one. Do not parse markdown output. -- Store generated review data and comments outside the repository. The default workspace is `$XDG_STATE_HOME/allagents/pr-interactive-review/` (or `~/.local/state/allagents/pr-interactive-review/`). -- Never put tokens, cookies, GitHub authentication, comments, or generated review data in the repository. -- The server binds to `127.0.0.1` by default. Non-loopback binding requires the explicit `--expose` option and emits a warning because findings and comments become network-visible. -- Comments are local only. Do not post them to GitHub. This skill does not implement GitHub posting. - -## Obtain a structured review - -From the repository that owns the PR, use an existing review skill when one is available. Ask it for structured JSON and pass the PR target directly. If no review skill is installed, use the host's available code-review capability and produce the same structured review artifact without inventing findings. - -The review artifact must contain `status`, `verdict`, `intent`, `scope.head_sha`, and the validated `findings` array. Require `status: complete`; stop on `failed`, `degraded`, or `skipped`. Save the artifact outside the repository as `review.json` and consume that JSON directly. Do not scrape, transform, or infer findings from a human markdown report. - -### Concrete finding scenarios - -After the review completes, actively enrich every finding without modifying `review.json`. For each stable `#`, inspect only its structured `evidence` and `first_evidence` plus the exact cited path at the reviewed commit. Write a separate JSON sidecar beside the review artifact (or in the external review workspace) as `interactive-scenarios.json`, keyed by the stable finding IDs: - -```json -{ - "#1": { - "what_actually_happens": "When an operator saves a label containing markup, the page renders it and the browser executes it.", - "what_actually_happens_evidence": { - "triggering_setup": "tests/config-label.test.ts creates and saves a label containing markup.", - "observable_outcome": "src/config-label.test.ts asserts the rendered label is interpreted as markup." - }, - "expected_suggested": "When that label is saved, the page displays the characters as text after encoding at the rendering boundary." - } -} -``` - -`what_actually_happens` must state a triggering setup/action and observable failure. It is rendered as a present defect only when `what_actually_happens_evidence` identifies both the concrete triggering setup/reachability and the observable outcome from a reviewed repository test or fixture, saved configuration/preset, production registry/declaration, reachable callsite, or explicit requirement. Without both, `prepare` removes the unsupported assertion and renders an evidence gap: treat it as an open question, not a defect. `expected_suggested` must state the expected resulting behavior and correction. Compatibility or future-risk observations must stay distinct from present defects and use an evidence-seeking open question until present reachability is proven. This sidecar is presentation context only: it must not change review scope, personas, validation, deduplication, or required response. `prepare` rejects malformed sidecars and IDs that are not review findings. - -Specifications may be private when the user authorizes access. Use the appropriate host tool to read or extract an authorized local file, document, or URL. Derive only concise labeled primer fields from that material, then pass the derived text with `--spec`; never put the original source content in this public repository. - -```text -Who configures: Release managers -Operational problem: Manual approval queues delay configuration changes -Intended outcome: Operators can complete the change without a queue handoff -Why it matters: Delays postpone customer-visible changes -Success criteria: A configured change completes and records its result -Scope: Configuration flow and audit record -Non-goals: Redesigning permission roles -``` - -`--requirements` remains a direct file-read option only for a repository-relative path. The helper rejects paths outside the repository and `.git`; use host extraction plus `--spec` for any external specification reference. -### Reproducible presentation - -Create `interactive-presentation.json` beside the scenario sidecar for every review. This is a constrained content model for the shared renderer, not a per-PR template: never generate custom HTML, CSS, or JavaScript. Ground every sentence in the structured review, supplied requirements, or reviewed repository evidence. - -```json -{ - "eyebrow": "Runtime seam review", - "headline": "One queue connects two services.", - "summary": "The change establishes the first validated Dispatcher-to-Runner workflow handoff.", - "context_cards": [ - { - "label": "System boundary", - "title": "Dispatcher to Runner", - "body": "The Dispatcher starts resolved work; the Runner validates and completes the workflow.", - "tone": "neutral" - }, - { - "label": "Operational risk", - "title": "A false-ready worker hides the broken seam", - "body": "The smoke must prove polling, not merely observe a startup log.", - "tone": "problem" - } - ], - "mental_model": { - "title": "The execution path", - "summary": "Each stage owns one boundary and hands a single contract forward.", - "steps": [ - { - "label": "01 / Dispatch", - "title": "Start workflow", - "body": "Create a workflow on the configured Runner queue." - }, - { - "label": "02 / Validate", - "title": "Reject malformed input", - "body": "Validate the shared payload before any execution work." - } - ] - } -} -``` - -`eyebrow`, `headline`, and `summary` are required. Use at most six context cards and six mental-model steps. Card `tone` is `neutral`, `problem`, or `outcome`. Keep labels short, make the headline state the review's central conclusion, and use the mental model only when a real sequence or system boundary helps the reviewer decide. Omit `mental_model` rather than inventing decorative steps. The renderer supplies the navigation rail, verdict, metrics, cards, disclosures, finding layout, and responsive behavior; the sidecar supplies only grounded copy. - - -## Create a reusable workspace - -Resolve this skill directory, then prepare the site from the structured `review.json`. - -```bash -SKILL_DIR="" -bun "$SKILL_DIR/scripts/review-site.ts" prepare \ - --review-json "/review.json" \ - --scenarios "/interactive-scenarios.json" \ - --presentation "/interactive-presentation.json" \ - --pr 123 \ - --spec "Who configures: Release managers -Operational problem: Manual approval queues delay configuration changes -Intended outcome: Operators complete changes without a queue handoff -Why it matters: Delays postpone customer-visible changes -Success criteria: A configured change completes and records its result -Scope: Configuration flow and audit record -Non-goals: Redesigning permission roles" -``` - -For a repository-relative requirements reference, use: - -```bash -bun "$SKILL_DIR/scripts/review-site.ts" prepare \ - --review-json "/review.json" \ - --scenarios "/interactive-scenarios.json" \ - --presentation "/interactive-presentation.json" \ - --pr https://github.com/example-org/sample-service/pull/123 \ - --requirements docs/requirements.md -``` - -`prepare` prints the per-repository, per-PR workspace path. It validates the review artifact, scenario and presentation sidecars, PR identifier, finding file paths, sizes, and requirements reference. It generates GitHub source links only when the runtime `origin` remote is GitHub. Links pin the reviewed commit and exact cited line range. Non-GitHub remotes receive no external link. Existing workspaces without presentation metadata still render through the same shell using the review title, intent, and primer fields as safe fallbacks. - - -For focused code context, `prepare` reads only the cited relative paths at the reviewed commit. Pass `--base-commit ` only when the review already supplied a verified exact base SHA; the helper does not rediscover PR scope. When no base or reviewed object is locally readable, the site labels that gap instead of substituting current-worktree content. - -## Host the site - -```bash -bun "$SKILL_DIR/scripts/review-site.ts" serve --workspace "" -``` - -Open the printed loopback URL. The shared page renders a sticky review map, current verdict, headline and metrics, business context, an optional mental model, status-grouped findings, search and filters, exact reviewed-commit links, paired actual/expected scenarios, required responses, and local comments. Supplied requirements, referenced evidence, lifecycle history, code excerpts, and per-finding discussion use collapsed disclosures so large reviews remain scannable. - - -LAN or public exposure is opt-in and must be deliberate: - -```bash -bun "$SKILL_DIR/scripts/review-site.ts" serve \ - --workspace "" \ - --host 0.0.0.0 \ - --expose -``` - -The warning is part of the command contract. Do not expose a review site that contains material the intended network audience may not read. - -## Comment handoff and lifecycle loop - -Reviewers can save general comments or comments attached to a finding. The browser sends only validated, bounded JSON to the local server. Comments are atomically written to `comments.json` in the external workspace; comment bodies are not logged. - -To respond as the assistant, first inspect only unanswered local comments: - -```bash -curl -sS "http://127.0.0.1:/api/comments?status=unanswered" -``` - -Re-ground every reviewer correction against the review artifact and reviewed code. Never defend an initial finding merely because it appears in the original artifact. Save the evidence-based reply locally, then revise the finding when the correction narrows, reclassifies, or disproves it: - -```bash -curl -sS -X POST "http://127.0.0.1:/api/comments//replies" \ - -H 'content-type: application/json' \ - --data '{"role":"assistant","author":"Assistant","body":"Verified response with the required next action."}' - -curl -sS -X POST "http://127.0.0.1:/api/findings/%231/revisions" \ - -H 'content-type: application/json' \ - --data '{"commentId":"","status":"withdrawn","rationale":"Reviewed fixture and registry show this configuration path is not reachable.","scenario":{"actualHappens":null,"actualTriggerEvidence":null,"actualOutcomeEvidence":null}}' -``` - -The lifecycle states are `active`, `question`, and `withdrawn`. Every revision requires the attached reviewer comment ID and a rationale. The site preserves the immutable original claim, comments/replies, and append-only revision history, while current status, severity, scenario, active counts, and verdict are recalculated for presentation. Withdrawn findings remain auditable but do not block the verdict. Use `question` when evidence remains incomplete rather than asserting a current defect. - -Refresh the site and unanswered queue until the queue is empty. Never treat this local operation as authority to post a GitHub comment; GitHub posting requires a separate explicit, user-confirmed feature. - -## Completion -1. Confirm the review artifact was consumed as JSON, not markdown. -2. Browser-check the local site with representative review data, not an empty fixture: the desktop view has the navigation rail, verdict, hero metrics, business context, and indexed finding links; the narrow view stacks cleanly with no document-level horizontal overflow; raw requirements, evidence, and code are closed by default; active findings, open questions, and withdrawn findings are visibly separate; verdict/counts exclude withdrawn findings; every finding shows paired `What actually happens` and `Expected / suggested` content (or an explicit evidence gap); severity/status filters and search work; a local comment, assistant reply, and lifecycle revision render; and a GitHub remote produces a reviewed-commit line link. -3. State the workspace path and loopback URL. Do not include comment text, credentials, or source contents in the report. diff --git a/plugins/engineering/skills/pr-interactive-review/scripts/review-site.ts b/plugins/engineering/skills/pr-interactive-review/scripts/review-site.ts deleted file mode 100644 index 31ea2b0c..00000000 --- a/plugins/engineering/skills/pr-interactive-review/scripts/review-site.ts +++ /dev/null @@ -1,2137 +0,0 @@ -import { createHash, randomUUID } from 'node:crypto'; -import { mkdir, open, readFile, rename, rm, writeFile } from 'node:fs/promises'; -import { homedir } from 'node:os'; -import { basename, isAbsolute, join, relative, resolve, sep } from 'node:path'; - -export const MAX_REQUEST_BYTES = 16 * 1024; -export const MAX_REQUIREMENTS_BYTES = 128 * 1024; -const MAX_COMMENT_LENGTH = 12 * 1024; -const MAX_EXCERPT_LINES = 120; -const LOOPBACK_HOSTS = new Set(['127.0.0.1', '::1', 'localhost']); -const SEVERITIES = new Set(['P0', 'P1', 'P2', 'P3']); - -type JsonObject = Record; - -export type FindingStatus = 'active' | 'question' | 'withdrawn'; - -export interface FindingScenario { - actualHappens: string | null; - expectedSuggested: string | null; - actualTriggerEvidence: string | null; - actualOutcomeEvidence: string | null; - actualEvidenceGap: string | null; - expectedEvidenceGap: string | null; -} - -export interface FindingClaim { - title: string; - severity: 'P0' | 'P1' | 'P2' | 'P3'; - requiredResponse: string; - scenario: FindingScenario; -} - -export interface FindingRevision { - id: string; - commentId: string; - status: FindingStatus; - rationale: string; - changes: Partial; - createdAt: string; -} - -export interface ReviewFinding extends FindingClaim { - id: string; - status: FindingStatus; - original: FindingClaim; - revisions: FindingRevision[]; - file: string; - line: number; - endLine: number; - confidence: number | string; - reviewers: string[]; - evidence: string[]; - firstEvidence: string | null; - sourceLink: string | null; - excerpts: { - before: CodeExcerpt | null; - after: CodeExcerpt | null; - }; -} - -export interface CodeExcerpt { - startLine: number; - endLine: number; - content: string; -} - -export interface BusinessPrimer { - whoConfigures: PrimerField; - operationalProblem: PrimerField; - intendedOutcome: PrimerField; - businessImportance: PrimerField; - successCriteria: PrimerField; - scope: PrimerField; - nonGoals: PrimerField; - providedRequirements: string | null; -} - -export interface PrimerField { - value: string | null; - evidenceGap: string | null; -} -export type PresentationTone = 'neutral' | 'problem' | 'outcome'; - -export interface PresentationCard { - label: string; - title: string; - body: string; - tone: PresentationTone; -} - -export interface PresentationStep { - label: string; - title: string; - body: string; -} - -export interface ReviewPresentation { - eyebrow: string; - headline: string; - summary: string; - contextCards: PresentationCard[]; - mentalModel: { - title: string; - summary: string | null; - steps: PresentationStep[]; - } | null; -} - - -export interface StoredReview { - version: 2; - repository: string; - githubRepository: string | null; - prNumber: number; - reviewedCommit: string; - title: string; - originalVerdict: string; - verdict: string; - intent: string; - primer: BusinessPrimer; - presentation: ReviewPresentation | null; - - findings: ReviewFinding[]; - generatedAt: string; -} - -export interface CommentReply { - id: string; - author: string; - role: 'assistant'; - body: string; - createdAt: string; -} - -export interface ReviewComment { - id: string; - findingId: string | null; - author: string; - role: 'reviewer'; - body: string; - createdAt: string; - replies: CommentReply[]; -} - -interface CommentStore { - version: 1; - comments: ReviewComment[]; -} - -interface ReviewInput { - status: string; - verdict: string; - intent: string; - scope: { head_sha: string }; - findings: unknown[]; - title?: string; -} - -export interface PrepareOptions { - reviewJsonPath: string; - pr: string; - repoPath?: string; - dataDir?: string; - scenariosPath?: string; - presentationPath?: string; - - specification?: string; - requirementsPath?: string; - baseCommit?: string; - now?: Date; -} - -export interface PreparedReview { - workspace: string; - review: StoredReview; -} - -function isRecord(value: unknown): value is JsonObject { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -function boundedString( - value: unknown, - field: string, - maxLength: number, - required = true, -): string | null { - if (value === undefined || value === null) { - if (required) throw new Error(`${field} is required`); - return null; - } - if (typeof value !== 'string') throw new Error(`${field} must be a string`); - if (value.includes('\0')) - throw new Error(`${field} must not contain null characters`); - const normalized = value.trim(); - if (required && normalized.length === 0) - throw new Error(`${field} must not be empty`); - if (normalized.length > maxLength) - throw new Error(`${field} exceeds ${maxLength} characters`); - return normalized; -} - -function stringArray(value: unknown, field: string, maxEntries = 32): string[] { - if (value === undefined) return []; - if (!Array.isArray(value) || value.length > maxEntries) - throw new Error( - `${field} must be an array of at most ${maxEntries} strings`, - ); - return value.map( - (item, index) => boundedString(item, `${field}[${index}]`, 2000) as string, - ); -} - -function safeRelativePath(value: unknown, field: string): string { - const path = boundedString(value, field, 1000) as string; - if ( - path.includes('\0') || - isAbsolute(path) || - path.split(/[\\/]/).includes('..') - ) { - throw new Error(`${field} must be a relative repository path`); - } - return path.replace(/\\/g, '/'); -} - -function findingId(value: unknown): string { - if (typeof value !== 'number' && typeof value !== 'string') - throw new Error('finding.# is required'); - const number = Number(String(value).replace(/^#/, '')); - if (!Number.isInteger(number) || number < 1 || number > 1000000) - throw new Error('finding.# must be a positive integer'); - return `#${number}`; -} - -function lineNumber(value: unknown, field: string): number { - const number = typeof value === 'number' ? value : Number(value); - if (!Number.isInteger(number) || number < 1 || number > 10000000) - throw new Error(`${field} must be a valid line number`); - return number; -} - -function readReviewInput(value: unknown): ReviewInput { - if (!isRecord(value)) - throw new Error('Structured review artifact must be an object'); - const status = boundedString(value.status, 'status', 32) as string; - if (status !== 'complete') - throw new Error('Structured review artifact must have complete status'); - if (!isRecord(value.scope)) throw new Error('scope is required'); - const headSha = boundedString( - value.scope.head_sha, - 'scope.head_sha', - 64, - ) as string; - if (!/^[0-9a-f]{7,64}$/i.test(headSha)) - throw new Error('scope.head_sha must be a commit SHA'); - if (!Array.isArray(value.findings)) - throw new Error('findings must be an array'); - return { - status, - verdict: boundedString(value.verdict, 'verdict', 200) as string, - intent: boundedString(value.intent, 'intent', 8000) as string, - scope: { head_sha: headSha.toLowerCase() }, - findings: value.findings, - ...(typeof value.title === 'string' - ? { title: boundedString(value.title, 'title', 500) as string } - : {}), - }; -} - -function missingScenario( - actualEvidenceGap = 'Evidence gap: the scenario sidecar does not provide a specific triggering setup/reachability and observable outcome.', -): FindingScenario { - return { - actualHappens: null, - expectedSuggested: null, - actualTriggerEvidence: null, - actualOutcomeEvidence: null, - actualEvidenceGap, - expectedEvidenceGap: - 'Evidence gap: the scenario sidecar does not provide a specific expected behavior and correction.', - }; -} - -function normalizeScenario(value: unknown, field: string): FindingScenario { - if (!isRecord(value)) throw new Error(`${field} must be an object`); - const actualHappens = boundedString( - value.what_actually_happens, - `${field}.what_actually_happens`, - 8000, - false, - ); - const expectedSuggested = boundedString( - value.expected_suggested, - `${field}.expected_suggested`, - 8000, - false, - ); - const rawEvidence = value.what_actually_happens_evidence; - if (rawEvidence !== undefined && !isRecord(rawEvidence)) - throw new Error(`${field}.what_actually_happens_evidence must be an object`); - const triggerEvidence = rawEvidence - ? boundedString( - rawEvidence.triggering_setup, - `${field}.what_actually_happens_evidence.triggering_setup`, - 8000, - false, - ) - : null; - const outcomeEvidence = rawEvidence - ? boundedString( - rawEvidence.observable_outcome, - `${field}.what_actually_happens_evidence.observable_outcome`, - 8000, - false, - ) - : null; - const hasActualEvidence = Boolean( - actualHappens && triggerEvidence && outcomeEvidence, - ); - const missing = missingScenario( - actualHappens - ? 'Evidence gap: the asserted scenario lacks both a cited triggering setup/reachability and observable outcome; treat it as an open question, not an active defect.' - : undefined, - ); - return { - actualHappens: hasActualEvidence ? actualHappens : null, - expectedSuggested, - actualTriggerEvidence: hasActualEvidence ? triggerEvidence : null, - actualOutcomeEvidence: hasActualEvidence ? outcomeEvidence : null, - actualEvidenceGap: hasActualEvidence ? null : missing.actualEvidenceGap, - expectedEvidenceGap: expectedSuggested ? null : missing.expectedEvidenceGap, - }; -} - -function normalizeFinding(value: unknown): ReviewFinding { - if (!isRecord(value)) throw new Error('finding must be an object'); - const line = lineNumber(value.line, 'finding.line'); - const endLine = - value.end_line === undefined - ? line - : lineNumber(value.end_line, 'finding.end_line'); - if (endLine < line) - throw new Error('finding.end_line must not precede finding.line'); - const severity = boundedString( - value.severity, - 'finding.severity', - 2, - ) as ReviewFinding['severity']; - if (!SEVERITIES.has(severity)) - throw new Error('finding.severity must be P0, P1, P2, or P3'); - const owner = boundedString(value.owner, 'finding.owner', 120, false); - const suggestedFix = boundedString( - value.suggested_fix, - 'finding.suggested_fix', - 8000, - false, - ); - const whyItMatters = boundedString( - value.why_it_matters, - 'finding.why_it_matters', - 8000, - false, - ); - const autofixClass = boundedString( - value.autofix_class, - 'finding.autofix_class', - 120, - false, - ); - const requiredResponse = - suggestedFix ?? - whyItMatters ?? - owner ?? - autofixClass ?? - 'Review the evidence and choose the next action.'; - const confidence = value.confidence; - if ( - typeof confidence !== 'string' && - (typeof confidence !== 'number' || !Number.isFinite(confidence)) - ) { - throw new Error('finding.confidence must be a string or number'); - } - const title = boundedString(value.title, 'finding.title', 1000) as string; - const scenario = missingScenario(); - const claim: FindingClaim = { title, severity, requiredResponse, scenario }; - return { - id: findingId(value['#']), - ...claim, - status: 'active', - original: { ...claim, scenario: { ...scenario } }, - revisions: [], - file: safeRelativePath(value.file, 'finding.file'), - line, - endLine, - confidence, - reviewers: stringArray(value.reviewers, 'finding.reviewers'), - evidence: stringArray(value.evidence, 'finding.evidence', 64), - firstEvidence: boundedString( - value.first_evidence, - 'finding.first_evidence', - 8000, - false, - ), - sourceLink: null, - excerpts: { before: null, after: null }, - }; -} - -export function parsePrTarget(value: string): { - number: number; - urlRepository: string | null; -} { - const input = value.trim(); - if (/^\d+$/.test(input)) { - const number = Number(input); - if (Number.isSafeInteger(number) && number > 0) - return { number, urlRepository: null }; - } - let url: URL; - try { - url = new URL(input); - } catch { - throw new Error( - 'pr must be a positive number or a https://github.com/owner/repo/pull/number URL', - ); - } - const match = - url.hostname === 'github.com' - ? /^\/([^/]+)\/([^/]+)\/pull\/(\d+)\/?$/.exec(url.pathname) - : null; - const number = Number(match?.[3]); - if (!match?.[1] || !match[2] || !Number.isSafeInteger(number) || number < 1) { - throw new Error( - 'pr must be a positive number or a https://github.com/owner/repo/pull/number URL', - ); - } - return { number, urlRepository: `${match[1]}/${match[2]}` }; -} - -export function githubRepositoryFromRemote( - remote: string | null, -): string | null { - if (!remote) return null; - const match = - /^(?:git@github\.com:|https:\/\/github\.com\/|ssh:\/\/git@github\.com\/)([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+?)(?:\.git)?\/?$/.exec( - remote.trim(), - ); - return match?.[1] && match[2] ? `${match[1]}/${match[2]}` : null; -} - -export function buildGitHubLineLink( - repository: string | null, - commit: string, - file: string, - startLine: number, - endLine: number, -): string | null { - if (!repository || !/^[0-9a-f]{7,64}$/i.test(commit)) return null; - const normalizedPath = safeRelativePath(file, 'finding.file'); - if ( - !Number.isInteger(startLine) || - !Number.isInteger(endLine) || - startLine < 1 || - endLine < startLine - ) - return null; - const escapedPath = normalizedPath - .split('/') - .map(encodeURIComponent) - .join('/'); - return `https://github.com/${repository}/blob/${commit}/${escapedPath}#L${startLine}-L${endLine}`; -} - -function parsePrimerField(text: string, labels: string[]): string | null { - const expression = new RegExp( - `(?:^|\\n)\\s*(?:#{1,6}\\s*)?(?:${labels.map((label) => label.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|')})\\s*:\\s*([^\\n]+)`, - 'i', - ); - const result = expression.exec(text); - return result?.[1]?.trim() || null; -} - -function primerValue(value: string | null, missing: string): PrimerField { - return value - ? { value, evidenceGap: null } - : { value: null, evidenceGap: missing }; -} - -export function buildBusinessPrimer( - specification: string | null, - intent: string, -): BusinessPrimer { - const text = specification?.trim() ?? ''; - const operationalGap = `The supplied requirements do not state the operational problem. Review intent: ${intent}`; - return { - whoConfigures: primerValue( - parsePrimerField(text, ['who configures', 'configured by', 'operator']), - 'The supplied requirements do not identify who configures this feature.', - ), - operationalProblem: primerValue( - parsePrimerField(text, ['operational problem', 'problem']), - operationalGap, - ), - intendedOutcome: primerValue( - parsePrimerField(text, ['intended outcome', 'before/after', 'outcome']), - 'The supplied requirements do not define the intended before-and-after outcome.', - ), - businessImportance: primerValue( - parsePrimerField(text, [ - 'why it matters', - 'business importance', - 'business value', - ]), - 'The supplied requirements do not state why this outcome matters to the business.', - ), - successCriteria: primerValue( - parsePrimerField(text, ['success criteria', 'acceptance criteria']), - 'The supplied requirements do not define measurable success criteria.', - ), - scope: primerValue( - parsePrimerField(text, ['scope', 'in scope']), - 'The supplied requirements do not define the intended scope.', - ), - nonGoals: primerValue( - parsePrimerField(text, ['non-goals', 'non goals', 'out of scope']), - 'The supplied requirements do not define non-goals.', - ), - providedRequirements: text || null, - }; -} - -function stateRoot(dataDir?: string): string { - if (dataDir) return resolve(dataDir); - const root = process.env.XDG_STATE_HOME || join(homedir(), '.local', 'state'); - return join(root, 'allagents', 'pr-interactive-review'); -} - -function repositoryStorageKey( - remote: string | null, - githubRepository: string | null, -): string { - if (githubRepository) - return `github.com-${githubRepository.replace('/', '-')}`.toLowerCase(); - const digest = createHash('sha256') - .update(remote ?? 'no-origin-remote') - .digest('hex') - .slice(0, 16); - return `non-github-${digest}`; -} - -function workspaceFor( - dataDir: string, - repositoryKey: string, - prNumber: number, -): string { - if ( - !/^[a-z0-9.-]+$/i.test(repositoryKey) || - !Number.isInteger(prNumber) || - prNumber < 1 - ) - throw new Error('Invalid workspace identifier'); - return join(dataDir, repositoryKey, `pr-${prNumber}`); -} - -async function readBoundedFile( - path: string, - maximumBytes: number, -): Promise { - const handle = await open(path, 'r'); - try { - const stat = await handle.stat(); - if (stat.size > maximumBytes) - throw new Error(`${basename(path)} exceeds ${maximumBytes} bytes`); - return await handle.readFile({ encoding: 'utf8' }); - } finally { - await handle.close(); - } -} - -function requirementsFilePath(repoPath: string, value: string): string { - if (isAbsolute(value)) - throw new Error( - 'requirements must be a repository-relative public-safe reference', - ); - const resolved = resolve(repoPath, value); - const fromRepo = relative(repoPath, resolved); - if ( - fromRepo === '' || - fromRepo.startsWith(`..${sep}`) || - fromRepo === '..' || - isAbsolute(fromRepo) || - fromRepo.split(sep).includes('.git') - ) { - throw new Error( - 'requirements must stay inside the repository and outside .git', - ); - } - return resolved; -} - -function runGit(repoPath: string, args: string[]): string | null { - const result = Bun.spawnSync(['git', '-C', repoPath, ...args], { - stdout: 'pipe', - stderr: 'pipe', - }); - if (result.exitCode !== 0) return null; - return new TextDecoder().decode(result.stdout).trim(); -} - -function codeExcerpt( - repoPath: string, - commit: string, - file: string, - line: number, -): CodeExcerpt | null { - const source = runGit(repoPath, ['show', `${commit}:${file}`]); - if (source === null) return null; - const lines = source.split('\n'); - const startLine = Math.max(1, line - 3); - const endLine = Math.min(lines.length, startLine + MAX_EXCERPT_LINES - 1); - return { - startLine, - endLine, - content: lines.slice(startLine - 1, endLine).join('\n'), - }; -} - -function validBaseCommit(value: string | undefined): string | null { - return value && /^[0-9a-f]{7,64}$/i.test(value) ? value.toLowerCase() : null; -} - -async function writeJsonAtomically( - path: string, - value: unknown, - maximumBytes = MAX_REQUIREMENTS_BYTES, -): Promise { - const serialized = `${JSON.stringify(value, null, 2)}\n`; - if (Buffer.byteLength(serialized, 'utf8') > maximumBytes) - throw new Error(`${basename(path)} exceeds ${maximumBytes} bytes`); - await mkdir(resolve(path, '..'), { recursive: true, mode: 0o700 }); - const temporary = `${path}.${randomUUID()}.tmp`; - await writeFile(temporary, serialized, { encoding: 'utf8', mode: 0o600 }); - await rename(temporary, path); -} - -async function readScenarioSidecar( - path: string, - findingIds: Set, -): Promise> { - const value = JSON.parse( - await readBoundedFile(resolve(path), MAX_REQUIREMENTS_BYTES), - ) as unknown; - if (!isRecord(value) || Array.isArray(value)) - throw new Error( - 'Scenario sidecar must be an object keyed by stable finding ID', - ); - const entries = Object.entries(value); - if (entries.length > 1000) - throw new Error('Scenario sidecar exceeds 1000 findings'); - const scenarios = new Map(); - for (const [id, scenario] of entries) { - if (!/^#[1-9]\d*$/.test(id)) - throw new Error( - 'Scenario sidecar keys must be stable finding IDs such as #1', - ); - if (!findingIds.has(id)) - throw new Error(`Scenario sidecar references unknown finding ${id}`); - scenarios.set(id, normalizeScenario(scenario, `scenario ${id}`)); - } - return scenarios; -} -function recordArray( - value: unknown, - field: string, - maximumEntries: number, -): JsonObject[] { - if (value === undefined) return []; - if (!Array.isArray(value) || value.length > maximumEntries) - throw new Error( - `${field} must be an array of at most ${maximumEntries} objects`, - ); - return value.map((item, index) => { - if (!isRecord(item)) throw new Error(`${field}[${index}] must be an object`); - return item; - }); -} - -function normalizePresentation( - value: unknown, - field = 'presentation', -): ReviewPresentation | null { - if (value === undefined || value === null) return null; - if (!isRecord(value)) throw new Error(`${field} must be an object`); - const contextCards = recordArray( - value.context_cards ?? value.contextCards, - `${field}.context_cards`, - 6, - ).map((card, index): PresentationCard => { - const tone = (boundedString( - card.tone, - `${field}.context_cards[${index}].tone`, - 16, - false, - ) ?? 'neutral') as PresentationTone; - if (!['neutral', 'problem', 'outcome'].includes(tone)) - throw new Error( - `${field}.context_cards[${index}].tone must be neutral, problem, or outcome`, - ); - return { - label: boundedString( - card.label, - `${field}.context_cards[${index}].label`, - 120, - ) as string, - title: boundedString( - card.title, - `${field}.context_cards[${index}].title`, - 300, - ) as string, - body: boundedString( - card.body, - `${field}.context_cards[${index}].body`, - 3000, - ) as string, - tone, - }; - }); - const rawMentalModel = value.mental_model ?? value.mentalModel; - let mentalModel: ReviewPresentation['mentalModel'] = null; - if (rawMentalModel !== undefined && rawMentalModel !== null) { - if (!isRecord(rawMentalModel)) - throw new Error(`${field}.mental_model must be an object`); - const steps = recordArray( - rawMentalModel.steps, - `${field}.mental_model.steps`, - 6, - ); - if (!steps.length) - throw new Error(`${field}.mental_model.steps must not be empty`); - mentalModel = { - title: boundedString( - rawMentalModel.title, - `${field}.mental_model.title`, - 300, - ) as string, - summary: boundedString( - rawMentalModel.summary, - `${field}.mental_model.summary`, - 3000, - false, - ), - steps: steps.map((step, index) => ({ - label: boundedString( - step.label, - `${field}.mental_model.steps[${index}].label`, - 120, - ) as string, - title: boundedString( - step.title, - `${field}.mental_model.steps[${index}].title`, - 300, - ) as string, - body: boundedString( - step.body, - `${field}.mental_model.steps[${index}].body`, - 3000, - ) as string, - })), - }; - } - return { - eyebrow: boundedString(value.eyebrow, `${field}.eyebrow`, 200) as string, - headline: boundedString(value.headline, `${field}.headline`, 500) as string, - summary: boundedString(value.summary, `${field}.summary`, 3000) as string, - contextCards, - mentalModel, - }; -} - -async function readPresentationSidecar( - path: string, -): Promise { - const value = JSON.parse( - await readBoundedFile(resolve(path), MAX_REQUIREMENTS_BYTES), - ) as unknown; - const presentation = normalizePresentation(value); - if (!presentation) throw new Error('presentation must be an object'); - return presentation; -} - - -function preserveFindingLifecycle( - fresh: ReviewFinding, - previous: ReviewFinding | undefined, -): ReviewFinding { - if (!previous?.revisions.length) return fresh; - return { - ...fresh, - title: previous.title, - severity: previous.severity, - requiredResponse: previous.requiredResponse, - scenario: { ...previous.scenario }, - status: previous.status, - original: { - ...previous.original, - scenario: { ...previous.original.scenario }, - }, - revisions: [...previous.revisions], - }; -} - -export async function prepareReview( - options: PrepareOptions, -): Promise { - const target = parsePrTarget(options.pr); - const repoPath = resolve(options.repoPath ?? process.cwd()); - const remote = runGit(repoPath, ['config', '--get', 'remote.origin.url']); - const githubRepository = githubRepositoryFromRemote(remote); - if ( - target.urlRepository && - githubRepository && - target.urlRepository.toLowerCase() !== githubRepository.toLowerCase() - ) { - throw new Error( - 'PR URL repository does not match the runtime origin remote', - ); - } - const rawArtifact = JSON.parse( - await readBoundedFile( - resolve(options.reviewJsonPath), - MAX_REQUIREMENTS_BYTES, - ), - ) as unknown; - const artifact = readReviewInput(rawArtifact); - const requirementText = options.requirementsPath - ? await readBoundedFile( - requirementsFilePath(repoPath, options.requirementsPath), - MAX_REQUIREMENTS_BYTES, - ) - : null; - const specification = options.specification ?? requirementText; - if ( - specification && - Buffer.byteLength(specification, 'utf8') > MAX_REQUIREMENTS_BYTES - ) { - throw new Error(`specification exceeds ${MAX_REQUIREMENTS_BYTES} bytes`); - } - const baseCommit = validBaseCommit(options.baseCommit); - const rawFindings = artifact.findings.map(normalizeFinding); - const scenarios = options.scenariosPath - ? await readScenarioSidecar( - options.scenariosPath, - new Set(rawFindings.map((finding) => finding.id)), - ) - : new Map(); - const preparedPresentation = options.presentationPath - ? await readPresentationSidecar(options.presentationPath) - : undefined; - - const findings = rawFindings.map((finding) => { - const scenario = scenarios.get(finding.id) ?? finding.scenario; - return { - ...finding, - scenario, - original: { ...finding.original, scenario: { ...scenario } }, - sourceLink: buildGitHubLineLink( - githubRepository, - artifact.scope.head_sha, - finding.file, - finding.line, - finding.endLine, - ), - excerpts: { - before: baseCommit - ? codeExcerpt(repoPath, baseCommit, finding.file, finding.line) - : null, - after: codeExcerpt( - repoPath, - artifact.scope.head_sha, - finding.file, - finding.line, - ), - }, - }; - }); - const workspace = workspaceFor( - stateRoot(options.dataDir), - repositoryStorageKey(remote, githubRepository), - target.number, - ); - const review = await withLock(join(workspace, 'review.json'), async () => { - let previousReview: StoredReview | null = null; - try { - previousReview = await loadStoredReview(workspace); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; - } - if ( - previousReview?.reviewedCommit !== artifact.scope.head_sha && - previousReview?.findings.some((finding) => finding.revisions.length) - ) { - throw new Error( - 'Refusing to replace lifecycle history from a different reviewed commit', - ); - } - if ( - previousReview?.findings.some( - (finding) => - finding.revisions.length && - !findings.some((candidate) => candidate.id === finding.id), - ) - ) { - throw new Error( - 'Refusing to discard lifecycle history for a finding missing from the review artifact', - ); - } - const previousFindings = new Map( - previousReview?.findings.map((finding) => [finding.id, finding]) ?? [], - ); - const lifecycleFindings = findings.map((finding) => - preserveFindingLifecycle(finding, previousFindings.get(finding.id)), - ); - const next: StoredReview = { - version: 2, - repository: githubRepository ?? repositoryStorageKey(remote, null), - githubRepository, - prNumber: target.number, - reviewedCommit: artifact.scope.head_sha, - title: artifact.title ?? `Pull request #${target.number}`, - originalVerdict: previousReview?.findings.some( - (finding) => finding.revisions.length, - ) - ? previousReview.originalVerdict - : artifact.verdict, - verdict: currentVerdict(lifecycleFindings), - intent: artifact.intent, - primer: buildBusinessPrimer(specification, artifact.intent), - presentation: - preparedPresentation ?? - (previousReview?.reviewedCommit === artifact.scope.head_sha - ? previousReview.presentation - : null), - findings: lifecycleFindings, - generatedAt: (options.now ?? new Date()).toISOString(), - }; - await writeJsonAtomically(join(workspace, 'review.json'), next); - return next; - }); - const commentsPath = join(workspace, 'comments.json'); - try { - await readFile(commentsPath, 'utf8'); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; - await writeJsonAtomically(commentsPath, { - version: 1, - comments: [], - } satisfies CommentStore); - } - return { workspace, review }; -} - -function storedScenario(value: unknown, field: string): FindingScenario { - if (!isRecord(value)) return missingScenario(); - return normalizeScenario( - { - what_actually_happens: value.actualHappens, - expected_suggested: value.expectedSuggested, - what_actually_happens_evidence: { - triggering_setup: value.actualTriggerEvidence, - observable_outcome: value.actualOutcomeEvidence, - }, - }, - field, - ); -} - -function legacyScenario(value: unknown, field: string): FindingScenario { - const normalized = storedScenario(value, field); - if (!isRecord(value)) return normalized; - const actualHappens = boundedString( - value.actualHappens, - `${field}.actualHappens`, - 8000, - false, - ); - return actualHappens - ? { - ...normalized, - actualHappens, - actualEvidenceGap: - 'Legacy scenario retained for audit: it lacks separate reachability and observable-outcome provenance.', - } - : normalized; -} -function currentVerdict(findings: ReviewFinding[]): string { - const active = findings.filter((finding) => finding.status === 'active'); - const questions = findings.filter((finding) => finding.status === 'question'); - if (!active.length) - return questions.length - ? `No active findings; ${questions.length} open question${questions.length === 1 ? '' : 's'}` - : 'No active findings'; - const bySeverity = [...SEVERITIES] - .filter((severity) => active.some((finding) => finding.severity === severity)) - .map( - (severity) => - `${severity}: ${active.filter((finding) => finding.severity === severity).length}`, - ); - return `${active.length} active finding${active.length === 1 ? '' : 's'} (${bySeverity.join(', ')})${questions.length ? `; ${questions.length} open question${questions.length === 1 ? '' : 's'}` : ''}`; -} - -function migrateStoredReview(value: unknown): StoredReview { - if (!isRecord(value) || !Array.isArray(value.findings)) - throw new Error('Invalid stored review'); - - if (value.version === 2) { - if (value.presentation !== undefined) { - normalizePresentation(value.presentation, 'review.presentation'); - return value as unknown as StoredReview; - } - return { - ...(value as unknown as StoredReview), - presentation: null, - }; - } - if (value.version !== 1) throw new Error('Unsupported stored review version'); - - const originalVerdict = boundedString(value.verdict, 'review.verdict', 200) as string; - const findings = value.findings.map((rawFinding, index) => { - const scenario = storedScenario(rawFinding.scenario, `review.findings[${index}].scenario`); - const originalScenario = legacyScenario( - rawFinding.scenario, - `review.findings[${index}].scenario`, - ); - const title = boundedString( - rawFinding.title, - `review.findings[${index}].title`, - 1000, - ) as string; - const severity = boundedString( - rawFinding.severity, - `review.findings[${index}].severity`, - 2, - ) as ReviewFinding['severity']; - if (!SEVERITIES.has(severity)) - throw new Error(`review.findings[${index}].severity is invalid`); - const requiredResponse = boundedString( - rawFinding.requiredResponse, - `review.findings[${index}].requiredResponse`, - 8000, - ) as string; - const claim: FindingClaim = { title, severity, requiredResponse, scenario }; - const original: FindingClaim = { - ...claim, - scenario: originalScenario, - }; - return { - ...(rawFinding as unknown as Omit), - ...claim, - status: 'active' as const, - original, - revisions: [], - }; - }); - return { - ...(value as unknown as Omit), - version: 2, - originalVerdict, - verdict: currentVerdict(findings), - presentation: normalizePresentation(value.presentation, 'review.presentation'), - - findings, - }; -} - -export async function loadStoredReview( - workspace: string, -): Promise { - const path = join(resolve(workspace), 'review.json'); - const raw = JSON.parse( - await readBoundedFile(path, MAX_REQUIREMENTS_BYTES), - ) as unknown; - const review = migrateStoredReview(raw); - if (raw !== review) await writeJsonAtomically(path, review); - return review; -} - -async function readCommentStore(workspace: string): Promise { - try { - const data = JSON.parse( - await readBoundedFile( - join(workspace, 'comments.json'), - MAX_REQUIREMENTS_BYTES, - ), - ) as unknown; - if (!isRecord(data) || data.version !== 1 || !Array.isArray(data.comments)) - throw new Error('Invalid comments store'); - return data as unknown as CommentStore; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') - return { version: 1, comments: [] }; - throw error; - } -} - -async function withLock( - path: string, - operation: () => Promise, -): Promise { - await mkdir(resolve(path, '..'), { recursive: true, mode: 0o700 }); - const lock = `${path}.lock`; - for (let attempt = 0; attempt < 100; attempt += 1) { - try { - await mkdir(lock, { mode: 0o700 }); - try { - return await operation(); - } finally { - await rm(lock, { recursive: true, force: true }); - } - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; - await Bun.sleep(10); - } - } - throw new Error('Comment store is busy; retry the request'); -} - -function commentBody(value: unknown): string { - return boundedString(value, 'body', MAX_COMMENT_LENGTH) as string; -} - -function commentAuthor(value: unknown): string { - const author = boundedString(value, 'author', 120, false); - return author ? author.replace(/[\r\n]/g, ' ') : 'Reviewer'; -} - -function revisedScenario( - value: unknown, - current: FindingScenario, -): FindingScenario { - if (!isRecord(value)) throw new Error('revision.scenario must be an object'); - return normalizeScenario( - { - what_actually_happens: - value.actualHappens === undefined - ? current.actualHappens - : value.actualHappens, - expected_suggested: - value.expectedSuggested === undefined - ? current.expectedSuggested - : value.expectedSuggested, - what_actually_happens_evidence: { - triggering_setup: - value.actualTriggerEvidence === undefined - ? current.actualTriggerEvidence - : value.actualTriggerEvidence, - observable_outcome: - value.actualOutcomeEvidence === undefined - ? current.actualOutcomeEvidence - : value.actualOutcomeEvidence, - }, - }, - 'revision.scenario', - ); -} - -function revisionFor( - body: JsonObject, - finding: ReviewFinding, -): Omit { - const status = boundedString(body.status, 'status', 16) as FindingStatus; - if (!['active', 'question', 'withdrawn'].includes(status)) - throw new Error('status must be active, question, or withdrawn'); - const commentId = boundedString(body.commentId, 'commentId', 36) as string; - if (!/^[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}$/.test(commentId)) - throw new Error('commentId must be a UUID'); - const rationale = boundedString(body.rationale, 'rationale', 8000) as string; - const changes: Partial = {}; - if (body.title !== undefined) - changes.title = boundedString(body.title, 'title', 1000) as string; - if (body.severity !== undefined) { - const severity = boundedString(body.severity, 'severity', 2) as ReviewFinding['severity']; - if (!SEVERITIES.has(severity)) - throw new Error('severity must be P0, P1, P2, or P3'); - changes.severity = severity; - } - if (body.requiredResponse !== undefined) - changes.requiredResponse = boundedString( - body.requiredResponse, - 'requiredResponse', - 8000, - ) as string; - if (body.scenario !== undefined) - changes.scenario = revisedScenario(body.scenario, finding.scenario); - return { commentId, status, rationale, changes }; -} - -function requestContentLength(request: Request): Response | null { - const contentLength = Number(request.headers.get('content-length') ?? '0'); - return contentLength > MAX_REQUEST_BYTES - ? textResponse('Request body is too large', 413) - : null; -} -export async function readBoundedRequestBody( - request: Request, -): Promise { - if (!request.body) return ''; - const reader = request.body.getReader(); - const chunks: Uint8Array[] = []; - let totalBytes = 0; - try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; - totalBytes += value.byteLength; - if (totalBytes > MAX_REQUEST_BYTES) { - try { - await reader.cancel(); - } catch { - // The request is already being terminated; preserve the size error. - } - throw new Error(`Request body exceeds ${MAX_REQUEST_BYTES} bytes`); - } - chunks.push(value); - } - } finally { - reader.releaseLock(); - } - const body = new Uint8Array(totalBytes); - let offset = 0; - for (const chunk of chunks) { - body.set(chunk, offset); - offset += chunk.byteLength; - } - return new TextDecoder().decode(body); -} - -function parseJsonRequest(text: string): JsonObject { - if (Buffer.byteLength(text, 'utf8') > MAX_REQUEST_BYTES) - throw new Error(`Request body exceeds ${MAX_REQUEST_BYTES} bytes`); - try { - const value = JSON.parse(text) as unknown; - if (!isRecord(value)) throw new Error('Request body must be an object'); - return value; - } catch (error) { - if ( - error instanceof Error && - error.message === 'Request body must be an object' - ) - throw error; - throw new Error('Request body must be valid JSON'); - } -} - -function json(value: unknown, status = 200): Response { - return new Response(JSON.stringify(value), { - status, - headers: { - 'content-type': 'application/json; charset=utf-8', - 'cache-control': 'no-store', - }, - }); -} - -function textResponse(value: string, status: number): Response { - return new Response(value, { - status, - headers: { - 'content-type': 'text/plain; charset=utf-8', - 'cache-control': 'no-store', - }, - }); -} - -function htmlEscape(value: string): string { - return value.replace( - /[&<>"']/g, - (character) => - ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[ - character - ] as string, - ); -} - -export function renderReviewPage(review: StoredReview): string { - return ` - - - - - -${htmlEscape(review.title)} - Interactive review - - - - -
- -
-
-

Structured pull request review

-

Loading review…

-

-
-
-
-
-
-

Business primer

Understand the change first

-

The operational context stays ahead of implementation detail. Missing context is explicit but does not dominate the review.

-
-
-
- - -
-

Review findings

Inspect the failure path

-
-
- -
-
-
-
-

Discussion

General comments

-

Add a general comment

-
-
-
-
-
- - -`; -} - -function validateWriteRequest(request: Request, url: URL): Response | null { - const contentType = request.headers.get('content-type'); - if ( - contentType?.split(';', 1)[0]?.trim().toLowerCase() !== 'application/json' - ) { - return textResponse('Content-Type must be application/json', 415); - } - const origin = request.headers.get('origin'); - if (origin && origin !== url.origin) { - return textResponse('Cross-origin writes are not allowed', 403); - } - return null; -} - -export function createReviewServer( - workspace: string, - host = '127.0.0.1', - port = 0, -) { - const resolvedWorkspace = resolve(workspace); - const review = (): Promise => loadStoredReview(resolvedWorkspace); - return Bun.serve({ - hostname: host, - port, - async fetch(request) { - const url = new URL(request.url); - try { - const pathname = decodeURIComponent(url.pathname); - if (request.method === 'GET' && pathname === '/') - return new Response(renderReviewPage(await review()), { - headers: { - 'content-type': 'text/html; charset=utf-8', - 'cache-control': 'no-store', - 'content-security-policy': - "default-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline'; connect-src 'self'; base-uri 'none'; form-action 'self'", - }, - }); - if (request.method === 'GET' && pathname === '/api/review') - return json(await review()); - if (request.method === 'GET' && pathname === '/api/comments') { - const comments = (await readCommentStore(resolvedWorkspace)).comments; - const unanswered = url.searchParams.get('status') === 'unanswered'; - return json({ - comments: unanswered - ? comments.filter( - (comment) => - !comment.replies.some( - (reply) => reply.role === 'assistant', - ), - ) - : comments, - }); - } - if (request.method === 'POST' && pathname === '/api/comments') { - const writeError = validateWriteRequest(request, url); - if (writeError) return writeError; - const lengthError = requestContentLength(request); - if (lengthError) return lengthError; - const body = parseJsonRequest(await readBoundedRequestBody(request)); - const finding = - body.findingId === null || body.findingId === undefined - ? null - : boundedString(body.findingId, 'findingId', 16); - const storedReview = await review(); - if ( - finding !== null && - !storedReview.findings.some((item) => item.id === finding) - ) - return textResponse('Unknown findingId', 400); - const comment = await withLock( - join(resolvedWorkspace, 'comments.json'), - async () => { - const store = await readCommentStore(resolvedWorkspace); - const next: ReviewComment = { - id: randomUUID(), - findingId: finding, - author: commentAuthor(body.author), - role: 'reviewer', - body: commentBody(body.body), - createdAt: new Date().toISOString(), - replies: [], - }; - store.comments.push(next); - await writeJsonAtomically( - join(resolvedWorkspace, 'comments.json'), - store, - ); - return next; - }, - ); - return json({ comment }, 201); - } - const replyMatch = /^\/api\/comments\/([0-9a-f-]{36})\/replies$/.exec( - pathname, - ); - if (request.method === 'POST' && replyMatch?.[1]) { - const writeError = validateWriteRequest(request, url); - if (writeError) return writeError; - const lengthError = requestContentLength(request); - if (lengthError) return lengthError; - const body = parseJsonRequest(await readBoundedRequestBody(request)); - if (body.role !== 'assistant') - return textResponse('Replies must declare assistant role', 400); - const comment = await withLock( - join(resolvedWorkspace, 'comments.json'), - async () => { - const store = await readCommentStore(resolvedWorkspace); - const parent = store.comments.find( - (item) => item.id === replyMatch[1], - ); - if (!parent) throw new Error('Comment not found'); - const reply: CommentReply = { - id: randomUUID(), - author: commentAuthor(body.author), - role: 'assistant', - body: commentBody(body.body), - createdAt: new Date().toISOString(), - }; - parent.replies.push(reply); - await writeJsonAtomically( - join(resolvedWorkspace, 'comments.json'), - store, - ); - return parent; - }, - ); - return json({ comment }, 201); - } - const revisionMatch = /^\/api\/findings\/(#[1-9]\d*)\/revisions$/.exec( - pathname, - ); - if (request.method === 'POST' && revisionMatch?.[1]) { - const writeError = validateWriteRequest(request, url); - if (writeError) return writeError; - const lengthError = requestContentLength(request); - if (lengthError) return lengthError; - const body = parseJsonRequest(await readBoundedRequestBody(request)); - const result = await withLock( - join(resolvedWorkspace, 'review.json'), - async () => { - const current = await loadStoredReview(resolvedWorkspace); - const target = current.findings.find( - (item) => item.id === revisionMatch[1], - ); - if (!target) throw new Error('Finding not found'); - const revision = revisionFor(body, target); - const comment = ( - await readCommentStore(resolvedWorkspace) - ).comments.find((item) => item.id === revision.commentId); - if (!comment || comment.findingId !== target.id) - throw new Error( - 'commentId must identify a comment on this finding', - ); - const applied: FindingRevision = { - ...revision, - id: randomUUID(), - createdAt: new Date().toISOString(), - }; - Object.assign(target, applied.changes); - target.status = applied.status; - target.revisions.push(applied); - current.verdict = currentVerdict(current.findings); - await writeJsonAtomically( - join(resolvedWorkspace, 'review.json'), - current, - ); - return { finding: target, verdict: current.verdict }; - }, - ); - return json(result, 201); - } - return textResponse('Not found', 404); - } catch (error) { - const message = - error instanceof Error ? error.message : 'Request failed'; - const status = - message === 'Comment not found' - ? 404 - : message.includes('busy') - ? 503 - : 400; - return textResponse(message, status); - } - }, - }); -} - -function readArguments(argumentsList: string[]): Map { - const values = new Map(); - for (let index = 0; index < argumentsList.length; index += 1) { - const item = argumentsList[index]; - if (!item?.startsWith('--')) - throw new Error(`Unexpected argument: ${item}`); - const key = item.slice(2); - const value = argumentsList[index + 1]; - if (value && !value.startsWith('--')) { - values.set(key, value); - index += 1; - } else { - values.set(key, true); - } - } - return values; -} - -function option( - values: Map, - key: string, - required = false, -): string | undefined { - const value = values.get(key); - if (value === true) throw new Error(`--${key} requires a value`); - if (required && value === undefined) throw new Error(`--${key} is required`); - return value; -} - -function rejectUnknownOptions( - values: Map, - allowed: string[], -): void { - for (const key of values.keys()) - if (!allowed.includes(key)) throw new Error(`Unknown option: --${key}`); -} - -export async function main( - argumentsList = process.argv.slice(2), -): Promise { - const [command, ...rest] = argumentsList; - const values = readArguments(rest); - if (command === 'prepare') { - rejectUnknownOptions(values, [ - 'review-json', - 'pr', - 'repo', - 'data-dir', - 'scenarios', - 'presentation', - - 'spec', - 'requirements', - 'base-commit', - ]); - const prepared = await prepareReview({ - reviewJsonPath: option(values, 'review-json', true) as string, - pr: option(values, 'pr', true) as string, - repoPath: option(values, 'repo'), - dataDir: option(values, 'data-dir'), - scenariosPath: option(values, 'scenarios'), - presentationPath: option(values, 'presentation'), - - specification: option(values, 'spec'), - requirementsPath: option(values, 'requirements'), - baseCommit: option(values, 'base-commit'), - }); - process.stdout.write( - `workspace: ${prepared.workspace}\nreviewed commit: ${prepared.review.reviewedCommit}\n`, - ); - return; - } - if (command === 'serve') { - rejectUnknownOptions(values, ['workspace', 'host', 'port', 'expose']); - const workspace = option(values, 'workspace', true) as string; - const host = option(values, 'host') ?? '127.0.0.1'; - const exposed = values.get('expose') === true; - if (!LOOPBACK_HOSTS.has(host) && !exposed) - throw new Error( - 'Refusing non-loopback binding without explicit --expose. Local review comments and findings may be visible on the network.', - ); - if (!LOOPBACK_HOSTS.has(host)) - process.stderr.write( - 'WARNING: review site is exposed beyond loopback; anyone who can reach this host can read review data and submit local comments or lifecycle revisions.\n', - ); - const portText = option(values, 'port'); - const port = portText === undefined ? 0 : Number(portText); - if (!Number.isInteger(port) || port < 0 || port > 65535) - throw new Error('--port must be an integer from 0 through 65535'); - const server = createReviewServer(workspace, host, port); - process.stdout.write( - `Interactive review: http://${host}:${server.port}\nworkspace: ${resolve(workspace)}\n`, - ); - return; - } - throw new Error( - 'Usage: review-site.ts prepare --review-json --scenarios --pr [--presentation ] [--spec | --requirements ] [--base-commit ]\n review-site.ts serve --workspace [--host 127.0.0.1] [--port 0] [--expose]', - ); -} - -if (import.meta.main) { - main().catch((error: unknown) => { - process.stderr.write( - `${error instanceof Error ? error.message : 'Interactive review failed'}\n`, - ); - process.exitCode = 1; - }); -} diff --git a/plugins/engineering/skills/pr-interactive-review/scripts/tsconfig.json b/plugins/engineering/skills/pr-interactive-review/scripts/tsconfig.json deleted file mode 100644 index 1f00e5c1..00000000 --- a/plugins/engineering/skills/pr-interactive-review/scripts/tsconfig.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "compilerOptions": { - "strict": true, - "noEmit": true, - "target": "ES2022", - "lib": ["ES2022", "DOM"], - "module": "ESNext", - "moduleResolution": "bundler", - "allowImportingTsExtensions": true, - "skipLibCheck": true, - "types": ["bun-types", "node"] - }, - "include": ["review-site.ts"] -} diff --git a/plugins/engineering/skills/worktree/SKILL.md b/plugins/engineering/skills/worktree/SKILL.md deleted file mode 100644 index 0c813a4e..00000000 --- a/plugins/engineering/skills/worktree/SKILL.md +++ /dev/null @@ -1,41 +0,0 @@ ---- -name: worktree -description: Creates and removes isolated Git worktrees under a consistent repository sibling directory. Use when starting feature or fix work in a dedicated worktree, or cleaning up a finished worktree. ---- - -# Git Worktrees - -Resolve this skill's directory, then invoke its bundled `scripts/worktree.sh`. Pass `-C ` when the shell is not already inside the target repository. - -## Add - -```bash -/scripts/worktree.sh -C add [start-point] -``` - -The script: - -1. uses an existing local branch when present; -2. creates a tracking branch when `/` exists; -3. otherwise creates the branch from `start-point`, the remote default branch, or `HEAD`; -4. prints the created worktree path. - -Fetch the remote before adding when the worktree must start from its latest state: - -```bash -git -C fetch origin -/scripts/worktree.sh -C add feat/example origin/main -``` - -Worktrees default to `__worktrees/`. Set `WORKTREE_ROOT` to use another parent and `WORKTREE_REMOTE` to use a remote other than `origin`. - -## Remove - -```bash -/scripts/worktree.sh -C remove -/scripts/worktree.sh -C remove --force -``` - -Normal removal preserves Git's dirty-worktree protection. Use `--force` only when discarding the worktree's uncommitted files is intended. Removal unregisters and deletes the worktree directory; it leaves the branch intact. - -An add is complete when the script prints the new path and `git -C worktree list` contains it. A removal is complete when that path is absent. diff --git a/plugins/engineering/skills/worktree/scripts/worktree.sh b/plugins/engineering/skills/worktree/scripts/worktree.sh deleted file mode 100755 index 08679e1e..00000000 --- a/plugins/engineering/skills/worktree/scripts/worktree.sh +++ /dev/null @@ -1,128 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -usage() { - cat <<'EOF' -Usage: - worktree.sh [-C ] add [start-point] - worktree.sh [-C ] remove [--force] - -Worktrees are created under __worktrees by default. Override the -location with WORKTREE_ROOT and the default remote with WORKTREE_REMOTE. -Branch slashes become dashes in directory names. -EOF -} - -fail() { - printf 'worktree: %s\n' "$*" >&2 - exit 1 -} - -repo_dir="." -if [[ "${1:-}" == "-C" ]]; then - [[ $# -ge 3 ]] || { - usage >&2 - exit 2 - } - repo_dir="$2" - shift 2 -fi - -repo_root="$(git -C "$repo_dir" rev-parse --show-toplevel 2>/dev/null)" || - fail "repository not found: $repo_dir" -remote="${WORKTREE_REMOTE:-origin}" -[[ -n "$remote" ]] || fail "WORKTREE_REMOTE must not be empty" - -configured_root="${WORKTREE_ROOT:-${repo_root}__worktrees}" -if [[ "$configured_root" = /* ]]; then - raw_worktree_root="${configured_root%/}" -else - raw_worktree_root="${repo_root}/${configured_root%/}" -fi - -worktree_path_for_branch() { - local branch="$1" - printf '%s/%s\n' "$worktree_root" "${branch//\//-}" -} - -default_start_point() { - local remote_head - remote_head="$(git -C "$repo_root" symbolic-ref --quiet --short "refs/remotes/$remote/HEAD" 2>/dev/null || true)" - printf '%s\n' "${remote_head:-HEAD}" -} - -command="${1:-}" -case "$command" in - add) - [[ $# -ge 2 && $# -le 3 ]] || { - usage >&2 - exit 2 - } - - branch="$2" - start_point="${3:-$(default_start_point)}" - git -C "$repo_root" check-ref-format --branch "$branch" >/dev/null 2>&1 || - fail "invalid branch name: $branch" - - mkdir -p "$raw_worktree_root" - worktree_root="$(cd "$raw_worktree_root" && pwd -P)" - path="$(worktree_path_for_branch "$branch")" - [[ ! -e "$path" ]] || fail "path already exists: $path" - - if git -C "$repo_root" show-ref --verify --quiet "refs/heads/$branch"; then - git -C "$repo_root" worktree add "$path" "$branch" - elif git -C "$repo_root" show-ref --verify --quiet "refs/remotes/$remote/$branch"; then - git -C "$repo_root" worktree add --track -b "$branch" "$path" "$remote/$branch" - else - git -C "$repo_root" worktree add -b "$branch" "$path" "$start_point" - fi - - printf '%s\n' "$path" - ;; - - remove) - shift - force=() - if [[ "${1:-}" == "--force" ]]; then - force=(--force) - shift - fi - [[ $# -eq 1 ]] || { - usage >&2 - exit 2 - } - - [[ -d "$raw_worktree_root" ]] || - fail "worktree root does not exist: $raw_worktree_root" - worktree_root="$(cd "$raw_worktree_root" && pwd -P)" - - target="$1" - if [[ "$target" = /* ]]; then - candidate="$target" - else - candidate="$repo_root/$target" - fi - if [[ -d "$candidate" ]]; then - path="$(cd "$candidate" && pwd -P)" - else - path="$(worktree_path_for_branch "$target")" - fi - - case "$path" in - "$worktree_root"/*) ;; - *) fail "refusing to remove a worktree outside $worktree_root" ;; - esac - - [[ -d "$path" ]] || fail "worktree does not exist: $path" - git -C "$repo_root" worktree remove "${force[@]}" "$path" - ;; - - -h|--help|help) - usage - ;; - - *) - usage >&2 - exit 2 - ;; -esac diff --git a/plugins/finance/.claude-plugin/plugin.json b/plugins/finance/.claude-plugin/plugin.json deleted file mode 100644 index 274aa5f8..00000000 --- a/plugins/finance/.claude-plugin/plugin.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "finance", - "description": "Reusable finance skills, starting with transparent spreadsheet financial modeling.", - "author": { - "name": "AllAgents" - }, - "version": "1.0.0", - "category": "finance", - "homepage": "https://github.com/allagentsdev/allagents/tree/main/plugins/finance" -} diff --git a/plugins/finance/skills/financial-modeling/SKILL.md b/plugins/finance/skills/financial-modeling/SKILL.md deleted file mode 100644 index 66f0877c..00000000 --- a/plugins/finance/skills/financial-modeling/SKILL.md +++ /dev/null @@ -1,54 +0,0 @@ ---- -name: financial-modeling -description: Build, refactor or review spreadsheet financial models with traceable assumptions, calculation blocks, consistent timelines and validated outputs. Use for forecasts, budgets, valuations or financial workpapers where formula structure and maintainability matter. ---- - -# Financial modeling - -Build models that another person can understand, change and reconcile. Use the FAST approach demonstrated by F1F9: each calculation block has its ingredients above one result, and periods run across columns. These are modeling conventions, not a certification of FAST compliance. Attribution and example-model observations are in [references/sources.md](references/sources.md). - -## Establish the model contract - -Identify the decision or report the model supports, required outputs, entities, ownership basis, currency, units, period frequency and source records. Distinguish actuals, forecasts, scenarios and unresolved assumptions. Preserve the user's chosen reporting layout and accepted business rules; restructuring formulas should not silently change their treatment. - -Inspect existing formulas, timelines and source conventions before editing. For a refactor, preserve a baseline of source inputs and key outputs so the structural change can be reconciled. Derive aggregates from canonical records; detailed evidence and overlapping statements support those records rather than adding to them again. - -## Structure the workbook - -Give each sheet a clear role. For a substantial model, the logical flow is **Inputs → Time / foundation → Workings → Outputs**, with checks visible near their results. A cover or comparison sheet may come first for navigation. Small models can keep these as clearly separated sections within one sheet. - -- **Inputs:** assumptions and source figures, recorded once with units, provenance and status. Put constants in a dedicated column and period-varying inputs against their timeline. Keep raw transaction evidence in a separate table when its grain differs from the model. -- **Time / foundation:** one canonical timeline, period starts/ends, actual/forecast boundaries, activity flags and exposure fractions. State whether boundaries are inclusive and whether the denominator is actual days, a contract basis or another convention. Calculate timing once and link it into the relevant blocks. -- **Workings:** blocks that own one calculation each. Put only the ingredients needed for that result above it; place the output at the bottom, visually separated from the next block. A schedule with several outputs, such as claim and closing balance, uses separate blocks. -- **Outputs:** direct links to calculated results, reporting subtotals and comparisons. Keep business calculations in Workings so the same result can feed several reports without duplicating its logic. - -Keep label, unit, constant and timeline columns in consistent positions across calculation sheets. Use descriptive labels that specify the entity and basis when ambiguous, such as whole-asset versus owner share, or cash proceeds versus taxable gain. Visually distinguish inputs, ingredient links, calculations and unresolved values with a small documented legend. Keep the periods and labels visible while scrolling. - -Default to parentheses for negative monetary amounts, such as `(1,234.56)`, using a number format rather than converting numbers to text or changing their signs. Preserve the font colours that identify inputs, links and calculations. An established workbook may also colour negatives red; document that preference in its legend. Apply the chosen monetary format consistently across comparable figures and preserve any explicit user convention. - -## Construct calculation blocks - -Read [references/calculation-blocks.md](references/calculation-blocks.md) when building blocks or refactoring a long formula. Open [references/basic-calculation-blocks.xlsx](references/basic-calculation-blocks.xlsx) to inspect the same conventions as an editable, original workbook with fictional inputs. - -1. Link each ingredient directly from its original input or the block that calculates it. Repeated ingredient links are intentional: they make each block readable in place. Keep the source value and calculation logic single-sourced; avoid chains of pass-through links. -2. Calculate the block's result from nearby ingredient cells. Default to direct references, simple arithmetic and plain `SUM` over contiguous ranges. Arrange contributing rows together and keep exclusions outside the range. Split nested business decisions into explicit ingredients or intermediate blocks. Add complexity only for an actual calculation requirement. -3. Separate **how much** from **when**. Link the magnitude and an activity flag or exposure fraction into the block, then combine them locally. Flags describe eligibility or timing; they do not make unknown amounts become known zeroes. -4. Use the same formula pattern for equivalent periods and entities. Apply a necessary guard consistently across its calculation block; choose the pattern from the model's supported input states rather than today's populated cells. Anchor constant references and retain relative period references. An opening balance normally links to the preceding closing balance; initialization, actual/forecast boundaries and disposal periods are explicit exceptions. Mark formula-pattern exceptions so a reviewer can find them. Keep missing historical records visible rather than fabricating a roll-forward. -5. Record signs and units before combining values. Signed cash schedules can use receipts positive and payments negative; a report can show deduction magnitudes when required. Label that reporting conversion. Keep allocation percentages, exchange rates and tax assumptions in identified inputs rather than embedding unexplained business constants in formulas. - -## Preserve missing-data semantics - -Identify actual missing inputs and permitted blank states before adding guards. A missing input remains unknown; a sourced zero remains zero. For validated numeric inputs, use direct links such as `=Inputs!B4` and totals such as `=SUM(B12:C12)`. Validate source completeness during import or export rather than repeating `COUNT`/`COLUMNS` checks in every total or adding count-check rows for fixed, populated ranges. - -Use a conditional guard only when a documented missing-input case could change the result or its meaning. For example, an outstanding quote must remain unknown rather than become zero through a direct reference; a minimal numeric link is `=IF(ISNUMBER(Inputs!B4),Inputs!B4,"")`. Follow the user's chosen SUM behavior for readable reports. Label genuinely partial calculations appropriately and keep inapplicable periods distinct from unknown amounts. A denominator of zero needs the chosen mathematical interpretation; suppressing all errors with `IFERROR(...,0)` conceals broken references. - -## Validate the delivered model - -Scale validation to the changed calculations. Check the actual saved workbook, not just its generating code: - -- Reconcile canonical source amounts and existing key outputs against the preserved baseline. Explain intended differences and keep excluded evidence out of aggregates. -- Recalculate formulas with the target spreadsheet application or an independent formula engine. Cached values are a preview, not evidence that formulas work. If full recalculation is unavailable, state which checks were actually performed. -- On a copy, change a meaningful input and confirm the expected effect through the relevant Workings, Outputs and comparison totals. Restore or discard the test copy. Exercise important boundaries: zero versus missing, period endpoints, remaining-balance caps, denominator zero and entity isolation as applicable. -- Check formula patterns across periods and trace a representative output back to its source. Inspect visible errors, unresolved inputs, check differences and unintended external workbook links. Confirm the delivered format preserves formulas and navigation. - -For reproducible generation, keep business rules separate from workbook layout and source-adapter code. Preserve an appropriate source snapshot or version reference, include enough calculation context to regenerate the model, and keep confidential records in the user's private store. Return the model link or path, what was validated, and remaining assumptions affecting its use. diff --git a/plugins/finance/skills/financial-modeling/references/basic-calculation-blocks.xlsx b/plugins/finance/skills/financial-modeling/references/basic-calculation-blocks.xlsx deleted file mode 100644 index 39fe7d314fb22de67adc5fd29a5ea04e1757c009..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 10748 zcmZ{K19T+XvUY4+6Wg9R6Wews>e#kzO)~MswvCB3u|2Wvyqvl3{pX(i_x9@Uy}H+` zuX|PPs$Ji=OI`{T3=Ief2-u$3~6 z36Hfs-m$q?TZL331mOBw&Lfjg$-YUlX52%I%lB06#?bLql2`yDfYcXuJnicNDa4dM zlL$_lu;)*Elu|`tCgTB7Nj*W5@}BUhr$OoA(C4~^4!!xcV9AOlybIZHKtUOrQa2dMLn>w~63`LF23L-1Elz=R;C1ljws^>G z_1Na+1M|2mUjqmBp!iZJ)E96MqIFW3X07HvM2#VAguqU_>=hK^H|W+%nzllAB9qu8 zZx_B|?~J^4;kC02**TGOR=iO`CKLO*=Ob0_CA`J0x6gCBI!qw)0n7g*8kboTO9$^! zkOBb$f_-20?2RoQ80i0cFOTn*?PY}fa^@+r^YqA`BdIn?r#dpfG~tKRZ3?yt6GUw~wh1T16)PEBh{iK0=ST$Sqq3?JGt!;Ry^C6C1b(Sc zX5E@;%unv8DzsoUp(jkyHQL}~$bWh*u<@E)6%+^v0R{*N>t9~`%U=$r#>S2g41X4; zzZ{pOI%2=ffz-A~gM@$EuqCu5q#Irwh5obnlAL;ZDDx{yqC62agfAV1UMHw;!Gmu@ zdVr2lcIz(|EGY4H-dS{aQf>J;OFW-V&QMlP8dHY_4!tvHA94f~l+WN5>O$<}a>VH_ zi)q(=_bzs%!o*0Xef^x0i@Hq}gE4vi2Hl!dUD89YQDmN%0W_MjwmA|`zacU)o7oDY z4AJQCEm+kxrqV4nUgZ-+agb%HX}dc(rc^mJO4JsvXss^%R!|=5R~?iU-3D$Mfh4%B zlUsO8vI+C2#L^RS_u75zepA5wKwhWh3=@d#i_74%*VLX4{<0*`5O!uGA!089Ll&)U zc8dK}v5^G>uAqwJpJ}}uJ{(o+I5|PD4V+OwpO^S8uY`S^-4SV=%!|~cY}jie&*%o8 z7cYMe655aq&?%n#>CiaoMm1NUBmgH$7L_NAfUCLOCO1_iF^uWrtcpU=HQ6-z*rH?# zzkgzO`M@f$oKifDfZDBBR!vwT$5fLSkTm(GAcLzjgu|a2J|&2sV;!+@H#dT1`5O^L z^>se->C^JLe>7&>`CKJ2J@-}(#p7maXlXQ7JJPlq@^&=Vn@PCDfRwv#$}_<)w*1-U zs-GwTDgZ^FlNGon*|nW*+?TXn^YeC`S%)gN!!Nuhx`%w!*!Q3&a(WsEG%|<~j-7=T zeZTRLBayWN)O)m3SG$3|T6OZRi{o^WqPq!9HV{E=Ss~6$HdcY~mapK--GVjE*ai)1 z=JEvs3n0$5*fKkqr+-z+`>lR4VnZ5(>q79k^8a2h71E^NJG{3F%fd)D7omd^F(Mt!z{K5|U$^gLtQ}At0D2PRDR_++) z%K9=P%%A(-OIa?&hmn6$G#iFqwCN|NzJJ=2Cnx1V!V&u>s zVxvzyPhF@T3y(UgN(! zNNGiE2ue^^S&EyA43Fh_BGog((&t;Te4f6)Hus5}Y3|jbu|-_S6`U{cH11RGAl3t< zNucCQvCitP>7f-CGgXvjgnP}gr~=E1g2kFiO5!4SF-jzvZ`#0%DJ!cZY^n56gwVhz z7Br1Q!e^JjDit`9V^=eq3flQ-eQud?2 zgh-jjiD@Z?FZ)fC#li>9L1L8jaii4@Qt(P+GXL1z)H17qpfqX8IACh8EIUMTJ$##u zg|f^yDl<%R(MH*>_KvY+Kj4M;Qu{2&I$`ooRRC&w7y*X)ZTs;h*?FbzcY)l>p@{yZ zzX0Up6{cS;hBlAiW<*yNc(0nKcEd`|_U?$DUtiK3eeZYk@4||Badq`7Rf$zLh2NwW z3Q`X|ZQM_=XrtWxP7?@;@5tdieUHxS)|_+5YNst3g=to1fwQp>GS+mreob0xp|YiuWDc8p-Ie3rI62hqu4^6;)+K|`wB zpy|=@QKs6sd?&j+FH3u_mMyC(m2KCxtr=Rg!j=HNVRsnx z$+I7@2$JNO+IY_Bk|JD{FkH5kUcz`*Ye7A}FhjgUCLcr1V+fp#LCkCA!NVaX4QUN- zH|T2$ER7vD?TzfeC^6e``VOq*bY)SX5-#lJ-g!tn5oi(kCZa=;a6XcqZl zTP}y}27?;s4Kk(E694w0yMlnY1s4Se0&072fB(y=$nqx>)&FFo*SelS>Zoj*1e~C! zyOm1=lO^-CGmBa4le85w%g<1dhyB;bb|}9%q_CG*!d^}_t~-345!yBWGnSLML>0-a z{AiPcia1Z9Ml$6Tw zpCip{NuHX%|MK5CC(ZfISFvg6W}_@3u|-F6!qUquGrurkR?&#O`mnX0^-36U8)x;3-rBAh(-;^B%-Bev z@EhHH@@gT6N2l)kVPmRV{kFts)mCWnbf2n|HD%Qv&^E?n1hEicY=uvq^00o(w{@Hh zu*-DX02f1MqS(p|P0tNwr)Yuv;ez&STlh>kjBoFOcj7{i7{|(71AETyQzZtO*OX-P zGK?g2NBu7Sg5&hc^W=*0L^kUfguC`4>~=LYs884NK|{PXOAA|eZss_WnqeRr@f;mU zZ>`Y{I{Qu<8!gdS@o9TuTMm#A&?*}LTx1R$U*39Iz2a$t(v(4^>>bER8^rr5&3U>W zfJOUIL`aLfgR|4_`3>>Dvm&`HnJ@(sn8=_q_W+(*P2N@|Cxk}87V*j$M* zpD@|fv{c!RM7!Ns+2V>UO-0_aX|X&O6~wBfjlrZ8+$4^_8+rym@KJWomunA?qY)iL z)npVx{YoJW7c^HeB^~~zi5)V?U6eM#ROlRQTwJK^Ds2sm7qCR`jM^SFs+-=eNMBL) zB+JGkS2zmGz4q8uJ60`|i_CJEvgsF=(Uk>W0KYC2y{`R}aGocWJ2v>2uzeL5r)xT% z96c2N>AjH1bBD1ys>9MD=gGVl&H&!>6xune=68QDVX0c}dM=l;@wnOM0NJa9U6zHh z7T-<>Mj$@Oiwxk{9ks+^i2I6sYZV?BF5e-EOEw5u@a0a-6^_WVx5>|5Uv&_7aK^e4!V(fK7q|=Rvz*KE9}Q zoEFa(lwiosG=I=#6v7BDJ?d5&o?uhGCy?6_^wRALzY=7;z!mV)twP@eUwp%Jh

- zr+LNfrg&Ocy#w>`RL6SAGK{|JgyQRdBfc~bVa?5U5gwyijBmgr$7C7 zc}*9B$XD=#Kd)K$UE(QiVzrvg012p4n zFeMm?C(cPpjvsWcL}d+owW7#R_OE(40yVI~8q~*##XKV|}iB?}9krY3+Zh zy{!LYBsvLc>}XzRmGZ9pb%@}7g|ok5$5S-h5 z3n)|XrFRd;uJ_JHB}~_B8NE!`Lj5zF@KK_%NV}fJgxm##u+S&DLs}{gnfay9OJ--e z1U)1coz9Swoc=&gjdKX;pJ{scr$Ec&WdZQzU&#ShNw%(*ccSxktV5*5)bJy$yt$I$ znnxsMOIik1JPe90Uk{{2EY={+%u;O^yj>{2%SH$`_f$mnkS|V)4QHk4L|ppwSAT-@ z6(0)~G!!-l4@9|Ckh-eB1+F}0L`Dj`Z_OUCo)TO2`{ZL2aokZvH3-oeh^~kAL|yPU z*cYqbQm}tgO$FOAX9@3x+Y%kjVR33U&OcS7ZGo#MBrzPJ)*=KN2eF4FQ!&fJ&V=a&?&0md&Yn;Hip1=gxjkEbmFy zO1SY-*JI@Fkkk;t!bLO;wb2(*IgQ03A7%8dW3wX3H6ssNW^bT%M}joJlir4@ZwRWd zD>2)Ct5;=yfDE^88DS5?B@*P;&&U~m`x#-cGQvKDk8euE*sex9%{v~#xs9p?UlYzr zVYHlu%dZ;(lzW=%Rl%aos3G`66*wV_kr^f}c7pp$p+NcdqaDR0Tu4{HPhNHC%+X;tIKK81XJ(kwM0MDSyLw$MnIReP>uJ0T z??4)t-A@V261nu|YlGz9W4@_rL?|m~qsl2c7rmn)?lyEgo>rI~e#`{4aITsvG?ig@&bEU{X`;>Q)n^j3ZNC+~;(N zvb=RyhE;G#kfp9DgiQ)fr5LC5sIt2~I>tRG4yVdJNqHyYHLJe+h7CCxN2-_GaW=^O zlhwvTM-P)rYDT4%aMH;Sx4SiAiVY4e1sLH;_a|(gW&H#5=PsbHTPNf!v;8~M#3f~N z60Hn}2ubovUx4#^AuCS!Z)y;o!jJvty`b&fxxNU9Yh8d5Y^F1LyEFD5p5zOZ(Pb|r zu&rs*i1(Ul~e73MLy1-3{cKd$+&A-pbENK>y4tGsqTO*DuJke-%g!5O;P7 z{q1!&G071E9W??TS$!Dw3i{7XNAk|iY16o;-T{5Krg!1@==;9_cxEiol$cyk|rb>$VM zn~LP}00Sk7pgRho$i|b;Yx0X?;L>(`{LlvqJFyC;?mSWp6<+HS=>8&YzD2G~Zh+LT zaF^k;qPjTQ6Ukau9SCFjRjK3EEIA^hf%ci6YtB4ur(3hgc0B+X|i<(`Ob2> zrljIK@W>&D0@7|a{+U`3I`_6fc#rT?gMl>{IRl~!>7UM z91aa1kEF878W~oY9##e;ss#UvEY*63_-CMdT-11r-%C2>JF)#sP=1%@98AC38yhJ(+M8Je9R4cj zQL4+RvecdKfl*{zt?0y@u^U{2OdExB*&)LN+^q+a!51Pu+~a^_p;TIjq{!y-VY>BBI{7H0=O~D)()w+xldSZr$}$z zE->E7YKpTa(;h6eG1Bo#`G$ z2$n)&p4&~?VTxZ;BTg@_{oR;?o5Q__S|+g?7tYj}5s3R~?ioYlHt73a)Vf84EOh3q|lvZc_&#Efn+8z3$#_ON}Rn~?t;K&so)V9e(Z0y zR(GH*DRI@iS)F6WvdK4nQ>#58VvChAa5}5x9qX>~JD05>Urmju;T_~jlZcJ@y_r-C z#Jv|_eaefZ4$cElrw3zTq84~=yC}%q5{AKKc~i`0IG!!3DVg(etxk4>Lq=G=Z70jT zG+u9J4B?+pDA5X#lN=n`Z$qHxv2Y8D<_XOfrG!21p$*UrEM7308uffZvS72O=*mRv zsE^pD1kc(M@wEY@8xiX%IMQtG6{B|@Q_KBdFh7rGj1oM>8ZB-giJ>@x`BNHz0 zehFBkFW0Bp_7r~fAsBHYY~y=CEqSY7+QaB^%q4oRg$C$p)`p<`jvi#hE!Q6M4UQ_)I2v*r3Uh4f-z=4S2OpUwgZtHbGf?lgz%|y z=L!C`J+WSlD`*_nF@w)hwrpiJm~$#^0QXaK)V|(nlj8$YV_B4XlX#iqbO>QHAb(## zHE@4kyptP3Fy+ZP?5UPd`Aw;BW&+zIOMN&fGtYFf4&+=v%c?RW<=YU22A92(V@h-` zgC2kUfSM#QoV0W|t%F8S_c{WRuKwkV-TXSb2t){+cXPY>rUQI|wXzAfWf>pVW4Avo!vb)9q?jHffAVuN`_{o4?mdXt|k*FQXT= zREQ@um@BfF>B%tm#p}f~>D1`)alLxW+Q zY8r7R=E{kQomL;|da6qiFpQ#dX|)H)*cd*nrEt-?;qCh0F{KNXhV~v3G>It-x%PX~ z7sY-#AY~KJ-3{zr6HcT~>;P^})T9O%6#Ew_`V)uGi%ichL%GvK6j-i^b zYkF6P6SNPsdGzf0xmbHF`zRGA^+3zP)lfqPPcfb5gkGYWF&8C&6G;-fASg0h&{i0} zd&#ho!t`_zszk&=Zt2C>oNVc8!!!-~Ty>IZf+^nx;!JK0{M2m5n6HUa$3f;&c4o2R zM^H6t0KahUg3!W-Mw5Q33s`GgyPb{iaC`;IS5!~1k+bUO_Nt}S+r{AHn?Z^vQ8%qx zw2{06Y0%({1&WZl9IGP4Nm+!@yNgG-ABdSa?HM`>inhV?+?eHs8)X(bJ#s)*NacmzrXZ?ceHaVAl9biT1SG%G)Q-fbm+_CN}FGmSBA* zRoLAn3&D3MB*ARpSwBq{u;qlG)4d2Anv8TLv6jN*KROr&h!`4ih_R{ES~K~#KcfQj zO!hKn8SEL9EoMjfr7Ld=WWZiN%RGPB`&f^n%2n?wks|KDZbaXYHAhopE8~B?|G73f z(U6H*7RKthq7m(Aj7H@itN32dzL*zu(oniuS2PgDTV06lQINRELFldlVVe731bgCrlrH1RTxFaJx zWY8Q(p!3VQ$T!e9MzrPS*jM-gmBK@oy@0LB z39>^pPIfHr#hCPAdenWW+6HiY5N{qr#Ka)1WEktZa?RK3(60oF{%xeKm927D z^#bRuAH_R&`oI+z-P6#|<=c6==JaZX+W9HS8QszP;;C~QdRt@H=WpjPG`KR2;S&PL zxkIkd89yEHQ#p8o6qI-D1?)`i{uh+o{nuOqaF`KZS#p`x)1Mu;D zTL4Se z#l6WH{h_UiL8nl3i8v+Hs5C8fY10K)CM6y&TR+ou+~?|WX-Rz9AU(S#Wu${x#%Ooo zxG7-=Eoqv=Q-gis{;IQTGi?zVkpc1@nbGFxmCbGI!0nkL78&ACPdRu(;RbaRNea-q z<*}CVc3nfmw(I%A8I@&clZ?z1;?4l#;R>CDP9~Xg^ZrP|&)(~ifO9KX)>O$YM_ApU zCj$1hPdpO0&^wP$9Zwx+9T8qGDs+|0R+#W}b9-CqdTsXN_zL|9vx#wO%r;{vi3pEX zsSNf%9;7oxZ8gG6Hw68zEJ&LkQ%d$3d#FO~?4fgLH9}DGwR5qPWfd9u)~`$e zU%nXD4+I#r=48R|SzfA8ItfHYcDMB4sIAGSC;`nU?z)01b6|9%1GlU@F5MEO<}trhgGHrP-_D=Nub zd(N7?7Qun+B(*!*o$WfSDE)Z<5e{%~W{WQ=K@!WCDrUnL5f63UyLFD{H1HLn;z7#X z>Z0H(-WIoqq#FBrcPpcan1;SNxaEZ+xxb$yG#3hmq(n)!O3}F^jNM}7(2zqEY$Jm$ zi4hq0PcRnI<)J4cA1#Tp1y`9bN|N+cyARY@jh*z9MVpQ#=;&_2c{($XVk=p1j?r5> zbEu+VS$}%C$P6)K_RopN5)|pfAz>MFE;TgCBZtr@iq4$DV&U0qWMlf*3@QuKdaS1( zNJe9Lm2N1oHZv+hvI_eP7AnON8tElDcLb*@&1Y4zh#F_5meNdbxrj$Y;Tp{@j~+M5 zvmTM>li05uWG@yR;sn3Eo$qW{qrFNlpD!A8d^!gW<@+v|2!(Q?SW4UZu#4nqZPssG_PWSqj_f}d?y9$QUd-8A z-LnXY@NfWlKJ;Lbbs6`Cm|pMPyRyAx ze4fb~$jm8?(l}=kdaw_<2_%LkURH9bXc|5#1QzEPq!s$?du%7r0$FMDCB`4kjF~C4 zrR?Vr?$FV>L2p?)XMLoH7=vca*47$$GghG)bz#Kf_uI;!G0Sp#d!-mtJ4Las^S2Aa z*yv94NTkeQL5MI0lBugy5r+Cc-Y+IvVa;ISzYyw_X2SECNG4H*k6LA_JQ#b+@GUgX zsItG&v$Ra;B3#1VWMYq`e~Up1`|YX60>5viKSv=aNs>l!G{9{kR*;7+#3{#7xErF1 zv}N^(q9yTi^IA9Bspv8IW+ri{FQgk#{V!fs+bF&M` zd@Xwen+YL$li>kraXZ`dX?%kHXKzY^!zx?%9zK(IeF5p;!}rzJ_OIxbEB~@iV?^^> zS7Yq_T%f|L5@)B9pT=dTl^f(7lK`Oard~)sgO|L$Vab-SDJW-+D%|!!92f|R_&Hlq zoPYs>17LCHlhOcDZS7f*=|8=a1AL!apC^n=q!L8mn-Bff+k zS5TXzO#j&sl`$10s-PVV?_zAmy285DNvlbPqe0VYjovA&Mz!i4^J79<*XmW8#z0_YJdEpB#25$wgVl2P5YafiS{CV|4 zFO^I9nD)4bAXp^|WVua=AMr!QF~n;xh9{3j-vHH4uz6RjeLhGA)Ngwn$umD0ZSMwe z54`SB`{}MBQB0kc*J~{oEq4XGh2s}$EGd4-BI5)IYJv(}x?W8Ef zA+hV93u0||&8-SxAw<0N;ZFG=Xm%9?MEI~F3-lZQDSx*JT{JGVmFFWrz|8XAYBg)6wfxl4c7d@&BhkeMI<3Z2v~Uc%LqN|9OZ1M{_@#e&hvzo1%RBWBQRLd<6J-?)(jKMf@L5 np^xSt>)zkyHKhM${=ZdFUJCquqVq5I4iBXGzT* 0) continue; const entry = manifest.data.plugins.find( (candidate) => candidate.name === installation.marketplace?.pluginName, @@ -898,8 +888,11 @@ async function skillUpdateDomainRootsReadable( } if (roots.size === 0) return false; if (includesMarketplace) { - const manifest = await parseMarketplaceManifest(node.cachePath); - if (!manifest.success || manifest.warnings.length > 0) return false; + for (const installation of unit.installations) { + if (installation.marketplace?.nodeId !== node.id) continue; + const manifest = await parseMarketplaceManifest(node.cachePath, installation.marketplace.pluginName); + if (!manifest.success || manifest.warnings.length > 0) return false; + } } const readable = await Promise.all( [...roots].map((root) => diff --git a/src/core/marketplace.ts b/src/core/marketplace.ts index a26a8d2b..d19c1943 100644 --- a/src/core/marketplace.ts +++ b/src/core/marketplace.ts @@ -23,12 +23,13 @@ import { } from '../utils/marketplace-manifest-parser.js'; import { normalizeGitRef } from '../utils/git-source.js'; import { - getPluginCachePath, isFilesystemRoot, parseGitHubUrl, parseMarketplaceLocation, isNativePackageSource, isRemotePluginSource, + parseRemotePluginSource, + resolveRemotePluginPath, } from '../utils/plugin-path.js'; import { checkRepositoryHealth, @@ -1919,7 +1920,10 @@ export async function resolvePluginSpec( } // Try manifest-based resolution first: look up plugin name in manifest entries - const manifestResult = await parseMarketplaceManifest(marketplacePath); + const manifestResult = await parseMarketplaceManifest(marketplacePath, parsed.plugin); + if (!manifestResult.success && !manifestResult.error.startsWith('Marketplace manifest not found')) { + return null; + } if (manifestResult.success) { const pluginEntry = manifestResult.data.plugins.find( (p) => p.name === parsed.plugin, @@ -1940,48 +1944,25 @@ export async function resolvePluginSpec( }; } } else { - if (options.offline) { - // Offline mode: check if plugin is already cached, don't fetch - const parsedUrl = parseGitHubUrl(pluginEntry.source.url); - if (parsedUrl) { - const cachePath = getPluginCachePath( - parsedUrl.owner, - parsedUrl.repo, - ); - if (existsSync(cachePath)) { - const fileArtifacts = - declaredFileArtifacts ?? - (await getEmbeddedMarketplaceFileArtifacts( - cachePath, - parsed.plugin, - )); - return { - path: cachePath, - marketplace: marketplaceName, - plugin: parsed.plugin, - ...(fileArtifacts && { fileArtifacts }), - }; - } - } - return null; - } - // URL source - fetch/clone the plugin - const fetchFn = options.fetchFn ?? fetchPlugin; - const fetchResult = await fetchFn(pluginEntry.source.url); + const remote = parseRemotePluginSource(pluginEntry.source.url); + if (!remote) return null; + const fetchResult = options.offline + ? { success: existsSync(remote.cachePath), cachePath: remote.cachePath } + : await (options.fetchFn ?? fetchPlugin)(pluginEntry.source.url); if (fetchResult.success && fetchResult.cachePath) { + const pluginPath = resolveRemotePluginPath(remote, fetchResult.cachePath); + if (!existsSync(pluginPath)) return null; const fileArtifacts = declaredFileArtifacts ?? - (await getEmbeddedMarketplaceFileArtifacts( - fetchResult.cachePath, - parsed.plugin, - )); + (await getEmbeddedMarketplaceFileArtifacts(pluginPath, parsed.plugin)); return { - path: fetchResult.cachePath, + path: pluginPath, marketplace: marketplaceName, plugin: parsed.plugin, ...(fileArtifacts && { fileArtifacts }), }; } + return null; } } } diff --git a/src/core/workspace-modify.ts b/src/core/workspace-modify.ts index a0cc429a..ecae9f09 100644 --- a/src/core/workspace-modify.ts +++ b/src/core/workspace-modify.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'node:crypto'; import { existsSync } from 'node:fs'; import { chmod, mkdir, rename, rm, stat, writeFile } from 'node:fs/promises'; -import { basename, dirname, join } from 'node:path'; +import { basename, dirname, join, posix } from 'node:path'; import { fetchPlugin } from './plugin.js'; import { dump } from 'js-yaml'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../constants.js'; @@ -836,6 +836,22 @@ async function findPluginEntryByGitHubIdentity( getPluginSource(entry), ); if (existingIdentity === identity) return i; + // Skill-source promotion may widen two direct skill selections to their + // common parent. Keep that operation within one plugin's skills tree; + // marketplace selections and sibling monorepo plugins stay distinct. + const current = parseRemotePluginSource(getPluginSource(entry)); + const next = parseRemotePluginSource(source); + if (current?.type !== 'github' || next?.type !== 'github') continue; + const skillRoot = (subpath: string | undefined): string | null => { + if (!subpath) return ''; + const parts = subpath.split('/'); + const index = parts.indexOf('skills'); + return index === -1 ? null : parts.slice(0, index).join('/'); + }; + const currentRoot = skillRoot(current.subpath); + if (currentRoot !== null && currentRoot === skillRoot(next.subpath) && + current.owner?.toLowerCase() === next.owner?.toLowerCase() && + current.repo.toLowerCase() === next.repo.toLowerCase()) return i; } return -1; @@ -1397,17 +1413,20 @@ export function pruneEnabledSkillsForPlugin( } /** - * Resolve a plugin source to its GitHub owner/repo identity, if it points to - * a GitHub repo (directly or via a marketplace URL source). Returns null for - * local-path plugins. + * Resolve a source's selected GitHub plugin identity, directly or through a + * marketplace. Subdirectories distinguish plugins sharing a repository; refs + * do not change the selected plugin's identity. */ export async function resolveGitHubIdentity( pluginSource: string, ): Promise { - if (isGitHubUrl(pluginSource)) { - const parsed = parseGitHubUrl(pluginSource); - return parsed ? `${parsed.owner}/${parsed.repo}`.toLowerCase() : null; - } + const identity = (source: string): string | null => { + const remote = parseRemotePluginSource(source); + if (remote?.type !== 'github') return null; + const repository = `${remote.owner}/${remote.repo}`.toLowerCase(); + return `${repository}${remote.subpath ? `/${posix.normalize(remote.subpath)}` : ''}`; + }; + if (isRemotePluginSource(pluginSource)) return identity(pluginSource); if (isPluginSpec(pluginSource)) { const parsed = parsePluginSpec(pluginSource); @@ -1417,18 +1436,14 @@ export async function resolveGitHubIdentity( if (!marketplace) return null; if (getMarketplaceAccessError(marketplace)) return null; - const manifestResult = await parseMarketplaceManifest(marketplace.path); + const manifestResult = await parseMarketplaceManifest(marketplace.path, parsed.plugin); if (!manifestResult.success) return null; const entry = manifestResult.data.plugins.find( (p) => p.name === parsed.plugin, ); if (!entry || typeof entry.source === 'string') return null; - - const parsedUrl = parseGitHubUrl(entry.source.url); - return parsedUrl - ? `${parsedUrl.owner}/${parsedUrl.repo}`.toLowerCase() - : null; + return identity(entry.source.url); } return null; diff --git a/src/models/marketplace-manifest.ts b/src/models/marketplace-manifest.ts index 98cbe5bb..3965a808 100644 --- a/src/models/marketplace-manifest.ts +++ b/src/models/marketplace-manifest.ts @@ -1,34 +1,61 @@ import { z } from 'zod'; +import { parseRemotePluginSource } from '../utils/plugin-path.js'; + +/** External sources share the ordinary remote descriptor used by file installs. */ +const ExternalPathSchema = z.string().min(1).refine((path) => { + try { + parseRemotePluginSource(`https://github.com/example/plugins#subdir=${encodeURIComponent(path)}`); + return true; + } catch { return false; } +}, 'Plugin path must stay inside its repository'); + +function externalSource(url: string, ref?: string, path?: string) { + const metadata = new URLSearchParams(); + if (ref) metadata.set('ref', ref); + if (path) metadata.set('subdir', path); + return { + source: 'url' as const, + url: metadata.size ? `${url}${url.includes('#') ? '&' : '#'}${metadata}` : url, + }; +} + +const externalFields = { + ref: z.string().min(1).optional(), + // Raw SHA checkout is not supported by the file installer. Reject rather + // than dropping the pin and installing a different revision. + sha: z.never().optional(), +}; -/** - * URL source object for plugins hosted externally - * e.g. { source: "url", url: "https://github.com/figma/mcp-server-guide.git" } - */ export const UrlSourceSchema = z.object({ source: z.literal('url'), url: z.string().url(), -}); + path: ExternalPathSchema.optional(), + ...externalFields, +}).transform((value) => externalSource(value.url, value.ref, value.path)); export type UrlSource = z.infer; -/** - * GitHub source object for plugins hosted on GitHub - * e.g. { source: "github", repo: "WiseTechGlobal/mcp-ediprod" } - * Normalized to UrlSource at parse time. - */ export const GitHubSourceSchema = z.object({ source: z.literal('github'), - repo: z.string().min(1), -}).transform((val) => ({ - source: 'url' as const, - url: `https://github.com/${val.repo}`, -})); - -/** - * Plugin source: a relative path string, a URL source object, or a GitHub source object. - * GitHub sources are normalized to URL sources during parsing. - */ -export const PluginSourceRefSchema = z.union([z.string(), UrlSourceSchema, GitHubSourceSchema]); + repo: z.string().regex(/^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/), + path: ExternalPathSchema.optional(), + ...externalFields, +}).transform((value) => externalSource(`https://github.com/${value.repo}`, value.ref, value.path)); + +export const GitSubdirSourceSchema = z.object({ + source: z.literal('git-subdir'), + url: z.string().min(1), + path: ExternalPathSchema, + ...externalFields, +}).transform((value) => externalSource(value.url, value.ref, value.path)); + +export const PluginSourceRefSchema = z.union([ + z.string(), UrlSourceSchema, GitHubSourceSchema, GitSubdirSourceSchema, +]).refine((source) => { + if (typeof source === 'string') return true; + try { return parseRemotePluginSource(source.url) !== null; } + catch { return false; } +}, 'Invalid external Git source'); export type PluginSourceRef = z.infer; diff --git a/src/utils/marketplace-manifest-parser.ts b/src/utils/marketplace-manifest-parser.ts index e620c5cf..bd509afe 100644 --- a/src/utils/marketplace-manifest-parser.ts +++ b/src/utils/marketplace-manifest-parser.ts @@ -33,6 +33,7 @@ export type ParseResult = */ export async function parseMarketplaceManifest( marketplacePath: string, + selectedPlugin?: string, ): Promise { const manifestPath = MANIFEST_PATHS .map((path) => join(marketplacePath, path)) @@ -65,6 +66,20 @@ export async function parseMarketplaceManifest( }; } + // A selected remote declaration must fail closed without preventing + // best-effort listing or resolution of other valid catalogue entries. + if (selectedPlugin && isRecord(json) && Array.isArray(json.plugins)) { + for (const [index, entry] of json.plugins.entries()) { + if (!isRecord(entry) || entry.name !== selectedPlugin || !isRecord(entry.source)) continue; + if (!PluginSourceRefSchema.safeParse(entry.source).success) { + return { + success: false, + error: `plugins[${index}]: invalid external source (SHA pins are not supported by file installations)`, + }; + } + } + } + // Tier 1: strict validation const strictResult = MarketplaceManifestSchema.safeParse(json); if (strictResult.success) { @@ -72,7 +87,7 @@ export async function parseMarketplaceManifest( } // Tier 2: lenient parsing - return parseLeniently(json); + return parseLeniently(json, selectedPlugin); } /** @@ -94,6 +109,7 @@ export async function getEmbeddedMarketplaceFileArtifacts( const rootEntries = manifestResult.data.plugins.filter( (entry) => typeof entry.source === 'string' && + entry.source.length > 0 && resolve(pluginPath, entry.source) === pluginRoot, ); const entry = @@ -108,7 +124,7 @@ export async function getEmbeddedMarketplaceFileArtifacts( * Requires at minimum a `plugins` array in the JSON. * Validates each plugin entry individually, collecting warnings. */ -function parseLeniently(json: unknown): ParseResult { +function parseLeniently(json: unknown, selectedPlugin?: string): ParseResult { const lenientResult = MarketplaceManifestLenientSchema.safeParse(json); if (!lenientResult.success) { return { @@ -135,7 +151,13 @@ function parseLeniently(json: unknown): ParseResult { } // Best-effort extraction - const extracted = extractPluginEntry(entry, i, warnings); + const entryWarnings: string[] = []; + const extracted = extractPluginEntry(entry, i, entryWarnings); + // Unknown identities remain relevant: malformed entries must not be + // interpreted as authoritative removal of the selected plugin. + if (!selectedPlugin || !isRecord(entry) || typeof entry.name !== 'string' || !entry.name || entry.name === selectedPlugin) { + warnings.push(...entryWarnings); + } if (extracted) { validPlugins.push(extracted); } diff --git a/tests/unit/cli/skill-update-command.test.ts b/tests/unit/cli/skill-update-command.test.ts index 5936de4c..d5260361 100644 --- a/tests/unit/cli/skill-update-command.test.ts +++ b/tests/unit/cli/skill-update-command.test.ts @@ -409,20 +409,20 @@ describe('skill update command adapters', () => { process.env.ALLAGENTS_TEST_HOME = home; try { - const external = getPluginCachePath('acme', 'external-skills'); + const external = getPluginCachePath('acme', 'external-skills', 'review'); await mkdir(join(workspace, '.allagents'), { recursive: true }); await mkdir(join(home, '.allagents'), { recursive: true }); await mkdir(join(marketplace, '.claude-plugin'), { recursive: true }); await mkdir(join(marketplace, 'plugins/embedded/skills/inside'), { recursive: true, }); - await mkdir(join(external, 'skills/outside'), { recursive: true }); + await mkdir(join(external, 'plugins/finance/skills/outside'), { recursive: true }); await writeFile( join(marketplace, 'plugins/embedded/skills/inside/SKILL.md'), '---\nname: inside\ndescription: embedded\n---\n', ); await writeFile( - join(external, 'skills/outside/SKILL.md'), + join(external, 'plugins/finance/skills/outside/SKILL.md'), '---\nname: outside\ndescription: external\n---\n', ); await writeFile( @@ -431,6 +431,7 @@ describe('skill update command adapters', () => { name: 'catalog', description: 'test catalog', plugins: [ + { name: 'unsupported', description: 'Pinned sibling', source: { source: 'github', repo: 'acme/unsupported', sha: 'a'.repeat(40) } }, { name: 'embedded', description: 'embedded plugin', @@ -440,8 +441,10 @@ describe('skill update command adapters', () => { name: 'external', description: 'external plugin', source: { - source: 'url', + source: 'git-subdir', url: 'https://github.com/acme/external-skills', + path: 'plugins/finance', + ref: 'review', }, }, ], @@ -487,6 +490,9 @@ describe('skill update command adapters', () => { marketplace, ]); expect(externalEntry?.nodes[0]?.role).toBe('dependency'); + expect(externalEntry?.rootSubpath).toBe('plugins/finance'); + expect(externalEntry?.nodes[0]?.ref).toBe('review'); + expect(externalEntry?.skills.map((skill) => skill.name)).toEqual(['outside']); expect(embedded?.marketplace?.nodeId).toBe(marketplace); expect(externalEntry?.marketplace?.nodeId).toBe(marketplace); } finally { @@ -545,6 +551,56 @@ describe('skill update command adapters', () => { } }); + test('classifies a rejected selected declaration as failure rather than removal', async () => { + const root = await mkdtemp(join(tmpdir(), 'allagents-skill-update-removed-')); + try { + await mkdir(join(root, '.claude-plugin'), { recursive: true }); + await writeFile( + join(root, '.claude-plugin/marketplace.json'), + JSON.stringify({ + name: 'catalog', + description: 'test catalog', + plugins: [{ name: 'gone', description: 'Selected pinned plugin', source: { source: 'github', repo: 'acme/pinned', sha: 'a'.repeat(40) } }], + }), + ); + await initCheckout(root); + const sha = runGit(root, ['rev-parse', 'HEAD']); + const result = await inspectSkillUpdateUnit({ + id: root, + nodes: [ + { + id: root, + cachePath: root, + remoteUrl: root, + role: 'root', + currentSha: sha, + }, + ], + installations: [ + { + id: 'project:0', + scope: 'project', + configIndex: 0, + rawSource: 'gone@catalog', + effectiveSource: 'gone@catalog', + pluginName: 'gone', + rootNodeId: root, + rootSubpath: 'plugins/gone', + nodes: [], + skills: [{ name: 'gone', subpath: 'gone', enabled: true }], + marketplace: { nodeId: root, pluginName: 'gone' }, + }, + ], + }); + + expect(result.outcome).toBe('failed'); + expect(result.installations[0]?.outcome).toBe('failed'); + expect(result.installations[0]?.error).toContain('SHA pins'); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + test('does not fetch an obsolete external dependency after its marketplace entry is removed', async () => { const root = await mkdtemp(join(tmpdir(), 'allagents-skill-update-obsolete-')); try { diff --git a/tests/unit/core/github-skill-source-promotion.test.ts b/tests/unit/core/github-skill-source-promotion.test.ts index 51b243bb..faf3bfce 100644 --- a/tests/unit/core/github-skill-source-promotion.test.ts +++ b/tests/unit/core/github-skill-source-promotion.test.ts @@ -35,6 +35,20 @@ describe('canonicalizeGitHubPluginSource', () => { }); describe('upsertGitHubPluginSourceAllowlist', () => { + it('keeps skill selections from sibling monorepo plugins distinct', async () => { + const finance = 'https://github.com/example/catalogue/tree/main/plugins/finance/skills/modeling'; + const engineering = 'https://github.com/example/catalogue/tree/main/plugins/engineering/skills/review'; + const config: WorkspaceConfig = { + version: 2, repositories: [], clients: ['universal'], + plugins: [{ source: finance, skills: ['modeling'] }], + }; + expect((await upsertGitHubPluginSourceAllowlistInConfig(config, engineering, ['review'])).success).toBe(true); + expect(config.plugins).toEqual([ + { source: finance, skills: ['modeling'] }, + { source: engineering, skills: ['review'] }, + ]); + }); + it('updates the exact generic object-ref declaration while preserving its source and another ref', async () => { const tmpDir = await mkdtemp(join(tmpdir(), 'allagents-generic-allowlist-')); const source = 'ssh://alice@forgejo.example:2222/team/skills.git#subdir=selected'; diff --git a/tests/unit/core/marketplace-external-subdir.test.ts b/tests/unit/core/marketplace-external-subdir.test.ts new file mode 100644 index 00000000..029b3311 --- /dev/null +++ b/tests/unit/core/marketplace-external-subdir.test.ts @@ -0,0 +1,174 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { addMarketplace, resolvePluginSpec } from '../../../src/core/marketplace.js'; +import { addPluginDeclaration, resolveGitHubIdentity } from '../../../src/core/workspace-modify.js'; +import { getEmbeddedMarketplaceFileArtifacts, parseMarketplaceManifest } from '../../../src/utils/marketplace-manifest-parser.js'; +import { parseRemotePluginSource } from '../../../src/utils/plugin-path.js'; + +let root: string; +let previousHome: string | undefined; +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'allagents-external-subdir-')); + previousHome = process.env.ALLAGENTS_TEST_HOME; + process.env.ALLAGENTS_TEST_HOME = join(root, 'user'); + mkdirSync(join(root, '.claude-plugin')); +}); +afterEach(() => { + if (previousHome === undefined) delete process.env.ALLAGENTS_TEST_HOME; + else process.env.ALLAGENTS_TEST_HOME = previousHome; + rmSync(root, { recursive: true, force: true }); +}); +function manifest(source: unknown) { + writeFileSync(join(root, '.claude-plugin', 'marketplace.json'), JSON.stringify({ + name: 'legacy', description: 'Legacy catalogue', + plugins: [{ name: 'finance', description: 'Finance', source }], + })); +} +function checkout(path: string) { + mkdirSync(join(path, 'plugins', 'finance', 'skills', 'financial-modeling'), { recursive: true }); + writeFileSync(join(path, 'plugins', 'finance', 'skills', 'financial-modeling', 'SKILL.md'), '---\nname: financial-modeling\n---\nModel'); + mkdirSync(join(path, 'plugins', 'engineering', 'skills', 'review'), { recursive: true }); +} + +describe('external marketplace subdirectories', () => { + for (const source of [ + { source: 'github', repo: 'example/agent-plugins', path: 'plugins/finance', ref: 'review' }, + { source: 'git-subdir', url: 'https://github.com/example/agent-plugins.git', path: 'plugins/finance', ref: 'review' }, + { source: 'url', url: 'https://github.com/example/agent-plugins.git', path: 'plugins/finance', ref: 'review' }, + ]) { + it(`resolves only the selected ${source.source} plugin online`, async () => { + manifest(source); + const cache = join(root, 'checkout'); + checkout(cache); + const result = await resolvePluginSpec('finance@legacy', { + marketplacePathOverride: root, + fetchFn: async () => ({ success: true, action: 'fetched', cachePath: cache }), + }); + expect(result?.path).toBe(join(cache, 'plugins', 'finance')); + }); + } + + it('uses the requested ref cache and selected plugin when offline', async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', path: 'plugins/finance', ref: 'review' }); + const remote = parseRemotePluginSource('https://github.com/example/agent-plugins#ref=review&subdir=plugins%2Ffinance')!; + checkout(remote.cachePath); + const result = await resolvePluginSpec('finance@legacy', { marketplacePathOverride: root, offline: true }); + expect(result?.path).toBe(join(remote.cachePath, 'plugins', 'finance')); + }); + + it('does not substitute a default-branch cache for a missing requested ref', async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', path: 'plugins/finance', ref: 'missing' }); + checkout(parseRemotePluginSource('https://github.com/example/agent-plugins')!.cachePath); + checkout(root); + expect(await resolvePluginSpec('finance@legacy', { marketplacePathOverride: root, offline: true })).toBeNull(); + }); + + + it('does not substitute a same-name local plugin when external fetching fails', async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', path: 'plugins/finance' }); + checkout(root); + expect(await resolvePluginSpec('finance@legacy', { + marketplacePathOverride: root, + fetchFn: async () => ({ success: false, action: 'skipped', cachePath: '', error: 'offline' }), + })).toBeNull(); + }); + + it('matches direct and forwarded plugin identities without merging sibling plugins', async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', path: 'plugins/finance', ref: 'review' }); + expect((await addMarketplace(root)).success).toBe(true); + const forwarded = await resolveGitHubIdentity('finance@legacy'); + expect(forwarded).toBe('example/agent-plugins/plugins/finance'); + expect(await resolveGitHubIdentity('https://github.com/example/agent-plugins/tree/main/plugins/finance')).toBe(forwarded); + expect(await resolveGitHubIdentity('https://github.com/example/agent-plugins#subdir=plugins%2Ffinance&ref=other')).toBe(forwarded); + expect(await resolveGitHubIdentity('https://github.com/example/agent-plugins/plugins/engineering')).not.toBe(forwarded); + expect(await resolveGitHubIdentity('https://github.com/EXAMPLE/AGENT-PLUGINS/plugins/Finance')).not.toBe(forwarded); + expect(await resolveGitHubIdentity('https://github.com/EXAMPLE/AGENT-PLUGINS#subdir=.%2Fplugins%2Ffinance')).toBe(forwarded); + const workspace = join(root, 'workspace'); + mkdirSync(join(workspace, '.allagents'), { recursive: true }); + writeFileSync(join(workspace, '.allagents', 'workspace.yaml'), 'version: 2\nrepositories: []\nplugins:\n - finance@legacy\nclients: [universal]\n'); + expect((await addPluginDeclaration('https://github.com/example/agent-plugins/tree/main/plugins/finance', workspace)).success).toBe(false); + expect((await addPluginDeclaration('https://github.com/example/agent-plugins/tree/main/plugins/engineering', workspace)).success).toBe(true); + }); + + it('preserves root resolution for legacy URL sources', async () => { + manifest({ source: 'url', url: 'https://github.com/example/finance.git' }); + const cache = join(root, 'legacy-checkout'); + mkdirSync(cache); + expect((await resolvePluginSpec('finance@legacy', { + marketplacePathOverride: root, + fetchFn: async () => ({ success: true, action: 'fetched', cachePath: cache }), + }))?.path).toBe(cache); + }); + + it('rejects a symlink that points outside the external checkout', async () => { + manifest({ source: 'git-subdir', url: 'example/agent-plugins', path: 'plugins/finance' }); + const cache = join(root, 'checkout'); + mkdirSync(join(cache, 'plugins'), { recursive: true }); + mkdirSync(join(root, 'outside')); + symlinkSync(join(root, 'outside'), join(cache, 'plugins', 'finance')); + await expect(resolvePluginSpec('finance@legacy', { + marketplacePathOverride: root, + fetchFn: async () => ({ success: true, action: 'fetched', cachePath: cache }), + })).rejects.toThrow('outside the repository'); + }); + + for (const path of ['../finance', '/finance', 'plugins/../../finance', 'C:/finance', 'plugins\\finance']) { + it(`rejects unsafe external path ${path}`, async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', path }); + expect((await parseMarketplaceManifest(root, 'finance')).success).toBe(false); + }); + } + + it('lists a mixed catalogue and resolves valid plugins without bypassing a rejected sibling', async () => { + checkout(root); + mkdirSync(join(root, 'plugins', 'unsupported'), { recursive: true }); + writeFileSync(join(root, '.claude-plugin', 'marketplace.json'), JSON.stringify({ + name: 'legacy', description: 'Mixed catalogue', plugins: [ + { name: 'finance', description: 'Finance', source: './plugins/finance' }, + { name: 'unsupported', description: 'Pinned plugin', source: { source: 'github', repo: 'example/catalogue', sha: 'a'.repeat(40) } }, + ], + })); + const listed = await parseMarketplaceManifest(root); + expect(listed.success).toBe(true); + if (listed.success) { + expect(listed.data.plugins.map((entry) => entry.name)).toEqual(['finance', 'unsupported']); + expect(listed.warnings.length).toBeGreaterThan(0); + } + const selected = await parseMarketplaceManifest(root, 'finance'); + expect(selected.success).toBe(true); + if (selected.success) expect(selected.warnings).toEqual([]); + expect((await resolvePluginSpec('finance@legacy', { marketplacePathOverride: root, offline: true }))?.path).toBe(join(root, 'plugins', 'finance')); + expect(await resolvePluginSpec('unsupported@legacy', { marketplacePathOverride: root, offline: true })).toBeNull(); + }); + + it('retains unknown empty-name warnings for a selected missing plugin', async () => { + writeFileSync(join(root, '.claude-plugin', 'marketplace.json'), JSON.stringify({ + name: 'legacy', description: 'Malformed catalogue', plugins: [ + { name: '', description: 'Unknown plugin', source: './plugins/finance' }, + ], + })); + const selected = await parseMarketplaceManifest(root, 'finance'); + expect(selected.success).toBe(true); + if (selected.success) expect(selected.warnings.some((warning) => warning.includes('missing "name"'))).toBe(true); + }); + + it('does not infer repository-root artifacts from an unsupported remote entry', async () => { + writeFileSync(join(root, '.claude-plugin', 'marketplace.json'), JSON.stringify({ + name: 'legacy', description: 'Mixed catalogue', plugins: [ + { name: 'unsupported', description: 'Unsupported source', source: { source: 'npm', package: 'example' }, strict: false, skills: ['./skills'] }, + ], + })); + expect(await getEmbeddedMarketplaceFileArtifacts(root, 'unsupported')).toBeUndefined(); + }); + + it('rejects an unsupported SHA instead of silently dropping the pin', async () => { + manifest({ source: 'github', repo: 'example/agent-plugins', sha: 'a'.repeat(40) }); + const result = await parseMarketplaceManifest(root, 'finance'); + expect(result.success).toBe(false); + if (!result.success) expect(result.error).toContain('SHA pins'); + checkout(root); + expect(await resolvePluginSpec('finance@legacy', { marketplacePathOverride: root, offline: true })).toBeNull(); + }); +}); diff --git a/tests/unit/plugins/pr-interactive-review.test.ts b/tests/unit/plugins/pr-interactive-review.test.ts deleted file mode 100644 index 990e7526..00000000 --- a/tests/unit/plugins/pr-interactive-review.test.ts +++ /dev/null @@ -1,820 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; -import { - mkdir, - mkdtemp, - readFile, - readdir, - rm, - writeFile, -} from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { - MAX_REQUEST_BYTES, - type StoredReview, - buildBusinessPrimer, - buildGitHubLineLink, - createReviewServer, - loadStoredReview, - prepareReview, - readBoundedRequestBody, - renderReviewPage, -} from '../../../plugins/engineering/skills/pr-interactive-review/scripts/review-site.ts'; - -const temporaryPaths: string[] = []; - -async function temporaryDirectory(): Promise { - const path = await mkdtemp(join(tmpdir(), 'allagents-pr-review-')); - temporaryPaths.push(path); - return path; -} - -async function fixture(withScenario = true) { - const root = await temporaryDirectory(); - const repository = join(root, 'repository'); - const state = join(root, 'state'); - await mkdir(repository); - const init = Bun.spawnSync(['git', '-C', repository, 'init', '--quiet']); - expect(init.exitCode).toBe(0); - const remote = Bun.spawnSync([ - 'git', - '-C', - repository, - 'remote', - 'add', - 'origin', - 'https://github.com/example-org/sample-service.git', - ]); - expect(remote.exitCode).toBe(0); - await writeFile( - join(repository, 'requirements.md'), - 'Who configures: Operations managers\nOperational problem: Manual approvals delay routine changes\nIntended outcome: Operators complete changes in one workflow\nWhy it matters: Delays postpone service changes\nSuccess criteria: A change completes and records its result\nScope: Change configuration\nNon-goals: Redesigning access roles\n', - ); - const artifact = join(root, 'review.json'); - await writeFile( - artifact, - JSON.stringify({ - status: 'complete', - verdict: 'Ready with fixes', - title: 'Example change', - intent: 'Review a configuration workflow.', - scope: { head_sha: '0123456789abcdef0123456789abcdef01234567' }, - findings: [ - { - '#': 1, - title: 'Escape untrusted label', - severity: 'P1', - file: 'src/config.ts', - line: 18, - end_line: 20, - confidence: 90, - owner: 'downstream-resolver', - suggested_fix: 'Escape the label before rendering it.', - first_evidence: 'The value enters the template without encoding.', - evidence: ['The changed template uses the raw value.'], - reviewers: ['security', 'correctness'], - }, - ], - }), - ); - const scenarios = join(root, 'interactive-scenarios.json'); - if (withScenario) { - await writeFile( - scenarios, - JSON.stringify({ - '#1': { - what_actually_happens: - 'When an operator submits a label containing markup, the page renders the markup and the browser executes it.', - what_actually_happens_evidence: { - triggering_setup: - 'tests/config-label.test.ts saves a label containing markup.', - observable_outcome: - 'tests/config-label.test.ts observes the label rendered as markup.', - }, - expected_suggested: - 'When an operator submits that label, the page displays the characters as text after encoding the value at the rendering boundary.', - }, - }), - ); - } - const prepared = await prepareReview({ - reviewJsonPath: artifact, - pr: '123', - repoPath: repository, - dataDir: state, - requirementsPath: 'requirements.md', - ...(withScenario ? { scenariosPath: scenarios } : {}), - now: new Date('2026-01-02T03:04:05.000Z'), - }); - return { ...prepared, artifact, repository, scenarios, state }; -} - -beforeEach(() => { - temporaryPaths.length = 0; -}); - -afterEach(async () => { - await Promise.all( - temporaryPaths.map((path) => rm(path, { recursive: true, force: true })), - ); -}); - -describe('pr-interactive-review', () => { - it('renders an evidence-based business primer and labels missing evidence', () => { - const primer = buildBusinessPrimer( - 'Who configures: Operations managers\nSuccess criteria: A change completes', - 'Review the changed flow.', - ); - expect(primer.whoConfigures.value).toBe('Operations managers'); - expect(primer.successCriteria.value).toBe('A change completes'); - expect(primer.nonGoals.value).toBeNull(); - expect(primer.nonGoals.evidenceGap).toContain('non-goals'); - expect(primer.operationalProblem.evidenceGap).toContain( - 'Review intent: Review the changed flow.', - ); - }); - - it('pins GitHub links to the reviewed commit and exact line range', () => { - expect( - buildGitHubLineLink( - 'example-org/sample-service', - '0123456789abcdef0123456789abcdef01234567', - 'src/a file.ts', - 18, - 20, - ), - ).toBe( - 'https://github.com/example-org/sample-service/blob/0123456789abcdef0123456789abcdef01234567/src/a%20file.ts#L18-L20', - ); - expect( - buildGitHubLineLink( - null, - '0123456789abcdef0123456789abcdef01234567', - 'src/a.ts', - 1, - 1, - ), - ).toBeNull(); - expect(() => - buildGitHubLineLink( - 'example-org/sample-service', - '0123456789abcdef0123456789abcdef01234567', - '../secrets', - 1, - 1, - ), - ).toThrow('relative repository path'); - }); - - it('cancels an oversized chunked request body before accepting it', async () => { - let cancelled = false; - const stream = new ReadableStream({ - pull(controller) { - controller.enqueue(new Uint8Array(MAX_REQUEST_BYTES + 1)); - }, - cancel() { - cancelled = true; - }, - }); - const request = new Request('http://127.0.0.1/api/comments', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: stream, - }); - expect(request.headers.get('content-length')).toBeNull(); - await expect(readBoundedRequestBody(request)).rejects.toThrow( - `Request body exceeds ${MAX_REQUEST_BYTES} bytes`, - ); - expect(cancelled).toBe(true); - }); - - it('labels missing concrete scenario evidence instead of inventing a claim', async () => { - const prepared = await fixture(false); - const scenario = prepared.review.findings[0]?.scenario; - expect(scenario?.actualHappens).toBeNull(); - expect(scenario?.expectedSuggested).toBeNull(); - expect(scenario?.actualEvidenceGap).toContain('triggering setup/reachability'); - expect(scenario?.expectedEvidenceGap).toContain( - 'expected behavior and correction', - ); - }); - it('downgrades sidecar scenarios without reachability and outcome evidence', async () => { - const prepared = await fixture(); - await writeFile( - prepared.scenarios, - JSON.stringify({ - '#1': { - what_actually_happens: - 'A polished but unsupported configuration path fails.', - expected_suggested: 'The configuration path succeeds.', - }, - }), - ); - const refreshed = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }); - expect(refreshed.review.findings[0]?.scenario).toEqual( - expect.objectContaining({ - actualHappens: null, - actualEvidenceGap: expect.stringContaining('asserted scenario lacks'), - expectedSuggested: 'The configuration path succeeds.', - }), - ); - }); - it('labels an evidence-only scenario as a gap without an actual claim', async () => { - const prepared = await fixture(); - await writeFile( - prepared.scenarios, - JSON.stringify({ - '#1': { - what_actually_happens_evidence: { - triggering_setup: 'A reachable test fixture.', - observable_outcome: 'An observed test result.', - }, - }, - }), - ); - const refreshed = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }); - expect(refreshed.review.findings[0]?.scenario).toEqual( - expect.objectContaining({ - actualHappens: null, - actualEvidenceGap: expect.stringContaining('specific triggering'), - }), - ); - }); - - it('merges scenario sidecars by stable finding ID and rejects invalid entries', async () => { - const prepared = await fixture(); - await writeFile( - prepared.scenarios, - JSON.stringify({ - '#99': { - what_actually_happens: 'A triggering action fails.', - expected_suggested: 'The action succeeds.', - }, - }), - ); - await expect( - prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }), - ).rejects.toThrow('unknown finding #99'); - await writeFile( - prepared.scenarios, - JSON.stringify({ '#1': { what_actually_happens: 7 } }), - ); - await expect( - prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }), - ).rejects.toThrow('what_actually_happens must be a string'); - }); - it('partitions review workspaces outside the repository and creates atomically readable stores', async () => { - const prepared = await fixture(); - expect(prepared.workspace).toContain( - 'state/github.com-example-org-sample-service/pr-123', - ); - expect(prepared.review.primer.scope.value).toBe('Change configuration'); - expect(prepared.review.findings[0]?.sourceLink).toContain( - '/blob/0123456789abcdef0123456789abcdef01234567/src/config.ts#L18-L20', - ); - expect(prepared.review.findings[0]?.scenario).toEqual({ - actualHappens: - 'When an operator submits a label containing markup, the page renders the markup and the browser executes it.', - expectedSuggested: - 'When an operator submits that label, the page displays the characters as text after encoding the value at the rendering boundary.', - actualTriggerEvidence: - 'tests/config-label.test.ts saves a label containing markup.', - actualOutcomeEvidence: - 'tests/config-label.test.ts observes the label rendered as markup.', - actualEvidenceGap: null, - expectedEvidenceGap: null, - }); - expect( - JSON.parse( - await readFile(join(prepared.workspace, 'comments.json'), 'utf8'), - ), - ).toEqual({ version: 1, comments: [] }); - expect( - (await readdir(prepared.workspace)).some((name) => name.endsWith('.tmp')), - ).toBe(false); - }); - it('stores a bounded presentation sidecar for reusable editorial context', async () => { - const prepared = await fixture(); - const presentation = join(prepared.repository, 'interactive-presentation.json'); - await writeFile( - presentation, - JSON.stringify({ - eyebrow: 'Runtime seam review', - headline: 'One queue connects two services.', - summary: 'Review the handoff before implementation detail.', - context_cards: [ - { - label: 'System boundary', - title: 'Dispatcher to Runner', - body: 'The Dispatcher starts work and the Runner owns execution.', - tone: 'problem', - }, - ], - mental_model: { - title: 'The execution path', - summary: 'One shared contract crosses the Temporal boundary.', - steps: [ - { - label: '01 / Dispatch', - title: 'Start workflow', - body: 'Create the workflow with the configured task queue.', - }, - ], - }, - }), - ); - const refreshed = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - presentationPath: presentation, - }); - expect(refreshed.review.presentation).toEqual({ - eyebrow: 'Runtime seam review', - headline: 'One queue connects two services.', - summary: 'Review the handoff before implementation detail.', - contextCards: [ - { - label: 'System boundary', - title: 'Dispatcher to Runner', - body: 'The Dispatcher starts work and the Runner owns execution.', - tone: 'problem', - }, - ], - mentalModel: { - title: 'The execution path', - summary: 'One shared contract crosses the Temporal boundary.', - steps: [ - { - label: '01 / Dispatch', - title: 'Start workflow', - body: 'Create the workflow with the configured task queue.', - }, - ], - }, - }); - const preserved = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }); - expect(preserved.review.presentation).toEqual(refreshed.review.presentation); - await writeFile( - presentation, - JSON.stringify({ - eyebrow: 'Review', - headline: 'Too many cards', - summary: 'Reject unbounded presentation content.', - context_cards: Array.from({ length: 7 }, (_, index) => ({ - label: `Card ${index}`, - title: 'Title', - body: 'Body', - })), - }), - ); - await expect( - prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - presentationPath: presentation, - }), - ).rejects.toThrow('array of at most 6 objects'); - await writeFile(presentation, 'null'); - await expect( - prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - presentationPath: presentation, - }), - ).rejects.toThrow('presentation must be an object'); - const changedArtifact = JSON.parse( - await readFile(prepared.artifact, 'utf8'), - ) as { scope: { head_sha: string } }; - changedArtifact.scope.head_sha = - 'fedcba9876543210fedcba9876543210fedcba98'; - await writeFile(prepared.artifact, JSON.stringify(changedArtifact)); - const changedCommit = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - }); - expect(changedCommit.review.presentation).toBeNull(); - }); - it('rejects invalid persisted presentation metadata before browser rendering', async () => { - const prepared = await fixture(); - const reviewPath = join(prepared.workspace, 'review.json'); - const stored = JSON.parse(await readFile(reviewPath, 'utf8')) as Record< - string, - unknown - >; - stored.presentation = { contextCards: {} }; - await writeFile(reviewPath, JSON.stringify(stored)); - await expect(loadStoredReview(prepared.workspace)).rejects.toThrow( - 'presentation.context_cards must be an array', - ); - }); - - - - it('escapes untrusted page values and keeps business context before findings', async () => { - const prepared = await fixture(); - const malicious: StoredReview = { - ...prepared.review, - title: '', - }; - const page = renderReviewPage(malicious); - expect(page).not.toContain(''); - expect(page.indexOf('id="business-context"')).toBeLessThan( - page.indexOf('id="findings"'), - ); - expect(page).not.toContain('Assistant reply'); - }); - it('renders a navigable review shell with heavy detail collapsed by default', async () => { - const prepared = await fixture(); - const page = renderReviewPage(prepared.review); - expect(page).toContain('class="review-shell"'); - expect(page).toContain('id="review-nav"'); - expect(page).toContain('class="hero-metrics"'); - expect(page).toContain("disclosure('Supplied requirements'"); - expect(page).toContain("disclosure('Referenced evidence"); - expect(page).toContain("disclosure('Focused code context'"); - expect(page).toContain('@media (max-width: 900px)'); - expect(page).toContain( - '', - ); - expect(page).toContain('renderFindingNav(visible);'); - const renderFindings = page.indexOf('function renderFindings()'); - expect( - page.indexOf('state.observer.disconnect()', renderFindings), - ).toBeLessThan(page.indexOf('root.replaceChildren()', renderFindings)); - }); - - - it('contains long prose fields and reviewed lines without breaking narrow cards', async () => { - const prepared = await fixture(); - const longText = 'x'.repeat(4096); - const review: StoredReview = { - ...prepared.review, - title: longText, - findings: prepared.review.findings.map((finding) => ({ - ...finding, - title: longText, - scenario: { ...finding.scenario, actualHappens: longText }, - excerpts: { - before: null, - after: { startLine: 1, endLine: 1, content: longText }, - }, - })), - }; - const page = renderReviewPage(review); - expect(review.findings[0]?.excerpts.after?.content).toHaveLength(4096); - expect(page).toContain('#findings { display: grid; min-width: 0;'); - expect(page).toContain('.finding { min-width: 0;'); - expect(page).toContain('aria-label="Finding status filters"'); - expect(page).toContain("withdrawn: 'Withdrawn findings'"); - expect(page).toContain("disclosure('Lifecycle history'"); - expect(page).toContain( - '.context-grid, .model-grid, .scenario-grid, .excerpt-grid { grid-template-columns: 1fr; }', - ); - expect(page).toContain('h1, h2, h3, h4, p, li, label, strong, .meta, .gap, .rail-link-label { overflow-wrap: anywhere; word-break: break-word; }'); - expect(page).toContain( - 'pre { max-width: 100%; min-width: 0; margin: 0; overflow-x: auto; white-space: pre;', - ); - }); - - it('migrates version-1 reviews to lifecycle records without asserting unproven scenarios', async () => { - const prepared = await fixture(); - const legacy = JSON.parse( - await readFile(join(prepared.workspace, 'review.json'), 'utf8'), - ) as Record; - legacy.version = 1; - delete legacy.originalVerdict; - const finding = (legacy.findings as Array>)[0]; - delete finding.status; - delete finding.original; - delete finding.revisions; - delete finding.scenario; - finding.scenario = { - actualHappens: 'An unproven path currently fails.', - expectedSuggested: 'The path succeeds.', - actualEvidenceGap: null, - expectedEvidenceGap: null, - }; - legacy.verdict = 'Ready with fixes'; - await writeFile(join(prepared.workspace, 'review.json'), JSON.stringify(legacy)); - const migrated = await loadStoredReview(prepared.workspace); - expect(migrated.version).toBe(2); - expect(migrated.originalVerdict).toBe('Ready with fixes'); - expect(migrated.verdict).toBe('1 active finding (P1: 1)'); - expect(migrated.findings[0]).toEqual( - expect.objectContaining({ status: 'active', revisions: [] }), - ); - expect(migrated.findings[0]?.scenario.actualHappens).toBeNull(); - expect(migrated.findings[0]?.scenario.actualEvidenceGap).toContain( - 'asserted scenario lacks', - ); - expect(migrated.findings[0]?.original.scenario.actualHappens).toBe( - 'An unproven path currently fails.', - ); - expect( - JSON.parse(await readFile(join(prepared.workspace, 'review.json'), 'utf8')), - ).toEqual(migrated); - }); - it('validates routes, atomically saves comments, and renders assistant replies', async () => { - const prepared = await fixture(); - const server = createReviewServer(prepared.workspace, '127.0.0.1', 0); - try { - const base = `http://127.0.0.1:${server.port}`; - const page = await fetch(`${base}/`); - expect(page.headers.get('content-security-policy')).toContain( - "default-src 'self'", - ); - expect(page.status).toBe(200); - expect((await page.text()).indexOf('Business context')).toBeGreaterThan( - -1, - ); - expect( - ( - await fetch(`${base}/api/comments`, { - method: 'POST', - headers: { 'content-type': 'text/plain' }, - body: '{"body":"not JSON content"}', - }) - ).status, - ).toBe(415); - expect( - ( - await fetch(`${base}/api/comments`, { - method: 'POST', - headers: { - 'content-type': 'application/json', - origin: 'https://untrusted.example', - }, - body: JSON.stringify({ body: 'cross-origin write' }), - }) - ).status, - ).toBe(403); - expect( - ( - await fetch(`${base}/api/comments`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ findingId: '#99', body: 'bad' }), - }) - ).status, - ).toBe(400); - const create = await fetch(`${base}/api/comments`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - findingId: '#1', - author: 'Reviewer', - body: '', - }), - }); - expect(create.status).toBe(201); - const created = (await create.json()) as { comment: { id: string } }; - const parallelWrites = await Promise.all( - Array.from({ length: 8 }, (_, index) => - fetch(`${base}/api/comments`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ body: `comment ${index}` }), - }), - ), - ); - expect( - ( - await fetch(`${base}/api/comments/${created.comment.id}/replies`, { - method: 'POST', - headers: { - 'content-type': 'application/json', - origin: 'https://untrusted.example', - }, - body: JSON.stringify({ - role: 'assistant', - body: 'cross-origin reply', - }), - }) - ).status, - ).toBe(403); - expect(parallelWrites.every((response) => response.status === 201)).toBe( - true, - ); - expect( - (await fetch(`${base}/api/comments?status=unanswered`)).status, - ).toBe(200); - const reply = await fetch( - `${base}/api/comments/${created.comment.id}/replies`, - { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - role: 'assistant', - author: 'Assistant', - body: 'Use the documented escape helper.', - }), - }, - ); - expect(reply.status).toBe(201); - const all = (await (await fetch(`${base}/api/comments`)).json()) as { - comments: Array<{ - body: string; - replies: Array<{ role: string; body: string }>; - }>; - }; - expect(all.comments).toHaveLength(9); - expect(all.comments[0]).toEqual( - expect.objectContaining({ - body: '', - replies: [ - expect.objectContaining({ - role: 'assistant', - body: 'Use the documented escape helper.', - }), - ], - }), - ); - const unanswered = (await ( - await fetch(`${base}/api/comments?status=unanswered`) - ).json()) as { comments: unknown[] }; - expect(unanswered.comments).toHaveLength(8); - expect( - JSON.parse( - await readFile(join(prepared.workspace, 'comments.json'), 'utf8'), - ).comments, - ).toHaveLength(9); - expect( - (await readdir(prepared.workspace)).some((name) => - name.endsWith('.tmp'), - ), - ).toBe(false); - } finally { - server.stop(true); - } - }); - it('reclassifies and withdraws findings through comment-linked atomic revisions', async () => { - const prepared = await fixture(); - const server = createReviewServer(prepared.workspace, '127.0.0.1', 0); - try { - const base = `http://127.0.0.1:${server.port}`; - const createComment = async (body: string) => { - const response = await fetch(`${base}/api/comments`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ findingId: '#1', body }), - }); - expect(response.status).toBe(201); - return (await response.json()) as { comment: { id: string } }; - }; - const first = await createComment( - 'The repository has no saved configuration that reaches this path.', - ); - const route = `${base}/api/findings/${encodeURIComponent('#1')}/revisions`; - expect( - ( - await fetch(route, { - method: 'POST', - headers: { 'content-type': 'text/plain' }, - body: '{}', - }) - ).status, - ).toBe(415); - expect( - ( - await fetch(route, { - method: 'POST', - headers: { - 'content-type': 'application/json', - origin: 'https://untrusted.example', - }, - body: JSON.stringify({}), - }) - ).status, - ).toBe(403); - expect( - ( - await fetch(route, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - commentId: '00000000-0000-0000-0000-000000000000', - status: 'withdrawn', - rationale: 'Not linked.', - }), - }) - ).status, - ).toBe(400); - const question = await fetch(route, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - commentId: first.comment.id, - status: 'question', - rationale: 'Reachability remains unproven after reviewing the correction.', - severity: 'P3', - scenario: { - actualHappens: null, - actualTriggerEvidence: null, - actualOutcomeEvidence: null, - }, - }), - }); - expect(question.status).toBe(201); - const questioned = (await (await fetch(`${base}/api/review`)).json()) as StoredReview; - expect(questioned.verdict).toBe('No active findings; 1 open question'); - expect(questioned.findings[0]).toEqual( - expect.objectContaining({ status: 'question', severity: 'P3' }), - ); - const second = await createComment( - 'The fixture and production registry confirm the path is unreachable.', - ); - const withdrawn = await fetch(route, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ - commentId: second.comment.id, - status: 'withdrawn', - rationale: 'The correction disproves the affected configuration path.', - }), - }); - expect(withdrawn.status).toBe(201); - const revised = (await (await fetch(`${base}/api/review`)).json()) as StoredReview; - expect(revised.verdict).toBe('No active findings'); - expect(revised.findings[0]).toEqual( - expect.objectContaining({ - status: 'withdrawn', - original: expect.objectContaining({ - title: 'Escape untrusted label', - severity: 'P1', - }), - revisions: [ - expect.objectContaining({ status: 'question', commentId: first.comment.id }), - expect.objectContaining({ status: 'withdrawn', commentId: second.comment.id }), - ], - }), - ); - const comments = (await (await fetch(`${base}/api/comments`)).json()) as { - comments: Array<{ body: string }>; - }; - expect(comments.comments.map((comment) => comment.body)).toEqual([ - 'The repository has no saved configuration that reaches this path.', - 'The fixture and production registry confirm the path is unreachable.', - ]); - expect( - (await readdir(prepared.workspace)).some((name) => name.endsWith('.tmp')), - ).toBe(false); - const reprepared = await prepareReview({ - reviewJsonPath: prepared.artifact, - pr: '123', - repoPath: prepared.repository, - dataDir: prepared.state, - scenariosPath: prepared.scenarios, - }); - expect(reprepared.review.findings[0]).toEqual( - expect.objectContaining({ - status: 'withdrawn', - severity: 'P3', - revisions: expect.arrayContaining([ - expect.objectContaining({ status: 'withdrawn' }), - ]), - }), - ); - } finally { - server.stop(true); - } - }); -}); From 9da921ebf0b09adfce3dca0bc48171006f468932 Mon Sep 17 00:00:00 2001 From: christso <3115311+christso@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:06:14 +1100 Subject: [PATCH 2/2] refactor(plugin): retain CLI guidance in the AllAgents repository --- .claude-plugin/marketplace.json | 6 +-- .github/plugin/marketplace.json | 6 +-- AGENTS.md | 2 +- README.md | 2 +- docs/src/content/docs/docs/guides/plugins.mdx | 8 +-- plugins/allagents/.claude-plugin/plugin.json | 10 ++++ plugins/allagents/skills/allagents/SKILL.md | 51 +++++++++++++++++++ 7 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 plugins/allagents/.claude-plugin/plugin.json create mode 100644 plugins/allagents/skills/allagents/SKILL.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 6913452c..d42c94ae 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,11 +10,7 @@ { "name": "allagents", "description": "End-user guidance for managing AllAgents through its CLI", - "source": { - "source": "git-subdir", - "url": "https://github.com/allagentsdev/agent-plugins.git", - "path": "plugins/allagents" - } + "source": "./plugins/allagents" }, { "name": "deepwiki", diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 9052571c..07feec18 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -10,11 +10,7 @@ { "name": "allagents", "description": "End-user guidance for managing AllAgents through its CLI", - "source": { - "source": "github", - "repo": "allagentsdev/agent-plugins", - "path": "plugins/allagents" - } + "source": "./plugins/allagents" }, { "name": "deepwiki", diff --git a/AGENTS.md b/AGENTS.md index 225e02b3..d1936835 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,7 @@ AllAgents is a CLI for managing AI coding-assistant plugins, workspaces, and cli - `tests/e2e/`: built CLI and filesystem workflows. - `docs/src/content/docs/`: public documentation. - `examples/workspaces/`: workspace templates that compose plugin capabilities. -- First-party plugin content lives in `allagentsdev/agent-plugins`; this repository retains forwarding marketplace manifests for existing installations. +- `plugins/allagents/`: guidance specific to this CLI. Reusable plugins live in `allagentsdev/agent-plugins`; forwarding marketplace entries preserve existing installations. - `.agents/`: task-specific repository guidance reached through the routing table below. ## Routing diff --git a/README.md b/README.md index d7a90a53..ad1e6ee0 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Install the end-user AllAgents skill when you want your coding agent to operate the CLI using its current machine-readable command contracts: ```bash -npx allagents plugin install allagents@allagentsdev/agent-plugins --scope user +npx allagents plugin install allagents@allagentsdev/allagents --scope user ``` The Finance plugin provides a `financial-modeling` skill for forecasts, budgets, diff --git a/docs/src/content/docs/docs/guides/plugins.mdx b/docs/src/content/docs/docs/guides/plugins.mdx index 36488976..7f9aa9eb 100644 --- a/docs/src/content/docs/docs/guides/plugins.mdx +++ b/docs/src/content/docs/docs/guides/plugins.mdx @@ -263,7 +263,9 @@ Plugins use the `plugin@marketplace` format: ## First-party Plugin Catalogue -Reusable AllAgents plugins live in [allagentsdev/agent-plugins](https://github.com/allagentsdev/agent-plugins). +Reusable plugins live in [allagentsdev/agent-plugins](https://github.com/allagentsdev/agent-plugins). +The CLI-specific `allagents` guidance plugin remains in this repository under +`plugins/allagents`, installed as `allagents@allagentsdev/allagents`. Workspace templates remain in [allagentsdev/allagents/examples/workspaces](https://github.com/allagentsdev/allagents/tree/main/examples/workspaces) and select capabilities from that catalogue: @@ -280,8 +282,8 @@ use their own marketplace source formats; update those clients if their installed versions do not support external plugin subdirectories. Direct old paths such as `allagentsdev/allagents/plugins/deepwiki` must be -changed to `deepwiki@allagentsdev/agent-plugins`. Plugin directories are no -longer stored in the CLI repository. Git refs that predate the move retain +changed to `deepwiki@allagentsdev/agent-plugins`. The four reusable plugin +directories move out of the CLI repository. Git refs that predate the move retain their original contents. External marketplace entries accept `github`, `url` and `git-subdir` source diff --git a/plugins/allagents/.claude-plugin/plugin.json b/plugins/allagents/.claude-plugin/plugin.json new file mode 100644 index 00000000..2c47f39e --- /dev/null +++ b/plugins/allagents/.claude-plugin/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "allagents", + "description": "End-user guidance for managing workspaces, plugins, skills, profiles, clients, and MCP servers with the AllAgents CLI.", + "author": { + "name": "AllAgents" + }, + "version": "1.0.0", + "category": "development", + "homepage": "https://github.com/allagentsdev/allagents/tree/main/plugins/allagents" +} diff --git a/plugins/allagents/skills/allagents/SKILL.md b/plugins/allagents/skills/allagents/SKILL.md new file mode 100644 index 00000000..6d40e09d --- /dev/null +++ b/plugins/allagents/skills/allagents/SKILL.md @@ -0,0 +1,51 @@ +--- +name: allagents +description: Manage AllAgents workspaces, plugins, skills, client targets, global profiles, and MCP servers through the allagents CLI. Use when a user asks to initialize, inspect, update, or configure AllAgents; install or remove plugins or skills; manage profiles; or add, inspect, authenticate, update, or remove MCP servers. Resolve current command syntax from machine-readable help instead of relying on remembered flags. +--- + +# AllAgents CLI + +The installed AllAgents CLI owns the current command surface and operational contract. This skill only resolves the runner, loads that contract, and applies it. Do not maintain a second command reference here. + +## Resolve the CLI runner + +Choose one runner at the start of the task and use it for discovery, execution, and verification: + +1. If `allagents` is on `PATH`, use `allagents`. +2. Otherwise, if `npx` is available, use `npx --yes allagents`. +3. If neither is available, stop and ask the user to install AllAgents or Node.js with npm. + +Examples and returned `help_command` values use the canonical `allagents` token. When `npx --yes allagents` is the selected runner, replace only that leading token before executing every discovery, execution, and verification command. Do not mix runners within one operation. + +## Load the current contract + +1. Confirm the selected runner with `allagents --version`. +2. Discover the concise top-level index with `allagents --help --json`. +3. Choose an entry, rewrite its leading runner token when required, and execute its `help_command`; group responses reveal only their immediate children. +4. Continue through nested groups until a leaf command returns its full contract. +5. Follow the leaf's `when_to_use`, positionals, options, examples, interaction requirement, expected output, output schema, and JSON field allowlist. + +The structured response is authoritative. Do not rely on remembered flags, copied examples, aliases, destination behavior, or mutation semantics. If the installed CLI does not advertise an operation, do not invent it. + +## Execute through the discovered surface + +- Run `allagents` without arguments in an interactive terminal only when the user wants to browse and choose in the TUI. +- Use the discovered direct command when the operation is already known. +- For automation, use `--json` plus only the explicit selectors and non-interactive options advertised by the leaf help. +- Use `--json=` only with fields in the leaf's JSON allowlist. Use `--jq` only with JSON output. +- Prefer an advertised CLI command over hand-editing an AllAgents declaration or generated client file. +- Preserve the exact scope, destination, profile, clients, and other ownership selectors requested by the user. If the request is insufficient and the CLI requires a choice, ask rather than guessing. +- Never echo credentials. Supply sensitive values only through mechanisms advertised by the current leaf help. + +Treat the process exit code and structured result as authoritative. After a mutation, rediscover and run the relevant read-only list, get, or status command against the same selectors. Do not claim success from a config write alone when the CLI reports a partial update or failed client reconciliation. + +## Recover from errors + +- Read the structured error before retrying. +- If command syntax is rejected, reload root, group, and leaf help; do not fall back to stale syntax. +- If a mutation may have partially applied, inspect current state before any retry. +- Follow only recovery operations exposed by the installed CLI. + +## Completion + +Report the selected runner, command path, explicit selectors, structured result, and verification command. Completion requires the declared state and corresponding live or generated state to agree, or an explicit partial-update result with the CLI-advertised recovery path.