From 03923894d1a26c8fa66df4bbc134a5f21344939d Mon Sep 17 00:00:00 2001 From: Codex_Lin_Lay Date: Fri, 2 Oct 2026 21:19:08 +0900 Subject: [PATCH] fix(memory): preserve resolved sources and canonical deletion state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 識別できない返却資料だけを除外し、正常資料を原子的に保存する。部分記録の状態・件数・理由を応答と履歴settingsに残し、全件未解決と正常0件を区別する。 graphの実経路nodeを検証し、不明経路を強化対象から除外する。issue_comment削除で索引片側の失敗後にcanonicalを消す欠陥を修正し、503と明示的な再配送を確認する。旧D1のID欠落原因と残存1件の未確定な起源を調査記録に分離した。 検証: unit/Workers 410件、bridge 33件、型検査、schema一致、diff whitespace成功。 Refs #263 --- README.ja.md | 2 + README.md | 2 + docs/0-requirements.ja.md | 4 + docs/0-requirements.md | 4 + docs/2-feedback-memory.ja.md | 14 +++- docs/2-feedback-memory.md | 12 ++- docs/3-unresolved-source-investigation.ja.md | 44 +++++++++++ mcp-server/server/memory-tools.json | 4 +- mcp-server/server/search-schema.json | 83 ++++++++++++++++++++ src/graph.ts | 15 ++-- src/mcp.ts | 5 +- src/memory-api.test.ts | 31 ++++++++ src/memory-api.ts | 57 +++++++++++--- src/memory-contract.ts | 4 +- src/memory-e2e.workers.test.ts | 74 +++++++++++++++++ src/search-contract.ts | 4 +- src/webhook-comment-deletion.workers.test.ts | 49 ++++++++++++ src/webhook.ts | 25 +++--- 18 files changed, 394 insertions(+), 39 deletions(-) create mode 100644 docs/3-unresolved-source-investigation.ja.md create mode 100644 src/memory-api.test.ts create mode 100644 src/webhook-comment-deletion.workers.test.ts diff --git a/README.ja.md b/README.ja.md index 583c621..39c0900 100644 --- a/README.ja.md +++ b/README.ja.md @@ -101,6 +101,8 @@ GitHub webhooks + GitHub API 成功検索は既定で UTC trace を保存し、資料の版を指す `source_id` と現在 activation を返します。`memory_history` で履歴・利用段階・取消し audit を読み、`record_source_use` で selected→validated→used、`record_outcome` で confirmed または receipt を指定した corrected/rolled_back を記録します。検索だけでは used/confirmed になりません。保存不可の結果には trace がなく feedback 不可です。半減期3600秒、activation は各 channel 上限10（total20）、graph strength 上限5です。成功検索に DO request と保存コストが加わります。 +識別できない資料が混ざっても正常資料の trace は保存します。`memory_recording` が部分記録の件数・理由を返し、履歴の `settings.memory_recording` に残ります。未解決 row は `feedback_available:false` で source_id を持ちません。全資料未解決は trace を発行せず、正常な0件検索は空 trace を保存します。検証不可の graph path は強化されず、正常な終点資料の利用記録は可能です。検索結果・順序は維持します。 + live inline doc/wiki 本文は返した本文の SHA-256 で版を識別し、`github_live` provenance を持ちます。stored fetch と graph 本文は index snapshot の版で、索引 timestamp が同じでも live 本文の変更は別 source_id になります。 [仕様・移行・合成実演](docs/2-feedback-memory.ja.md) を参照してください。0008 を Worker deploy より先に適用し、新 schema JSON を bridge artifact に同梱します。検索品質向上は未評価です。 diff --git a/README.md b/README.md index 1b242af..7f6b74c 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,8 @@ This MCP server exposes `search` and three private feedback tools. All retrieval Successful retrieval writes a UTC trace by default, returning version-specific `source_id` and current activation. `memory_history` reads history, stages and reversal audit; `record_source_use` records selected→validated→used; `record_outcome` confirms a saved path or corrects/rolls back one named receipt. Retrieval alone is never usage or confirmation. Memory failure emits no usable trace. Half-life is 3600 seconds, each activation channel caps at 10 (total 20), and relation strength caps at 5. Successful retrieval adds a DO request and storage cost. +Unresolved identities exclude only affected sources; valid sources still save a trace. `memory_recording` reports partial counts/reasons and persists in history settings. Unresolved rows have feedback_available:false and no source_id. All-unresolved results save no trace; genuine zero-hit retrieval saves an empty trace. Unverifiable graph paths cannot receive credit, while resolved terminals can record usage. Retrieval rows and order remain intact. + Live inline doc/wiki bodies use the returned-text SHA-256 version and `github_live` provenance. Stored fetch and graph bodies refer to the index snapshot; changed live text has another source ID even with the same index timestamp. See the [specification, migration and synthetic lifecycle](docs/2-feedback-memory.md). Apply 0008 before Worker deployment and include the new schema JSON in bridge artifacts. Retrieval quality improvement has not been evaluated. diff --git a/docs/0-requirements.ja.md b/docs/0-requirements.ja.md index f66d394..01f24be 100644 --- a/docs/0-requirements.ja.md +++ b/docs/0-requirements.ja.md @@ -666,6 +666,10 @@ embedding が失敗した record は incomplete と分かる形で残し、次 新 API は `memory_history`、`record_source_use`、`record_outcome`。設定値、上限、ID/version、batch/idempotency、ledger、error、0008先行移行、既存行 backfill と artifact の契約は [2-feedback-memory.ja.md](2-feedback-memory.ja.md) に定義する。会話/本文/credential の複製は行わず、品質向上の主張は別評価を必要とする。 +Issue #263: canonical identity の失敗は資料単位で除外し、正常資料は既存の原子的保存に渡す。検索結果と順序は維持し、部分記録の状態・除外件数・理由を応答と保存 settings に残す。全資料未解決は正常な0件検索と区別する。未解決資料に source_id を発行せず、feedback を不可とする。経路の起点・中間・終点を確認できない graph path は強化対象から除外する。保存失敗と実際の部分取得は引き続き trace を発行しない。 + +Issue #263 の調査で再現した issue_comment.deleted の部分削除: Vectorize とFTSの両方が削除成功するまでDO canonicalを保持し、片側失敗とDO削除失敗は503を返す。自動再試行は追加しない。対象の残存1件との因果は未確認であり、[調査記録](3-unresolved-source-investigation.ja.md) に事実と仮説を分けて保存する。本番DB変更・一括補修・版番号更新・releaseはこの修正に含めない。 + #259 の版契約: live inline doc/wiki は返した本文の SHA-256 を version とし、index snapshot と provenance を分ける。同じ索引 timestamp でも live 本文更新は別 source_id にする。本文を memory に複製しない。 ## リリース metadata の一致（Issue #261） diff --git a/docs/0-requirements.md b/docs/0-requirements.md index 403aee2..9a2dea9 100644 --- a/docs/0-requirements.md +++ b/docs/0-requirements.md @@ -671,6 +671,10 @@ Implement retrieval history, selected/validated/used records, retrieved/usage ac The APIs are `memory_history`, `record_source_use`, and `record_outcome`. [2-feedback-memory.md](2-feedback-memory.md) owns constants, bounds, source version identity, batch/idempotency, ledger, errors, migration 0008-before-deployment, historical backfill and bridge artifacts. Do not duplicate conversations, bodies or credentials. Claims of retrieval quality improvement require separate evaluation. +Issue #263: exclude canonical identity failures per source and pass valid sources to the existing atomic save. Preserve retrieval rows and order; return and persist partial-recording status, exclusion counts and reasons in settings. Distinguish all-unresolved from a genuine zero-hit search. Unresolved rows receive no source_id and cannot accept feedback. Exclude graph paths from reinforcement unless the origin, intermediates and terminal can be verified. Storage failures and incomplete retrieval continue to emit no trace. + +Issue #263 investigation reproduces partial issue_comment.deleted teardown: retain DO canonical identity until both Vectorize and FTS deletions succeed. Index or DO deletion failure returns 503; no automatic retry is added. Causality for the remaining production row is unverified; [investigation](3-unresolved-source-investigation.ja.md) separates evidence from hypotheses. Production DB mutation, bulk repair, version updates and releases are outside this fix. + Issue #259 version contract: live inline doc/wiki uses the returned-text SHA-256 version and provenance separate from the index snapshot. Changed live text gets a different source ID even at the same index timestamp. Do not duplicate the body in memory. ## Release metadata consistency (Issue #261) diff --git a/docs/2-feedback-memory.ja.md b/docs/2-feedback-memory.ja.md index 6df0c1c..522fd9c 100644 --- a/docs/2-feedback-memory.ja.md +++ b/docs/2-feedback-memory.ja.md @@ -18,7 +18,17 @@ principal は検証済み MCP OAuth props の numeric GitHub user ID からサ limit は1..50、trace の資料 snapshot は300件、保存入力は300,000文字、query は4096文字、metadata string は256文字、usage batch は100 entry、reason/key は1000/128文字まで。SQL は owner/cursor index と有界 page を使う。全履歴をまとめて読む API はない。 -未成立・失敗を成功 trace として保存しない。一部 scan source、sparse retrieval、graph expansion の失敗、canonical identity 未解決、memory の原子的書込失敗では検索結果を返せるが、`memory_unavailable:true`、`feedback_available:false` とし、**trace_id を発行しない**。この結果への feedback は不可。拒否 error は段階違反、key conflict、confirmation 等の安全な code を返し、query/handle を error や log へ転記しない。検索の error log は固定文言にする。 +未成立・失敗を成功 trace として保存しない。一部 scan source、sparse retrieval、graph expansion の失敗と memory の原子的書込失敗では検索結果を返せるが、`memory_unavailable:true`、`feedback_available:false` とし、**trace_id を発行しない**。`memory_error` はそれぞれ `retrieval_incomplete` / `memory_save_failed`。拒否 error は段階違反、key conflict、confirmation 等の安全な code を返し、query/handle を error や log へ転記しない。検索の error log は固定文言にする。 + +### 資料単位の部分記録（Issue #263） + +canonical identity または live version が未解決の資料は、検索結果・順序・本文を維持して、その資料だけ保存から除外する。正常資料があれば既存 transaction で原子的に保存し `trace_id` と全体の `feedback_available:true` を返す。未解決 row は `feedback_available:false`、`memory_exclusion_reason:canonical_identity_unavailable` または `live_version_unavailable` を持ち、source_id/provenance/activation を付けない。`results`、`graph_results`、両軸の `same_entity.others` に適用する。 + +応答の `memory_recording` は `status:complete|partial|unresolved`、`recorded_sources`（保存した distinct source 数）、`excluded_sources`（除外した返却 row 数）、`exclusions:[{location,reason}]` を返す。同じ未解決資料が複数箇所にあれば各 row を数える。location は `results[0].same_entity.others[1]` 等の返却位置のみ。本文・タイトル・vector ID を除外記録へコピーしない。同じ object を `settings.memory_recording` として保存し、history 一覧・詳細の両方で再読できる。古い trace の settings にこの field が無い場合は従来の記録。 + +全資料未解決は `status:unresolved`、`memory_error:all_sources_unresolved`、memory_unavailable/feedback不可、trace無し。正常な0件検索は `status:complete`、件数0の空 trace を保存する。識別以外の例外とDB書込失敗は資料単位の除外として扱わない。 + +graph path の起点・中間・終点は実際に探索した vector ID 列から索引 metadata を読む。返却枠外・dangling・identity 未解決の中間も検証し、repo と directional mention の両端 slug を照合する。確認不可の path は保存 snapshot で `path:[]` とし、返却 graph_path は維持する。終点自体が正常なら利用段階は記録可能だが `graph_feedback_available:false` と `graph_feedback_exclusion_reason:unverifiable_graph_path` を返し、confirmed は `no_graph_path` で非適用。`excluded_graph_paths` / `graph_exclusions` を memory_recording に保存する。確認できた他の path の強化・receipt・取消しは継続する。内部 node snapshot は返却・settings・history に含めない。graph metadata 読取は返却 path 当たり最大3 node の集合に限定し、保存形式の移行は不要。 ## 利用段階と retry @@ -72,7 +82,7 @@ npx wrangler d1 execute github-rag-fts --remote --command "SELECT type,COUNT(*) deploy 後の認可された `POST /admin/backfill-source-identities?repo=owner/repo&limit=50[&cursor=TYPE:ID]` は、既存 DO の canonical event ID から過去の unchanged 行を補修する。既存管理 credential は非公開 header で渡し、URL / artifact / shell history に token を載せない。next_cursor を渡して done:true まで進める。limit は1..100、1 page は最大3つの有界 DO read と一つの原子的 D1 batch。再開・再実行は安全で、embedding / GitHub 本文再取得 / index reset は不要。 -通常の unchanged ingest も hash skip の前に ID を補修する。DO に canonical event が無い行は未解決のままなので、別の再取り込み前に欠損を調べる。未解決 source を含む検索は memory を fail-closed にする。backfill は private trace/credit を変更しない。 +通常の unchanged ingest も hash skip の前に ID を補修する。DO に canonical event が無い行は未解決のままなので、別の再取り込み前に欠損を調べる。未解決 source 自体は feedback 不可とし、正常 source は上記の部分記録を適用する。backfill は private trace/credit を変更しない。旧ID欠落と残存1件の調査、再現した削除経路の修正は [調査記録](3-unresolved-source-investigation.ja.md) を参照。 bridge には `server/search-schema.json` と `server/memory-tools.json` を同梱する。`node scripts/generate-tool-contracts.mjs` で Worker Zod contract から生成し、`node scripts/check-schema-drift.mjs` が全 tool の実 protocol と nested 入出力 schema / bounds / defaults / annotations の完全一致を確認する。npm / mcpb の両 artifact にこの JSON が必要。stdio client の tool discovery には更新 bridge の公開が必要。公開 version と publication は後続の運用段階で決める。 diff --git a/docs/2-feedback-memory.md b/docs/2-feedback-memory.md index ddbb475..8597a23 100644 --- a/docs/2-feedback-memory.md +++ b/docs/2-feedback-memory.md @@ -14,7 +14,15 @@ The server derives `github:` from verified MCP OAuth pro `memory_history {limit:10}` returns newest trace summaries and `next_cursor`; pass that cursor for the next page. `memory_history {trace_id,limit:10}` returns sources, current stages/activation, settings and oldest-first receipt audit. Its `next_cursor` pages **receipts within that trace**, not trace summaries. Cursors belong to the authenticated principal and, for details, the trace. Limits are 1..50; source snapshots cap at 300 per trace, saved trace input at 300,000 characters, queries at 4096 characters, metadata strings at 256, usage batches at 100 entries, and reason/idempotency handles at 1000/128 characters. SQL reads use owner/cursor indexes and bounded pages; no full-history fetch is used. -A failed retrieval is not recorded as successful. If retrieval is partial (a scan surface, sparse retrieval, or graph expansion failed), identity cannot be resolved, or the atomic memory write fails, retrieval can still return with `memory_unavailable: true`, `feedback_available: false` and **no `trace_id`**. It cannot accept feedback. Memory rejection codes describe ownership, stage, key conflict and confirmation errors without echoing the submitted query or handles. Retrieval error logging emits fixed messages. +A failed retrieval is not recorded as successful. If retrieval is partial (a scan surface, sparse retrieval, or graph expansion failed), or the atomic memory write fails, retrieval can still return with `memory_unavailable: true`, `feedback_available: false` and **no `trace_id`**. `memory_error` distinguishes `retrieval_incomplete` and `memory_save_failed`. Memory rejection codes describe ownership, stage, key conflict and confirmation errors without echoing the submitted query or handles. Retrieval error logging emits fixed messages. + +### Partial source recording (Issue #263) + +Canonical identity and live-version failures exclude only the affected rows from storage; retrieval rows, order and text remain intact. Valid sources commit atomically with a trace and overall `feedback_available:true`. Unresolved rows return `feedback_available:false` and `memory_exclusion_reason:canonical_identity_unavailable|live_version_unavailable`, without source_id/provenance/activation. This applies to results, graph_results and their same_entity.others members. + +`memory_recording` returns status complete/partial/unresolved, recorded_sources (distinct saved sources), excluded_sources (excluded returned occurrences), and exclusions [{location,reason}]. Locations name response positions only, e.g. results[0].same_entity.others[1]; no text, title or vector ID is copied. The same object persists as settings.memory_recording in history summaries and details. Old traces lack this field. All-unresolved results emit memory_error:all_sources_unresolved, memory_unavailable and no trace. Genuine zero-hit retrieval saves an empty complete trace. Unexpected exceptions and storage failures are not classified as row exclusions. + +Graph origins, intermediates and terminals are checked against indexed metadata using the actual traversed vector IDs, including nodes omitted from the output and dangling nodes. Verify canonical identity, repository and directional mention endpoint slugs. An unverifiable path saves path:[] while retaining the returned graph_path. A resolved terminal can record usage but returns graph_feedback_available:false and graph_feedback_exclusion_reason:unverifiable_graph_path; confirmation yields no_graph_path without credit. memory_recording retains excluded_graph_paths and graph_exclusions. Other verified paths keep confirmation receipts and reversal. Internal node snapshots are removed before response/storage; graph metadata reads cover the union of at most three nodes per returned path. No storage migration is needed. ## Usage and idempotency @@ -62,7 +70,7 @@ npx wrangler d1 execute github-rag-fts --remote --command "PRAGMA table_info(sea npx wrangler d1 execute github-rag-fts --remote --command "SELECT type,COUNT(*) AS unresolved FROM search_docs WHERE (type IN ('issue_comment','pr_review_comment') AND comment_id=0) OR (type='pr_review' AND review_id=0) GROUP BY type" ``` -After deployment, authorized `POST /admin/backfill-source-identities?repo=owner/repo&limit=50[&cursor=TYPE:ID]` repairs historical unchanged rows from existing canonical DO events. Supply the existing administrative credential through a private header; no token belongs in a URL, artifact or shell history. Follow `next_cursor` until `done:true`. Limit 1..100, at most three bounded DO reads and one atomic D1 batch per page. Restarting is safe; no embeddings, GitHub body refetch or index reset is needed. Normal unchanged comment/review ingest also repairs IDs before its hash-skip return. Rows whose canonical event is absent from the DO remain unresolved; inspect that gap before any separate reingest. A trace containing unresolved source identity fails memory closed. Backfill does not rewrite private traces/credits. +After deployment, authorized `POST /admin/backfill-source-identities?repo=owner/repo&limit=50[&cursor=TYPE:ID]` repairs historical unchanged rows from existing canonical DO events. Supply the existing administrative credential through a private header; no token belongs in a URL, artifact or shell history. Follow `next_cursor` until `done:true`. Limit 1..100, at most three bounded DO reads and one atomic D1 batch per page. Restarting is safe; no embeddings, GitHub body refetch or index reset is needed. Normal unchanged comment/review ingest also repairs IDs before its hash-skip return. Rows whose canonical event is absent from the DO remain unresolved; inspect that gap before any separate reingest. Unresolved sources cannot accept feedback; valid sources use partial recording above. Backfill does not rewrite private traces/credits. See the [investigation](3-unresolved-source-investigation.ja.md) for historical ID omission, the remaining row, and the reproduced deletion-path fix. The bridge includes `server/search-schema.json` and `server/memory-tools.json`. `node scripts/generate-tool-contracts.mjs` regenerates them from Worker Zod schemas; `node scripts/check-schema-drift.mjs` compares all shipped tools' exact nested input/output contracts, bounds, defaults and annotations against the actual protocol. Package both JSON files in npm/mcpb artifacts. Worker deployment enables direct-client tools; the published bridge must include the new artifacts for stdio clients to discover them. Public version selection and publication are subsequent operations. diff --git a/docs/3-unresolved-source-investigation.ja.md b/docs/3-unresolved-source-investigation.ja.md new file mode 100644 index 0000000..c2cdf51 --- /dev/null +++ b/docs/3-unresolved-source-investigation.ja.md @@ -0,0 +1,44 @@ +# 未解決 source の調査（Issue #263） + +調査日: 2026-10-02 JST。対象: [Issue #263](https://github.com/Liplus-Project/github-rag-mcp/issues/263)。基準は v0.12.0 / `c3e5126363386a7665cbe56087ecd5cbc5cf7a6a`。本番管理操作・DB変更・一括修復の再実行は行っていない。使った証拠は既存の補修CSV/結果、Git履歴、公開GitHub API、通常ユーザー向けRAGの限定stored-content fetch。 + +## 1. 旧行の comment_id/review_id が0になった理由（確定） + +[コメント取り込み導入 1edf74d](https://github.com/Liplus-Project/github-rag-mcp/commit/1edf74d3d0729096fbc9890a8a3d847fb776a580)（2026-04-23）では、Vectorize metadata と DO canonical record に GitHub event ID を保存していた。一方 `src/pipeline.ts` のコメント/review用 `upsertFtsRow` はIDを渡さず、当時のFTSスキーマにも両列が無かった。 + +[memory実装 4594bb6](https://github.com/Liplus-Project/github-rag-mcp/commit/4594bb6801f61f9a19e15bbee47e242d2bef3cbb) の `migrations/0008_source_event_identity.sql` は両列を `INTEGER NOT NULL DEFAULT 0` で追加した。そのため旧行のIDは0となる。取り込みのhash一致skipで本文が更新されなければ、旧行にevent IDを補う経路が必要だった。同実装は通常取り込みでIDを保存し、unchanged ingest と DO canonical backfill でも補修するよう変更済み。 + +既存Owner UI補修記録は issue_comment 952、pr_review 553、pr_review_comment 14、計1519件を修復し、総行数10545を維持した。未解決は1520→1。本修正はこれらの成功済み補修や個人memoryを変更しない。 + +## 2. 残存1件が照合できない理由（未確定） + +既存証拠の位置は `D:/Users/hal/Codex/release-staging/github-rag-259-operations/`。`OWNER-UI-ID-REPAIR-RESULT.ja.md`、`canonical-event-ids.csv`、`unmapped-event-ids.csv`、`verified-id-repair-map.csv` を根拠とする。秘密情報を含むファイルは参照していない。 + +| 観測 | 値 | +|---|---| +| repo/type/parent | Liplus-Project/liplus-language / issue_comment / #1428 | +| vector_id | `ic:B9kDYNPm4eKCVyazFWiY_sjR6YvOPt6Cw-OVIOi1OEE` | +| ID列 | comment_id=0、既存補修証拠ではreview_id=0 | +| indexed row updated_at | 2026-05-30T11:15:02Z | +| indexed body | 著者prefix `liplus-lin-lay`、見出し「中間検証報告（feasibility-first / 進捗ログ）」、2443文字、content_truncated=false | +| 現在の #1428 コメント | ID 4582731637、created/updated 2026-05-30T11:47:48Z、重複close報告 | + +限定fetchは該当vector IDの1件のみを指定し、本文とmetadataを返した。調査時の現行Workerはその1件だけでmemory_unavailable=true、feedback_available=false、trace無し。旧行の本文・timestampは[現在の #1428 コメント](https://github.com/Liplus-Project/liplus-language/issues/1428#issuecomment-4582731637)と異なる。[#1430](https://github.com/Liplus-Project/liplus-language/issues/1430)の現在のコメント2件（ID4582772963、4582774279）も12:07:52Z/12:08:28Zのverdict/close報告で、同じ本文ではない。 + +comment導入時と現行のvector ID生成はどちらも `ic:base64url(SHA256(repo + NUL + String(comment_id)))`。このsurfaceの方式変更はGit履歴で見つからなかった。取得済みcanonical event集合で照合できず、現在の公開コメントでも出典を復元できない、というところまでが確認できた事実。IDの推測・brute force・新しいsource_idの捏造は行わない。 + +過去のコメント削除、取り込み時の不整合、別の運用によるcanonical欠落は候補であり、対象event ID・削除delivery・当時のbinding失敗ログが無いので原因の断定はできない。「現在のコメントにない」だけでは「削除済み」は確定しない。 + +## 3. 再現した削除経路の欠陥（確定、対象1件との因果は未確認） + +コメント導入時から現行まで `issue_comment.deleted` は Vectorize とFTSの削除例外を個別にcatchして、DOのcanonical削除を続行し、202 `result:deleted` を返す。FTS削除だけ失敗した場合、索引行は残るがcanonicalは消える。逆にVectorizeだけ失敗した場合も、その索引行の出典をDOで追えなくなる。DO fetchの非2xxも成功扱いだった。 + +Issue #263 ではこのhandlerだけを修正した。索引両surfaceの削除成功までDO canonicalを保持し、どちらかの失敗を503 `partial_delete` として返す。DO削除の例外・非2xxも503とする。ログは失敗surface、vector IDまたはrepo/comment ID、bindingエラー理由を保持し、本文や認証propsを記録しない。成功時は従来の202 deleted。部分削除後の明示的な再配送と既に削除済みの再配送も正常に扱う。自動再試行/queueは追加しておらず、503を自動回復の保証としない。 + +この再現は残存1件を説明できる機構を示すが、その行で実際に起きた証拠ではない。過去の既存行は本変更で修復されない。 + +## 4. 検証と適用範囲 + +`src/webhook-comment-deletion.workers.test.ts` は署名付きsynthetic deliveryとローカルDO/D1でFTSのみ失敗、Vectorizeのみ失敗、DO非2xx、DO例外、正常削除を確認する。失敗時のcanonical保持、失敗surfaceの索引状態、明示的再配送、削除済み再配送も確認する。 + +`src/memory-e2e.workers.test.ts` と `src/memory-api.test.ts` は両軸/同一entity、全件未解決と正常0件、原子的保存失敗、history一覧/詳細再読、未解決/返却されない中間を通るgraph path、正常pathのconfirmed receiptと取消し、識別以外の例外を確認する。仕様は [2-feedback-memory.ja.md](2-feedback-memory.ja.md)。新migration・binding・版番号更新は不要。品質や性能向上の測定はしていない。 diff --git a/mcp-server/server/memory-tools.json b/mcp-server/server/memory-tools.json index 6e383c7..7c0a0f9 100644 --- a/mcp-server/server/memory-tools.json +++ b/mcp-server/server/memory-tools.json @@ -1,9 +1,9 @@ { - "instructions": "Successful search/scan/fetch writes a private trace. Retrieval is not usage or confirmation. Call record_source_use with selected (chosen for investigation), then validated (exact source checked and judged usable), then used (actually used in an answer or decision). record_outcome confirmed reinforces only saved graph paths of used sources. corrected/rolled_back names the confirmation receipt to reverse. Use a new idempotency_key for each operation; retry the same payload with the same key. memory_history lists traces or reads one trace. use_memory only orders graph candidates within the same hop; keyword scores/ranks never change. memory_unavailable means no usable trace was saved. Live inline doc/wiki bodies use returned-content SHA-256 versions and github_live provenance; stored-content fetch refers to the indexed snapshot.", + "instructions": "Successful search/scan/fetch writes a private trace. memory_recording reports complete/partial/unresolved source recording, counts and exclusion reasons; saved settings retain it in memory_history. Unresolved rows have feedback_available=false and no source_id. All-unresolved results have no trace; a genuine zero-hit search saves an empty trace. Unverifiable graph paths cannot be reinforced; a resolved terminal can still record usage. Retrieval is not usage or confirmation. Call record_source_use with selected (chosen for investigation), then validated (exact source checked and judged usable), then used (actually used in an answer or decision). record_outcome confirmed reinforces only saved graph paths of used sources. corrected/rolled_back names the confirmation receipt to reverse. Use a new idempotency_key for each operation; retry the same payload with the same key. memory_history lists traces or reads one trace. use_memory only orders graph candidates within the same hop; keyword scores/ranks never change. memory_unavailable means no usable trace was saved; memory_error distinguishes retrieval_incomplete, memory_save_failed and all_sources_unresolved. Live inline doc/wiki bodies use returned-content SHA-256 versions and github_live provenance; stored-content fetch refers to the indexed snapshot.", "tools": [ { "name": "memory_history", - "description": "Read your private, bounded trace history. trace_id returns source usage, current activation, saved paths, and outcome/reversal audit. Otherwise limit/cursor pages trace summaries.", + "description": "Read your private, bounded trace history. settings.memory_recording retains source-recording status, exclusion counts and reasons in summaries and details. trace_id returns source usage, current activation, saved paths, and outcome/reversal audit. Otherwise limit/cursor pages trace summaries.", "inputSchema": { "$schema": "https://json-schema.org/draft/2020-12/schema", "type": "object", diff --git a/mcp-server/server/search-schema.json b/mcp-server/server/search-schema.json index 44c8f34..b0e212e 100644 --- a/mcp-server/server/search-schema.json +++ b/mcp-server/server/search-schema.json @@ -193,6 +193,89 @@ "memory_unavailable": { "type": "boolean" }, + "memory_error": { + "type": "string", + "enum": [ + "retrieval_incomplete", + "memory_save_failed", + "all_sources_unresolved" + ] + }, + "memory_recording": { + "type": "object", + "properties": { + "status": { + "type": "string", + "enum": [ + "complete", + "partial", + "unresolved" + ] + }, + "recorded_sources": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "excluded_sources": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "exclusions": { + "type": "array", + "items": { + "type": "object", + "properties": { + "location": { + "type": "string" + }, + "reason": { + "type": "string" + } + }, + "required": [ + "location", + "reason" + ], + "additionalProperties": false + } + }, + "excluded_graph_paths": { + "type": "integer", + "minimum": 0, + "maximum": 9007199254740991 + }, + "graph_exclusions": { + "type": "array", + "items": { + "type": "object", + "properties": { + "location": { + "type": "string" + }, + "reason": { + "type": "string" + } + }, + "required": [ + "location", + "reason" + ], + "additionalProperties": false + } + } + }, + "required": [ + "status", + "recorded_sources", + "excluded_sources", + "exclusions", + "excluded_graph_paths", + "graph_exclusions" + ], + "additionalProperties": false + }, "feedback_available": { "type": "boolean" } diff --git a/src/graph.ts b/src/graph.ts index a730fc2..2ed726c 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -31,6 +31,7 @@ export interface GraphNeighbor { hop: number; fromVectorId: string; path?: import("./memory.js").SavedEdge[]; + pathNodes?: string[]; } /** Escape a string for safe inclusion in a RegExp. */ @@ -166,24 +167,25 @@ export async function queryNeighbors( const limit = Math.max(1, Math.min(200, opts.limit ?? 50)); // Anchor: each seed at depth 0 with itself as origin. - const seedValues = seeds.map(() => "(?, 0, ?, json_array())").join(", "); + const seedValues = seeds.map(() => "(?, 0, ?, json_array(), json_array(?))").join(", "); const repoFilter = opts.repo ? "AND e.repo = ?" : ""; const sql = ` - WITH RECURSIVE reach(id, depth, origin, path) AS ( + WITH RECURSIVE reach(id, depth, origin, path, nodes) AS ( SELECT * FROM (VALUES ${seedValues}) UNION SELECT CASE WHEN e.src_vector_id = r.id THEN e.dst_vector_id ELSE e.src_vector_id END, r.depth + 1, r.origin, - json_insert(r.path, '$[#]', json_object('repo', e.repo, 'src', e.src_slug, 'dst', e.dst_slug, 'kind', e.edge_kind)) + json_insert(r.path, '$[#]', json_object('repo', e.repo, 'src', e.src_slug, 'dst', e.dst_slug, 'kind', e.edge_kind)), + json_insert(r.nodes, '$[#]', CASE WHEN e.src_vector_id = r.id THEN e.dst_vector_id ELSE e.src_vector_id END) FROM doc_edges e JOIN reach r ON (e.src_vector_id = r.id OR e.dst_vector_id = r.id) WHERE r.depth < ? ${repoFilter} ) - SELECT id, MIN(depth) AS hop, origin, path + SELECT id, MIN(depth) AS hop, origin, path, nodes FROM reach WHERE depth > 0 GROUP BY id @@ -192,7 +194,7 @@ export async function queryNeighbors( `; const binds: (string | number)[] = []; - for (const s of seeds) binds.push(s, s); // (id, origin) per seed VALUES row + for (const s of seeds) binds.push(s, s, s); // (id, origin, first node) binds.push(hops); if (opts.repo) binds.push(opts.repo); binds.push(limit); @@ -200,7 +202,7 @@ export async function queryNeighbors( const res = await db .prepare(sql) .bind(...binds) - .all<{ id: string; hop: number; origin: string; path: string }>(); + .all<{ id: string; hop: number; origin: string; path: string; nodes: string }>(); const seedSet = new Set(seeds); return (res.results ?? []) @@ -209,6 +211,7 @@ export async function queryNeighbors( hop: Number(r.hop ?? 0), fromVectorId: String(r.origin ?? ""), path: JSON.parse(r.path), + pathNodes: JSON.parse(r.nodes), })) .filter((n) => n.vectorId.length > 0 && !seedSet.has(n.vectorId)); } diff --git a/src/mcp.ts b/src/mcp.ts index ecd00d3..7cc62e3 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -1107,7 +1107,7 @@ export function createRagMcpServer(env: Env): McpServer { if (fresh.length > 0) { const enrich = await getDocsByVectorIds( env.DB_FTS, - fresh.map((n) => n.vectorId), + fresh.flatMap((n) => n.pathNodes ?? [n.vectorId]), ); const slugOf = (vid: string): string => { const p = payload.get(vid); @@ -1117,7 +1117,8 @@ export function createRagMcpServer(env: Env): McpServer { const row = enrich.get(n.vectorId); if (!row) continue; // dangling edge (target not indexed) — skip graphItems.push( - { ...buildGraphItem(n, row, slugOf(n.fromVectorId), includeContent), ...(use_memory ? { learned_strength: Number((n as any).learnedStrength) } : {}) }, + Object.assign({ ...buildGraphItem(n, row, slugOf(n.fromVectorId), includeContent), ...(use_memory ? { learned_strength: Number((n as any).learnedStrength) } : {}) }, + { memory_graph_nodes: (n.pathNodes ?? []).map(id => enrich.get(id) ?? null) }), ); } } diff --git a/src/memory-api.test.ts b/src/memory-api.test.ts new file mode 100644 index 0000000..1f92961 --- /dev/null +++ b/src/memory-api.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect, vi } from 'vitest'; +import { rememberResult } from './memory-api.js'; +import type { Env } from './types.js'; +vi.mock('agents/mcp/server', () => ({ getMcpAuthContext: () => ({props:{githubUserId:9263}}) })); +const row = (identity: string) => ({repo:'synthetic/unit',type:'wiki_doc',wiki_path:identity,updated_at:'2026-01-01T00:00:00Z'}); +function fixture() { + const saved: any[] = []; + const env = { ISSUE_STORE:{idFromName:() => 'global',get:() => ({ fetch:async (r: Request) => { + const data = await r.json() as any; saved.push(data.args); + return Response.json({trace_id:'synthetic-trace',timestamp:'2026-01-01T00:00:00Z',policy:{},sources:data.args.sources.map((s: any) => ({...s,activation:{retrieved:0.1,usage:0,total:0.1,updated_at:'2026-01-01T00:00:00Z'}}))}); + }})} } as unknown as Env; + return {env,saved}; +} +describe('source-level memory failure classification', () => { + it('handles mixed axes and folded members independently, including invalid live versions', async () => { + const {env,saved} = fixture(); + const bad = {repo:'synthetic/unit',type:'pr_review',number:1,review_id:0,updated_at:'2026-01-01T00:00:00Z'}; + const payload = {count:1,mode:'search',results:[{...bad,same_entity:{others:[row('good-fold')]}}],graph_results:[{...row('good-graph'),same_entity:{others:[{...row('bad-live'),content_source:'github_live',content_version:'invalid'}]}}]}; + const result = await rememberResult(env,payload,{query:'synthetic'}); + expect(result.memory_recording).toMatchObject({status:'partial',recorded_sources:2,excluded_sources:2,exclusions:[{location:'results[0]',reason:'canonical_identity_unavailable'},{location:'graph_results[0].same_entity.others[0]',reason:'live_version_unavailable'}]}); + expect(result.results[0].source_id).toBeUndefined(); expect(result.results[0].same_entity.others[0].source_id).toBeTruthy(); + expect(saved[0].sources).toHaveLength(2); expect(saved[0].settings.memory_recording).toEqual(result.memory_recording); + expect(saved[0].settings.results).toBeUndefined(); expect(saved[0].settings.graph_results).toBeUndefined(); + }); + it('does not classify an unexpected processing exception as an excluded source', async () => { + const {env,saved} = fixture(); const bad = {...row('unexpected'),get updated_at(): string {throw new Error('Synthetic unexpected exception');}}; + const result = await rememberResult(env,{count:2,mode:'fetch',results:[row('valid'),bad]},{}); + expect(result.memory_error).toBe('memory_save_failed'); expect(result.memory_recording).toBeUndefined(); + expect(result.trace_id).toBeUndefined(); expect(result.feedback_available).toBe(false); expect(saved).toEqual([]); + }); +}); diff --git a/src/memory-api.ts b/src/memory-api.ts index 9cad143..af75c05 100644 --- a/src/memory-api.ts +++ b/src/memory-api.ts @@ -23,15 +23,18 @@ export function registerMemoryTools(server: McpServer, env: Env) { } async function digest(s: string) { return Array.from(new Uint8Array(await crypto.subtle.digest('SHA-256', new TextEncoder().encode(s))), x => x.toString(16).padStart(2, '0')).join(''); } export async function hashReturnedContent(content: string): Promise { return 'sha256:' + await digest(content); } +class SourceIdentityError extends Error { + constructor(public readonly reason: 'canonical_identity_unavailable' | 'live_version_unavailable') { super(reason); } +} export async function identifySource(row: Record, axis: string): Promise { const repo = String(row.repo ?? ''); const type = String(row.type ?? ''); const path = type === 'wiki_doc' ? (row.wiki_path || row.doc_path || '') : type === 'doc' ? (row.doc_path || '') : (row.file_path || ''); const event = type === 'pr_review' ? row.review_id : ['issue_comment', 'pr_review_comment'].includes(type) ? row.comment_id : null; - if (!repo || !type || !row.updated_at || (event !== null && !event)) throw new Error('Canonical source identity unavailable'); + if (!repo || !type || !row.updated_at || (event !== null && (!Number.isSafeInteger(event) || event <= 0))) throw new SourceIdentityError('canonical_identity_unavailable'); const identity = ['doc', 'wiki_doc'].includes(type) ? path : type === 'diff' ? [row.commit_sha, path] : type === 'release' ? row.tag_name : event ?? row.number; - if (!identity || (Array.isArray(identity) && identity.some(x => !x))) throw new Error('Canonical source identity unavailable'); + if (!identity || (Array.isArray(identity) && identity.some(x => !x))) throw new SourceIdentityError('canonical_identity_unavailable'); const live = row.content_source === 'github_live'; - if (live && (!['doc', 'wiki_doc'].includes(type) || !/^sha256:[0-9a-f]{64}$/.test(row.content_version ?? ''))) throw new Error('Live source version unavailable'); + if (live && (!['doc', 'wiki_doc'].includes(type) || !/^sha256:[0-9a-f]{64}$/.test(row.content_version ?? ''))) throw new SourceIdentityError('live_version_unavailable'); const canonical = { repo, type, identity, version: live ? row.content_version : row.updated_at, content_source: live ? 'github_live' : 'index' }; const provenance = { ...canonical, ...(live ? { index_updated_at: row.updated_at } : {}) }; return { source_id: 's:' + await digest(JSON.stringify(canonical)), provenance, axes: [axis], path: row.graph_path ?? [] }; @@ -39,21 +42,57 @@ export async function identifySource(row: Record, axis: string): Pr /** Commit a successful result before emitting its trace. No body, title, handle, or auth props is stored. */ export async function rememberResult(env: Env, payload: any, request: unknown) { const incomplete = payload.memory_incomplete; delete payload.memory_incomplete; - if (incomplete) { return { ...payload, memory_unavailable: true, feedback_available: false }; } + // Internal traversal snapshots include nodes omitted by the graph output cap. + const graphNodes = new Map(); + for (const row of payload.graph_results ?? []) { graphNodes.set(row, row.memory_graph_nodes ?? []); delete row.memory_graph_nodes; } + if (incomplete) { return { ...payload, memory_unavailable: true, memory_error: 'retrieval_incomplete', feedback_available: false }; } try { const entries: { row: Record; source: SavedSource }[] = []; + const excluded: { location: string; reason: string }[] = []; + const graphExcluded: { location: string; reason: string }[] = []; + const add = async (row: Record, axis: string, location: string) => { + try { entries.push({ row, source: await identifySource(row, axis) }); } + catch (err) { + if (!(err instanceof SourceIdentityError)) throw err; + excluded.push({ location, reason: err.reason }); + row.feedback_available = false; row.memory_exclusion_reason = err.reason; + } + }; for (const axis of ['results', 'graph_results']) for (const row of payload[axis] ?? []) { - entries.push({ row, source: await identifySource(row, axis === 'results' ? 'keyword' : 'graph') }); - for (const other of row.same_entity?.others ?? []) entries.push({ row: other, source: await identifySource(other, 'keyword') }); + const location = `${axis}[${payload[axis].indexOf(row)}]`; + await add(row, axis === 'results' ? 'keyword' : 'graph', location); + for (const [i, other] of (row.same_entity?.others ?? []).entries()) await add(other, 'keyword', `${location}.same_entity.others[${i}]`); + } + for (const { row, source } of entries) { + if (!source.path.length) continue; + const nodes = graphNodes.get(row) ?? []; + let safe = nodes.length === source.path.length + 1 && source.path.length <= 2; + for (const node of nodes) { + if (!node || node.type !== 'wiki_doc') { safe = false; continue; } + try { await identifySource({ ...node, wiki_path: node.doc_path }, 'graph'); } + catch (err) { if (!(err instanceof SourceIdentityError)) throw err; safe = false; } + } + for (const [i, edge] of source.path.entries()) { + const a = nodes[i], b = nodes[i + 1]; + if (!a || !b || a.repo !== edge.repo || b.repo !== edge.repo || edge.kind !== 'mention' || + !((a.doc_path === edge.src && b.doc_path === edge.dst) || (a.doc_path === edge.dst && b.doc_path === edge.src))) safe = false; + } + if (!safe) { + source.path = []; + row.graph_feedback_available = false; row.graph_feedback_exclusion_reason = 'unverifiable_graph_path'; + graphExcluded.push({ location: `graph_results[${payload.graph_results.indexOf(row)}]`, reason: 'unverifiable_graph_path' }); + } } const unique = new Map(); for (const { source } of entries) { const old = unique.get(source.source_id); if (old) { old.axes = [...new Set([...old.axes, ...source.axes])]; if (source.path.length) old.path = source.path; } else unique.set(source.source_id, source); } const { results, graph_results, ...settings } = payload; + const recording = { status: excluded.length ? (entries.length ? 'partial' : 'unresolved') : 'complete', recorded_sources: unique.size, excluded_sources: excluded.length, exclusions: excluded, excluded_graph_paths: graphExcluded.length, graph_exclusions: graphExcluded }; + if (!entries.length && excluded.length) return { ...payload, memory_recording: recording, memory_unavailable: true, memory_error: 'all_sources_unresolved', feedback_available: false }; // Settings only contain retrieval controls/outcomes, not document text. - const saved = await memoryCall(env, 'save', { request, settings, sources: [...unique.values()] }); + const saved = await memoryCall(env, 'save', { request, settings: { ...settings, memory_recording: recording }, sources: [...unique.values()] }); const snapshots = new Map(saved.sources.map((s: any) => [s.source_id, s])); for (const { row, source } of entries) { row.source_id = source.source_id; row.provenance = source.provenance; row.activation = (snapshots.get(source.source_id) as any).activation; } - return { ...payload, trace_id: saved.trace_id, timestamp: saved.timestamp, memory_policy: saved.policy, feedback_available: true }; - } catch { return { ...payload, memory_unavailable: true, feedback_available: false }; } + return { ...payload, memory_recording: recording, trace_id: saved.trace_id, timestamp: saved.timestamp, memory_policy: saved.policy, feedback_available: true }; + } catch { return { ...payload, memory_unavailable: true, memory_error: 'memory_save_failed', feedback_available: false }; } } export { MEMORY_INSTRUCTIONS }; diff --git a/src/memory-contract.ts b/src/memory-contract.ts index e3ef179..34a28cb 100644 --- a/src/memory-contract.ts +++ b/src/memory-contract.ts @@ -1,6 +1,6 @@ import { z } from 'zod'; -export const MEMORY_INSTRUCTIONS = 'Successful search/scan/fetch writes a private trace. Retrieval is not usage or confirmation. Call record_source_use with selected (chosen for investigation), then validated (exact source checked and judged usable), then used (actually used in an answer or decision). record_outcome confirmed reinforces only saved graph paths of used sources. corrected/rolled_back names the confirmation receipt to reverse. Use a new idempotency_key for each operation; retry the same payload with the same key. memory_history lists traces or reads one trace. use_memory only orders graph candidates within the same hop; keyword scores/ranks never change. memory_unavailable means no usable trace was saved. Live inline doc/wiki bodies use returned-content SHA-256 versions and github_live provenance; stored-content fetch refers to the indexed snapshot.'; +export const MEMORY_INSTRUCTIONS = 'Successful search/scan/fetch writes a private trace. memory_recording reports complete/partial/unresolved source recording, counts and exclusion reasons; saved settings retain it in memory_history. Unresolved rows have feedback_available=false and no source_id. All-unresolved results have no trace; a genuine zero-hit search saves an empty trace. Unverifiable graph paths cannot be reinforced; a resolved terminal can still record usage. Retrieval is not usage or confirmation. Call record_source_use with selected (chosen for investigation), then validated (exact source checked and judged usable), then used (actually used in an answer or decision). record_outcome confirmed reinforces only saved graph paths of used sources. corrected/rolled_back names the confirmation receipt to reverse. Use a new idempotency_key for each operation; retry the same payload with the same key. memory_history lists traces or reads one trace. use_memory only orders graph candidates within the same hop; keyword scores/ranks never change. memory_unavailable means no usable trace was saved; memory_error distinguishes retrieval_incomplete, memory_save_failed and all_sources_unresolved. Live inline doc/wiki bodies use returned-content SHA-256 versions and github_live provenance; stored-content fetch refers to the indexed snapshot.'; const handle = z.string().min(1).max(128); const reason = z.string().min(1).max(1000); export const memorySchemas = { @@ -9,7 +9,7 @@ export const memorySchemas = { record_outcome: z.strictObject({ trace_id: handle, idempotency_key: handle, outcome: z.enum(['confirmed', 'corrected', 'rolled_back']), source_ids: z.array(handle).min(1).max(100).optional(), confirmation_id: handle.optional(), reason }), }; export const memoryDescriptions = { - memory_history: 'Read your private, bounded trace history. trace_id returns source usage, current activation, saved paths, and outcome/reversal audit. Otherwise limit/cursor pages trace summaries.', + memory_history: 'Read your private, bounded trace history. settings.memory_recording retains source-recording status, exclusion counts and reasons in summaries and details. trace_id returns source usage, current activation, saved paths, and outcome/reversal audit. Otherwise limit/cursor pages trace summaries.', record_source_use: 'Atomically record selected -> validated -> used for sources in your trace. selected = chosen for investigation; validated = exact source checked and usable; used = actually used in an answer/decision. Search alone is never used. Same key/payload returns the saved receipt; conflicting payload is rejected. Repeating a completed stage adds no activation.', record_outcome: 'confirmed requires source_ids already used in this trace and strengthens only saved actual graph paths. One trace contributes to each edge once. corrected/rolled_back requires confirmation_id and reverses only that receipt contribution, preserving other traces. reason is saved for audit. Lexical sources have no graph credit. Idempotent by key/payload.', }; diff --git a/src/memory-e2e.workers.test.ts b/src/memory-e2e.workers.test.ts index b0739f2..9428259 100644 --- a/src/memory-e2e.workers.test.ts +++ b/src/memory-e2e.workers.test.ts @@ -27,6 +27,80 @@ async function client(principal = 9101, failMemory = false, failScan = false) { } const uses = (source_id: string) => ['selected', 'validated', 'used'].map(stage => ({ source_id, stage })); describe('synthetic in-process MCP lifecycle with real DO/D1', () => { + it('preserves mixed fetch rows and saves only resolved sources with durable partial status', async () => { + const repo = 'synthetic/partial-fetch'; + await seed(repo, 'partial-good', 'good', 'resolved'); + await upsertFtsRow(env.DB_FTS, { vectorId:'partial-bad', repo, type:'issue_comment', state:'active', labels:'', milestone:'', assignees:'', updatedAt:timestamp, number:1428, content:'unresolved' }); + const c = await client(9110); + const found = (await c.call('search', { vector_ids:['partial-bad','partial-good'] })).payload; + expect(found.results.map((r: any) => r.vector_id)).toEqual(['partial-bad','partial-good']); + expect(found.feedback_available).toBe(true); expect(found.trace_id).toBeTruthy(); + expect(found.memory_recording).toMatchObject({ status:'partial', recorded_sources:1, excluded_sources:1, exclusions:[{location:'results[0]', reason:'canonical_identity_unavailable'}] }); + expect(found.results[0]).toMatchObject({ feedback_available:false, memory_exclusion_reason:'canonical_identity_unavailable' }); + expect(found.results[0].source_id).toBeUndefined(); expect(found.results[0].activation).toBeUndefined(); + const history = (await c.call('memory_history', { trace_id:found.trace_id })).payload.data; + expect(history.sources.map((s: any) => s.source_id)).toEqual([found.results[1].source_id]); + expect(history.settings.memory_recording).toEqual(found.memory_recording); + expect((await c.call('memory_history', {})).payload.data.traces[0].settings.memory_recording).toEqual(found.memory_recording); + await c.call('record_source_use', { trace_id:found.trace_id, idempotency_key:'partial-used', uses:uses(found.results[1].source_id) }); + expect((await c.call('memory_history', { trace_id:found.trace_id })).payload.data.sources[0].stage).toBe('used'); + expect((await c.call('record_source_use', { trace_id:found.trace_id, idempotency_key:'missing-source', uses:uses('unresolved') })).payload.error).toBe('unknown_source'); + const unresolved = (await c.call('search', { vector_ids:['partial-bad'] })).payload; + expect(unresolved.memory_recording.status).toBe('unresolved'); expect(unresolved.memory_error).toBe('all_sources_unresolved'); expect(unresolved.trace_id).toBeUndefined(); + const empty = (await c.call('search', { vector_ids:['partial-absent'] })).payload; + expect(empty.memory_recording).toMatchObject({status:'complete', recorded_sources:0, excluded_sources:0}); expect(empty.trace_id).toBeTruthy(); + const broken = await client(9111, true); + const failed = (await broken.call('search', {vector_ids:['partial-bad','partial-good']})).payload; + expect(failed.memory_error).toBe('memory_save_failed'); expect(failed.trace_id).toBeUndefined(); expect(failed.results[1].source_id).toBeUndefined(); + await c.reset(); await broken.reset(); + }); + it('keeps valid folded rows when a same_entity member lacks canonical identity', async () => { + const repo = 'synthetic/partial-fold'; + await seed(repo, 'partial-doc', 'docs/a.md', 'partialfoldneedle', 'doc'); + await upsertFtsRow(env.DB_FTS, {vectorId:'partial-diff',repo,type:'diff',state:'active',labels:'',milestone:'',assignees:'',updatedAt:timestamp,filePath:'docs/a.md',content:'partialfoldneedle'}); + const c = await client(9112); + const found = (await c.call('search', {query:'partialfoldneedle',repo,fusion:'sparse_only',rerank:false})).payload; + expect(found.results).toHaveLength(1); expect(found.results[0].same_entity.others).toHaveLength(1); + expect(found.memory_recording).toMatchObject({status:'partial',recorded_sources:1,excluded_sources:1}); + const rows = [found.results[0], ...found.results[0].same_entity.others]; + expect(rows.filter(r => r.source_id)).toHaveLength(1); expect(rows.filter(r => r.feedback_available === false)).toHaveLength(1); + await c.reset(); + }); + it('excludes unresolved graph intermediates and their paths but preserves safe credits and reversal', async () => { + const repo = 'synthetic/partial-graph'; + await seed(repo,'partial-root','root','partialgraphneedle'); + await seed(repo,'partial-middle','middle','middle'); + await env.DB_FTS.prepare('UPDATE search_docs SET updated_at=? WHERE vector_id=?').bind('', 'partial-middle').run(); + await seed(repo,'partial-terminal','terminal','terminal'); await seed(repo,'partial-safe','safe','safe'); + await upsertEdges(env.DB_FTS,'partial-root',repo,'root',[{dstVectorId:'partial-middle',dstSlug:'middle',edgeKind:'mention'},{dstVectorId:'partial-safe',dstSlug:'safe',edgeKind:'mention'}]); + await upsertEdges(env.DB_FTS,'partial-middle',repo,'middle',[{dstVectorId:'partial-terminal',dstSlug:'terminal',edgeKind:'mention'}]); + const c = await client(9113); + const found = (await c.call('search',{query:'partialgraphneedle',repo,fusion:'sparse_only',rerank:false,graph_expand:true,graph_hops:2})).payload; + expect(found.memory_recording).toMatchObject({status:'partial',excluded_sources:1,excluded_graph_paths:1}); + const middle = found.graph_results.find((r: any) => r.wiki_path === 'middle'); const terminal = found.graph_results.find((r: any) => r.wiki_path === 'terminal'); const safe = found.graph_results.find((r: any) => r.wiki_path === 'safe'); + expect(middle.source_id).toBeUndefined(); expect(terminal.graph_feedback_available).toBe(false); expect(terminal.graph_path).toHaveLength(2); + expect(JSON.stringify(found)).not.toContain('memory_graph_nodes'); + let history = (await c.call('memory_history',{trace_id:found.trace_id})).payload.data; + expect(history.sources.find((s: any) => s.source_id === terminal.source_id).path).toEqual([]); + for (const source_id of [terminal.source_id,safe.source_id]) await c.call('record_source_use',{trace_id:found.trace_id,idempotency_key:'used-'+source_id,uses:uses(source_id)}); + const denied = (await c.call('record_outcome',{trace_id:found.trace_id,idempotency_key:'unsafe-confirm',outcome:'confirmed',source_ids:[terminal.source_id],reason:'Synthetic terminal verified'})).payload.data; + expect(denied.changes).toEqual([]); expect(denied.non_applied_reason).toBe('no_graph_path'); + const confirmed = (await c.call('record_outcome',{trace_id:found.trace_id,idempotency_key:'safe-confirm',outcome:'confirmed',source_ids:[safe.source_id],reason:'Synthetic safe path verified'})).payload.data; + expect(confirmed.changes).toHaveLength(1); + const reversal = (await c.call('record_outcome',{trace_id:found.trace_id,idempotency_key:'safe-reverse',outcome:'rolled_back',confirmation_id:confirmed.receipt_id,reason:'Synthetic rollback'})).payload.data; + expect(reversal.changes).toHaveLength(1); + history = (await c.call('memory_history',{trace_id:found.trace_id})).payload.data; + expect(history.settings.memory_recording).toEqual(found.memory_recording); + expect(history.audit.find((r: any) => r.receipt_id === confirmed.receipt_id).active_credits[0].active).toBe(0); + // A dangling intermediate remains traversable but is omitted from output. + await env.DB_FTS.prepare('DELETE FROM search_docs WHERE vector_id=?').bind('partial-middle').run(); + const dangling = (await c.call('search',{query:'partialgraphneedle',repo,fusion:'sparse_only',rerank:false,graph_expand:true,graph_hops:2})).payload; + expect(dangling.graph_results.some((r: any) => r.wiki_path === 'middle')).toBe(false); + const danglingTerminal = dangling.graph_results.find((r: any) => r.wiki_path === 'terminal'); + expect(danglingTerminal.graph_feedback_available).toBe(false); + expect((await c.call('memory_history',{trace_id:dangling.trace_id})).payload.data.sources.find((s: any) => s.source_id === danglingTerminal.source_id).path).toEqual([]); + await c.reset(); + }); it('search -> explicit used -> confirmed two-hop path -> corrected; source versions survive vector migration', async () => { const repo = 'synthetic/e2e-path'; await seed(repo, 'e2e-a', 'a', 'syntheticneedle'); await seed(repo, 'e2e-b', 'b', 'middle page'); await seed(repo, 'e2e-c', 'c', 'terminal page'); diff --git a/src/search-contract.ts b/src/search-contract.ts index b7d07a6..de6d15c 100644 --- a/src/search-contract.ts +++ b/src/search-contract.ts @@ -172,4 +172,6 @@ export const searchInputSchema = z.object({ "Graph traversal depth for graph_expand (1 or 2). Default 1. Ignored when graph_expand is false.", ), }); -export const searchOutputSchema = z.object({ count: z.number().int().nonnegative(), mode: z.enum(['search','scan','fetch']), results: z.array(z.record(z.string(), z.unknown())).max(50), graph_results: z.array(z.record(z.string(), z.unknown())).max(30).optional(), trace_id: z.string().optional(), timestamp: z.string().optional(), memory_policy: z.record(z.string(), z.unknown()).optional(), memory_unavailable: z.boolean().optional(), feedback_available: z.boolean() }).passthrough(); +const exclusion = z.object({ location: z.string(), reason: z.string() }); +export const searchOutputSchema = z.object({ count: z.number().int().nonnegative(), mode: z.enum(['search','scan','fetch']), results: z.array(z.record(z.string(), z.unknown())).max(50), graph_results: z.array(z.record(z.string(), z.unknown())).max(30).optional(), trace_id: z.string().optional(), timestamp: z.string().optional(), memory_policy: z.record(z.string(), z.unknown()).optional(), memory_unavailable: z.boolean().optional(), memory_error: z.enum(['retrieval_incomplete', 'memory_save_failed', 'all_sources_unresolved']).optional(), + memory_recording: z.object({ status: z.enum(['complete', 'partial', 'unresolved']), recorded_sources: z.number().int().nonnegative(), excluded_sources: z.number().int().nonnegative(), exclusions: z.array(exclusion), excluded_graph_paths: z.number().int().nonnegative(), graph_exclusions: z.array(exclusion) }).optional(), feedback_available: z.boolean() }).passthrough(); diff --git a/src/webhook-comment-deletion.workers.test.ts b/src/webhook-comment-deletion.workers.test.ts new file mode 100644 index 0000000..158db04 --- /dev/null +++ b/src/webhook-comment-deletion.workers.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { env, applyD1Migrations } from 'cloudflare:test'; +import { handleWebhook } from './webhook.js'; +import { issueCommentVectorId } from './pipeline/vector-id.js'; +import { upsertFtsRow } from './fts.js'; +import type { Env } from './types.js'; + +vi.mock('./github-ip.js', () => ({ isGitHubWebhookIP: async () => true })); +beforeAll(() => applyD1Migrations(env.DB_FTS, env.TEST_MIGRATIONS)); +const timestamp = '2026-01-01T00:00:00Z'; +const secret = 'synthetic-signature-test'; + +async function delivery(repo: string, id: number, bindings: Env) { + const body = JSON.stringify({ action:'deleted', repository:{full_name:repo}, issue:{number:1428}, comment:{id} }); + const key = await crypto.subtle.importKey('raw', new TextEncoder().encode(secret), {name:'HMAC',hash:'SHA-256'}, false, ['sign']); + const signature = Array.from(new Uint8Array(await crypto.subtle.sign('HMAC',key,new TextEncoder().encode(body))), b => b.toString(16).padStart(2,'0')).join(''); + return handleWebhook(new Request('https://synthetic.example/webhook',{method:'POST',body,headers:{'X-GitHub-Event':'issue_comment','X-Hub-Signature-256':'sha256='+signature}}), bindings); +} +describe('comment deletion preserves canonical identity on partial teardown', () => { + for (const failure of ['fts','vector','store-response','store-throw','none']) it(`handles ${failure} and explicit redelivery`, async () => { + const repo = 'synthetic/delete-'+failure; const id = 263; const vid = await issueCommentVectorId(repo,id); + const store = env.ISSUE_STORE.get(env.ISSUE_STORE.idFromName(crypto.randomUUID())); + await store.fetch(new Request('http://store/upsert-comment',{method:'POST',body:JSON.stringify({repo,commentId:id,number:1428,author:'synthetic',bodyHash:'synthetic',createdAt:timestamp,updatedAt:timestamp})})); + await upsertFtsRow(env.DB_FTS,{vectorId:vid,repo,type:'issue_comment',commentId:id,number:1428,state:'active',labels:'',milestone:'',assignees:'',updatedAt:timestamp,content:'Synthetic indexed comment'}); + let failing = true; + const calls: string[] = []; + const bindings = { + GITHUB_WEBHOOK_SECRET:secret, + VECTORIZE:{deleteByIds:async () => { calls.push('vector'); if (failing && failure === 'vector') throw new Error('Synthetic vector failure'); }}, + DB_FTS:{prepare:(query: string) => { if (failing && failure === 'fts') throw new Error('Synthetic FTS failure'); return env.DB_FTS.prepare(query); }}, + ISSUE_STORE:{idFromName:() => 'synthetic',get:() => ({fetch:async (r: Request) => { calls.push('store'); if (failing && failure === 'store-response') return new Response('failed',{status:503}); if (failing && failure === 'store-throw') throw new Error('Synthetic store failure'); return store.fetch(r); }})}, + } as unknown as Env; + const result = await delivery(repo,id,bindings); + const canonical = () => store.fetch(new Request(`http://store/comment?repo=${encodeURIComponent(repo)}&comment_id=${id}`)); + const rows = () => env.DB_FTS.prepare('SELECT vector_id FROM search_docs WHERE vector_id=?').bind(vid).all(); + if (failure !== 'none') { + expect(result.status).toBe(503); expect((await result.json() as any).result).toBe('partial_delete'); + expect((await canonical()).status).toBe(200); + if (failure === 'fts' || failure === 'vector') expect(calls).toEqual(['vector']); + expect((await rows()).results).toHaveLength(failure === 'fts' ? 1 : 0); + failing = false; + expect((await delivery(repo,id,bindings)).status).toBe(202); + } else { + expect(result.status).toBe(202); expect((await result.json() as any).result).toBe('deleted'); + } + expect((await canonical()).status).toBe(404); expect((await rows()).results).toHaveLength(0); + expect((await delivery(repo,id,bindings)).status).toBe(202); + }); +}); diff --git a/src/webhook.ts b/src/webhook.ts index bbc81d2..66c05e1 100644 --- a/src/webhook.ts +++ b/src/webhook.ts @@ -591,34 +591,33 @@ async function handleIssueCommentEvent( // Deletion: drop from Vectorize + FTS5 + store if (action === "deleted") { const vid = await issueCommentVectorId(repo, commentId); + let indexDeletionFailed = false; try { await env.VECTORIZE.deleteByIds([vid]); } catch (err) { - console.error( - `Failed to delete comment vector ${vid}:`, - err instanceof Error ? err.message : String(err), - ); + indexDeletionFailed = true; + console.error(`Failed to delete comment vector ${vid} (${repo}#${commentId}):`, err instanceof Error ? err.message : 'binding_error'); } try { await deleteFtsRow(env.DB_FTS, vid); } catch (err) { - console.error( - `Failed to delete FTS5 row ${vid}:`, - err instanceof Error ? err.message : String(err), - ); + indexDeletionFailed = true; + console.error(`Failed to delete comment FTS5 row ${vid} (${repo}#${commentId}):`, err instanceof Error ? err.message : 'binding_error'); } + // Keep canonical identity until both index surfaces are removed. A failed + // delivery can be explicitly redelivered; this does not schedule a retry. + if (indexDeletionFailed) return jsonResponse(503, { received:true, event:'issue_comment', action, result:'partial_delete', canonical_retained:true }); try { - await storeStub.fetch( + const response = await storeStub.fetch( new Request( `http://store/comment?repo=${encodeURIComponent(repo)}&comment_id=${commentId}`, { method: "DELETE" }, ), ); + if (!response.ok) throw new Error(`Store deletion failed (status ${response.status})`); } catch (err) { - console.error( - `Failed to delete comment record ${repo}#${commentId}:`, - err instanceof Error ? err.message : String(err), - ); + console.error(`Failed to delete comment record ${repo}#${commentId}:`, err instanceof Error ? err.message : 'binding_error'); + return jsonResponse(503, { received:true, event:'issue_comment', action, result:'partial_delete' }); } return jsonResponse(202, { received: true,