docs: n8nワークフローPRレビューのスキルを追加 #980
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| --- | ||
| description: keito4-org/n8n_custom_node の n8n ワークフロー/テンプレートPRをレビューする。ワークフロー同期PR（workflow-sync/*）の退行判定、資格情報のMASKED破損チェック、typeVersion・.item/.first() の意味論判定、lockfile起因のCI失敗の切り分けを行う。n8nのPRレビュー・マージ可否判断を依頼されたときに使う。 | ||
| --- | ||
|
|
||
| # n8n ワークフローPRレビュー | ||
|
|
||
| 対象リポジトリ: `keito4-org/n8n_custom_node`（テンプレートの正本） | ||
| ワークフロー実体は elu / OYKOT の n8n インスタンスにあり、リポジトリのテンプレートJSONと双方向に同期される。 | ||
|
|
||
| ## 0. PRの分類 | ||
|
|
||
| まず種別を判定する。判断基準が異なる。 | ||
|
|
||
| | 種別 | 見分け方 | 判断基準 | | ||
| | ---------------- | --------------------------------------------- | ---------------------------------- | | ||
| | ワークフロー同期 | ブランチ `workflow-sync/*`、`*.template.json` | **退行していないか**（下記1〜3） | | ||
| | ワークフロー修正 | 人手の `fix:`/`feat:` + `*.template.json` | 意図通りか＋正常系の配線が無変更か | | ||
| | パッケージ更新 | `package.json` / `pnpm-lock.yaml` | CI green＋lockfile整合＋peer整合 | | ||
|
|
||
| ## 1. 最優先: 資格情報のMASKED破損チェック | ||
|
|
||
| 同期PRで**最も重大な退行**。これが混入していたら即クローズ。 | ||
|
|
||
| ```bash | ||
| gh pr diff <PR> | grep -i masked # 0件であること | ||
| gh pr diff <PR> | grep -i credential # 資格情報の書き換えが無いこと | ||
| ``` | ||
|
|
||
| n8n の API は資格情報を `"MASKED"` で返すため、**live → repo → live** の往復で資格情報が破壊される。 | ||
| 実害例: 2026-06 に `githubApi/common_github` が MASKED 化 → Issue作成が全件失敗。 | ||
|
|
||
| ## 2. 「同期PR＝退行」ではない — 差分の意味論を読む | ||
|
|
||
| 同期PRは n8n UI での編集が機械的に落ちてくるだけなので、**差分の1行ずつが fix なのか退行なのかを判定する**。 | ||
| 機械的な「同期だから怪しい」判断はしない。 | ||
|
|
||
| ### `.item` → `.first()` | ||
|
|
||
| - `$('X').item` = paired item 解決。Switch/If の分岐をまたぐと解決に失敗して実行時エラーになりうる | ||
| （関連: [n8n#14568](https://github.com/n8n-io/n8n/issues/14568)。**ソース未確認 — 一般論として鵜呑みにしない**）。 | ||
| - `$('X').first()` = 常に先頭アイテム。例外を投げない。 | ||
| - **アイテムが1件しか流れない場合、両者は等価** → 退行ではない。 | ||
| - 複数アイテムが流れうる場合は、全アイテムが先頭の値で処理される**サイレントな退行**になる。 | ||
| Slackなら「別のユーザー宛に返信する」等の実害になり、CIでは検出できない。 | ||
|
|
||
| 一般論に頼らず、**そのワークフローで複数アイテムが流れうるか**を構造から確定させるのが確実。 | ||
|
|
||
| 判定手順 — fan-out ノードの有無とトリガー特性を必ず確認する: | ||
|
|
||
| ```bash | ||
| F=packages/common_module/nodes/CommonModule/templates/<name>.template.json | ||
| # fan-out ノード（複数アイテム化）の有無 | ||
| jq -r '.nodes[] | select(.type|test("splitOut|splitInBatches|itemLists|code|aggregate";"i")) | "\(.name)\t\(.type)"' $F | ||
| # トリガー種別 | ||
| jq -r '.nodes[] | select(.type|test("trigger|webhook";"i")) | "\(.name)\t\(.type)"' $F | ||
| ``` | ||
|
|
||
| Slack Trigger / Webhook は1イベント=1アイテム。fan-out が無ければ `.first()` は安全。 | ||
|
|
||
| ### `typeVersion` の変化 | ||
|
|
||
| n8n UI で開くと自動マイグレーションで上がる。**上がること自体は退行ではない**。 | ||
| 「どのバージョンで何の挙動が変わるか」を n8n のソースで裏取りしてから判定する。**推測しない**。 | ||
|
|
||
| `n8n-nodes-base.executeWorkflow`（`version: [1, 1.1, 1.2, 1.3]` の単一クラス）で確認済みの事実: | ||
|
|
||
| - **`workflowInputs` は typeVersion で実行時ガードされていない。** バージョン条件は | ||
| `displayOptions: { show: { '@version': [{_cnd:{gte:1.2}}] } }` ＝**エディタでの表示条件のみ**。 | ||
| 実行時は `getNodeParameter` が `node.parameters` を生で読むため、**1.1 でも `workflowInputs` は効く**。 | ||
| → 「1.1 だからマッピングが死んでいる」は**誤り**。1.2 への引き上げ＝有効化、でもない。 | ||
| - **1.3 が 1.2 に追加したのはエラー出力の統合のみ。** `outputIndex = nodeVersion >= 1.3 ? 0 : i`。 | ||
| **`onError: continueErrorOutput`（エラー出力モード）を使っているノードにしか影響しない。** | ||
| - サブWFへ渡るフィールドの絞り込みは、呼び出し側の typeVersion ではなく | ||
| **サブWF側 `ExecuteWorkflowTrigger` の `inputSource`** が支配する（`passthrough` 以外なら schema に切り詰め）。 | ||
| - 1.1 のままエディタで開くと `workflowInputs` が非表示のため、保存時に**JSONから消える恐れ**がある。 | ||
| 1.2+ への引き上げはこの取りこぼしを防ぐ方向に働く。 | ||
|
|
||
| → 実務上の判定: **1.1→1.3 のバンプは、そのノードが `onError: continueErrorOutput` を使い | ||
| かつエラー出力を配線している場合のみ挙動が変わる。** それ以外は中立。 | ||
|
|
||
| ```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 | ||
|
Comment on lines
+81
to
+85
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win jq例を実際のノード名から解決するよう修正してください。
Also applies to: 94-99 🤖 Prompt for AI Agents |
||
| ``` | ||
|
|
||
| 出典: [ExecuteWorkflow.node.ts](https://github.com/n8n-io/n8n/blob/master/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/ExecuteWorkflow.node.ts) / [ExecuteWorkflowTrigger.node.ts](https://github.com/n8n-io/n8n/blob/master/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflowTrigger/ExecuteWorkflowTrigger.node.ts) | ||
|
|
||
| ## 3. ワークフロー修正PRの検証 | ||
|
|
||
| 配線をJSONから直接検証する。PR説明を信用しない。 | ||
|
|
||
| ```bash | ||
| # エラー出力(main[1])がどこへ行くか / 正常系(main[0])が無変更か | ||
| jq -r '.connections | to_entries[] | select(.key|test("^Create an issue")) | "\(.key)\n success-> \([.value.main[0][]?.node])\n error -> \([.value.main[1][]?.node])"' $F | ||
|
|
||
| # If ノードの true(main[0]) / false(main[1]) の接続先 | ||
| jq -r '.connections | to_entries[] | select(.key|test("Guard")) | "\(.key): true->\([.value.main[0][]?.node]) | false->\([.value.main[1][]?.node])"' $F | ||
| ``` | ||
|
|
||
| チェック観点: | ||
|
|
||
| - **fail-safe か**: 条件不一致時の最悪ケースが「何もしない」に倒れているか（台帳を壊す方向でないか） | ||
| - **正常系が無変更か**: `main[0]` の配線に差分が無いこと | ||
| - `onError: continueErrorOutput` のエラー出力が、**理由を問わず**破壊的操作に直結していないか | ||
|
|
||
| ## 4. CI失敗の切り分け — 自分の変更が原因か | ||
|
|
||
| **他PRのlockfile破損に巻き込まれている**ケースが多い。原因を必ず特定する。 | ||
|
|
||
| ```bash | ||
| gh pr view <PR> --json statusCheckRollup --jq '[.statusCheckRollup[]? | select(.conclusion=="FAILURE") | {name, detailsUrl}]' | ||
| gh run view --repo keito4-org/n8n_custom_node --job <jobId> --log-failed | grep -iE "ERR_PNPM|error|lockfile|frozen" | head | ||
| ``` | ||
|
|
||
| | エラー | 原因 | 対応 | | ||
| | ---------------------------- | ------------------------------------ | ---------------------------- | | ||
| | `ERR_PNPM_BROKEN_LOCKFILE` | lockfileの重複キー（自PRとは無関係） | 修正PRを先にマージ → rebase | | ||
| | `ERR_PNPM_OUTDATED_LOCKFILE` | package.jsonとlockfileの不整合 | lockfile未更新。そのPRの欠陥 | | ||
| | `Generated Docs Sync` 失敗 | ノード数変更後にdocs未再生成 | **自PRの責任。下記で修正** | | ||
|
Comment on lines
+117
to
+121
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エラーは条件付きで切り分けるようにしてください。 🤖 Prompt for AI Agents |
||
|
|
||
| ### Generated Docs Sync の直し方 | ||
|
|
||
| ノードを増減させたら生成ドキュメントの再生成が必要。`pnpm` が無くても素の node で走る: | ||
|
|
||
| ```bash | ||
| node scripts/generate-docs.js # docs/GENERATED_TEMPLATES.md を更新 | ||
| git add docs/GENERATED_TEMPLATES.md && git commit -m "docs: 生成ドキュメントを同期" | ||
| ``` | ||
|
|
||
| ## 5. パッケージ更新PR | ||
|
|
||
| ```bash | ||
| gh pr diff <PR> # package.json だけで pnpm-lock.yaml が無い = frozen-lockfile で必ず落ちる | ||
| ``` | ||
|
|
||
| - **メジャーバンプは peer 依存を必ず確認**する。例: `@typescript-eslint/*` の peer は `typescript: '>=4.8.4 <6.1.0'` → TypeScript 7 は非適合。 | ||
| ```bash | ||
| grep -A4 "'@typescript-eslint/parser@" pnpm-lock.yaml | grep typescript | head | ||
| ``` | ||
| - lockfile が壊れている間は dependabot が lockfile を更新できず、**package.json だけのPRが量産される**。 | ||
| lockfile修正を先にマージし、dependabot に作り直させる（`@dependabot recreate`）。 | ||
|
|
||
| ## 6. マージ順序 | ||
|
|
||
| 依存関係を解いてから流す。 | ||
|
|
||
| 1. lockfile / インフラ修正（他の全PRのCIをブロックしているもの） | ||
| 2. rebase → CI green を確認 → ワークフロー修正 | ||
| 3. 同期PRは退行判定後 | ||
|
|
||
| ```bash | ||
| gh pr merge <PR> --squash --delete-branch | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: keito4/config
Length of output: 5498
🏁 Script executed:
Repository: 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$('X').first and $ ('X').item have distinct semantics regarding how they resolve data from previous nodes [1]. Semantics of.first vs.item: - $('X').first: Explicitly targets the first item from the output of node 'X' [2][3][1]. It is a deterministic method that ignores the current execution context's paired item linking, making it useful when you need a specific item regardless of branch structure [2][3]. - $ ('X').item: Relies on "paired item linking" [2][4]. n8n attempts to trace the relationship between the current item and the source item from node 'X' based on the workflow execution path [2][5]. If the connection is broken (e.g., due to branching with If/Switch nodes) or ambiguous, n8n may fail to resolve the item, often resulting in an error or empty result [2]. Missing Fields and Exceptions: - Missing Fields: Accessing a non-existent field via dot notation (e.g., $('X').first.json.missingField) typically returns undefined rather than throwing an immediate exception [6]. However, attempting to access properties on that undefined result (e.g., $ ('X').first.json.missingField.subProperty) will cause a TypeError [6]. - Empty Input: If node 'X' produced no output, or if.first is called on an empty dataset, it may return undefined or null, leading to similar TypeError issues if subsequent property access is attempted [1][6]. Best Practices for Robustness: - Use Optional Chaining and Nullish Coalescing: Protect expressions with standard JavaScript patterns to handle missing data gracefully [7][6]: {{ $('X').first?.json?.field?? 'fallback-value' }} - Use $ifEmpty: n8n provides a built-in helper function to handle empty or missing values: {{ $ifEmpty($('X').first.json.field, 'default') }} [1] - Verify Data Paths: If an expression returns undefined in the editor but the data appears to exist, ensure the node has been executed and the expression path matches the actual JSON structure (use bracket notation for keys with spaces/dots: $json['field name']) [8]. - Partial Execution: Be aware that in partial executions (running only a sub-section of a workflow),.item expressions can fail because they lack the full "connectionInputData" chain, whereas.first may still resolve by reading directly from the target node's run data [4].
Citations:
.first()の安全性は条件付きに書き換えてください。.first()自体は先頭アイテムを返しますが、空入力ではundefined/nullになり得て、jsonなどの後続参照で落ちます。「例外を投げない」ではなく、1件以上の入力があることと欠落フィールドをガードすることを明記してください。🤖 Prompt for AI Agents