diff --git a/.gitignore b/.gitignore index 8c4bf47..69db55d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ bin/ tmp/ -prism +/prism CLAUDE.md .claude/ diff --git a/README.md b/README.md index c2e4b7d..7002294 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,9 @@ prism prompt --template my.tmpl # Custom template # Debug prism fetch --format json # Raw PR data prism fetch --format text # Raw PR data as text + +# Provider selection (available on all commands) +prism analyze --provider github # Explicit provider ``` @@ -226,12 +229,35 @@ prism fetch --format text # Raw PR data as text --- -## Supported Providers +## Providers + +### Supported providers + +| Provider | Type | Status | +|----------|------|--------| +| GitHub | Built-in | Supported | +| AWS CodeCommit | Plugin | In progress (v0.2.0) | + +### Provider selection + +By default, prism auto-detects the provider from the PR URL. Use `--provider` to specify explicitly: + +```bash +prism analyze https://github.com/owner/repo/pull/123 # auto-detected as GitHub +prism analyze --provider github # explicit GitHub +prism analyze --provider codecommit # explicit CodeCommit (requires plugin) +``` + +### Plugin providers + +External providers are distributed as separate binaries named `prism-provider-` and discovered on PATH. Plugins receive a PR URL and return structured JSON to stdout. + +```bash +# Plugin invocation (called by prism internally): +prism-provider-codecommit fetch +``` -| Provider | Status | -|----------|--------| -| GitHub | Supported | -| AWS CodeCommit | Planned (v0.2.0) | +See [ADR-0001](docs/adr/0001-provider-plugin-architecture.md) for design details. --- @@ -324,7 +350,7 @@ make clean # Remove bin/ ## Roadmap - **v0.1.0** — GitHub provider, analyze/prompt/fetch commands, JSON/Markdown/text output, light/detailed/cross modes, config/lang/template support, exit codes -- **v0.2.0** — AWS CodeCommit provider +- **v0.2.0** — Provider plugin architecture, `--provider` flag, AWS CodeCommit provider - **v0.3.0** — Policy files, custom review axes, project-specific rules - **v0.4.0+** — Review policy as code, SARIF output, metrics, IDE/CI integration diff --git a/cmd/prism/integration_test.go b/cmd/prism/integration_test.go index 0401c60..84addbf 100644 --- a/cmd/prism/integration_test.go +++ b/cmd/prism/integration_test.go @@ -64,8 +64,20 @@ func TestCLIAnalyzeInvalidURL(t *testing.T) { if err == nil { t.Fatal("expected error") } - if !strings.Contains(err.Error(), "invalid PR URL") { - t.Errorf("error = %q, want 'invalid PR URL'", err.Error()) + if !strings.Contains(err.Error(), "cannot auto-detect provider") { + t.Errorf("error = %q, want 'cannot auto-detect provider'", err.Error()) + } +} + +func TestCLIAnalyzeWithUnknownProvider(t *testing.T) { + cmd := rootCmd() + cmd.SetArgs([]string{"analyze", "https://example.com/pr/1", "--provider", "nonexistent"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error = %q, want 'not found'", err.Error()) } } diff --git a/cmd/prism/main.go b/cmd/prism/main.go index 8250ab7..7d4bf2b 100644 --- a/cmd/prism/main.go +++ b/cmd/prism/main.go @@ -9,11 +9,11 @@ import ( "github.com/hidetzu/prism/internal/config" "github.com/hidetzu/prism/internal/domain" - ghprovider "github.com/hidetzu/prism/internal/provider/github" + "github.com/hidetzu/prism/internal/provider" "github.com/hidetzu/prism/internal/usecase" ) -const version = "0.1.0-dev" +const version = "0.2.0-alpha.1" // Exit codes as defined in docs/spec.md. const ( @@ -53,6 +53,8 @@ func rootCmd() *cobra.Command { SilenceErrors: true, } + cmd.PersistentFlags().String("provider", "", "Provider name (e.g. github, codecommit); auto-detected from URL if omitted") + cmd.AddCommand( analyzeCmd(), promptCmd(), @@ -107,6 +109,20 @@ func fetchCmd() *cobra.Command { return cmd } +func resolveProvider(cmd *cobra.Command, cfg config.Config, prURL string) (provider.Provider, domain.PRRef, error) { + providerName, _ := cmd.Flags().GetString("provider") + reg := provider.NewRegistry(cfg.GitHubToken) + p, err := reg.Resolve(providerName, prURL) + if err != nil { + return nil, domain.PRRef{}, fmt.Errorf("%w: %v", domain.ErrInvalidArgs, err) + } + ref, err := p.Parse(prURL) + if err != nil { + return nil, domain.PRRef{}, fmt.Errorf("%w: invalid PR URL: %v", domain.ErrInvalidArgs, err) + } + return p, ref, nil +} + func runAnalyze(cmd *cobra.Command, args []string) error { if len(args) == 0 { return fmt.Errorf("%w: PR URL is required", domain.ErrInvalidArgs) @@ -118,10 +134,9 @@ func runAnalyze(cmd *cobra.Command, args []string) error { return fmt.Errorf("load config: %w", err) } - p := newProvider(cfg) - ref, err := p.Parse(args[0]) + p, ref, err := resolveProvider(cmd, cfg, args[0]) if err != nil { - return fmt.Errorf("%w: invalid PR URL: %v", domain.ErrInvalidArgs, err) + return err } format, _ := cmd.Flags().GetString("format") @@ -145,10 +160,9 @@ func runPrompt(cmd *cobra.Command, args []string) error { return fmt.Errorf("load config: %w", err) } - p := newProvider(cfg) - ref, err := p.Parse(args[0]) + p, ref, err := resolveProvider(cmd, cfg, args[0]) if err != nil { - return fmt.Errorf("%w: invalid PR URL: %v", domain.ErrInvalidArgs, err) + return err } mode, _ := cmd.Flags().GetString("mode") @@ -182,10 +196,9 @@ func runFetch(cmd *cobra.Command, args []string) error { return fmt.Errorf("load config: %w", err) } - p := newProvider(cfg) - ref, err := p.Parse(args[0]) + p, ref, err := resolveProvider(cmd, cfg, args[0]) if err != nil { - return fmt.Errorf("%w: invalid PR URL: %v", domain.ErrInvalidArgs, err) + return err } format, _ := cmd.Flags().GetString("format") @@ -194,8 +207,3 @@ func runFetch(cmd *cobra.Command, args []string) error { Format: format, }, os.Stdout) } - -func newProvider(cfg config.Config) *ghprovider.Provider { - token := cfg.GitHubToken - return ghprovider.NewProvider(token) -} diff --git a/cmd/prism/main_test.go b/cmd/prism/main_test.go index cbdbfd5..d0c8e65 100644 --- a/cmd/prism/main_test.go +++ b/cmd/prism/main_test.go @@ -109,3 +109,10 @@ func TestFetchRequiresURL(t *testing.T) { t.Fatal("expected error, got nil") } } + +func TestProviderFlag(t *testing.T) { + cmd := rootCmd() + if cmd.PersistentFlags().Lookup("provider") == nil { + t.Error("missing persistent flag --provider") + } +} diff --git a/docs/adr/0001-provider-plugin-architecture.md b/docs/adr/0001-provider-plugin-architecture.md new file mode 100644 index 0000000..6deb6a8 --- /dev/null +++ b/docs/adr/0001-provider-plugin-architecture.md @@ -0,0 +1,142 @@ +# ADR-0001: Provider Plugin Architecture for v0.2.0 + +## Status +Accepted + +## Date +2026-04-11 + +## Context + +`prism` v0.2.0 では AWS CodeCommit の Pull Request 取得を追加したい。 +ただし、`prism` 本体の責務は **Pull Request を構造化コンテキストへ分解すること** であり、 +各ホスティングサービス固有の API 実装や認証処理を大量に抱え込むことではない。 + +現状の懸念は次の2点である。 + +1. **プロバイダー検出の設計** + - 現在は GitHub 前提で実装されている + - GitHub / CodeCommit / 将来の Bitbucket などにどう拡張するかを整理する必要がある + +2. **認証と依存の違い** + - GitHub は `GITHUB_TOKEN` を用いた Bearer 認証 + - CodeCommit は AWS credential chain / IAM / region を前提とする + - Go 本体に AWS SDK を直接組み込むと、依存と責務が膨らむ + +また、CodeCommit については既存資産として `ccpr` が存在している。 +この資産を活用しつつ、`prism` 本体の責務を守る必要がある。 + +--- + +## Decision + +v0.2.0 では、**Provider を外部バイナリプラグイン方式で拡張する**。 + +`prism` 本体は provider interface と plugin execution layer のみを持ち、 +CodeCommit 対応は `ccpr` 系の別バイナリとして実装・連携する。 + +### 採用する方針 + +- Provider 拡張は **外部バイナリ方式** を採用する +- `prism-provider-codecommit` のような実行可能バイナリを想定する +- `prism` はサブプロセスとして provider plugin を呼び出す +- provider plugin は PR データを共通 JSON 形式で返す +- `prism` は返却された共通 JSON を `PullRequest` ドメインモデルへ変換して解析を続行する + +### GitHub provider の扱い + +- v0.2.0 では **GitHub は本体内蔵（built-in）** とする +- `go install` だけで GitHub に対して即座に使えるユーザー体験を維持する +- ただし、内部的には plugin と同等の境界を保ち、将来的に plugin として外出し可能な構造にする +- CodeCommit 以降の外部 provider は plugin 方式とする + +### Provider 検出方針 + +- デフォルトは **URL からの自動判定** +- `--provider` が明示指定された場合は **URL による自動判定を行わない** +- URL 自動判定が難しいケースや GitHub Enterprise などでは `--provider` で明示できる + +### 初期ルール + +- `github.com` を含む URL は GitHub provider +- CodeCommit URL パターンに一致するものは CodeCommit provider +- `--provider github` などが指定された場合はその provider を強制利用する +- 自動判定できない場合は、分かりやすいエラーメッセージで `--provider` の指定を促す + +--- + +## Rationale + +この判断の理由は次の通り。 + +### 1. `prism` の責務を保てる +`prism` の本質は PR の取得ではなく、**取得済み PR をレビュー用コンテキストへ変換すること** にある。 +プロバイダー固有の実装を本体に抱え込むと、この責務が曖昧になる。 + +### 2. `ccpr` を活かしやすい +CodeCommit 対応については既存の `ccpr` を活かせる。 +`prism` に AWS SDK を直接入れず、外部 provider としてラップすることで、資産再利用と責務分離を両立できる。 + +### 3. 将来の拡張に強い +Bitbucket、GitHub Enterprise、GitLab など将来的な provider を想定したとき、 +プラグイン方式のほうが `prism` 本体の変更を最小化しやすい。 + +### 4. 依存分離が明確 +GitHub provider は軽量に保てる一方、CodeCommit provider は AWS 認証や SDK を独立して持てる。 +これにより本体のビルド、保守、配布がシンプルになる。 + +--- + +## Alternatives Considered + +## A. 外部バイナリ方式 +採用。 + +### メリット +- 依存が完全に分離できる +- `prism` 本体の責務が明確 +- `ccpr` を活かしやすい +- 将来 provider を増やしやすい + +### デメリット +- プラグインとの入出力仕様を安定させる必要がある +- サブプロセス実行とエラー処理が必要 +- 配布方法を設計する必要がある + +## B. Go internal 方式 +不採用。 + +### 理由 +- `prism` 本体に provider 固有依存が入りやすい +- AWS SDK などの依存が本体に混ざる +- `prism` の思想である責務分離が崩れやすい + +--- + +## Consequences + +### Positive +- v0.2.0 で CodeCommit 対応を追加しつつ、本体をシンプルに保てる +- `ccpr` を provider plugin として再利用する道が開ける +- 将来の provider 拡張戦略が明確になる + +### Negative +- plugin protocol を設計する必要がある +- plugin discovery の仕様が必要になる +- ユーザー向けにはインストール説明が少し増える + +--- + +## Plugin Protocol + +plugin の呼び出し規約、JSON schema、discovery 仕様の詳細は [Provider Plugin Protocol](../provider-plugin-protocol.md) を参照。 + +--- + +## Implementation Status + +- [x] plugin protocol の JSON schema を定義する +- [x] provider registry を実装する +- [x] provider auto detection ロジックを実装する +- [x] `--provider` 優先ルールを実装する +- [ ] CodeCommit plugin の第一候補として `ccpr` ラップ方式を検討する diff --git a/docs/architecture.md b/docs/architecture.md index 9bad342..702e220 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,8 +28,11 @@ graph TD FMT --> DOM PMT --> DOM - PRV --- GH["GitHub"] - PRV --- CC["CodeCommit
planned"] + PRV --- REG["Registry"] + REG --- GH["GitHub
built-in"] + REG --- PLG["Plugin Executor"] + PLG --- CC["prism-provider-codecommit"] + PLG --- ETC["prism-provider-..."] FMT --- JSON["JSON"] FMT --- MD["Markdown"] @@ -87,6 +90,12 @@ type Provider interface { All provider-specific data is normalized into domain models at the provider boundary. +The provider layer consists of: + +- **Registry** — resolves a provider by name or auto-detects from URL. GitHub is built-in; other providers are discovered as external plugin binaries (`prism-provider-`) on PATH. +- **Plugin Executor** — runs an external provider binary as a subprocess, parses its stdout JSON into domain models, and enriches files with language/test/config classification. +- **Plugin Protocol** — plugins are invoked as `prism-provider- fetch ` and return a JSON object to stdout. See [ADR-0001](adr/0001-provider-plugin-architecture.md) for details. + ### `internal/classifier` Determines change type based on PR title, description, file paths, and diff content. @@ -111,7 +120,7 @@ Orchestrates the pipeline: fetch → classify → analyze → format/render. Eac ## Design Principles -1. **Provider abstraction first** — New PR sources should only require implementing the `Provider` interface +1. **Provider abstraction first** — New PR sources are added as external plugin binaries without modifying prism itself 2. **Domain models are the contract** — All packages communicate through domain types 3. **Output stability** — JSON schema must remain backward-compatible within a major version 4. **Testability** — All external dependencies are behind interfaces; use fixtures and golden files for output verification diff --git a/docs/implementation/v0.2.0-provider-plugin.md b/docs/implementation/v0.2.0-provider-plugin.md new file mode 100644 index 0000000..ac6c54b --- /dev/null +++ b/docs/implementation/v0.2.0-provider-plugin.md @@ -0,0 +1,157 @@ +# prism v0.2.0 Implementation Guide + +## Goal + +v0.2.0 では **外部バイナリ方式の provider plugin architecture** を導入し、 +AWS CodeCommit 対応を追加する。 + +### Design decisions (see ADR-0001) + +- Provider 拡張は外部バイナリ方式 +- GitHub は本体内蔵（built-in）、将来的に plugin 化可能な構造を保つ +- CodeCommit 以降の外部 provider は plugin 方式 +- Provider 検出はデフォルト URL 自動判定、`--provider` 指定時は自動判定しない +- plugin invocation: `prism-provider- fetch ` +- plugin は共通 JSON を stdout に返す + +--- + +## Scope + +### Must + +- provider registry の導入 +- provider autodetection の導入 +- `--provider` 明示指定の導入（analyze / prompt / fetch） +- plugin executor の導入 +- provider plugin protocol の定義（JSON schema） +- GitHub provider を plugin と同等の境界を保つ構造へ整理 +- CodeCommit provider plugin の最小実装 +- `ccpr` 利用方針の具体化 +- テスト追加 +- README / docs 更新 + +### Nice to have + +- plugin discovery path の将来拡張を見越したインターフェース整理 + +### Out of scope + +- Bitbucket 対応 +- plugin marketplace +- ネットワーク越し plugin +- 動的ロードライブラリ方式 + +--- + +## Implementation Steps + +### Step 1: Provider detection と `--provider` を導入する + +- `--provider` フラグを analyze / prompt / fetch に追加 +- URL からの provider 自動判定ロジックを実装 +- `--provider` 指定時は自動判定をスキップ +- 判定不能時は `--provider` を案内するエラーメッセージを返す + +期待動作: +```bash +prism analyze https://github.com/owner/repo/pull/123 +# → GitHub を自動選択 + +prism analyze +# → CodeCommit を自動選択 + +prism analyze --provider github +# → GitHub を強制利用 +``` + +### Step 2: Provider registry / plugin discovery / plugin executor を導入する + +- provider registry: built-in（GitHub）と plugin の両方を管理 +- plugin discovery: PATH 上の `prism-provider-` バイナリを探索 +- plugin executor: サブプロセスとして plugin を実行し、stdout JSON をパース + +plugin invocation: +```bash +prism-provider-codecommit fetch +``` + +設計指針: +- stdout のみを機械読取対象とする +- stderr はデバッグ用 +- exit code でエラー判定 + +### Step 3: ダミー plugin で integration test を通す + +- テスト用のダミー provider plugin を作成 +- 正常系: 共通 JSON を返す +- 異常系: 不正 JSON、exit code 非 0、タイムアウト +- plugin discovery のテスト（PATH 上にバイナリがある / ない） + +### Step 4: CodeCommit plugin の最小対応を追加する + +方針: +1. まず `ccpr` の出力形式を調査し、共通 JSON への変換可否を確認 +2. 変換可能なら `ccpr` ラップ方式で `prism-provider-codecommit` を実装 +3. 困難なら新規 plugin の雛形を作成 + +### Step 5: README / docs を更新する + +- README に provider architecture 概要を追加 +- URL autodetection と `--provider` の説明 +- plugin install 方法 +- `docs/provider-plugins.md` を追加 + +--- + +## Plugin Protocol + +詳細は [Provider Plugin Protocol](../provider-plugin-protocol.md) を参照。 + +--- + +## Test Strategy + +### Unit tests + +- provider autodetection（GitHub URL, CodeCommit URL, unknown URL） +- `--provider` 優先ロジック +- plugin discovery（PATH 上にバイナリがある / ない） +- plugin stdout JSON parsing（正常 / 不正 / 空） +- error mapping（plugin exit code → domain error） + +### Integration tests + +- ダミー provider plugin バイナリを使った動作確認 +- 正常系 / 異常系 +- stdout / stderr / exit code の扱い + +### Important + +- 実際の AWS / GitHub ネットワーク依存テストに寄せすぎない +- plugin protocol と app 層の安定性を重視する + +--- + +## Acceptance Criteria + +- [ ] `--provider` が analyze / prompt / fetch に実装されている +- [ ] URL 自動判定が機能する +- [ ] provider 判定不能時のエラーが分かりやすい +- [ ] plugin executor が動く +- [ ] provider plugin protocol が文書化されている +- [ ] CodeCommit 対応経路が存在する（plugin 最小実装 or ccpr ラップ） +- [ ] `ccpr` 活用方針がコードまたは文書で具体化されている +- [ ] tests が追加されている +- [ ] README / docs が更新されている + +--- + +## Principles + +- 1 PR 1 責務 +- 先に protocol と境界を固める +- いきなり CodeCommit の詳細実装に入らない +- `prism` 本体へ AWS SDK を直接追加しない +- 過剰設計しない +- ただし将来の provider 追加で破綻しない構造にする diff --git a/docs/provider-plugin-protocol.md b/docs/provider-plugin-protocol.md new file mode 100644 index 0000000..a26ea1b --- /dev/null +++ b/docs/provider-plugin-protocol.md @@ -0,0 +1,195 @@ +# Provider Plugin Protocol v1 + +This document defines the contract between `prism` and provider plugin binaries. Any binary that conforms to this protocol can serve as a prism provider. + +## Protocol Version + +Current version: **1** + +The `version` field in plugin output is required. `prism` will reject output with a mismatched version. + +--- + +## Invocation + +`prism` invokes a provider plugin as a subprocess: + +``` +prism-provider- fetch +``` + +### Arguments + +| Position | Value | Description | +|----------|-------|-------------| +| 1 | `fetch` | Subcommand (currently the only supported command) | +| 2 | PR URL | The raw PR URL as provided by the user | + +### Environment + +The plugin inherits the parent process environment. Plugins may use environment variables for authentication (e.g., `AWS_PROFILE`, `GITHUB_TOKEN`). + +`prism` does NOT pass authentication credentials to plugins. Each plugin is responsible for its own authentication. + +--- + +## Output + +### stdout — Structured JSON (machine-readable) + +The plugin MUST write a single JSON object to stdout. `prism` parses only stdout. + +```json +{ + "version": "1", + "provider": "codecommit", + "repository": "my-repo", + "id": "42", + "title": "Add retry handling for payment API", + "author": "dev", + "source_branch": "feature/payment-retry", + "target_branch": "main", + "description": "Adds retry logic for transient payment API failures", + "changed_files": [ + { + "path": "internal/payment/client.go", + "status": "modified", + "additions": 45, + "deletions": 3, + "patch": "@@ -1,3 +1,45 @@\n+func retry() { ... }" + }, + { + "path": "internal/payment/client_test.go", + "status": "added", + "additions": 80, + "deletions": 0, + "patch": "@@ -0,0 +1,80 @@\n+package payment_test ..." + } + ] +} +``` + +### stderr — Debug/diagnostic output (human-readable) + +The plugin MAY write diagnostic messages to stderr. `prism` captures stderr and includes it in error messages when the plugin fails. stderr is NOT parsed as structured data. + +--- + +## Field Reference + +### Top-level fields + +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `version` | string | **yes** | Protocol version. Must be `"1"`. | +| `provider` | string | **yes** | Provider name (e.g., `"github"`, `"codecommit"`). | +| `repository` | string | **yes** | Repository identifier (e.g., `"owner/repo"`, `"my-repo"`). | +| `id` | string | **yes** | Pull request identifier (e.g., `"123"`, `"42"`). | +| `title` | string | **yes** | Pull request title. | +| `author` | string | **yes** | Author name or identifier. | +| `source_branch` | string | **yes** | Source branch name. | +| `target_branch` | string | **yes** | Target (base) branch name. | +| `description` | string | **yes** | Pull request description body. May be empty string `""`. | +| `changed_files` | array | **yes** | List of changed file objects. May be empty array `[]`. | + +### changed_files element + +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `path` | string | **yes** | File path relative to repository root. | +| `status` | string | **yes** | One of: `"added"`, `"modified"`, `"removed"`, `"renamed"`. | +| `additions` | integer | **yes** | Number of added lines. Must be >= 0. | +| `deletions` | integer | **yes** | Number of deleted lines. Must be >= 0. | +| `patch` | string | **yes** | Unified diff patch content. May be empty string `""` for binary files. | + +### Notes on field values + +- **`version`**: `prism` validates this field. Unknown versions are rejected with an error. +- **`status`**: Unknown values are treated as `"modified"`. Plugins SHOULD use the defined values. +- **`patch`**: Should be in unified diff format. `prism` passes this through to output without parsing. +- **`description`**: Use `""` (empty string) if no description exists. Do NOT omit the field. +- **`changed_files`**: Use `[]` (empty array) if the PR has no file changes. Do NOT omit the field. + +--- + +## Exit Codes + +| Exit Code | Meaning | prism behavior | +|-----------|---------|----------------| +| 0 | Success | Parse stdout JSON | +| non-zero | Failure | Report error with stderr content | + +When exit code is non-zero, `prism` wraps the error as a provider error (exit code 3). + +--- + +## Plugin Discovery + +`prism` discovers plugins by searching for executables on `PATH`: + +``` +prism-provider- +``` + +The `` corresponds to the provider name used with `--provider ` or auto-detected from the URL. + +### Naming convention + +| Provider | Binary name | +|----------|-------------| +| GitHub | `prism-provider-github` | +| CodeCommit | `prism-provider-codecommit` | +| GitLab | `prism-provider-gitlab` | +| Bitbucket | `prism-provider-bitbucket` | + +--- + +## Context and Timeouts + +`prism` may terminate the plugin process if a context deadline is exceeded (e.g., user cancellation, timeout). Plugins SHOULD handle `SIGTERM` / `SIGINT` gracefully. + +--- + +## File Enrichment + +`prism` automatically enriches each `changed_files` entry after receiving plugin output: + +- `language` — detected from file extension +- `is_test` — detected from file name and path patterns +- `is_config` — detected from file name patterns +- `is_generated` — detected from file name patterns + +Plugins do NOT need to provide these fields. They are computed by `prism`. + +--- + +## Versioning Policy + +- Protocol version `"1"` is the initial stable version. +- Additive changes (new optional fields) do NOT require a version bump. +- Breaking changes (removing fields, changing types, changing semantics) require a new version. +- `prism` will support multiple protocol versions simultaneously when needed. + +--- + +## Example: Minimal Plugin (shell script) + +```bash +#!/bin/sh +cat <<'EOF' +{ + "version": "1", + "provider": "example", + "repository": "my-org/my-repo", + "id": "1", + "title": "Example PR", + "author": "developer", + "source_branch": "feature", + "target_branch": "main", + "description": "", + "changed_files": [] +} +EOF +``` + +Save as `prism-provider-example`, make executable, and place on PATH. diff --git a/docs/spec.md b/docs/spec.md index c5fad9b..82c7495 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -1,5 +1,17 @@ # CLI Specification +## Global Flags + +The following flag is available on all subcommands: + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--provider` | string | (auto-detect) | Provider name (e.g. `github`, `codecommit`); auto-detected from URL if omitted | + +When `--provider` is specified, URL-based auto-detection is skipped and the given provider is used directly. + +--- + ## Commands ### `prism analyze` @@ -94,7 +106,7 @@ prism fetch [flags] ### Config file -Loaded from `~/.config/prism/config.yaml` by default. Override with `--config` flag or `PRCTX_CONFIG` environment variable. +Loaded from `~/.config/prism/config.yaml` by default. Override with `--config` flag or `PRISM_CONFIG` environment variable. ```yaml github_token: ghp_xxxxxxxxxxxx @@ -108,7 +120,7 @@ default_lang: en | Variable | Description | |----------|-------------| | `GITHUB_TOKEN` | GitHub API authentication token (overrides config file) | -| `PRCTX_CONFIG` | Override config file path | +| `PRISM_CONFIG` | Override config file path | ### Custom Templates diff --git a/internal/provider/github/github.go b/internal/provider/github/github.go index 37ec5cf..da92b90 100644 --- a/internal/provider/github/github.go +++ b/internal/provider/github/github.go @@ -5,7 +5,6 @@ import ( "net/http" "github.com/hidetzu/prism/internal/domain" - "github.com/hidetzu/prism/internal/provider" ) const apiBaseURL = "https://api.github.com" @@ -15,9 +14,6 @@ type Provider struct { client *Client } -// Verify interface compliance at compile time. -var _ provider.Provider = (*Provider)(nil) - // NewProvider creates a GitHub provider with the given token. func NewProvider(token string) *Provider { return &Provider{ diff --git a/internal/provider/plugin/plugin.go b/internal/provider/plugin/plugin.go new file mode 100644 index 0000000..ffd63a0 --- /dev/null +++ b/internal/provider/plugin/plugin.go @@ -0,0 +1,131 @@ +package plugin + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os/exec" + + "github.com/hidetzu/prism/internal/domain" + "github.com/hidetzu/prism/internal/fileutil" +) + +const protocolVersion = "1" + +// pluginOutput represents the JSON schema that provider plugins must produce. +type pluginOutput struct { + Version string `json:"version"` + Provider string `json:"provider"` + Repository string `json:"repository"` + ID string `json:"id"` + Title string `json:"title"` + Author string `json:"author"` + SourceBranch string `json:"source_branch"` + TargetBranch string `json:"target_branch"` + Description string `json:"description"` + ChangedFiles []pluginChangedFile `json:"changed_files"` +} + +type pluginChangedFile struct { + Path string `json:"path"` + Status string `json:"status"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + Patch string `json:"patch"` +} + +// Provider implements provider.Provider by executing an external plugin binary. +type Provider struct { + name string + binaryPath string + prURL string +} + +// NewProvider creates a plugin-based provider. +func NewProvider(name, binaryPath, prURL string) *Provider { + return &Provider{ + name: name, + binaryPath: binaryPath, + prURL: prURL, + } +} + +// Parse returns a minimal PRRef. For plugin providers, the binary does the real parsing. +func (p *Provider) Parse(input string) (domain.PRRef, error) { + return domain.PRRef{Provider: p.name}, nil +} + +// FetchPullRequest executes the plugin binary and converts its JSON output to a domain.PullRequest. +func (p *Provider) FetchPullRequest(ctx context.Context, ref domain.PRRef) (domain.PullRequest, error) { + cmd := exec.CommandContext(ctx, p.binaryPath, "fetch", p.prURL) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return domain.PullRequest{}, fmt.Errorf( + "%w: plugin %s failed: %v: %s", domain.ErrProvider, p.name, err, stderr.String(), + ) + } + + var out pluginOutput + if err := json.Unmarshal(stdout.Bytes(), &out); err != nil { + return domain.PullRequest{}, fmt.Errorf( + "%w: plugin %s returned invalid JSON: %v", domain.ErrProvider, p.name, err, + ) + } + + if out.Version != protocolVersion { + return domain.PullRequest{}, fmt.Errorf( + "%w: plugin %s uses protocol version %q, expected %q", + domain.ErrProvider, p.name, out.Version, protocolVersion, + ) + } + + return convertOutput(out), nil +} + +func convertOutput(out pluginOutput) domain.PullRequest { + files := make([]domain.ChangedFile, len(out.ChangedFiles)) + for i, f := range out.ChangedFiles { + files[i] = domain.ChangedFile{ + Path: f.Path, + Status: mapFileStatus(f.Status), + Additions: f.Additions, + Deletions: f.Deletions, + Language: fileutil.DetectLanguage(f.Path), + IsTest: fileutil.IsTestFile(f.Path), + IsConfig: fileutil.IsConfigFile(f.Path), + IsGenerated: fileutil.IsGeneratedFile(f.Path), + Patch: f.Patch, + } + } + + return domain.PullRequest{ + Repository: out.Repository, + ID: out.ID, + Title: out.Title, + Author: out.Author, + SourceBranch: out.SourceBranch, + TargetBranch: out.TargetBranch, + Description: out.Description, + ChangedFiles: files, + } +} + +func mapFileStatus(status string) domain.FileStatus { + switch status { + case "added": + return domain.FileStatusAdded + case "modified": + return domain.FileStatusModified + case "removed": + return domain.FileStatusRemoved + case "renamed": + return domain.FileStatusRenamed + default: + return domain.FileStatusModified + } +} diff --git a/internal/provider/plugin/plugin_test.go b/internal/provider/plugin/plugin_test.go new file mode 100644 index 0000000..82215b6 --- /dev/null +++ b/internal/provider/plugin/plugin_test.go @@ -0,0 +1,203 @@ +package plugin_test + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/hidetzu/prism/internal/domain" + "github.com/hidetzu/prism/internal/provider/plugin" +) + +// TestHelperProcess is re-executed by tests as a fake plugin binary. +// It is not a real test — it exits immediately when not invoked by the test harness. +func TestHelperProcess(t *testing.T) { + if os.Getenv("PRISM_TEST_PLUGIN") == "" { + return + } + behavior := os.Getenv("PRISM_TEST_PLUGIN") + switch behavior { + case "valid": + fmt.Print(`{ + "version": "1", + "provider": "dummy", + "repository": "owner/repo", + "id": "99", + "title": "Test PR", + "author": "tester", + "source_branch": "feature", + "target_branch": "main", + "description": "A test pull request", + "changed_files": [ + { + "path": "internal/handler.go", + "status": "modified", + "additions": 10, + "deletions": 3, + "patch": "@@ -1,3 +1,10 @@" + }, + { + "path": "internal/handler_test.go", + "status": "added", + "additions": 30, + "deletions": 0, + "patch": "@@ -0,0 +1,30 @@" + } + ] + }`) + case "invalid_json": + fmt.Print(`{not valid json`) + case "wrong_version": + fmt.Print(`{"version": "99", "provider": "dummy"}`) + case "stderr_only": + fmt.Fprint(os.Stderr, "debug info") + os.Exit(1) + } + os.Exit(0) +} + +func helperBinary(t *testing.T) string { + t.Helper() + return os.Args[0] +} + +func newTestProvider(t *testing.T, behavior string) *plugin.Provider { + t.Helper() + t.Setenv("PRISM_TEST_PLUGIN", behavior) + return plugin.NewProvider("dummy", helperBinary(t), "https://example.com/pr/1") +} + +func TestFetchPullRequestValid(t *testing.T) { + p := newTestProvider(t, "valid") + pr, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "dummy"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pr.Repository != "owner/repo" { + t.Errorf("Repository = %q, want %q", pr.Repository, "owner/repo") + } + if pr.ID != "99" { + t.Errorf("ID = %q, want %q", pr.ID, "99") + } + if pr.Title != "Test PR" { + t.Errorf("Title = %q, want %q", pr.Title, "Test PR") + } + if pr.Author != "tester" { + t.Errorf("Author = %q, want %q", pr.Author, "tester") + } + if pr.SourceBranch != "feature" { + t.Errorf("SourceBranch = %q, want %q", pr.SourceBranch, "feature") + } + if pr.TargetBranch != "main" { + t.Errorf("TargetBranch = %q, want %q", pr.TargetBranch, "main") + } + if len(pr.ChangedFiles) != 2 { + t.Fatalf("ChangedFiles length = %d, want 2", len(pr.ChangedFiles)) + } + + f := pr.ChangedFiles[0] + if f.Path != "internal/handler.go" { + t.Errorf("file[0].Path = %q", f.Path) + } + if f.Language != "Go" { + t.Errorf("file[0].Language = %q, want %q", f.Language, "Go") + } + if f.IsTest { + t.Error("file[0].IsTest = true, want false") + } + + f = pr.ChangedFiles[1] + if !f.IsTest { + t.Error("file[1].IsTest = false, want true") + } +} + +func TestFetchPullRequestInvalidJSON(t *testing.T) { + p := newTestProvider(t, "invalid_json") + _, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "dummy"}) + if err == nil { + t.Fatal("expected error for invalid JSON") + } +} + +func TestFetchPullRequestWrongVersion(t *testing.T) { + p := newTestProvider(t, "wrong_version") + _, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "dummy"}) + if err == nil { + t.Fatal("expected error for wrong protocol version") + } +} + +func TestFetchPullRequestPluginFailure(t *testing.T) { + p := newTestProvider(t, "stderr_only") + _, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "dummy"}) + if err == nil { + t.Fatal("expected error for plugin failure") + } +} + +func TestFetchPullRequestTimeout(t *testing.T) { + // Use a command that sleeps, then cancel quickly. + p := plugin.NewProvider("dummy", "sleep", "10") + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + _, err := p.FetchPullRequest(ctx, domain.PRRef{Provider: "dummy"}) + if err == nil { + t.Fatal("expected timeout error") + } +} + +func TestFetchPullRequestBinaryNotFound(t *testing.T) { + p := plugin.NewProvider("dummy", "/nonexistent/binary", "https://example.com/pr/1") + _, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "dummy"}) + if err == nil { + t.Fatal("expected error for missing binary") + } +} + +func TestParseReturnsMinimalPRRef(t *testing.T) { + p := plugin.NewProvider("codecommit", "/usr/bin/true", "https://example.com/pr/1") + ref, err := p.Parse("https://example.com/pr/1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ref.Provider != "codecommit" { + t.Errorf("Provider = %q, want %q", ref.Provider, "codecommit") + } +} + +func TestPluginDiscoveryOnPATH(t *testing.T) { + // Create a dummy plugin script on PATH. + dir := t.TempDir() + script := filepath.Join(dir, "prism-provider-testplugin") + content := `#!/bin/sh +echo '{"version":"1","provider":"testplugin","repository":"r","id":"1","title":"t","author":"a","source_branch":"s","target_branch":"m","description":"d","changed_files":[]}' +` + if err := os.WriteFile(script, []byte(content), 0755); err != nil { + t.Fatal(err) + } + + path, err := exec.LookPath(script) + if err != nil { + // Add dir to PATH for lookup + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + path, err = exec.LookPath("prism-provider-testplugin") + if err != nil { + t.Fatalf("LookPath: %v", err) + } + } + + p := plugin.NewProvider("testplugin", path, "https://example.com/pr/1") + pr, err := p.FetchPullRequest(context.Background(), domain.PRRef{Provider: "testplugin"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pr.ID != "1" { + t.Errorf("ID = %q, want %q", pr.ID, "1") + } +} diff --git a/internal/provider/registry.go b/internal/provider/registry.go new file mode 100644 index 0000000..7e53b8d --- /dev/null +++ b/internal/provider/registry.go @@ -0,0 +1,70 @@ +package provider + +import ( + "fmt" + "net/url" + "os/exec" + + "github.com/hidetzu/prism/internal/domain" + "github.com/hidetzu/prism/internal/provider/github" + "github.com/hidetzu/prism/internal/provider/plugin" +) + +// Registry manages built-in and plugin providers. +type Registry struct { + githubToken string +} + +// NewRegistry creates a provider registry. +func NewRegistry(githubToken string) *Registry { + return &Registry{githubToken: githubToken} +} + +// Resolve returns a Provider for the given PR URL. +// If providerName is non-empty, it uses that provider directly. +// If providerName is empty, it auto-detects the provider from the URL. +func (r *Registry) Resolve(providerName string, prURL string) (Provider, error) { + if providerName == "" { + detected, err := detectProvider(prURL) + if err != nil { + return nil, err + } + providerName = detected + } + + switch providerName { + case "github": + return github.NewProvider(r.githubToken), nil + default: + return r.resolvePlugin(providerName, prURL) + } +} + +func detectProvider(prURL string) (string, error) { + u, err := url.Parse(prURL) + if err != nil || u.Host == "" { + return "", fmt.Errorf( + "cannot auto-detect provider from %q; use --provider to specify one", prURL, + ) + } + + switch u.Host { + case "github.com": + return "github", nil + default: + return "", fmt.Errorf( + "cannot auto-detect provider for host %q; use --provider to specify one", u.Host, + ) + } +} + +func (r *Registry) resolvePlugin(name string, prURL string) (Provider, error) { + binary := "prism-provider-" + name + path, err := exec.LookPath(binary) + if err != nil { + return nil, fmt.Errorf( + "%w: provider %q not found: %s is not on PATH", domain.ErrProvider, name, binary, + ) + } + return plugin.NewProvider(name, path, prURL), nil +} diff --git a/internal/provider/registry_test.go b/internal/provider/registry_test.go new file mode 100644 index 0000000..83e9716 --- /dev/null +++ b/internal/provider/registry_test.go @@ -0,0 +1,67 @@ +package provider + +import ( + "testing" +) + +func TestDetectProviderGitHub(t *testing.T) { + name, err := detectProvider("https://github.com/owner/repo/pull/123") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != "github" { + t.Errorf("name = %q, want %q", name, "github") + } +} + +func TestDetectProviderUnknownHost(t *testing.T) { + _, err := detectProvider("https://gitlab.com/owner/repo/-/merge_requests/1") + if err == nil { + t.Fatal("expected error for unknown host") + } +} + +func TestDetectProviderInvalidURL(t *testing.T) { + _, err := detectProvider("not-a-url") + if err == nil { + t.Fatal("expected error for invalid URL") + } +} + +func TestResolveGitHub(t *testing.T) { + reg := NewRegistry("test-token") + p, err := reg.Resolve("github", "https://github.com/o/r/pull/1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if p == nil { + t.Fatal("provider is nil") + } +} + +func TestResolveAutoDetectGitHub(t *testing.T) { + reg := NewRegistry("test-token") + p, err := reg.Resolve("", "https://github.com/o/r/pull/1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if p == nil { + t.Fatal("provider is nil") + } +} + +func TestResolveAutoDetectFails(t *testing.T) { + reg := NewRegistry("") + _, err := reg.Resolve("", "https://unknown.example.com/pr/1") + if err == nil { + t.Fatal("expected error for unknown host") + } +} + +func TestResolvePluginNotFound(t *testing.T) { + reg := NewRegistry("") + _, err := reg.Resolve("nonexistent", "https://example.com/pr/1") + if err == nil { + t.Fatal("expected error for missing plugin binary") + } +}