feat(chat): Atlassian MCP C phase — auth_request UX + long-lived Query + YAML fix#18
Closed
shomatan wants to merge 34 commits into
Closed
feat(chat): Atlassian MCP C phase — auth_request UX + long-lived Query + YAML fix#18shomatan wants to merge 34 commits into
shomatan wants to merge 34 commits into
Conversation
office-hours + plan-eng-review で固めた設計を元に、TDD-style で 19 task に分割。 buildMcpServers utility の抽出、chat-runner / agent-runner の refactor、 duplicate-guards/ の strategy-pattern 化、T1/T4 fix を含む。A フェーズ (専用エージェント + ボタン UI) は C dogfood 結果を踏まえて別 plan で書く。 Related docs: - ~/.gstack/projects/ignission-tally/knowbe01-main-design-20260423-164810.md - ~/.gstack/projects/ignission-tally/knowbe01-main-eng-review-test-plan-20260423-212143.md
- Task 1: McpServerConfigSchema を Basic/Bearer 両対応に拡張 (Cloud=Basic email:token base64、Server/DC=Bearer) - Task 5: buildMcpServers の auth header 構築を scheme 分岐 - 関連テストケース更新
…set / envVar 名 regex)
…lt []) Atlassian MCP 連携の Project レベル設定を保持するため、 ProjectSchema と ProjectMetaSchema に mcpServers: McpServerConfig[] を追加。 default は [] とし、mcpServers フィールドを持たない既存 YAML も後方互換で読み込める。 宣言順の都合で MCP セクションを ProjectMetaSchema より前に移動。 ProjectMeta / Project 型は z.input 由来に切り替え、書き込み側で mcpServers 省略可とした (output は parse 時に default [] が解決される)。
3987780 で導入した z.input 由来の Project / ProjectMeta / ProjectMetaPatch を z.infer (output 型) に戻す。foundation schema は parse 後の値を表すべきで、 input 緩和は downstream で ?? [] ガードを書く debt を生むため。 これに伴い必要となる既存 fixture 17 ファイルに mcpServers: [] を機械的に追加: - packages/storage/src/init-project.ts (1) - packages/storage/src/project-store.test.ts (5) - packages/ai-engine の test 4 ファイル (9) - packages/frontend の test 11 ファイル (32) 新規 test の追加は無し。既存 546 tests は同数のまま全 pass。
…owedTools wildcard 合成)
… (Task 12) - ChatEvent に chat_tool_external_use / chat_tool_external_result を追加 - extractAssistantBlocks を拡張: text + 外部 MCP tool_use + tool_result を返す - mcp__tally__* は intercept 経路で処理されるため拾わない (重複回避) - chatStore.appendBlockToMessage で source: 'external' で永続化、approval 省略 - console.log は redactMcpSecrets でラップ済み (Task 11 で導入済)
大規模 epic 取り込みで 1 ターン 500KB+ になり得る tool_result の YAML 永続化を 4KB + "(truncated, N chars total)" マーカーに切り詰める。event はフルを流すので UI セッション内では全文展開可、リロード後は truncated 版のみ。
…k 14, T4 fix) multi-turn 対話で AI が前ターンの外部 MCP 出力を覚えるように buildChatPrompt を拡張。 旧版は text block のみ replay していたため、AI が 1 ターン目で @jira EPIC-1 を読んで proposal を生成しても、2 ターン目「続けて子チケット STORY-2 も見て」では Jira の内容を 覚えていない (= AI が summarize した assistant text しか残らない) 問題があった。 各 block を順に replay: - text → そのまま (assistant / user の自然言語) - tool_use → <tool_use id="..." name="..." source="external"?>...input...</tool_use> - tool_result → <tool_result id="..." ok="...">...output...</tool_result> ChatRunner.runUserTurn の prompt 構築タイミングは現状維持 (user message append → contextNodes load → buildChatPrompt → 空 assistant append)。
agent-runner.ts:114 の hardcoded mcpServers / allowedTools を chat-runner と 共通の buildMcpServers utility 経由に変更。プロジェクト設定 mcpServers[] が agent 側でも有効化され、Task 16+ で実装する ingest-jira-epic agent がそのまま 動く土台が整う。 - 既存 5 agent (decompose-to-stories / extract-questions / find-related-code / analyze-impact / ingest-document) の動作不変を 193 既存 test で regression 担保 - 新規 2 test: 外部 MCP 合成 (Bearer auth) / env 未設定で error event - agent 固有 allowedTools (mcp__tally__find_related 等) と外部 MCP wildcard (mcp__<id>__*) を合流、tally の wildcard は dedup - env 未設定 throw は既存の catch (err) で agent_failed error event に流れる
…k 16) - core: ProjectMetaPatchSchema に mcpServers: McpServerConfig[] optional を追加 (.strict() なので明示追加が必要) - frontend: PATCH /api/projects/:id で parsed.data.mcpServers を next に流す - 新規 test 3 件: round-trip / hardening (http://example.com で 400) / 空配列で全消去
- mcpServers[] の追加 / 削除 / 編集 (id / name / url / scheme / envVar) - Bearer (Server/DC) と Basic (Cloud) 切替で emailEnvVar 入力欄が出現 - secret 値はフォームに無し → caption で .env への誘導 - store の patchProjectMeta type に mcpServers? を追加 - 新規 test 4 件: 追加/削除/Basic 切替/secret 欄が無いこと
- store.ts: chat_tool_external_use / chat_tool_external_result の event handler を追加 - 内部 tool_use とは別 handler で source='external' の block を append - external は承認概念なし (approval は付与しない) - tool-approval-card.tsx: source 分岐 - source='external' は <details> 折り畳み + 「外部ソース」ラベル + 承認 / 却下なし - source='internal' は既存通り承認 / 却下 button (退行なし) - 新規 test 2 件: external で承認 button 非表示 + details 折り畳み
10 個の Jira エピックで Success Criteria を測定する手順と記録テンプレート。 - Setup: sooperset/mcp-atlassian の起動方法 + Tally 設定 - 各 Epic ごとの記録項目 (turn 1 / turn 2 / 気づかなかった論点 / 重複ガード) - 集計: 採用率 50%+ / 質的記録 3+ / multi-turn / システム動作 - A フェーズ ingest-jira-epic 設計の入力 (プロンプト改善点 / tool 呼び出しパターン)
Premise 9 撤回 (PAT only → MCP プロトコルの OAuth 2.1 採用、user sovereignty)。 理由 (user 指摘): - Atlassian 公式 Rovo MCP は OAuth 2.1 ネイティブ、Claude Agent SDK は MCP の 'needs-auth' status を扱う built-in support を持つ → SDK 任せで auto auth flow が回る - sooperset/mcp-atlassian を使う場合も PAT は MCP server 自身の env で持つ前提に - Tally プロセスから PAT/API key の概念が完全消滅 (project.yaml / メモリ / ログのいずれにも残らない) 実装変更: - core: McpServerConfigSchema から auth フィールド削除、関連 hardening (envVar regex / Basic+Bearer 分岐) も削除。schema test を auth-less に整理 (84 tests) - ai-engine: buildMcpServers の auth header 構築ロジック削除、url のみで HTTP config (req は SDK が必要に応じて WWW-Authenticate を解釈)。chat-runner / agent-runner も auth に依存しない。env 未設定 throw test は仕様変更のため削除 (189 tests) - frontend: 設定 dialog から auth scheme dropdown / envVar 入力欄を削除、URL のみの シンプル UI、caption も「OAuth 2.1 / API token は MCP 任せ」に書き換え (264 tests) - 全 624 tests pass、typecheck clean、lint errors 0 docs: - design doc Premise 9 を「MCP プロトコル / SDK 側に完全委譲」に更新 - dogfood log: sooperset の env 受け渡し例 + Atlassian 公式 Rovo MCP 経由の OAuth 2.1 フロー手順を併記
実機 Tally dev server で dogfood する前の事前 PoC として、Claude が atlassian plugin tool 経由で PMDEV-165 (ホーム画面+認証、子チケット 50+) の context を取り、 「もし Tally AI agent が同じ context で論点抽出したら」のシミュレーション出力を 6 件の question proposal として記録。 論点候補: - ログインセッションの「保持」既定状態 (UX vs リテンション) - パスワード再発行メールの送信元・文言所有権 (法務/マーケ判断) - ゲストの価格・在庫「非表示」の具体実装 (ぼかし/CTA/完全隠し) - 退会後のデータ保持期間 (電帳法 7 年 vs GDPR 削除権) - パフォーマンス "3 秒以内" の計測条件 (dev vs Core Web Vitals) - サプライヤー初回パスワード変更 UX (子チケット PMDEV-264/266 由来) うち「気づかなかった論点」候補 2 件 (退会後の法定保持 / パフォーマンス二重基準)。 実機実装後に同エピックで dogfood し、この期待値と比べる。
4000/4001 ポートが他プロジェクト (ark) と衝突しがちなので default を 5050 に。 env override (AI_ENGINE_PORT / NEXT_PUBLIC_AI_ENGINE_URL) は引き続き有効。 - packages/ai-engine/src/config.ts: default port 5050 - packages/ai-engine/src/config.test.ts: 期待値 5050 - packages/frontend/src/lib/ws.ts: default URL ws://localhost:5050 (2 箇所) - .env.example: コメントと値を 5050 に - README.md: ai-engine の URL 表記を 5050 に
空配列 (`messages: []`) を初回書き込みすると yaml lib は flow style `[]` で 出力する。次回書き込み時に既存 seq の flow=true が残ったまま新しい map を 追加すると、フロー集約の中に複数行 string の block scalar が混在して 再パース不能な YAML が生成されていた (Atlassian MCP 連携で複数行応答が 来始めて顕在化)。 - packages/storage/src/yaml.ts: forceBlockStyle ヘルパで Map/Seq の flow フラグを再帰的にオフ。mergeSeqById と doc.set の両経路で適用。 - packages/storage/src/chat-store.ts: listChats を 1 ファイルずつ try/catch にし、破損 1 件で全 chat 一覧が 500 で死ぬのを回避 (warn は残す)。 - yaml.test.ts: 空 seq → 複数行 string 追加が再パース可能であることを regression として固定。
外部 MCP (Atlassian Rovo 等) の OAuth 2.1 認証フローで以下を解消する: 1. 認証 URL とその後の callback URL paste 操作が assistant 文中に埋もれて UX が破綻していた問題 2. user_message のたびに sdk.query() を呼び直していたため SDK subprocess が 再生成され、MCP HTTP transport の OAuth state (PKCE / token) が turn 跨ぎ で消えてしまう問題 ## auth_request ブロックで OAuth UI を 1 等地化 - packages/core: ChatBlock に `auth_request` variant を追加 (mcpServerId / mcpServerLabel / authUrl / status / failureMessage) - packages/ai-engine/auth-detector.ts: `mcp__<id>__authenticate` / `complete_authentication` のパターン検出と auth URL 抽出 - chat-runner: tool_use を stash して tool_result とペアになった瞬間に auth_request ブロックへ変換 (raw な tool_use/tool_result は出さない)。 complete_authentication 受領時は同 mcpServerId の最新 pending を更新。 - frontend: `AuthRequestCard` を新設し、認証ボタン (新規タブ) と callback URL paste 入力を 1 カードに集約。chat-message から dispatch。 - store.ts: chat_auth_request イベントで pending append / completed・failed in-place 更新。 ## ChatRunner を long-lived Query 化 - packages/ai-engine/async-input.ts: push 可能な AsyncIterable<SDKUserMessage>。 - agent-runner.ts: SdkLike.query を `prompt: string | AsyncIterable<...>` に 拡張、SdkQueryHandle 型 (close 任意) を追加。 - chat-runner.ts: 1 ChatRunner = 1 sdk.query() = 1 subprocess に固定。 ensureQuery / runOutputLoop / dispatchSdkMessage / close を新設し、 runUserTurn は input.push + queue drain に簡素化。MCP ハンドラは currentTurn から assistantMsgId / emit を解決する。 - server.ts: WS close 時に runner.close() で SDK subprocess を片付ける。 注: 実 SDK の Query iter が turn 単位で切れる挙動を持つ場合 OAuth state が それでも復元されないケースがあるため、debug 用 console.log を残してある。 本番化時に削除予定。
3000 (Next.js デフォルト) は claude.ai のローカル OAuth callback / 他の React dev と衝突する。5050 も別プロジェクトと衝突実例あり。3321/3322 は セットで覚えやすく現状空き。 - frontend: package.json の dev/start に `-p 3321` を固定、ws.ts と playwright.config / README / docs を 3321 に統一 - ai-engine: loadConfig の default を 3322 に変更、config.test.ts 追従 - README / .env.example / docs / examples の URL を一斉更新
📝 WalkthroughWalkthroughWebSocketエンジンポートを4000から3322に、フロントエンドポートを3000から3321に変更。MCP（Model Context Protocol）サーバー設定、OAuth 2.1認証フロー、重複チェックシステムの実装を追加し、長時間実行のチャットランナーアーキテクチャを導入。 Changes
Sequence DiagramssequenceDiagram
actor User as ユーザー
participant UI as チャットUI
participant SDK as AI SDK
participant MCP as MCPサーバー
User->>UI: メッセージ送信
UI->>SDK: チャットメッセージ送信
alt MCPツール実行
SDK->>MCP: ツール呼び出し
MCP-->>SDK: ツール結果
SDK-->>UI: 外部tool_useイベント
SDK-->>UI: 外部tool_resultイベント
UI->>UI: ツール結果を表示
end
alt OAuth認証が必要
SDK-->>UI: authenticate ツール呼び出し
UI-->>UI: 認証リクエストカード表示
User->>UI: OAuth認証URL開く
User->>UI: コールバックURL入力
UI->>SDK: complete_authentication送信
SDK->>MCP: 認証完了通知
MCP-->>SDK: 認証結果
SDK-->>UI: auth_request更新イベント
UI->>UI: 認証ステータス更新
end
sequenceDiagram
participant Client as クライアント
participant Agent as エージェント
participant Config as 設定管理
participant Guard as 重複ガード
participant Store as ストア
Client->>Agent: ノード作成依頼
Agent->>Config: プロジェクトMCP設定取得
Config-->>Agent: MCPサーバー構成
Agent->>Agent: MCPサーバー構成を統合
Agent->>Guard: duplicateGuard.check()実行
Guard->>Store: findRelatedNodes()照会
Store-->>Guard: 関連ノード一覧
alt 重複検出
Guard-->>Agent: 重複理由を返却
Agent-->>Client: エラー（重複あり）
else 重複なし
Guard-->>Agent: null返却
Agent->>Store: addNode()実行
Agent->>Guard: notifyCreated()通知
Guard->>Guard: セッションメモ更新
Agent-->>Client: 成功（ノード作成完了）
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase.md (1)
1-2526:⚠️ Potential issue | 🔴 Critical計画ドキュメントとPR実装の間に重大な不一致があります。
このPRの実装内容（PR summary参照）:
- OAuth 2.1 認証フロー（auth_request ChatBlock variant）
- WWW-Authenticate ハンドリング
- 長寿命 ChatRunner (async-input AsyncIterable)
- auth_request has status: pending/completed/failed
しかし、この計画ドキュメントでは:
- PAT (Personal Access Token) ベースの認証のみ（Basic/Bearer schemes）
- Line 168: "OAuth は MVP 非対応、Premise 9"
- Line 24: "PAT 認証が使えることが必須 (Premise 9)"
- auth_request や OAuth 2.1 への言及なし
- async-input や長寿命 ChatRunner への言及なし
この計画ドキュメントは古いバージョンか、別のPR向けのものと思われます。開発者がこの計画に従うと、実際のPR実装（OAuth 2.1）と矛盾した実装（PAT）を作ってしまいます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase.md` around lines 1 - 2526, Summary: The PR implements OAuth2/auth_request and long‑lived async-input behavior but the design doc and tests expect PAT-based Basic/Bearer handling (McpServerConfigSchema + buildMcpServers) and no auth_request/async-input, causing a spec mismatch. Fix by choosing one approach and making code/tests/docs consistent: either (A) revert PR OAuth/auth_request/WWW-Authenticate/async-input changes and implement PAT flows (ensure McpServerConfigSchema, buildMcpServers, redactMcpSecrets, ChatBlockSchema.source, ChatRunner.runUserTurn use PAT Basic/Bearer headers as documented) or (B) update the plan/tests/docs to accept OAuth2: add auth_request ChatBlock variant to schema, document OAuth flows, update buildMcpServers/tests to accept OAuth config, and adapt ChatRunner (including async-input handling and WWW-Authenticate logic); specifically align McpServerConfigSchema, buildMcpServers, ChatRunner.runUserTurn, and any auth_request handling so code and plan/tests match. Ensure unit tests reference the chosen symbols (McpServerConfigSchema, buildMcpServers, ChatBlockSchema auth fields, auth_request, ChatRunner.runUserTurn, async-input) are updated accordingly.packages/core/src/schema.ts (1)
263-302:⚠️ Potential issue | 🟠 Major
mcpServers[].idの重複も schema 側で弾いた方がよいです。Line 271 / 285 / 299 では配列要素の shape は検証されていますが、
idの一意性は未検証です。ここはmcp__<id>__*に埋め込まれる前提なので、重複するとツール解決が衝突して別サーバーを区別できなくなります。codebasesと同じく project/meta/patch の 3 箇所で一意性チェックを入れておくのが安全です。🧩 修正例
export const ProjectMetaSchema = z .object({ id: z.string().min(1), name: z.string().min(1), description: z.string().optional(), // 0 件以上。code ノードが存在するときは最低 1 件必要（整合性は storage 層で検証）。 codebases: z.array(CodebaseSchema), // Atlassian 等の MCP サーバー設定。既存 YAML (フィールド無し) は default [] で読み込める。 mcpServers: z.array(McpServerConfigSchema).default([]), createdAt: z.string(), updatedAt: z.string(), }) - .superRefine((meta, ctx) => checkUniqueCodebaseIds(meta.codebases, ctx)); + .superRefine((meta, ctx) => { + checkUniqueCodebaseIds(meta.codebases, ctx); + checkUniqueMcpServerIds(meta.mcpServers, ctx); + }); export const ProjectSchema = z .object({ id: z.string().min(1), name: z.string().min(1), description: z.string().optional(), codebases: z.array(CodebaseSchema), // Atlassian 等の MCP サーバー設定。ProjectMetaSchema と整合。 mcpServers: z.array(McpServerConfigSchema).default([]), createdAt: z.string(), updatedAt: z.string(), nodes: z.array(NodeSchema), edges: z.array(EdgeSchema), }) - .superRefine((p, ctx) => checkUniqueCodebaseIds(p.codebases, ctx)); + .superRefine((p, ctx) => { + checkUniqueCodebaseIds(p.codebases, ctx); + checkUniqueMcpServerIds(p.mcpServers, ctx); + }); export const ProjectMetaPatchSchema = z .object({ name: z.string().min(1).optional(), description: z.string().nullable().optional(), codebases: z.array(CodebaseSchema).optional(), mcpServers: z.array(McpServerConfigSchema).optional(), }) .strict() - .superRefine((patch, ctx) => checkUniqueCodebaseIds(patch.codebases, ctx)); + .superRefine((patch, ctx) => { + checkUniqueCodebaseIds(patch.codebases, ctx); + checkUniqueMcpServerIds(patch.mcpServers, ctx); + });function checkUniqueMcpServerIds( servers: { id: string }[] | undefined, ctx: z.RefinementCtx, ): void { if (!servers) return; const seen = new Set<string>(); for (const s of servers) { if (seen.has(s.id)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `mcpServers[].id 重複: ${s.id}`, path: ['mcpServers'], }); return; } seen.add(s.id); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/schema.ts` around lines 263 - 302, Add a uniqueness check for mcpServers[].id similar to checkUniqueCodebaseIds: implement a function (e.g., checkUniqueMcpServerIds(servers, ctx)) that returns early if servers is undefined, iterates servers, records seen ids, and calls ctx.addIssue with path ['mcpServers'] when a duplicate is found; then call this function inside the existing .superRefine callbacks for ProjectMetaSchema, ProjectSchema, and ProjectMetaPatchSchema (in addition to checkUniqueCodebaseIds) so mcpServers id collisions are rejected at schema validation time.
🧹 Nitpick comments (7)
packages/frontend/src/lib/store.test.ts (1)
20-20: テストフィクスチャの重複が大きく、今後のスキーマ変更に弱いです。
mcpServersのような共通フィールド追加時に修正点が広がるため、hydrateProject(overrides)のようなヘルパーに寄せると保守性が上がります。あわせてこのファイルは 500 行を大きく超えているので、機能単位で分割した方が安全です。As per coding guidelines: Keep individual files under 500 lines; split larger files into multiple modules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/lib/store.test.ts` at line 20, テストフィクスチャが各ケースにベタ書きされており、共通フィールド（例: mcpServers）を追加すると修正箇所が増えるため、共通のテストフィクスチャ生成ヘルパーを導入してください（例: hydrateProject(overrides)）。具体的には packages/frontend/src/lib/store.test.ts 内のテストデータ作成箇所を抽出して hydrateProject という関数に集約し、各テストでは必要な差分のみ overrides で渡すように置き換え、またファイルが500行を超えているので関連するテスト群（state/selectors/actions など）ごとにモジュール分割してテストケースを別ファイルに移してください。packages/storage/src/chat-store.ts (1)
96-109: 例外を広く握りつぶしており、実運用エラーが見えなくなります。壊れた YAML をスキップする方針は良いですが、現状は権限/IO 系エラーまで
warn + skipになり、障害検知が遅れます。パース失敗相当のみスキップし、それ以外は再 throw する分岐に絞るのが安全です。改善例
- } catch (err) { - console.warn(`[chat-store] skip broken chat file: ${file}`, err); - return null; - } + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + // ファイル破損相当はスキップ、それ以外の運用エラーは上位へ + if (code && code !== 'ENOENT') throw err; + console.warn(`[chat-store] skip broken chat file: ${file}`, err); + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage/src/chat-store.ts` around lines 96 - 109, The catch is too broad around the readYaml(...) call and hides non-parse IO/permission errors; change the error handling in the readYaml + ChatThreadSchema block so that only YAML parse/validation errors are swallowed (log a warn and return null) while other errors (e.g., ENOENT, EACCES, unexpected exceptions) are re-thrown; detect parse/validation failures by checking the error type or message from readYaml/validator (or introduce a helper like isYamlParseError) and use that in the catch to decide between console.warn + return null and throw err, keeping references to readYaml, ChatThreadSchema and the surrounding function/method that reads chat files to locate the change.packages/storage/src/project-store.test.ts (1)
33-47:mcpServersの往復アサーションを1ケース追加するとより安全です。フィクスチャには
mcpServersを入れていますが、保存/読込で欠落しても現在のテストでは検知できません。少なくとも1つの roundtrip テストで明示確認しておくのを推奨します。差分案
const meta = await store.getProjectMeta(); expect(meta?.name).toBe('テストプロジェクト'); expect(meta?.codebases).toEqual([]); + expect(meta?.mcpServers).toEqual([]);Also applies to: 353-382
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storage/src/project-store.test.ts` around lines 33 - 47, Add an assertion to verify mcpServers is preserved by the roundtrip: in the test that constructs FileSystemProjectStore and calls saveProjectMeta/getProjectMeta (the "saveProjectMeta → getProjectMeta で往復できる" test), assert that the returned meta.mcpServers matches the fixture (e.g. expect(meta?.mcpServers).toEqual([...]) or expect(meta?.mcpServers).toHaveLength(1) etc. Do the same fix for the other analogous test around the second occurrence (related to saveProjectMeta/getProjectMeta) so missing mcpServers regressions are caught.packages/ai-engine/src/tools/create-node.test.ts (1)
699-853: この suite は別 spec に分けたいです。
create-node.test.tsはすでにかなり大きく、ここに sourceUrl guard の統合ケースを足すと失敗時の切り分けがさらにしづらくなります。duplicate-guards/source-url.test.tsかcreate-node.source-url.test.tsに分離した方が保守しやすいです。Based on learnings
Keep individual files under 500 lines; split larger files into multiple modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/tools/create-node.test.ts` around lines 699 - 853, The test suite "createNodeHandler — sourceUrl 重複ガード (Task 9 連動)" is too large for create-node.test.ts and should be extracted into its own spec file (e.g. duplicate-guards/source-url.test.ts or create-node.source-url.test.ts); cut the entire describe(...) block and paste it into the new test file, keep the same tests and imports, ensure createNodeHandler and ProjectStore mocks are imported/available in the new file, remove the block from create-node.test.ts, and run tests to verify the new file is discovered by the test runner.packages/ai-engine/src/duplicate-guards/source-url.test.ts (1)
37-43:sourceUrl正規化の回帰テストを追加しておくと安全です
' 'と' https://... 'のケースを追加すると、trim()導入後の重複判定退行を防げます。Also applies to: 123-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/duplicate-guards/source-url.test.ts` around lines 37 - 43, Add regression tests to source-url.test.ts to verify sourceUrl normalization: extend the existing sourceUrlGuard.check tests (referencing sourceUrlGuard.check) to include a case where additional.sourceUrl is a string of only spaces (' ') and a case with leading/trailing spaces around a URL (' https://... '), asserting the same expected behavior (e.g., returns null or identical duplicate detection as trimmed input). Ensure these new tests mirror the existing test structure used at the example block (and the related area around the lines indicated) so trim-based normalization regressions are caught.packages/ai-engine/src/chat-runner.ts (1)
262-311: console.log のデバッグログが残存
ensureQueryとrunOutputLoopにconsole.logが残っています。PR 説明に「debug logs retained」とありますが、これらはプロダクション前に削除するか、適切なロギングフレームワークに置き換えることを推奨します。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/chat-runner.ts` around lines 262 - 311, Remove the stray console.log calls left in ensureQuery and runOutputLoop and replace them with the project's logging facility at debug level (e.g., processLogger.debug or the logger exposed on this.deps). Specifically, in ensureQuery (the messages logging reuse/creation and outputLoopFailed) and any console.log inside runOutputLoop, call the existing logger (processLogger.debug or this.deps.processLogger.debug) with the same message/context instead of console.log, or remove the logs if not needed; use the symbols ensureQuery and runOutputLoop to locate the code and ensure no console.* calls remain.packages/ai-engine/src/duplicate-guards/index.ts (1)
38-42: guard 登録は idempotent にしておくとテスト時に安全です。今は
registerGuard()が無条件pushで、Line 81-83 の副作用登録やテスト側の再登録が重なると同じ guard が複数回実行されます。notifyCreated()の hook まで多重発火するので、組み込み guard の登録を helper 化して重複防止を入れておく方が扱いやすいです。🧩 修正例
const REGISTRY = new Map<AdoptableType, DuplicateGuard[]>(); +let builtinGuardsRegistered = false; export function registerGuard(guard: DuplicateGuard): void { const list = REGISTRY.get(guard.adoptAs) ?? []; + if (list.includes(guard)) return; list.push(guard); REGISTRY.set(guard.adoptAs, list); } @@ export function __resetGuardsForTest(): void { REGISTRY.clear(); + builtinGuardsRegistered = false; } @@ -// 個別 guard を register する (module load 時の副作用)。 -// テストは __resetGuardsForTest でクリアした後、必要な guard を再登録すること。 -registerGuard(coderefGuard); -registerGuard(questionGuard); -registerGuard(sourceUrlGuard); +export function registerBuiltinGuards(): void { + if (builtinGuardsRegistered) return; + builtinGuardsRegistered = true; + registerGuard(coderefGuard); + registerGuard(questionGuard); + registerGuard(sourceUrlGuard); +} + +registerBuiltinGuards();Also applies to: 71-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/duplicate-guards/index.ts` around lines 38 - 42, The registerGuard function currently always pushes the guard into REGISTRY causing duplicate executions; make registration idempotent in registerGuard by checking REGISTRY.get(guard.adoptAs) for an existing entry equivalent to the incoming DuplicateGuard (compare a unique identifier property on the guard such as guard.id or fall back to object identity) and only push/set when not already present; update logic used by notifyCreated-related registrations (the code paths around the existing notifyCreated hooks) to rely on this idempotent registerGuard so repeated helper-based registrations or tests won't create duplicate hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/03-architecture.md`:
- Around line 140-141: Update the documented environment variable port examples
so they match the process config ports (change any remaining ":3001" references
to ":3322") in the same doc section that shows the process list (the entries
that list "frontend (Next.js dev server, :3321)" and "ai-engine (WebSocket
server, :3322)"); ensure you update the environment variable example blocks and
the other occurrence referenced (around the second mention at the later
location) so all port numbers are consistent across the file.
In `@docs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase-dogfood-log.md`:
- Around line 9-15: The dogfood log asserts OAuth 2.1 is used (heading
"Atlassian 公式 Rovo MCP — OAuth 2.1 (推奨)") but the implementation plan still
mandates PAT/Premise 9 and says "OAuth は MVP 非対応" in the other doc; update the
implementation plan document to match the dogfood/PR by removing or revising
Premise 9 references and the PAT requirement, mark OAuth 2.1 as supported for
the implementation (update the "Premise 9" mentions and the OAuth MVP status in
the 2026-04-24-atlassian-mcp-c-phase.md document) so both docs consistently
state OAuth 2.1 as the chosen approach.
In `@packages/ai-engine/src/async-input.ts`:
- Around line 11-12: AsyncIterableInput currently overwrites the single waiter
(private waiter) on concurrent next() calls, violating the AsyncIterator
contract; change the implementation to maintain a FIFO queue of waiters instead
of a single waiter so multiple pending next() Promises are queued and resolved
in order when push() is called, update the logic in AsyncIterableInput.next(),
push(), and any places that set/clear finished to consume/resolve waiters from
the queue, and add a unit test that calls next() twice before calling push()
once and asserts the second Promise remains unresolved while the first resolves.
In `@packages/ai-engine/src/auth-detector.ts`:
- Around line 29-33: The regex match (urlRe) currently returns URLs including
trailing sentence punctuation (e.g., "state=xyz." or "state=xyz)"), breaking
auth links; after calling unfolded.match(urlRe) and before returning m[0], strip
any trailing punctuation characters like . , ) ] } > : ; and similar from the
end of the matched string (ensure you only trim these characters at the end, not
inside the URL), then return the cleaned URL (update the return logic that
currently returns m ? m[0] : null). Also add a regression test that feeds
sentences with URLs followed by punctuation (.,),), etc.) to the function to
assert the returned URL excludes the trailing punctuation.
In `@packages/ai-engine/src/duplicate-guards/source-url.ts`:
- Around line 20-21: Normalize and reuse a trimmed sourceUrl instead of the raw
input value: where the code currently reads and checks const sourceUrl =
input.additional?.sourceUrl and later repeats the raw checks (the same variable
used in the memo key and comparison logic), replace those checks with a single
normalized value (e.g., const normalizedSourceUrl = typeof sourceUrl ===
'string' ? sourceUrl.trim() : '') and use normalizedSourceUrl for the emptiness
check, for building the memo key, and for any comparisons; apply the same change
at the other occurrence noted (the block around the later sourceUrl usage) so
both places use the same trimmed value.
In `@packages/ai-engine/src/server.ts`:
- Around line 257-261: runner をクリーンアップする際に呼んでいる r.close() の返す Promise
が拒否される可能性があり未処理拒否になるので、r.close() に対して最低でも .catch(...)
を追加して拒否を捕捉して握りつぶさないでください（たとえば processLogger.error / console.error
でエラーを記録するか適切なエラーハンドラに渡す）。対象はローカル変数 r とグローバル変数 runner を扱っている箇所（ if (runner) {
const r = runner; runner = null; void r.close(); } ）で、void r.close() を
r.close().catch(err => /* log or handle */) に置き換えてください。
In `@packages/core/src/schema.ts`:
- Around line 335-342: The current auth_request z.object in
packages/core/src/schema.ts allows inconsistent states because failureMessage is
optional regardless of status; change the schema for the "auth_request" shape to
enforce the status/failureMessage relationship by using a discriminated union
(z.discriminatedUnion on status) or an explicit union of three objects: one for
status: 'pending' and one for 'completed' both without failureMessage, and one
for status: 'failed' that requires failureMessage (z.string().min(1)); ensure
you reference the existing fields mcpServerId, mcpServerLabel, authUrl, status
and failureMessage when building the new union so validation rejects invalid
combinations.
In `@packages/frontend/next-env.d.ts`:
- Line 3: packages/frontend/next-env.d.ts is an auto-generated Next.js artifact
that currently imports ".next/dev/types/routes.d.ts", causing CI/production type
resolution failures; remove the file from version control and add
"packages/frontend/next-env.d.ts" (or a glob covering next-env.d.ts) to
.gitignore, and ensure CI runs "next typegen" before type-check/build to produce
the correct .next/types output; alternatively, if you must keep the file in git,
update the import inside next-env.d.ts to reference the stable path
".next/types/routes.d.ts" so production builds resolve types consistently.
In `@packages/frontend/src/app/api/projects/`[id]/route.test.ts:
- Around line 120-136: PATCH
呼び出しのレスポンスを検証しておらず、事前登録が失敗しても後続の「空配列で削除」ケースが通ってしまう可能性があるため、await PATCH(...)
の結果を受け取り（例: const res = await PATCH(...)）、res.status が 200
であることを明示的にアサートしてください；対象の呼び出しはテスト内の PATCH 関数呼び出しと new Request(...)、および params:
Promise.resolve({ id: projectId }) に紐づくブロックなので、その直後にステータス検証を追加してください。
In `@packages/frontend/src/components/dialog/project-settings-dialog.tsx`:
- Around line 84-96: addMcpServer currently calls
makeDefaultMcpServer(mcpServers.length + 1) which can produce duplicate ids
after removals; change addMcpServer to generate a unique id suffix by reading
existing mcpServers.map(s => s.id) and selecting the lowest unused numeric
suffix (or use a UUID) and pass that to makeDefaultMcpServer (or accept an id
param), so new servers never collide with existing ids; alternatively,
de-duplicate on save by checking mcpServers ids and rejecting/renaming
duplicates in the save handler (refer to addMcpServer, makeDefaultMcpServer,
removeMcpServer and state mcpServers).
---
Outside diff comments:
In `@docs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase.md`:
- Around line 1-2526: Summary: The PR implements OAuth2/auth_request and
long‑lived async-input behavior but the design doc and tests expect PAT-based
Basic/Bearer handling (McpServerConfigSchema + buildMcpServers) and no
auth_request/async-input, causing a spec mismatch. Fix by choosing one approach
and making code/tests/docs consistent: either (A) revert PR
OAuth/auth_request/WWW-Authenticate/async-input changes and implement PAT flows
(ensure McpServerConfigSchema, buildMcpServers, redactMcpSecrets,
ChatBlockSchema.source, ChatRunner.runUserTurn use PAT Basic/Bearer headers as
documented) or (B) update the plan/tests/docs to accept OAuth2: add auth_request
ChatBlock variant to schema, document OAuth flows, update buildMcpServers/tests
to accept OAuth config, and adapt ChatRunner (including async-input handling and
WWW-Authenticate logic); specifically align McpServerConfigSchema,
buildMcpServers, ChatRunner.runUserTurn, and any auth_request handling so code
and plan/tests match. Ensure unit tests reference the chosen symbols
(McpServerConfigSchema, buildMcpServers, ChatBlockSchema auth fields,
auth_request, ChatRunner.runUserTurn, async-input) are updated accordingly.
In `@packages/core/src/schema.ts`:
- Around line 263-302: Add a uniqueness check for mcpServers[].id similar to
checkUniqueCodebaseIds: implement a function (e.g.,
checkUniqueMcpServerIds(servers, ctx)) that returns early if servers is
undefined, iterates servers, records seen ids, and calls ctx.addIssue with path
['mcpServers'] when a duplicate is found; then call this function inside the
existing .superRefine callbacks for ProjectMetaSchema, ProjectSchema, and
ProjectMetaPatchSchema (in addition to checkUniqueCodebaseIds) so mcpServers id
collisions are rejected at schema validation time.
---
Nitpick comments:
In `@packages/ai-engine/src/chat-runner.ts`:
- Around line 262-311: Remove the stray console.log calls left in ensureQuery
and runOutputLoop and replace them with the project's logging facility at debug
level (e.g., processLogger.debug or the logger exposed on this.deps).
Specifically, in ensureQuery (the messages logging reuse/creation and
outputLoopFailed) and any console.log inside runOutputLoop, call the existing
logger (processLogger.debug or this.deps.processLogger.debug) with the same
message/context instead of console.log, or remove the logs if not needed; use
the symbols ensureQuery and runOutputLoop to locate the code and ensure no
console.* calls remain.
In `@packages/ai-engine/src/duplicate-guards/index.ts`:
- Around line 38-42: The registerGuard function currently always pushes the
guard into REGISTRY causing duplicate executions; make registration idempotent
in registerGuard by checking REGISTRY.get(guard.adoptAs) for an existing entry
equivalent to the incoming DuplicateGuard (compare a unique identifier property
on the guard such as guard.id or fall back to object identity) and only push/set
when not already present; update logic used by notifyCreated-related
registrations (the code paths around the existing notifyCreated hooks) to rely
on this idempotent registerGuard so repeated helper-based registrations or tests
won't create duplicate hooks.
In `@packages/ai-engine/src/duplicate-guards/source-url.test.ts`:
- Around line 37-43: Add regression tests to source-url.test.ts to verify
sourceUrl normalization: extend the existing sourceUrlGuard.check tests
(referencing sourceUrlGuard.check) to include a case where additional.sourceUrl
is a string of only spaces (' ') and a case with leading/trailing spaces
around a URL (' https://... '), asserting the same expected behavior (e.g.,
returns null or identical duplicate detection as trimmed input). Ensure these
new tests mirror the existing test structure used at the example block (and the
related area around the lines indicated) so trim-based normalization regressions
are caught.
In `@packages/ai-engine/src/tools/create-node.test.ts`:
- Around line 699-853: The test suite "createNodeHandler — sourceUrl 重複ガード (Task
9 連動)" is too large for create-node.test.ts and should be extracted into its own
spec file (e.g. duplicate-guards/source-url.test.ts or
create-node.source-url.test.ts); cut the entire describe(...) block and paste it
into the new test file, keep the same tests and imports, ensure
createNodeHandler and ProjectStore mocks are imported/available in the new file,
remove the block from create-node.test.ts, and run tests to verify the new file
is discovered by the test runner.
In `@packages/frontend/src/lib/store.test.ts`:
- Line 20: テストフィクスチャが各ケースにベタ書きされており、共通フィールド（例:
mcpServers）を追加すると修正箇所が増えるため、共通のテストフィクスチャ生成ヘルパーを導入してください（例:
hydrateProject(overrides)）。具体的には packages/frontend/src/lib/store.test.ts
内のテストデータ作成箇所を抽出して hydrateProject という関数に集約し、各テストでは必要な差分のみ overrides
で渡すように置き換え、またファイルが500行を超えているので関連するテスト群（state/selectors/actions
など）ごとにモジュール分割してテストケースを別ファイルに移してください。
In `@packages/storage/src/chat-store.ts`:
- Around line 96-109: The catch is too broad around the readYaml(...) call and
hides non-parse IO/permission errors; change the error handling in the readYaml
+ ChatThreadSchema block so that only YAML parse/validation errors are swallowed
(log a warn and return null) while other errors (e.g., ENOENT, EACCES,
unexpected exceptions) are re-thrown; detect parse/validation failures by
checking the error type or message from readYaml/validator (or introduce a
helper like isYamlParseError) and use that in the catch to decide between
console.warn + return null and throw err, keeping references to readYaml,
ChatThreadSchema and the surrounding function/method that reads chat files to
locate the change.
In `@packages/storage/src/project-store.test.ts`:
- Around line 33-47: Add an assertion to verify mcpServers is preserved by the
roundtrip: in the test that constructs FileSystemProjectStore and calls
saveProjectMeta/getProjectMeta (the "saveProjectMeta → getProjectMeta で往復できる"
test), assert that the returned meta.mcpServers matches the fixture (e.g.
expect(meta?.mcpServers).toEqual([...]) or
expect(meta?.mcpServers).toHaveLength(1) etc. Do the same fix for the other
analogous test around the second occurrence (related to
saveProjectMeta/getProjectMeta) so missing mcpServers regressions are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8ad42163-2623-4837-bf70-341cfb5aa8ff
📒 Files selected for processing (69)
.env.exampleREADME.mddocs/03-architecture.mddocs/04-roadmap.mddocs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase-dogfood-log.mddocs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase.mdexamples/sample-project/README.mdpackages/ai-engine/src/agent-runner.test.tspackages/ai-engine/src/agent-runner.tspackages/ai-engine/src/agents/find-related-code.test.tspackages/ai-engine/src/async-input.test.tspackages/ai-engine/src/async-input.tspackages/ai-engine/src/auth-detector.test.tspackages/ai-engine/src/auth-detector.tspackages/ai-engine/src/chat-runner.test.tspackages/ai-engine/src/chat-runner.tspackages/ai-engine/src/config.test.tspackages/ai-engine/src/config.tspackages/ai-engine/src/duplicate-guards/coderef.test.tspackages/ai-engine/src/duplicate-guards/coderef.tspackages/ai-engine/src/duplicate-guards/index.test.tspackages/ai-engine/src/duplicate-guards/index.tspackages/ai-engine/src/duplicate-guards/question.test.tspackages/ai-engine/src/duplicate-guards/question.tspackages/ai-engine/src/duplicate-guards/source-url.test.tspackages/ai-engine/src/duplicate-guards/source-url.tspackages/ai-engine/src/mcp/build-mcp-servers.test.tspackages/ai-engine/src/mcp/build-mcp-servers.tspackages/ai-engine/src/mcp/redact.test.tspackages/ai-engine/src/mcp/redact.tspackages/ai-engine/src/server.test.tspackages/ai-engine/src/server.tspackages/ai-engine/src/stream.tspackages/ai-engine/src/tools/create-node.test.tspackages/ai-engine/src/tools/create-node.tspackages/core/src/schema.test.tspackages/core/src/schema.tspackages/core/src/types.tspackages/frontend/README.mdpackages/frontend/next-env.d.tspackages/frontend/package.jsonpackages/frontend/playwright.config.tspackages/frontend/src/app/api/projects/[id]/chats/chats-route.test.tspackages/frontend/src/app/api/projects/[id]/edges/edges-route.test.tspackages/frontend/src/app/api/projects/[id]/nodes/[nodeId]/adopt/adopt-route.test.tspackages/frontend/src/app/api/projects/[id]/nodes/nodes-route.test.tspackages/frontend/src/app/api/projects/[id]/route.test.tspackages/frontend/src/app/api/projects/[id]/route.tspackages/frontend/src/components/ai-actions/analyze-impact-button.test.tsxpackages/frontend/src/components/ai-actions/codebase-agent-button.test.tsxpackages/frontend/src/components/ai-actions/extract-questions-button.test.tsxpackages/frontend/src/components/ai-actions/find-related-code-button.test.tsxpackages/frontend/src/components/chat/auth-request-card.test.tsxpackages/frontend/src/components/chat/auth-request-card.tsxpackages/frontend/src/components/chat/chat-message.tsxpackages/frontend/src/components/chat/tool-approval-card.test.tsxpackages/frontend/src/components/chat/tool-approval-card.tsxpackages/frontend/src/components/details/usecase-detail.test.tsxpackages/frontend/src/components/dialog/project-settings-dialog.test.tsxpackages/frontend/src/components/dialog/project-settings-dialog.tsxpackages/frontend/src/lib/store.test.tspackages/frontend/src/lib/store.tspackages/frontend/src/lib/ws.tspackages/storage/src/chat-store.test.tspackages/storage/src/chat-store.tspackages/storage/src/init-project.tspackages/storage/src/project-store.test.tspackages/storage/src/yaml.test.tspackages/storage/src/yaml.ts
Comment on lines
+9
to
+15
| #### (A) Atlassian 公式 Rovo MCP — OAuth 2.1 (推奨) | ||
|
|
||
| - URL: `https://mcp.atlassian.com/v1/mcp` | ||
| - 認証: ユーザーが初回 Tally Chat 利用時に Claude Agent SDK が WWW-Authenticate を解釈し、 | ||
| ブラウザ経由で OAuth 2.1 を実行。token は SDK が管理し、Tally process には保存されない。 | ||
| - 制約: Atlassian Cloud 専用 (Server/DC 非対応)。 | ||
|
|
There was a problem hiding this comment.
このdogfoodログは正しいOAuth 2.1実装を前提としていますが、実装計画ドキュメント（同じディレクトリ内の 2026-04-24-atlassian-mcp-c-phase.md）との不一致があります。
このdogfoodログ:
- Line 12: "ブラウザ経由で OAuth 2.1 を実行"
- Line 35: "Premise 9 撤回後の設計です"
実装計画ドキュメント:
- Line 24: "PAT 認証が使えることが必須 (Premise 9)"
- Line 168: "OAuth は MVP 非対応、Premise 9"
PRの実装はOAuth 2.1を採用しているため、このdogfoodログの方が正しく、実装計画ドキュメントを更新する必要があります。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-24-atlassian-mcp-c-phase-dogfood-log.md`
around lines 9 - 15, The dogfood log asserts OAuth 2.1 is used (heading
"Atlassian 公式 Rovo MCP — OAuth 2.1 (推奨)") but the implementation plan still
mandates PAT/Premise 9 and says "OAuth は MVP 非対応" in the other doc; update the
implementation plan document to match the dogfood/PR by removing or revising
Premise 9 references and the PAT requirement, mark OAuth 2.1 as supported for
the implementation (update the "Premise 9" mentions and the OAuth MVP status in
the 2026-04-24-atlassian-mcp-c-phase.md document) so both docs consistently
state OAuth 2.1 as the chosen approach.
PR #18 のレビューから明確にバグの 5 件を一括修正。 - async-input: waiter スロットを単一から FIFO キューに変更。next() を 並行で 2 回呼んで push 1 回だけのとき 1 つ目の resolver が捨てられ Promise が永遠に未解決になる仕様違反を解消。close 時に残った全 waiter を done で倒す。回帰テスト 2 件追加。 - auth-detector.extractAuthUrl: 自然文末尾の `.` `,` `;` `:` `!` `?` `)` を URL に含めないよう trailing strip。`...state=xyz.` のような出力で 認可リンクが壊れていた問題を修正。テスト 3 件追加。 - server.ts: WS close 時の `r.close()` を `.catch` で握って unhandled rejection 化を防ぐ。失敗は warn で観測可能にする。 - project-settings-dialog.tsx: addMcpServer の id 採番を `mcpServers.length + 1` から「未使用 suffix を探索」方式に変更。 例えば atlassian-1/atlassian-2 がある状態で 1 件削除して追加すると 再び atlassian-2 が生成されて key 衝突するバグを修正。回帰テスト追加。 - next-env.d.ts: Next.js 自動生成ファイルなので .gitignore に追加して リポジトリから untrack。dev/build で内容が変動するため tracking する意味が無い。
PR #18 のレビューから残り Minor 4 件を一括対応。 - docs/03-architecture.md: 環境変数例の `TALLY_AI_PORT=3001` を実装と整合 する `AI_ENGINE_PORT=3322` (ai-engine の loadConfig が読む env 名) に 修正。同ファイル内で port が複数表記されていた不整合を解消。 - duplicate-guards/source-url.ts: sourceUrl を生値のまま比較していたため 前後空白付き入力で memo / 永続化済み URL と比較がずれて重複検知を すり抜けていた。`normalizeSourceUrl` で trim 正規化し、check / onCreated / 既存ノード比較すべてで揃える。回帰テスト 4 件追加。 - core/schema.ts: auth_request の status と failureMessage の整合を superRefine で固定。failed なのに message 無し / pending・completed に message が付くケースを永続化レイヤで弾く。テスト 3 件追加。 - frontend/route.test.ts: mcpServers 全消去テストの事前 PATCH の ステータスを assert。失敗していると後続の「空配列で削除」ケースが 偽の成功 (空 → 空) で通る恐れがあった。
中間 planning doc と実装の不整合が CodeRabbit に毎回指摘される (Premise 9 撤回後の OAuth 2.1 採用 / 長寿命 ChatRunner 等が plan doc に未反映)。 これらは実装の正史ではなく作業メモなので tracking しない方針に。 - .gitignore に docs/superpowers/ 追加 - 既存の plans/ specs/ をリポジトリから untrack (ローカルファイルは残す)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/schema.ts (1)
263-302:⚠️ Potential issue | 🟠 Major
mcpServers[].idの重複も schema で拒否してください。今は
codebasesだけが重複検査対象で、mcpServersは同じidをそのまま保存できます。idはmcp__<id>__*の名前空間や auth_request の紐付けに使われるので、重複するとツール解決や状態更新が曖昧になります。ProjectMetaSchema/ProjectSchema/ProjectMetaPatchSchemaにも配列レベルの一意性チェックを入れて、あわせて重複 reject の回帰テストを足したいです。🧩 差分案
function checkUniqueCodebaseIds( codebases: { id: string }[] | undefined, ctx: z.RefinementCtx, ): void { if (!codebases) return; const seen = new Set<string>(); for (const c of codebases) { if (seen.has(c.id)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `codebases[].id 重複: ${c.id}`, path: ['codebases'], }); return; } seen.add(c.id); } } + +function checkUniqueMcpServerIds( + mcpServers: { id: string }[] | undefined, + ctx: z.RefinementCtx, +): void { + if (!mcpServers) return; + const seen = new Set<string>(); + for (const server of mcpServers) { + if (seen.has(server.id)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `mcpServers[].id 重複: ${server.id}`, + path: ['mcpServers'], + }); + return; + } + seen.add(server.id); + } +} @@ export const ProjectMetaSchema = z .object({ @@ }) - .superRefine((meta, ctx) => checkUniqueCodebaseIds(meta.codebases, ctx)); + .superRefine((meta, ctx) => { + checkUniqueCodebaseIds(meta.codebases, ctx); + checkUniqueMcpServerIds(meta.mcpServers, ctx); + }); @@ export const ProjectSchema = z .object({ @@ }) - .superRefine((p, ctx) => checkUniqueCodebaseIds(p.codebases, ctx)); + .superRefine((p, ctx) => { + checkUniqueCodebaseIds(p.codebases, ctx); + checkUniqueMcpServerIds(p.mcpServers, ctx); + }); @@ export const ProjectMetaPatchSchema = z .object({ @@ }) .strict() - .superRefine((patch, ctx) => checkUniqueCodebaseIds(patch.codebases, ctx)); + .superRefine((patch, ctx) => { + checkUniqueCodebaseIds(patch.codebases, ctx); + checkUniqueMcpServerIds(patch.mcpServers, ctx); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/schema.ts` around lines 263 - 302, Project schemas currently only validate uniqueness of codebase ids via checkUniqueCodebaseIds; add array-level duplicate-id rejection for mcpServers across ProjectMetaSchema, ProjectSchema, and ProjectMetaPatchSchema by creating/using a validator analogous to checkUniqueCodebaseIds (e.g., checkUniqueMcpServerIds) that inspects mcpServers array and calls ctx.addIssue on duplicate mcpServers[].id, then wire that validator into each schema's .superRefine; also add a regression test asserting that saving/patching a project with duplicate mcpServers[].id is rejected.
🧹 Nitpick comments (1)
packages/ai-engine/src/async-input.ts (1)
33-37: 単一利用前提ならiterable()の多重呼び出しを防ぐと安全です。コメントで前提は示されていますが、実装で2回目以降を防止すると誤用時の調査コストを下げられます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ai-engine/src/async-input.ts` around lines 33 - 37, The iterable() method currently assumes it will only be called once but doesn't enforce it; add a guard (e.g., a private boolean flag like iterableConsumed or _iterableTaken on the AsyncInput instance) that is checked at the start of iterable() and set when iterable() is first called, and if iterable() is called again throw a clear Error indicating multiple iterable() calls are unsupported; update any related logic that constructs the object returned by [Symbol.asyncIterator] to rely on this guard so misuse is detected early.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ai-engine/src/async-input.ts`:
- Around line 39-45: The next() implementation in AsyncInput checks this.buf
before this.finished so callers can still receive buffered items after close();
change next() to check this.finished first (and return {value: undefined, done:
true}) before reading this.buf so close() makes iteration immediately done;
update both instances (the block around lines handling buf/finished and the
similar block at the earlier occurrence) and ensure references to
AsyncInput.next(), this.buf and this.finished are used when making the change.
In `@packages/ai-engine/src/duplicate-guards/source-url.test.ts`:
- Around line 2-4: This test unnecessarily calls __resetGuardsForTest() which
mutates the global guard registry and can create test-order dependence; remove
the call to __resetGuardsForTest() from this file (or at least from the
beforeEach in this test) since this spec does not use dispatchDuplicateGuard(),
and instead only reset guards in tests that actually exercise
dispatchDuplicateGuard() or use a local/isolated registry; if isolation is
required keep state restoration within that specific test (e.g., save/restore or
use a dedicated before/after in the relevant describe) so the global registry
isn’t cleared here.
---
Outside diff comments:
In `@packages/core/src/schema.ts`:
- Around line 263-302: Project schemas currently only validate uniqueness of
codebase ids via checkUniqueCodebaseIds; add array-level duplicate-id rejection
for mcpServers across ProjectMetaSchema, ProjectSchema, and
ProjectMetaPatchSchema by creating/using a validator analogous to
checkUniqueCodebaseIds (e.g., checkUniqueMcpServerIds) that inspects mcpServers
array and calls ctx.addIssue on duplicate mcpServers[].id, then wire that
validator into each schema's .superRefine; also add a regression test asserting
that saving/patching a project with duplicate mcpServers[].id is rejected.
---
Nitpick comments:
In `@packages/ai-engine/src/async-input.ts`:
- Around line 33-37: The iterable() method currently assumes it will only be
called once but doesn't enforce it; add a guard (e.g., a private boolean flag
like iterableConsumed or _iterableTaken on the AsyncInput instance) that is
checked at the start of iterable() and set when iterable() is first called, and
if iterable() is called again throw a clear Error indicating multiple iterable()
calls are unsupported; update any related logic that constructs the object
returned by [Symbol.asyncIterator] to rely on this guard so misuse is detected
early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c9236047-47f5-4137-80db-9eb9d5209a0e
📒 Files selected for processing (17)
.gitignoredocs/03-architecture.mddocs/superpowers/plans/2026-04-21-project-storage-redesign.mddocs/superpowers/specs/2026-04-21-project-storage-redesign-design.mdpackages/ai-engine/src/async-input.test.tspackages/ai-engine/src/async-input.tspackages/ai-engine/src/auth-detector.test.tspackages/ai-engine/src/auth-detector.tspackages/ai-engine/src/duplicate-guards/source-url.test.tspackages/ai-engine/src/duplicate-guards/source-url.tspackages/ai-engine/src/server.tspackages/core/src/schema.test.tspackages/core/src/schema.tspackages/frontend/next-env.d.tspackages/frontend/src/app/api/projects/[id]/route.test.tspackages/frontend/src/components/dialog/project-settings-dialog.test.tsxpackages/frontend/src/components/dialog/project-settings-dialog.tsx
💤 Files with no reviewable changes (2)
- packages/frontend/next-env.d.ts
- docs/superpowers/specs/2026-04-21-project-storage-redesign-design.md
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- packages/ai-engine/src/auth-detector.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/ai-engine/src/server.ts
- docs/03-architecture.md
- packages/ai-engine/src/auth-detector.test.ts
- packages/ai-engine/src/async-input.test.ts
Comment on lines
+39
to
+45
| if (this.buf.length > 0) { | ||
| const v = this.buf.shift() as T; | ||
| return Promise.resolve({ value: v, done: false }); | ||
| } | ||
| if (this.finished) { | ||
| return Promise.resolve({ value: undefined as never, done: true }); | ||
| } |
There was a problem hiding this comment.
close() 後にもバッファ済み値を返し続けるため、終了が即時になっていません。
next() が finished より先に buf を確認するため、close() 呼び出し後でもキュー済みメッセージを消費します。接続切断時の teardown 用 close() では、想定より長く query が継続するリスクがあります。
修正案
close(): void {
if (this.finished) return;
this.finished = true;
- while (this.waiters.length > 0) {
- this.waiters.shift()?.({ value: undefined as never, done: true });
- }
+ this.buf.length = 0;
+ while (this.waiters.length > 0) {
+ this.waiters.shift()?.({ value: undefined as never, done: true });
+ }
}
@@
next: (): Promise<IteratorResult<T>> => {
+ if (this.finished) {
+ return Promise.resolve({ value: undefined as never, done: true });
+ }
if (this.buf.length > 0) {
const v = this.buf.shift() as T;
return Promise.resolve({ value: v, done: false });
}
- if (this.finished) {
- return Promise.resolve({ value: undefined as never, done: true });
- }
return new Promise<IteratorResult<T>>((resolve) => {
this.waiters.push(resolve);
});
},Also applies to: 25-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai-engine/src/async-input.ts` around lines 39 - 45, The next()
implementation in AsyncInput checks this.buf before this.finished so callers can
still receive buffered items after close(); change next() to check this.finished
first (and return {value: undefined, done: true}) before reading this.buf so
close() makes iteration immediately done; update both instances (the block
around lines handling buf/finished and the similar block at the earlier
occurrence) and ensure references to AsyncInput.next(), this.buf and
this.finished are used when making the change.
Comment on lines
+2
to
+4
| import { beforeEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { __resetGuardsForTest, type DuplicateGuardContext } from './index'; |
There was a problem hiding this comment.
__resetGuardsForTest() の単独実行はテスト順依存を作る可能性があります。
Line 23 で毎回グローバルな guard レジストリを空にしていますが、このテストは dispatchDuplicateGuard を使っておらず reset が不要です。後続テストが既定登録 guard を前提にしている場合に不安定化します。
修正案（副作用を避ける）
-import { beforeEach, describe, expect, it } from 'vitest';
+import { describe, expect, it } from 'vitest';
-import { __resetGuardsForTest, type DuplicateGuardContext } from './index';
+import type { DuplicateGuardContext } from './index';
import { sourceUrlGuard } from './source-url';
describe('sourceUrlGuard', () => {
- beforeEach(() => __resetGuardsForTest());
-
it('adoptAs は "requirement"', () => {Also applies to: 23-23
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai-engine/src/duplicate-guards/source-url.test.ts` around lines 2 -
4, This test unnecessarily calls __resetGuardsForTest() which mutates the global
guard registry and can create test-order dependence; remove the call to
__resetGuardsForTest() from this file (or at least from the beforeEach in this
test) since this spec does not use dispatchDuplicateGuard(), and instead only
reset guards in tests that actually exercise dispatchDuplicateGuard() or use a
local/isolated registry; if isolation is required keep state restoration within
that specific test (e.g., save/restore or use a dedicated before/after in the
relevant describe) so the global registry isn’t cleared here.
This was referenced Apr 28, 2026
shomatan
added a commit
that referenced
this pull request
Apr 28, 2026
CodeRabbit 指摘 (PR #18 2nd review): このテストは sourceUrlGuard を 直接呼んでおり、dispatchDuplicateGuard 経由のグローバル registry は 触らないため、beforeEach の reset 呼び出しは不要 + テスト順依存を 作るリスクがあった。
This was referenced Apr 28, 2026
Contributor
Author
shomatan
added a commit
that referenced
this pull request
Apr 30, 2026
* feat(core): McpServerConfigSchema を追加 (Atlassian MCP 連携の基盤、Basic/Bearer 両対応) * feat(core): McpServerConfigSchema に hardening 3 件 (https 強制 / id charset / envVar 名 regex) * feat(core): ProjectSchema/ProjectMetaSchema に mcpServers[] を追加 (default []) Atlassian MCP 連携の Project レベル設定を保持するため、 ProjectSchema と ProjectMetaSchema に mcpServers: McpServerConfig[] を追加。 default は [] とし、mcpServers フィールドを持たない既存 YAML も後方互換で読み込める。 宣言順の都合で MCP セクションを ProjectMetaSchema より前に移動。 ProjectMeta / Project 型は z.input 由来に切り替え、書き込み側で mcpServers 省略可とした (output は parse 時に default [] が解決される)。 * refactor(core): Project 型を z.infer 復帰、既存 fixture に mcpServers: [] を追加 3987780 で導入した z.input 由来の Project / ProjectMeta / ProjectMetaPatch を z.infer (output 型) に戻す。foundation schema は parse 後の値を表すべきで、 input 緩和は downstream で ?? [] ガードを書く debt を生むため。 これに伴い必要となる既存 fixture 17 ファイルに mcpServers: [] を機械的に追加: - packages/storage/src/init-project.ts (1) - packages/storage/src/project-store.test.ts (5) - packages/ai-engine の test 4 ファイル (9) - packages/frontend の test 11 ファイル (32) 新規 test の追加は無し。既存 546 tests は同数のまま全 pass。 * feat(core): ChatBlock.tool_use に source 追加、RequirementNode に sourceUrl 追加 * feat(core): RequirementNode.sourceUrl を https-only に hardening (Task 1 url との整合) * feat(ai-engine): redactMcpSecrets utility を追加 (Authorization header の log 漏洩予防) * feat(ai-engine): redactMcpSecrets に case-sensitive 注記と配列値 test を追加 * feat(ai-engine): buildMcpServers utility を追加 (Basic/Bearer auth + allowedTools wildcard 合成) * feat(ai-engine): duplicate-guards 骨格 (interface + strategy dispatcher) を追加 * feat(ai-engine): coderef 重複ガードを duplicate-guards/coderef.ts に分離 * feat(ai-engine): question 重複ガードを duplicate-guards/question.ts に分離 (anchorId 空は skip) * feat(ai-engine): sourceUrl ベース重複ガードを追加 (T1 fix: chat anchor 無しでも動く) * refactor(ai-engine): create-node を duplicate-guards に委譲、sourceUrl guard を有効化 * feat(ai-engine): ChatRunner が buildMcpServers で外部 MCP を合成 (Task 11) * feat(ai-engine): 外部 MCP の tool_use/tool_result を source=external で永続化 (Task 12) - ChatEvent に chat_tool_external_use / chat_tool_external_result を追加 - extractAssistantBlocks を拡張: text + 外部 MCP tool_use + tool_result を返す - mcp__tally__* は intercept 経路で処理されるため拾わない (重複回避) - chatStore.appendBlockToMessage で source: 'external' で永続化、approval 省略 - console.log は redactMcpSecrets でラップ済み (Task 11 で導入済) * feat(ai-engine): tool_result output を永続化時 4KB に truncate (Task 13) 大規模 epic 取り込みで 1 ターン 500KB+ になり得る tool_result の YAML 永続化を 4KB + "(truncated, N chars total)" マーカーに切り詰める。event はフルを流すので UI セッション内では全文展開可、リロード後は truncated 版のみ。 * feat(ai-engine): buildChatPrompt が tool_use/tool_result も replay (Task 14, T4 fix) multi-turn 対話で AI が前ターンの外部 MCP 出力を覚えるように buildChatPrompt を拡張。 旧版は text block のみ replay していたため、AI が 1 ターン目で @jira EPIC-1 を読んで proposal を生成しても、2 ターン目「続けて子チケット STORY-2 も見て」では Jira の内容を 覚えていない (= AI が summarize した assistant text しか残らない) 問題があった。 各 block を順に replay: - text → そのまま (assistant / user の自然言語) - tool_use → <tool_use id="..." name="..." source="external"?>...input...</tool_use> - tool_result → <tool_result id="..." ok="...">...output...</tool_result> ChatRunner.runUserTurn の prompt 構築タイミングは現状維持 (user message append → contextNodes load → buildChatPrompt → 空 assistant append)。 * feat(ai-engine): agent-runner refactor + buildMcpServers 共有 (Task 15) agent-runner.ts:114 の hardcoded mcpServers / allowedTools を chat-runner と 共通の buildMcpServers utility 経由に変更。プロジェクト設定 mcpServers[] が agent 側でも有効化され、Task 16+ で実装する ingest-jira-epic agent がそのまま 動く土台が整う。 - 既存 5 agent (decompose-to-stories / extract-questions / find-related-code / analyze-impact / ingest-document) の動作不変を 193 既存 test で regression 担保 - 新規 2 test: 外部 MCP 合成 (Bearer auth) / env 未設定で error event - agent 固有 allowedTools (mcp__tally__find_related 等) と外部 MCP wildcard (mcp__<id>__*) を合流、tally の wildcard は dedup - env 未設定 throw は既存の catch (err) で agent_failed error event に流れる * feat(core,frontend): Project API で mcpServers[] の round-trip を実装 (Task 16) - core: ProjectMetaPatchSchema に mcpServers: McpServerConfig[] optional を追加 (.strict() なので明示追加が必要) - frontend: PATCH /api/projects/:id で parsed.data.mcpServers を next に流す - 新規 test 3 件: round-trip / hardening (http://example.com で 400) / 空配列で全消去 * feat(frontend): プロジェクト設定 dialog に MCP サーバー CRUD UI を追加 (Task 17) - mcpServers[] の追加 / 削除 / 編集 (id / name / url / scheme / envVar) - Bearer (Server/DC) と Basic (Cloud) 切替で emailEnvVar 入力欄が出現 - secret 値はフォームに無し → caption で .env への誘導 - store の patchProjectMeta type に mcpServers? を追加 - 新規 test 4 件: 追加/削除/Basic 切替/secret 欄が無いこと * feat(frontend): Chat UI で外部 MCP の tool_use を折り畳み表示 (Task 18) - store.ts: chat_tool_external_use / chat_tool_external_result の event handler を追加 - 内部 tool_use とは別 handler で source='external' の block を append - external は承認概念なし (approval は付与しない) - tool-approval-card.tsx: source 分岐 - source='external' は <details> 折り畳み + 「外部ソース」ラベル + 承認 / 却下なし - source='internal' は既存通り承認 / 却下 button (退行なし) - 新規 test 2 件: external で承認 button 非表示 + details 折り畳み * fix(frontend): MCP サーバー addMcpServer の id 採番衝突を回避 旧実装は \`mcpServers.length + 1\` で id を採番していたため、 \`atlassian-1\` / \`atlassian-2\` の状態で 1 件削除して追加すると、 再び \`atlassian-2\` が生成されて key 衝突 → 1 件を黙って上書きしていた。 未使用 suffix を探索する方式に変更。 副次効果として React key も id だけで一意になるため、配列 index を key に使う必要が無くなり biome の \`noArrayIndexKey\` lint error を解消。 - addMcpServer: usedIds set を作り、最小未使用 suffix を採番 - map の key を \`s.id\` のみに変更 (index 削除) - regression test 追加: 削除→追加で id 衝突しない * test(ai-engine): source-url.test.ts の不要な __resetGuardsForTest を削除 CodeRabbit 指摘 (PR #18 2nd review): このテストは sourceUrlGuard を 直接呼んでおり、dispatchDuplicateGuard 経由のグローバル registry は 触らないため、beforeEach の reset 呼び出しは不要 + テスト順依存を 作るリスクがあった。 * fix(core): mcpServers[] の id 重複を superRefine で検出する buildMcpServers が Record<id, ...> にマップするため、重複 id を 登録すると後勝ちで silent override されつつ allowedTools には 両方残るため整合性が崩れる。codebases[] と対称に checkUniqueMcpServerIds を ProjectMetaSchema / ProjectSchema / ProjectMetaPatchSchema の superRefine に組み込む。 codex セカンドオピニオン (PR #19) Major 1 指摘対応。 * fix(ai-engine): 空 assistant message の永続化を MCP 構築成功後に移動 env 未設定で buildMcpServers が throw した場合、これまでは空 blocks の assistant message が YAML に永続化された後に error event を出して return していた。リロード時には buildChatPrompt が空 message を スキップするため AI への影響はないが、chat 履歴 UI には空のアシスタント バブルが蓄積する問題があった。 assistantMsgId の生成は handler が参照するため先に行い、永続化のみを MCP 構築成功後に遅延させる。 codex セカンドオピニオン (PR #19) Major 2 指摘対応。 * fix(ai-engine): buildChatPrompt の tool_result.output を XML エスケープ tool_result の output は外部 MCP の生出力 (Jira 本文・epic 説明等) で </tool_result> や < を含む可能性があり、prompt の XML 構造を 壊して AI が前ターンの context を誤読する恐れがあった。 escapeXmlText で & < > の最低限のみエスケープ。tool_use.input 側は JSON.stringify が < を \\u003c にエスケープするため対象外。 codex セカンドオピニオン (PR #19) Major 3 指摘対応。 * fix(ai-engine): tool_use/属性も XML エスケープし、コメントの誤りを訂正 前 commit で追加した escapeXmlText は tool_result.output のみを対象として おり「JSON.stringify が < を \\u003c にエスケープするので tool_use 側は 安全」とコメントしていたが、これは事実誤認だった。JSON.stringify が エスケープするのは " \\ と control chars のみで、< > & は素通しする。 このため input オブジェクトに </tool_use> や < が含まれると prompt の XML 構造が壊れて AI が前ターンの context を誤読する恐れがあった。 - escapeXmlAttr (& " < > エスケープ) を追加 - tool_use の input は escapeXmlText(JSON.stringify(...)) でエスケープ - 属性値 (toolUseId / name / role) は escapeXmlAttr でラップ - コメントの誤りを訂正 CodeRabbit (PR #19) Major 1 指摘対応。 * fix(core): MCP server URL の userinfo (user:pass@) を拒否 これまでの url validator は \`https://user:pass@host/mcp\` を許容していた。 URL 内資格情報はログ・プロキシ・ブラウザ履歴に漏洩しやすく、本実装が Authorization header 経由で PAT を送る設計とも不整合。 parsed.username / parsed.password が非空なら reject する。 CodeRabbit (PR #19) Major 2 指摘対応 (Quick win)。 * fix(frontend): MCP サーバー一覧の React key を不変 _uid に分離 React key として s.id を使っていたが、id はフォームから編集可能で あり、編集中に key が変わると React が要素を再マウントしてしまい 入力フォーカスや state がリセットされるリスクがあった。重複入力時 には reconciliation の混乱で間違った DOM ノードが再利用される 可能性もある。 UI ローカルの不変 ID として _uid をマウント時に 1 度だけ割り当て、 React key には _uid を使う。永続化前 (onSave) に _uid を剥がす。 CodeRabbit (PR #19) Major 3 指摘対応。 * test(frontend): CR 指摘のテスト精度向上 (mcp 置換 / details 中身 / secret UI 前提) CodeRabbit (PR #19) Minor 3 件 (Quick win) 対応: - route.test.ts: mcpServers 全置換テストに事前データを投入し、 「マージ追記」では通らないようにする。 - tool-approval-card.test.tsx: <details> の存在だけでなく、その内側に input preview の <pre> が含まれていることまで検証する。 - project-settings-dialog.test.tsx: secret UI 検証を実 MCP 行に対する ものに変更 (空 list だと退行検知が効かないため、+ MCP サーバーを追加 をクリックしてから tokenEnvVar 存在 + secret 入力欄非存在を確認)。 * fix(ai-engine): tool_result の external 誤分類防止 + text 本文も XML エスケープ CodeRabbit (PR #19) 2 周目 Major 2 件対応: [Major 1] tool_result を無条件に external として永続化していたため、SDK 仕様 変更や edge case で内部 (mcp__tally__*) の tool_result が SDK ストリームに 流れた場合に誤分類されるリスクがあった。同 turn 内で観測した外部 tool_use の toolUseId を Set に保持し、対応する tool_result のみ external として扱う。 集合に無い toolUseId (内部 / intercept 経路 / 想定外) は無視する。 [Major 2] buildChatPrompt で <message> 内の text 本文と <current_user_message> の本文が未エスケープだった。前 commit で tool_use/tool_result はエスケープ したが、自由入力の text block 自体は <message>/</message> や < & が混入し うる。escapeXmlText でラップ。 test: 既存の Task 13 テスト 2 件 (4KB 超 truncate / 4KB 以下 not truncate) は tool_result のみ yield していたが、Major 1 ガード後は tool_use 先行が 必須なので mock を整備 (tool_use → tool_result の順)。 * test(frontend): 「mcpServers 全消去」テストの seed PATCH に status assert CodeRabbit (PR #19) 2 周目 Minor 1 件対応 (Quick win)。 事前登録ステップが silent fail すると後段の「空配列で消す」前提が崩れて 偽の成功で通る恐れがあったため、seed PATCH の status を 200 で assert する。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Atlassian MCP C フェーズの仕上げと、その過程で見つかった storage 周りのバグ修正、ポート整理をまとめた PR。
変更内容
`265a43c` fix(storage): YAML flow→block 強制
`031d705` feat(chat): OAuth 2.1 auth フロー 1 等地化 + long-lived Query
`a676e67` chore: default port 変更
テスト
```
pnpm typecheck # 4/4 packages 通過
pnpm test # 87 + 88 + 207 + 271 = 653 PASS
```
新規テスト:
残作業 / 既知の課題
Test plan
Summary by CodeRabbit
リリースノート
新機能
バグ修正
ドキュメント