docs: n8nワークフローPRレビューのスキルを追加 - #980
Conversation
n8n_custom_node のワークフロー同期PR・テンプレート修正PRをレビューする際の 判定手順をスキル化。 - 資格情報のMASKED破損チェック（同期PRで最も重大な退行） - .item/.first() の退行判定（fan-outノードとトリガー特性から確定させる） - executeWorkflow の typeVersion 意味論（n8nソースで裏取り済み） - lockfile起因のCI失敗と自PR起因の切り分け - Generated Docs Sync の直し方
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an n8n workflow PR review procedure covering classification, credential safety, semantic validation, CI triage, package updates, generated documentation, and merge order. Registers the skill in Changesn8n review guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 5
🧹 Nitpick comments (1)
.claude/skills/n8n-workflow-pr-review.md (1)
63-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winn8nソースの参照先を対象版で固定してください。
masterを根拠にすると、対象インスタンスと異なる実装でtypeVersionの結論を出す危険があります。対象 n8n のバージョンまたはコミットを必須入力にして、この節の参照先をその版へ固定してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/n8n-workflow-pr-review.md around lines 63 - 88, Update the n8n source-verification guidance in this section to require the target instance’s n8n version or commit as an input, and use that exact revision for the ExecuteWorkflow.node.ts and ExecuteWorkflowTrigger.node.ts references instead of master. Ensure typeVersion conclusions are based only on the matched source revision.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/n8n-workflow-pr-review.md:
- Around line 81-85: jq例の接続確認を、固定文字列「Execute Workflow」「Create an
issue」「Guard」ではなく、まず.nodes[]から対象ノード名を取得して参照する形に更新してください。.connectionsのキーと実際のノード名を突き合わせ、ノード名変更・複数配置・自動リネーム後も各対象ノードの配線を確認できるようにしてください。該当するtypeや役割で対象ノードを特定し、配線が見つからない場合に未確認を問題なしと扱わない出力を維持してください。
- Around line 39-44: 「.first()
は例外を投げない」という断定を、「1件以上の入力がある場合に先頭アイテムを返す」と修正し、空入力では undefined/null
になり得ることを明記してください。後続で json
などを参照する場合は、対象アイテムと必要フィールドの存在をガードする条件も追記し、既存の単一・複数アイテムに関する説明は維持してください。
- Around line 117-121: Revise the ERR_PNPM_BROKEN_LOCKFILE guidance in the
troubleshooting table so it is not categorically treated as unrelated to the PR.
Instruct reviewers to determine the cause by checking the base diff, changed
files, and failed job logs, while preserving the existing conditional guidance
for ERR_PNPM_OUTDATED_LOCKFILE and Generated Docs Sync failures.
In `@AGENTS.md`:
- Around line 152-158: 修正Quality
Gates表の列数を整え、shellcheck行のコマンド内パイプをMarkdown上で列区切りとして解釈されないようエスケープするか表外へ移動してください。追加テストコマンドもScript・Command・Purposeの3列を持つ正しい表行に変更してください。
- Line 168: AGENTS.mdのblock_managed_file_edit.pyのHook
TriggerとPurposeを、PreToolUseで「Managed by
keito4/config」マーカー付きファイルの編集をブロックする実装内容に合わせて更新してください。TriggerのUnknownを実際のフック種別に置き換え、Purposeには適用条件とブロック対象を明記してください。
---
Nitpick comments:
In @.claude/skills/n8n-workflow-pr-review.md:
- Around line 63-88: Update the n8n source-verification guidance in this section
to require the target instance’s n8n version or commit as an input, and use that
exact revision for the ExecuteWorkflow.node.ts and
ExecuteWorkflowTrigger.node.ts references instead of master. Ensure typeVersion
conclusions are based only on the matched source revision.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 762cd8e5-653f-4932-af84-a3a34a995cc7
📒 Files selected for processing (2)
.claude/skills/n8n-workflow-pr-review.mdAGENTS.md
| - `$('X').item` = paired item 解決。Switch/If の分岐をまたぐと解決に失敗して実行時エラーになりうる | ||
| （関連: [n8n#14568](https://github.com/n8n-io/n8n/issues/14568)。**ソース未確認 — 一般論として鵜呑みにしない**）。 | ||
| - `$('X').first()` = 常に先頭アイテム。例外を投げない。 | ||
| - **アイテムが1件しか流れない場合、両者は等価** → 退行ではない。 | ||
| - 複数アイテムが流れうる場合は、全アイテムが先頭の値で処理される**サイレントな退行**になる。 | ||
| Slackなら「別のユーザー宛に返信する」等の実害になり、CIでは検出できない。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".claude/skills/n8n-workflow-pr-review.md"
echo "== file exists =="
git ls-files "$file"
echo
echo "== line count =="
wc -l "$file"
echo
echo "== relevant excerpt =="
sed -n '1,120p' "$file"Repository: keito4/config
Length of output: 5498
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo references to first() / item linking =="
rg -n --hidden --glob '!**/.git/**' '\.first\(\)|item linking|paired item|item-linking|item linking' . || true
echo
echo "== nearby references in the reviewed file =="
sed -n '30,55p' .claude/skills/n8n-workflow-pr-review.mdRepository: keito4/config
Length of output: 2387
🌐 Web query:
n8n $('X').first() item linking semantics empty input missing field exceptions docs source
💡 Result:
In n8n, expressions using
Citations:
- 1: https://docs.n8n.io/data/expression-reference/
- 2: https://community.n8n.io/t/expression-missing-from-previous-node-using-item-why/54602
- 3: https://community.n8n.io/t/cant-understand-first/146033/1
- 4: fix(core): Resolve $().item expressions in partial executions n8n-io/n8n#27338
- 5: https://docs.n8n.io/connect/create-nodes/build-your-node/reference/item-linking
- 6: https://debugalo.com/n8n/expression-undefined
- 7: https://github.com/n8n-io/skills/blob/main/plugins/n8n-skills/skills/n8n-expressions/SKILL.md
- 8: https://community.n8n.io/t/undefined-expressions/300852
.first() の安全性は条件付きに書き換えてください。
.first() 自体は先頭アイテムを返しますが、空入力では undefined/null になり得て、json などの後続参照で落ちます。「例外を投げない」 ではなく、1件以上の入力があること と 欠落フィールドをガードすること を明記してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/n8n-workflow-pr-review.md around lines 39 - 44, 「.first()
は例外を投げない」という断定を、「1件以上の入力がある場合に先頭アイテムを返す」と修正し、空入力では undefined/null
になり得ることを明記してください。後続で json
などを参照する場合は、対象アイテムと必要フィールドの存在をガードする条件も追記し、既存の単一・複数アイテムに関する説明は維持してください。
| ```bash | ||
| # typeVersion と、エラー出力モードを使っているか | ||
| jq -r '.nodes[] | select(.type=="n8n-nodes-base.executeWorkflow") | "\(.name)\ttypeVersion=\(.typeVersion)\tonError=\(.onError // "none")\thasWorkflowInputs=\(.parameters.workflowInputs != null)"' $F | ||
| # エラー出力の配線があるか（無ければ 1.3 の変更は無影響） | ||
| jq -r '.connections | to_entries[] | select(.key|test("Execute Workflow")) | "\(.key): -> \([.value.main[]?[]?.node])"' $F |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
jq例を実際のノード名から解決するよう修正してください。
.connectionsのキーはワークフロー内のノード名ですが、ここでは Execute Workflow、Create an issue、Guard という固定文字列に依存しています。ノード名変更・複数配置・自動リネーム時に何も出力されず、配線未確認を「問題なし」と誤判定します。まず.nodes[]から対象ノード名を取得してから接続を参照してください。
Also applies to: 94-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/n8n-workflow-pr-review.md around lines 81 - 85,
jq例の接続確認を、固定文字列「Execute Workflow」「Create an
issue」「Guard」ではなく、まず.nodes[]から対象ノード名を取得して参照する形に更新してください。.connectionsのキーと実際のノード名を突き合わせ、ノード名変更・複数配置・自動リネーム後も各対象ノードの配線を確認できるようにしてください。該当するtypeや役割で対象ノードを特定し、配線が見つからない場合に未確認を問題なしと扱わない出力を維持してください。
| | エラー | 原因 | 対応 | | ||
| | ---------------------------- | ------------------------------------ | ---------------------------- | | ||
| | `ERR_PNPM_BROKEN_LOCKFILE` | lockfileの重複キー（自PRとは無関係） | 修正PRを先にマージ → rebase | | ||
| | `ERR_PNPM_OUTDATED_LOCKFILE` | package.jsonとlockfileの不整合 | lockfile未更新。そのPRの欠陥 | | ||
| | `Generated Docs Sync` 失敗 | ノード数変更後にdocs未再生成 | **自PRの責任。下記で修正** | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".claude/skills/n8n-workflow-pr-review.md"
echo "== file exists =="
ls -l "$file"
echo
echo "== lines 100-140 =="
sed -n '100,140p' "$file" | cat -n
echo
echo "== search for lockfile mentions =="
rg -n "ERR_PNPM_BROKEN_LOCKFILE|ERR_PNPM_OUTDATED_LOCKFILE|lockfile|Generated Docs Sync" "$file"Repository: keito4/config
Length of output: 3186
lockfileエラーは条件付きで切り分けるようにしてください。
ERR_PNPM_BROKEN_LOCKFILE を「自PRとは無関係」と断定せず、base差分・変更ファイル・失敗ジョブのログを見て原因を判定する形に直してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/n8n-workflow-pr-review.md around lines 117 - 121, Revise the
ERR_PNPM_BROKEN_LOCKFILE guidance in the troubleshooting table so it is not
categorically treated as unrelated to the PR. Instruct reviewers to determine
the cause by checking the base diff, changed files, and failed job logs, while
preserving the existing conditional guidance for ERR_PNPM_OUTDATED_LOCKFILE and
Generated Docs Sync failures.
| | `block_dangerous_commands.py` | Pre Bash | Block destructive commands | | ||
| | `block_git_no_verify.py` | Pre git commit/push | Block `--no-verify` and `HUSKY=0` | | ||
| | `block_inline_secrets.py` | Pre Bash | Block commands embedding literal credentials | | ||
| | `block_managed_file_edit.py` | Unknown | block_managed_file_edit | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
HookのTriggerとPurposeを実装内容に合わせてください。
.claude/hooks/block_managed_file_edit.pyはPreToolUse hookで、Managed by keito4/configマーカー付きファイルの編集をブロックします。Unknownと自身のファイル名だけのPurposeでは、利用者が適用条件を判断できません。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 168, AGENTS.mdのblock_managed_file_edit.pyのHook
TriggerとPurposeを、PreToolUseで「Managed by
keito4/config」マーカー付きファイルの編集をブロックする実装内容に合わせて更新してください。TriggerのUnknownを実際のフック種別に置き換え、Purposeには適用条件とブロック対象を明記してください。
|
🎉 This PR is included in version 1.124.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Why
n8n（
keito4-org/n8n_custom_node）のPRレビューには、このリポジトリ固有の判断が要る。今回4件をレビューして得た手順を、次回セッションに残すためスキル化する（AGENTS.md「セッションを跨ぐ情報はgitにコミットする」に従う）。特に、推測すると誤判定する箇所が実在した。今回レビュー中に「executeWorkflow の typeVersion 1.1 は
workflowInputsを無視する」と想定したが、n8nのソースを読むと誤りだった（displayOptionsはエディタでの表示条件で、実行時はgetNodeParameterが生パラメータを読む）。スキルには裏取り済みの事実と出典のみを載せている。What
.claude/skills/n8n-workflow-pr-review.mdを追加。"MASKED"で返すため live→repo→live の往復で壊れる.item→.first()の退行判定 — 一般論に頼らず、fan-outノードの有無とトリガー特性から「複数アイテムが流れうるか」を構造で確定させる。1アイテムなら等価＝退行ではないtypeVersionの意味論 — 1.3 の変更点はエラー出力の統合のみで、onError: continueErrorOutputを使うノードにしか影響しない（n8nソースの出典付き）ERR_PNPM_BROKEN_LOCKFILE（他PR起因）とERR_PNPM_OUTDATED_LOCKFILE（自PRの欠陥）とGenerated Docs Sync（自PRの責任）を区別するAGENTS.md の Skills 表は自動生成ブロック内のため、手編集せず
script/update-agents-md.shで生成した（同スクリプトが Quality Gates 表の既存の崩れも併せて整形している）。How
今回の実作業（PR #2727 → #2726 → #2718 をこの順でマージ、#2684 は保留）で実際に使ったコマンドをそのまま収録している。
Risk
ドキュメントのみでコード影響なし。n8nソースの参照は master ブランチのため、対象インスタンスのバージョン差異で結論が変わりうる点はスキル内に明記済み。
Summary by CodeRabbit