diff --git a/.claude/review-lessons.md b/.claude/review-lessons.md index d6ce5b3..44ce7f2 100644 --- a/.claude/review-lessons.md +++ b/.claude/review-lessons.md @@ -29,3 +29,13 @@ - **対策**: 実時刻（`os.time()`）と比較される経路を通るfixtureのタイムスタンプは、固定の十分未来の日付（例: `2126-01-01T00:00:00Z`）を使う。過去日付のハードコードは時限爆弾、現在時刻の動的生成は再現性低下。時刻を注入できる純粋関数（`prune(t, now, days)`等）のテストは固定`now`を渡して書く - **該当箇所**: tests/fude/drafts_spec.lua +### テスト: N個の並列実装に対する代表1件だけの検証 +- **問題**: 複数の並列関数（同種のbuilder、同種のvariant実装等）に同じ変更を加えたとき、検証は代表1件だけに書き、残りは未検証のまま「全て検証した」とPR説明やコミットメッセージに書いてしまうことがある +- **対策**: 変更を加えた関数・variantの数だけ検証を書く。「〜を全て確認/検証」と書く前に、実際のアサーション数・チェック対象数が変更対象の数と一致しているか数え直す。pj-checklistの「全 variant を Grep で列挙」原則（ドキュメント整合性）と同じ考え方をテスト網羅性にも適用する +- **該当箇所**: 汎用パターン（個別事例は git log 参照） + +### コード品質: 依存プリミティブの保証と重複する検証コード (PR #169, 2026-07-23) +- **問題**: シェルスクリプトで`jq -c`によるJSON生成後、生成した行が実際にファイル末尾に追記されたかを`tail -n 1`で再読み込みして比較検証していたが、(1) `jq -c`は出力が単一行のcompact JSONになることを既に保証しており、(2) 追記コマンド自体の失敗は`set -eu`が既に検知するため、この検証は実質どの失敗も追加で捕捉していなかった。むしろ同時書き込み（人間側の操作等）が割り込むと、追記自体は成功しているのに検証だけ失敗する誤検知を生んでいた +- **対策**: 検証コードを書く前に「この検証は、依存しているプリミティブ（jqのフォーマット保証、`set -e`のfail-fast等）が既にカバーしていない失敗モードを捕捉しているか」を確認する。捕捉対象が存在しないなら、検証の精度を上げる（マッチ窓を広げる等）のではなく検証自体を削除する +- **該当箇所**: contrib/skills/fude-watch/fude-watch-reply.sh + diff --git a/CLAUDE.md b/CLAUDE.md index 8e18482..0de0c9e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,7 +33,7 @@ All plugin code lives under `lua/fude/`. The plugin entry point is `plugin/fude. - **`util.lua`** — Shared utility functions. `is_null(v)` checks for both `nil` and `vim.NIL` (JSON null compatibility across Neovim 0.11/0.12). `all_comments_resolved(comments)` checks whether every comment in a list is resolved (false for empty lists); shared by `comments/data.lua`, `ui/format.lua`, and `ui/extmarks.lua` so the "all resolved" rule stays consistent. - **`drafts.lua`** — Local on-disk storage for *unsubmitted* comment input ("drafts"), distinct from the in-session GitHub pending review (`state.pending_comments`). Persists to `stdpath("state")/fude/drafts.json` so input can be paused (jump back to the diff) and resumed later, across PR switches and Neovim restarts. Pure functions (no state/IO): `make_draft_key(repo, pr_number, kind, ...)` builds opaque keys (kind = `line`/`suggest`/`issue`/`reply`/`edit`), `repo_slug(pr_url)`, `serialize`/`deserialize` (corrupt JSON → `{}`), `prune(drafts, now, retention_days)`. IO/state helpers: `current_key(kind, ...)` (derives repo/PR from `config.state`), `enabled()` (reads `config.opts.drafts.enabled`), `load`/`save`/`get`/`set`/`remove` (file is the source of truth — no `config.state` field; `set` with empty body removes, `load` prunes by `config.opts.drafts.retention_days`), `file_markers(rel_path)` (one load → `{ lines, comment_ids }` used by `ui/extmarks.lua` to render the `draft` indicator: `line`/`suggest` drafts mark their own line, `reply`/`edit` drafts mark the targeted comment's line via the comment map), and `list_drafts()` (one load → array of `{ key, kind, body, saved_at (normalized to ISO), path?, start_line?, end_line?, comment_id? }` for the active PR, used by the comment browser; skips non-string bodies). `M._dir` overrides the storage dir for tests. Wired into comment creation/reply/edit in `comments.lua`, PR-level comments in `overview.lua`, and the `comment_browser` lower pane (prefill on entry navigation via `lower_key_for_entry`, save/discard on close, remove on submit) and list (draft rows + markers via `merge_draft_entries`). Comment input UIs receive `opts.allow_draft`/`initial_lines`/`on_save_draft`/`on_discard_draft` and report the close action (`submit`/`draft`/`discard`/`cancel`). - **`diff.lua`** — Local git operations (sync). Gets repo root, converts paths to repo-relative, retrieves base branch file content via `git show`, generates file diffs, and computes merge-base for gitsigns (avoids merge commit noise). Falls back to `origin/` when local ref isn't available. Also provides local-session helpers: `get_current_branch`, `get_head_sha`, `get_empty_tree` (empty-tree hash, diff base for zero-commit repos), `get_upstream_ref` (`@{upstream}`, diff base for the unpushed scope), `get_git_user`, `get_name_status`, `get_numstat`, `get_untracked`, `get_review_patch` (the last four take a `cwd` repo root so paths resolve from a subdirectory). -- **`local/store.lua`** — Append-only JSONL event store for local (pre-PR) review sessions, persisted inside the worktree (`.fude/reviews/.jsonl` + `.fude/current.json` pointer; `M._dir` overrides the base dir for tests). One JSON event per line; kinds: `session`/`comment`/`reply`/`edit`/`move`/`resolve`/`reopen`/`delete`/`viewed`. Pure functions: `generate_uuid`, `make_session_id`, `serialize_event`, `parse_event_line`/`parse_events` (corrupt lines skipped), `materialize(events)` (rebuilds GitHub-compatible comment objects + thread resolved state + a per-path viewed map; replies inherit the root's path/line including moves; deleted comments are hidden but stay on disk as audit trail), `apply_outdated(comments, line_counts)` (file gone or line > EOF → `is_outdated`), `reanchor(comments, file_lines_map)` (context-based re-anchor: moves a comment to the unique location of its saved `context` block, re-propagates to replies, returns the moves to persist), and `build_*_event` constructors. IO: `append_event`, `read_events`, and the current-session pointer helpers `write_current(root, branch, session)`/`read_current(root, branch)`/`clear_current(root, branch)`. `.fude/current.json` is a **branch-keyed map** (`{ [branch] = session }`) so reviewing several branches in the same worktree doesn't collide; a legacy flat single-session pointer is migrated on read. +- **`local/store.lua`** — Append-only JSONL event store for local (pre-PR) review sessions, persisted inside the worktree (`.fude/reviews/.jsonl` + `.fude/current.json` pointer; `M._dir` overrides the base dir for tests). One JSON event per line; kinds: `session`/`comment`/`reply`/`edit`/`move`/`resolve`/`reopen`/`delete`/`viewed`. Pure functions: `generate_uuid`, `make_session_id`, `serialize_event`, `parse_event_line`/`parse_events` (corrupt lines skipped), `materialize(events)` (rebuilds GitHub-compatible comment objects + thread resolved state + a per-path viewed map; replies inherit the root's path/line including moves; deleted comments are hidden but stay on disk as audit trail), `apply_outdated(comments, line_counts)` (file gone or line > EOF → `is_outdated`), `reanchor(comments, file_lines_map)` (context-based re-anchor: moves a comment to the unique location of its saved `context` block, re-propagates to replies, returns the moves to persist), and `build_*_event` constructors — every action-event constructor (`build_comment_event`/`build_reply_event`/`build_edit_event`/`build_move_event`/`build_status_event`/`build_delete_event`/`build_viewed_event`) stamps `author_type` (`opts.author_type or "human"`); `build_session_event` intentionally does not, since the session header is metadata, not a user action. IO: `append_event`, `read_events`, and the current-session pointer helpers `write_current(root, branch, session)`/`read_current(root, branch)`/`clear_current(root, branch)`. `.fude/current.json` is a **branch-keyed map** (`{ [branch] = session }`) so reviewing several branches in the same worktree doesn't collide; a legacy flat single-session pointer is migrated on read. - **`local/session.lua`** — Local review session lifecycle, parallel in shape to `init.lua` start/stop/reload. `start(base)` resolves the base ref (arg → `current.json` → default branch), computes the diff base from the scope, synthesizes `changed_files` from `git diff --name-status/--numstat` + untracked files (pure helpers: `status_word`, `resolve_rename_path`, `parse_name_status`, `parse_numstat`, `build_changed_files`), creates or resumes the session for the current branch (`.fude/current.json` is keyed by branch, so different branches in one worktree get separate sessions), sets state (`review_mode = "local"`, `local_session`), reuses `init.setup_review_autocmds`, registers local-only autocmds (BufWritePost re-anchor, BufEnter tracker sync), and starts the auto-reload timer. `stop()` tears down and clears `current.json` (the JSONL file is kept). `toggle(base)` stops an active local session or starts one (refuses while a GitHub review is active). `reload(silent)` re-reads git state and the JSONL synchronously. **Scope**: `local_session.scope` is `"base"` (merge-base with the base branch — whole branch diff), `"unpushed"` (the `@{upstream}` tracking ref — changes not yet pushed), or `"uncommitted"` (HEAD — staged + unstaged only). All three compare the working tree against a ref, so comments stay anchored (per-commit was deferred for exactly this reason — its right side is a commit, not the working tree). `resolve_scope_base(scope, base_ref, cwd)` returns the `git diff` base and the `git show` content ref (nil when the scope is unavailable). `scope_specs(session)` builds the adaptive list of available scopes with labels (base only on a non-base branch; unpushed only with an upstream; uncommitted always). `set_scope(scope)` re-derives the base, reloads changed files/comments, re-applies the gitsigns base and preview, and updates the statusline; `select_scope()` is the `vim.ui.select` picker. Comments are scope-independent (they anchor to the working tree). - **`local/tracker.lua`** — Extmark-based line tracking for local review comments (dedicated `fude_local_track` namespace so `refresh_extmarks` never clears it). `sync_buffer`/`sync_all` place invisible extmarks for root comments; `collect_moves(buf)` computes drifted positions from extmark rows; `on_buf_write(buf)` persists drift as `move` events via `local_sync.move_comments`. `teardown()` clears marks on session stop. Module-local registry, not `config.state`. - **`comments/local_sync.lua`** — Local comment backend mirroring `comments/sync.lua`'s external shape (`load_comments`, `reply_to_comment`, `edit_comment`, `delete_comment` with identical callback signatures) plus `create_comment` (with best-effort `context` capture), `move_comments` (batch re-anchor), `toggle_resolved`, and `set_viewed` (local viewed state, no GitHub round-trip). `load_comments` also populates `state.viewed_files` from the materialized viewed map, and runs `store.reanchor` (reading current file lines, preferring loaded buffers) to recover drifted comments, persisting confident re-anchors as `move` events before `apply_outdated`. After `apply_outdated` it normalizes the local thread-level `resolved` flag onto the display-facing `is_resolved`, gated by `resolved.show` (mirroring how `sync.lua` only sets `is_resolved` at fetch time), so the whole display layer reads `is_resolved` alone; `resolved` stays as the toggle source of truth. All operations are synchronous JSONL appends followed by one re-materialize into `state.comments`/`state.comment_map`; `comments.lua` dispatches to this backend when `state.review_mode == "local"`. diff --git a/README.md b/README.md index 8fdbfd9..fd440c0 100644 --- a/README.md +++ b/README.md @@ -326,8 +326,12 @@ Each line of `.fude/reviews/.jsonl` is one JSON event: Other event kinds: `edit` (body replacement), `move` (line re-anchor), `reopen`, `delete` (hides the comment; the log line remains as an audit -trail), and `viewed` (per-file viewed state). Agents should **append only** -— never rewrite existing lines. +trail), and `viewed` (per-file viewed state). Every action event — +`comment`/`reply`/`edit`/`move`/`resolve`/`reopen`/`delete`/`viewed` — +carries `author_type` (`"human"` or `"agent"`, default `"human"`), so a +watcher can mechanically filter events by who wrote them; the `session` +header is metadata, not a user action, and has no `author_type`. Agents +should **append only** — never rewrite existing lines. For a resident Claude Code session, `contrib/skills/fude-watch/` provides a skill scaffold that tails the active session file and responds to new diff --git a/contrib/skills/fude-watch/SKILL.md b/contrib/skills/fude-watch/SKILL.md index 3f21d87..9377671 100644 --- a/contrib/skills/fude-watch/SKILL.md +++ b/contrib/skills/fude-watch/SKILL.md @@ -41,15 +41,25 @@ REVIEW_FILE = .fude/reviews/.jsonl `comment` が thread root、`reply` は `in_reply_to` で root を指す。`resolve` 済みの thread は対応不要）。未対応の open コメントがあれば、この時点で Step 4 の対応を行う。 -### 3. Monitor を張る +### 3. 同梱フィルタを挟んで Monitor を張る -Monitor ツールで新規イベントを待ち受ける: +tail の生出力には agent 自身が追記した行や `viewed` / `move` などの非対象イベントも +流れてくる。これらを LLM の判断で無視するのではなく、スキルに同梱の +`fude-watch-filter.sh`（この SKILL.md と同じディレクトリ。スキル起動時に通知される +base directory 配下）をパイプに挟んで機械的に落とす。判定は `jq` で `.event` / +`.author_type` を構造的に抽出して行う（文字列の部分一致ではないので、JSON の +空白の有無や、コメント本文にたまたま `"event":"comment"` 等の文字列が含まれる +ケースの誤判定を避けられる）: -- command: `tail -n 0 -f ` +- command: `tail -n 0 -f | bash <スキルの base directory>/fude-watch-filter.sh` - description: `fude local review comments` - persistent: true -各 stdout 行が 1 イベントとして通知される。 +通知される stdout 行は「human が書いた comment / reply / resolve / reopen」だけになる。 +`viewed` / `move` / `edit` / `delete` / `session` の各イベントと、`author_type` が +`agent` の行（自分の追記の echo）はフィルタで落ちる。fude.nvim は全アクション +イベントに `author_type`（デフォルト `"human"`）を付与するので、この2軸 +（イベント種別・書き手）のフィルタで過不足なく絞れる。 ### 4. イベントへの対応 @@ -62,20 +72,23 @@ Monitor ツールで新規イベントを待ち受ける: - **`reply`**（人間からの追い返信）: スレッド文脈を読み直して同様に対応する - **`reopen`**: そのスレッドの対応を再開する - **`resolve`**: そのスレッドはクローズ。対応中なら打ち切ってよい -- 自分（agent）が追記したイベントの echo は無視する（`author_type` が `agent`） - -### 5. 返信の追記ルール - -`REVIEW_FILE` に **1行の JSON を append する**（既存行の書き換え禁止）: - -```json -{"event":"reply","id":"<新規UUID>","thread_id":"","in_reply_to":"","body":"対応内容の説明","author":"claude","author_type":"agent","created_at":""} -``` - -- `id` は新規 UUID v4 を生成する -- `thread_id` / `in_reply_to` は **root コメントの id**（reply への reply でも root を指す） -- `author_type` は必ず `"agent"` -- 追記は `printf '%s\n' '' >> ` のようにアトミックな1行 append で行う +- 上記以外のイベント（`viewed` / `move` / `edit` / `delete` / `session`）や + `author_type` が `agent` の行は Step 3 のフィルタで届かないはずだが、 + 万一届いた場合は黙って無視する（返信も報告もしない） + +### 5. 返信の追記 + +返信は同梱の `fude-watch-reply.sh` で `REVIEW_FILE` に append する（既存行の +書き換え禁止）。UUID・タイムスタンプ・`author_type: "agent"` の付与、1行の +compact JSON への正規化（fude.nvim の行単位パーサが前提とする JSONL の形式）は +スクリプトが保証する: + +1. 返信本文だけを scratchpad のテキストファイルに Write する（Markdown 可） +2. `bash <スキルの base directory>/fude-watch-reply.sh <本文ファイル>` を実行する + - 第2引数は **root コメントの id**（reply への reply でも root を指す） + - 成功すると追記したイベントの 1 行 JSON を stdout に出力する。非 0 で + 終了した場合は追記が行われていない可能性が高いので、REVIEW_FILE の末尾を + 確認してユーザーに報告する コード修正を伴う場合は、修正 → テスト/lint 確認 → reply 追記の順で行い、 reply の body には何をどう変えたかを簡潔に書く。 diff --git a/contrib/skills/fude-watch/fude-watch-filter.sh b/contrib/skills/fude-watch/fude-watch-filter.sh new file mode 100644 index 0000000..da6c365 --- /dev/null +++ b/contrib/skills/fude-watch/fude-watch-filter.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# fude-watch: pass through only actionable, human-authored review events. +# Reads JSONL lines on stdin (from tail -f) and prints only lines the watch +# session should react to: comment / reply / resolve / reopen written by a +# human. Agent-authored lines (author_type "agent") and non-actionable kinds +# (viewed / move / edit / delete / session) are dropped. +# +# Fields are extracted with jq rather than string-matched, so formatting +# (spaces) and free-text fields (e.g. a comment body that happens to contain +# the literal text `"event":"comment"`) can't cause a false match. +command -v jq >/dev/null 2>&1 || { + echo 'fude-watch-filter: jq is required but not found in PATH' >&2 + exit 1 +} + +while IFS= read -r line; do + event=$(printf '%s' "$line" | jq -r '.event // empty' 2>/dev/null) || continue + author_type=$(printf '%s' "$line" | jq -r '.author_type // empty' 2>/dev/null) + + if [ "$author_type" = "agent" ]; then + continue + fi + + case $event in + comment | reply | resolve | reopen) + printf '%s\n' "$line" ;; + esac +done diff --git a/contrib/skills/fude-watch/fude-watch-reply.sh b/contrib/skills/fude-watch/fude-watch-reply.sh new file mode 100644 index 0000000..415c54f --- /dev/null +++ b/contrib/skills/fude-watch/fude-watch-reply.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# fude-watch: append an agent reply event to a local review JSONL. +# +# Usage: fude-watch-reply.sh +# +# The body is passed as a file to avoid shell quoting issues. The event is +# serialized with jq -c, which guarantees a single compact (no-space) line — +# the format fude.nvim's line-based parser and fude-watch-filter.sh both +# rely on. On success the appended line is printed to stdout. +set -euo pipefail + +review_file=$1 +thread_id=$2 +body_file=$3 + +id=$(uuidgen | tr 'A-Z' 'a-z') +created_at=$(date -u +%Y-%m-%dT%H:%M:%SZ) + +line=$(jq -cn \ + --arg id "$id" \ + --arg thread "$thread_id" \ + --rawfile body "$body_file" \ + --arg ts "$created_at" \ + '{event:"reply",id:$id,thread_id:$thread,in_reply_to:$thread,body:($body|sub("\n+$";"")),author:"claude",author_type:"agent",created_at:$ts}') + +printf '%s\n' "$line" >> "$review_file" + +printf '%s\n' "$line" diff --git a/doc/fude.txt b/doc/fude.txt index 03cb667..650f958 100644 --- a/doc/fude.txt +++ b/doc/fude.txt @@ -948,7 +948,11 @@ Add `.fude/` to your `.gitignore`. Event kinds: `session` (header), `comment` (thread root with path/start_line/end_line/body/author/ author_type/context), `reply`, `edit`, `move` (line re-anchor), `resolve`, `reopen`, `delete` (hides the comment from the view; the log line -remains as an audit trail), and `viewed` (per-file viewed state). +remains as an audit trail), and `viewed` (per-file viewed state). Every +action event (`comment`/`reply`/`edit`/`move`/`resolve`/`reopen`/`delete`/ +`viewed`) carries `author_type` (`"human"` or `"agent"`, default `"human"`) +so consumers can filter events by writer; the `session` header is metadata, +not a user action, and has no `author_type`. Behavior~ diff --git a/lua/fude/local/store.lua b/lua/fude/local/store.lua index a21ca93..5ebe07f 100644 --- a/lua/fude/local/store.lua +++ b/lua/fude/local/store.lua @@ -421,7 +421,7 @@ function M.build_reply_event(opts) end --- Build an edit event. ---- @param opts table { id, body, author, created_at } +--- @param opts table { id, body, author, author_type, created_at } --- @return table edit event function M.build_edit_event(opts) return { @@ -429,12 +429,13 @@ function M.build_edit_event(opts) id = opts.id, body = opts.body, author = opts.author, + author_type = opts.author_type or "human", created_at = opts.created_at, } end --- Build a move (line re-anchor) event. ---- @param opts table { id, path, start_line, end_line, created_at } +--- @param opts table { id, path, start_line, end_line, author_type, created_at } --- @return table move event function M.build_move_event(opts) return { @@ -443,13 +444,14 @@ function M.build_move_event(opts) path = opts.path, start_line = opts.start_line, end_line = opts.end_line, + author_type = opts.author_type or "human", created_at = opts.created_at, } end --- Build a resolve or reopen event. --- @param kind string "resolve"|"reopen" ---- @param opts table { id, thread_id, author, created_at } +--- @param opts table { id, thread_id, author, author_type, created_at } --- @return table event function M.build_status_event(kind, opts) return { @@ -457,24 +459,26 @@ function M.build_status_event(kind, opts) id = opts.id, thread_id = opts.thread_id, author = opts.author, + author_type = opts.author_type or "human", created_at = opts.created_at, } end --- Build a delete event (removes a comment from the materialized view). ---- @param opts table { id, author, created_at } +--- @param opts table { id, author, author_type, created_at } --- @return table delete event function M.build_delete_event(opts) return { event = "delete", id = opts.id, author = opts.author, + author_type = opts.author_type or "human", created_at = opts.created_at, } end --- Build a viewed event (marks a file viewed/unviewed). ---- @param opts table { id, path, viewed (boolean), author, created_at } +--- @param opts table { id, path, viewed (boolean), author, author_type, created_at } --- @return table viewed event function M.build_viewed_event(opts) return { @@ -483,6 +487,7 @@ function M.build_viewed_event(opts) path = opts.path, viewed = opts.viewed and true or false, author = opts.author, + author_type = opts.author_type or "human", created_at = opts.created_at, } end diff --git a/tests/fude/local_store_spec.lua b/tests/fude/local_store_spec.lua index 098a4c0..23f9320 100644 --- a/tests/fude/local_store_spec.lua +++ b/tests/fude/local_store_spec.lua @@ -20,6 +20,53 @@ describe("store.make_session_id", function() end) end) +describe("store event builders author_type", function() + it("defaults author_type to human when not given", function() + assert.equals("human", store.build_edit_event({ id = "c1", body = "x" }).author_type) + assert.equals( + "human", + store.build_move_event({ id = "c1", path = "a.lua", start_line = 1, end_line = 1 }).author_type + ) + assert.equals( + "human", + store.build_status_event("resolve", { id = "e1", thread_id = "c1", author = "shusann" }).author_type + ) + assert.equals("human", store.build_delete_event({ id = "c1", author = "shusann" }).author_type) + assert.equals("human", store.build_viewed_event({ id = "v1", path = "a.lua", viewed = true }).author_type) + end) + + it("passes through an explicit author_type", function() + assert.equals("agent", store.build_edit_event({ id = "c1", body = "x", author_type = "agent" }).author_type) + assert.equals( + "agent", + store.build_move_event({ + id = "c1", + path = "a.lua", + start_line = 1, + end_line = 1, + author_type = "agent", + }).author_type + ) + assert.equals( + "agent", + store.build_status_event("resolve", { + id = "e1", + thread_id = "c1", + author = "claude", + author_type = "agent", + }).author_type + ) + assert.equals( + "agent", + store.build_delete_event({ id = "c1", author = "claude", author_type = "agent" }).author_type + ) + assert.equals( + "agent", + store.build_viewed_event({ id = "v1", path = "a.lua", viewed = true, author_type = "agent" }).author_type + ) + end) +end) + describe("store.parse_event_line", function() it("parses a valid comment event", function() local line = vim.json.encode({ event = "comment", id = "abc", path = "f.lua", body = "hi" })