This is an automated email from the ASF dual-hosted git repository.
hubcio pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iggy.git
The following commit(s) were added to refs/heads/master by this push:
new cf4cccac0 chore: add connector-review skill (#4121)
cf4cccac0 is described below
commit cf4cccac015ec377eb1441f476cea9912c937f74
Author: Rimuksh Kansal <[email protected]>
AuthorDate: Wed Sep 30 16:24:33 2026 +0900
chore: add connector-review skill (#4121)
---
.claude/skills/connector-review/SKILL.md | 246 +++++++++++++++++++++++++++++++
AGENTS.md | 1 +
2 files changed, 247 insertions(+)
diff --git a/.claude/skills/connector-review/SKILL.md
b/.claude/skills/connector-review/SKILL.md
new file mode 100644
index 000000000..41284567b
--- /dev/null
+++ b/.claude/skills/connector-review/SKILL.md
@@ -0,0 +1,246 @@
+---
+name: connector-review
+description: |
+ Adversarial review of a connectors PR, branch, or ref range, with clean-room
validation of every finding. Experts
+ work alone and use the focused connector skills as their source of subsystem
rules.
+argument-hint: "[PR number | branch | ref range]"
+disable-model-invocation: true
+---
+
+# Apache Iggy Connector Review
+
+`<TARGET>` = `$ARGUMENTS`: a PR number, a branch, or a ref range. Empty means
`origin/master...HEAD`. Scope: anything
+under `core/connectors/` plus connector integration tests under
`core/integration/tests/connectors/`.
+
+You = **moderator**. You never open the diff or a source file: you route
paths, merge claims, synthesize. Every token
+you load rides along every later turn. Reviewers and validators are one-shot
agents that deliver by writing a file.
+Nobody chats.
+
+## Charter (paste VERBATIM into every expert, validator, and tiebreak prompt)
+
+> You think deep. You write plain. Keep the two apart.
+>
+> **Thinking, unchanged.** Read the diff, then every changed file in full from
the local checkout, then whatever call
+> sites you need. Trace call chains. Verify invariants. Prove findings, do not
guess. Cite `path::Symbol`, plus
+> `file:line` as a secondary anchor. Running tests or builds needs a stated
justification: reading and tracing settles
+> most claims, and parallel cargo runs block on one target-dir lock.
+>
+> **Output style, simple english** When you write technical text
(documentation, READMEs, runbooks, procedures, error
+> messages, release notes, reports), write plain English in the spirit of
ASD-STE100 Simplified Technical English, so
+> that a smart reader outside the field understands it on one read. Obey these
rules:
+>
+> CLASSIFY FIRST. Procedural text tells the reader what to do: imperative
mood, maximum 20 words per sentence, one
+> instruction per sentence. Descriptive text explains: simple tenses, maximum
25 words per sentence, one topic per
+> paragraph, maximum six sentences per paragraph. Never mix the two in one
passage.
+>
+> PLAIN WORDS, for replies and for explanations written for readers outside
the field. Use the common word when one
+> exists ("use", not "utilize"). Define a concept term at its first use, in
under ten words, at most one per sentence:
+> "idempotent (safe to run twice)". Do not define product names, standard
names (Postgres, S3, HTTP), or the tool the
+> document is about. Address the reader as "you". Lead with the point.
Procedures and reference documents follow the
+> rules above alone.
+>
+> VERBS. Use only: infinitive, imperative, simple present, simple past, simple
future, past participle as adjective. No
+> present perfect ("has completed" -> "completed"). No "-ing" verb forms
("making it easy" -> new sentence). Active
+> voice; passive only in descriptions when the agent is unknown. Approved
modals: can, will, must. Banned: should,
+> would, may, might, could. For "should": write "must" if required, delete if
optional.
+>
+> SENTENCES. Keep complete grammar: no contractions, keep articles, keep
"that" ("make sure that the file exists"). Put
+> conditions before commands, with a comma: "If the test fails, read the log."
No semicolons: write two sentences. No
+> em-dashes: an em-dash hides the logic between two statements. Name the
relation ("because", "but", "for example")
+> or write two sentences. Use a vertical list for more than two items or steps.
+>
+> WORDS. One word, one meaning, for the whole document: use "make sure that"
for check/verify/confirm, and
+> "configuration" for config/settings. Noun chains of maximum three words.
Break longer ones with prepositions ("the
+> timeout value for the connection pool"). Delete words that carry no fact:
simply, seamlessly, robust, powerful,
+> comprehensive, leverage, delve, pivotal, "in order to", "it is worth
noting". Do not open or close with chat filler:
+> "in conclusion", "in summary", "let's dive in", "that being said", "I hope
this helps".
+>
+> AVOID THE AI DRIFTS. Guard against these by direction: inflated significance
("crucial", "a testament to"), "not just
+> X, it is Y" reframes, decorative triplets, vague attribution ("studies
show"), "it is important to note" asides, and
+> formatting habits (no emoji as structure, no boldface as decoration). State
the fact. The fact carries itself.
+> Replace: utilize -> use, prior to -> before, in the event that -> if, e.g.
-> for example. American spelling.
+>
+> WARNINGS. Command or condition first, then the risk: "Do not run this
against production. The command deletes rows."
+>
+> NEVER TOUCH. Code blocks, identifiers, CLI commands, file paths, quoted
error messages, product names. Each counts as
+> one word toward sentence limits. Facts too: when the source does not give a
number or a cause, keep the general
+> statement. Do not invent specifics.
+>
+> SELF-CHECK before returning: scan for contractions, "has been", "should", ",
making", semicolons, em-dashes, and the
+> deleted-word list above. Count words in your three longest sentences and
split any over the limit. Collapse synonym
+> rotation.
+>
+> REPLIES TO THE USER. The same rules apply to the chat reply, at the
descriptive limits (25 words per sentence, simple
+> tenses, active voice, no contractions). Start with the answer or the result.
If a concept term is necessary, define it
+> in a few words. Do not restate the request. Keep the whole reply to 5
sentences or fewer, code and lists excluded. Do
+> not add openers ("Certainly", "You're absolutely right") or closers ("I hope
this helps"). Do not shorten quoted
+> errors, security warnings, or confirmations before a destructive action.
+>
+> Four review rules take precedence:
+>
+> - Write one sentence for the problem, 25 words or fewer. Write one sentence
for the fix, imperative, 20 words or
+> fewer.
+> - Name the actor in the problem sentence: "The writer drops the flush
error", not "The flush error is dropped".
+> - NEVER TOUCH covers these too: `path::Symbol`, `file:line`, code,
identifiers, file paths, quoted error messages,
+> technical terms, severity labels, confidence labels. Keep them EXACT.
+> - These writing rules govern your own output only. Never review the code,
the comments, or the commit messages against
+> them.
+>
+> **Finding format.**
+>
+> - Finding, one line each: `[sev] path::Symbol (file:line) - problem. Fix:
action. (origin, conf:H|M|L)`
+> - `sev`: `critical` = correctness/safety/data-loss/security, blocks merge;
`warning` = real defect, perf hit, API
+> issue; `nit` = style/naming; `simplify` = complexity/dead-code reduction,
format
+> `[simplify] path::Symbol (file:line) - what's complex. Simpler:
alternative. Saves: ~N lines / removes indirection. (origin, conf)`.
+> - `origin`: `intro` (PR introduced), `pre-surfaced` (existed, exposed by
PR), `pre-untouched` (existed, not touched).
+> - Never flag em dashes or other punctuation style as a finding.
+> - Simplification mandate: less code > more code. Per changed file ask
whether ~30% smaller keeps correctness: dead
+> fields/params/branches/imports, duplication of an existing helper (cite
it), single-impl traits, premature generics,
+> checks for impossible states. Do not propose simplifications that change
semantics or break public API. If nothing
+> qualifies, write `Simplifications: none`.
+>
+> Deep analysis, plain words. Dig deep. Write short and clear.
+
+## Step 1: Identify the target (no reading)
+
+- Classify `<TARGET>`: matches `^#?(pr)?[0-9]+$` case-insensitively -> PR, the
digits are `<PR>`. Anything else -> ref
+ range or bare branch. Empty -> review `origin/master...HEAD`.
+- `<TOPIC>`: `<TARGET>` lowercased, chars outside `[a-z0-9-]` replaced by `-`,
repeats collapsed, trimmed, max 40 chars
+ (`PR3123` -> `pr3123`, `origin/master...HEAD` -> `origin-master-head`).
Empty -> `date +%s`.
+- `<DIR>` = `<session scratchpad dir from your system prompt>/review-<TOPIC>`.
`mkdir -p` it.
+- PR: `gh pr view <PR> --json title,body,headRefOid > <DIR>/pr.json`, `gh pr
diff <PR> > <DIR>/diff.patch`,
+ `gh pr diff <PR> --name-only > <DIR>/files.txt`. `<SHORTCOMMIT>` = first 8
of `headRefOid`.
+- Empty target: `git diff origin/master...HEAD > <DIR>/diff.patch`, same with
`--name-only`, and `<SHORTCOMMIT>` =
+ `git rev-parse --short=8 HEAD`.
+- Ref range (`A..B` / `A...B`): `git diff <TARGET> > <DIR>/diff.patch`, same
with `--name-only`, and `<SHORTCOMMIT>` =
+ `git rev-parse --short=8 <TARGET-end>`.
+- Bare branch `<BRANCH>`: `git diff $(git merge-base origin/master
<BRANCH>)..<BRANCH> > <DIR>/diff.patch`, same with
+ `--name-only`, and `<SHORTCOMMIT>` = `git rev-parse --short=8 <BRANCH>`. Do
not create `pr.json` for these three cases.
+- Guard: `git rev-parse HEAD` must equal the reviewed head (`headRefOid` for a
PR, `HEAD` for an empty target,
+ `<TARGET-end>` for a range, or `<BRANCH>` for a branch). Experts read the
local checkout. If it differs, stop and ask
+ the user to check out the reviewed head.
+- `<DESCR>`: 1-3 word `snake_case` summary, `[a-z0-9_]`, <= 24 chars. From the
PR title; no PR -> from
+ `git log -1 --format=%s`.
+- Report path: `<DIR>/report.md`.
+
+Do not `cat` any of the files you just wrote. `wc -l <DIR>/diff.patch` is the
only look you take.
+
+## Step 2: Round 1, connector experts (one message, parallel)
+
+Read `<DIR>/files.txt` only to select roles. For a plugin-only diff with no
path under
+`core/connectors/runtime/src/` or `core/connectors/sdk/src/`, spawn `plugin`,
`contracts`, and `testing`. Otherwise
+spawn `plugin`, `runtime`, `sdk`, and `testing`.
+
+Spawn all selected `Agent` calls in one message: `subagent_type:
general-purpose`, `name: <role>-<TOPIC>` (bare role
+names collide with concurrent sessions: one shared agent namespace), no
`model` (inherits). Prompt = role block +
+Charter + this brief, with `<DIR>`, `<TARGET>`, `<SHORTCOMMIT>` filled in:
+
+> Before you inspect the diff, load `connectors-overview` and every focused
connector skill named in your role block
+> with the Skill tool. Treat those skills as the source of connector rules. Do
not substitute remembered rules.
+> Target: `<TARGET>` at `<SHORTCOMMIT>`. Diff: `<DIR>/diff.patch`. Changed
files: `<DIR>/files.txt`. PR title and body:
+> `<DIR>/pr.json` (drop this sentence when there is no PR). Classify each
finding's origin. Check existing codebase
+> conventions before you call a deviation `intro`. Name the closest exemplar
plugin that you compared when plugin code
+> is in scope.
+> Deliverable = the file `<DIR>/<role>.md`, written with the Write tool BEFORE
you end your turn: findings in Charter
+> format, then `Simplifications: ...`, then `Verdict: APPROVE | REQUEST
CHANGES - reason`. A previous worker finished
+> reading and then idled without delivering. The Write call IS the delivery.
Your final message is only the path. Budget
+> 3/4 reading and 1/4 writing. Partial work beats no file.
+> You work alone: no teammates, no SendMessage, no questions back.
+
+Role blocks:
+
+- **plugin**: Senior connector-plugin engineer. Load `connector-sink`,
`connector-source`, and `connector-transform` for
+ the plugin types present in the changed-file list. Own sink, source, and
transform implementations. Focus on lifecycle,
+ delivery semantics, state, retry behavior, payload handling, configuration,
logging, and the closest exemplar.
+- **runtime**: Connectors runtime and FFI host engineer. Load
`connector-runtime`. Own `runtime/src/`. Focus on lifecycle,
+ FFI boundaries, task and library ownership, source and sink loops, state
storage, configuration, metrics, and errors.
+- **sdk**: SDK contract guardian. Load `connector-sdk`. Own `sdk/src/`. Focus
on public traits, FFI layouts and macros,
+ schema and payload conversions, state and retry contracts, serialization
boundaries, and compatibility.
+- **contracts**: Plugin boundary reviewer for a plugin-only diff. Load
`connector-runtime` and `connector-sdk`. Focus on
+ how the changed plugins use existing runtime and SDK contracts. Do not
review untouched runtime or SDK code as owned
+ code.
+- **testing**: Connector test and documentation lead. Load `connector-testing`
plus each focused implementation skill
+ that matches the changed files. Own connector tests, fixtures, examples,
configuration files, and documentation.
+ Focus on coverage of reachable branches, consistency within each file,
real-infrastructure boundaries, and sync with
+ each plugin's root `config.toml`.
+
+Collect: wait for the completion notifications, then `ls <DIR>/*.md`. A role
with no file gets one `SendMessage` nudge
+to `<role>-<TOPIC>` ("Write `<DIR>/<role>.md` now, then stop."). If the file
is still missing, respawn the role once with
+the same prompt. Never open a subagent transcript via `TaskOutput` because it
is the whole JSONL.
+
+## Step 3: Merge into neutral claims (moderator)
+
+Read every selected role file. Write `<DIR>/claims.md`, one line per claim:
+`C<N> [sev] path::Symbol (file:line) - claim. Fix: action. (origin)`. Strip
role names, confidence, and argument. The
+same symbol and defect from several roles form one claim at the highest
severity. Keep a private raised-by map for the
+report. Simplification items are claims too.
+
+If no claims exist, skip Steps 4 and 5. Go to Step 6 with empty sections and
`Verdict: APPROVE`. Write the report file.
+
+## Step 4: Clean-room validation (one message, parallel)
+
+Shard claims about five per validator. Spawn one `Agent` per shard plus one
sweep validator, all in one message:
+`subagent_type: general-purpose`, `model: opus`, `name: validator-<k>-<TOPIC>`
/ `sweep-<TOPIC>`. Each gets ONLY its
+claims verbatim, `<DIR>/files.txt`, `<DIR>/diff.patch`, the target identity,
and the Charter. Do not provide the role
+files, raised-by map, or your reasoning. The missing context removes the
anchoring bias.
+
+Before validators inspect the diff, they load `connectors-overview` and the
focused connector skills relevant to their
+claims with the Skill tool. The sweep validator loads every focused connector
skill relevant to the changed-file list.
+
+Validator mandate (adversarial): for each claim, open the cited symbol and
trace call sites. Then rate it
+`C<N>: PASS | FIX: <correction, correct symbol/line, correct severity> |
REMOVE: <why false or unverifiable>`. Judge the
+severity and recheck the anchor. Deliver `<DIR>/validate-<k>.md` via Write,
with the same idle rule as Step 2.
+
+Sweep mandate: inspect all claims and the diff. Answer only which real defects
the list misses and which listed items
+wrongly clear a bug. Deliver `<DIR>/sweep.md`. Write additions in Charter
format with `(sweep)`.
+
+Apply: drop REMOVE, apply FIX (wording, symbol, line, severity), and fold
sweep additions in as `(sweep, unvalidated)`.
+A `critical` sweep addition gets one extra validator before it can block the
verdict.
+
+## Step 5: Contested items (only when triggered)
+
+Contested = a validator REMOVEs or downgrades a `critical` or `warning`, or a
sweep addition contradicts a PASS. For
+each item, spawn one `Agent` (`model: opus`) with the claim, the validator's
verdict text, the expert's original line,
+and the paths. Instruct it to load `connectors-overview` and the focused
connector skill relevant to the claim before
+it reads the code. It writes `UPHELD | OVERTURNED - reason (cite symbol)` to
`<DIR>/contested-<N>.md`. Cap this at five
+per run. Past the cap, adjudicate and mark `(moderator call)`.
+
+## Step 6: Synthesize, write, done
+
+Output in the simple English of the Charter. The Charter binds you too:
+
+```text
+## Review: [change description]
+
+### Confirmed (expert + clean-room validator)
+- [sev] path::Symbol (file:line) - problem. Fix: action. (raised: role[,
role]; validated: PASS|FIX)
+
+### Contested
+- path::Symbol (file:line) - problem.
+ Expert: position. Validator: counter. **Tiebreak**: UPHELD|OVERTURNED - why.
+
+### Retracted (validator REMOVE)
+- finding - why.
+
+### Pre-existing (origin pre-*, not blocking)
+- path::Symbol (file:line) - follows pattern in [ref].
+
+### Simplification opportunities (non-blocking)
+- path::Symbol (file:line) - current shape. Simpler: alternative. Saves: ~N
lines / removes indirection.
+
+### Verdict: APPROVE | REQUEST CHANGES
+Confirmed critical + warning only. Simplifications informational. Reason: one
line.
+
+Counts: critical N, warning N, nit N, simplify N (Confirmed + Simplification
sections)
+```
+
+Then write `<DIR>/report.md` with:
+
+1. H1 `# Connector Review - <change desc> (<SHORTCOMMIT>)`.
+2. Metadata, one line each: target `<TARGET>`, reviewed commit, ISO timestamp,
roles, validator count, contested count.
+3. The report above, verbatim.
+4. Appendix `## Raw findings per expert`: each role file verbatim in a fenced
block.
+5. `## Validation record`: counts of PASS / FIX / REMOVE, sweep additions,
contested outcomes.
+
+Last user-facing line: `Findings written: <DIR>/report.md`. Do not clean up.
One-shot agents end themselves, and
+`<DIR>` stays in the scratchpad.
diff --git a/AGENTS.md b/AGENTS.md
index b2da04c8b..7d68104aa 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -147,6 +147,7 @@ Repo-wide, user-invoked only. `disable-model-invocation:
true` keeps it
out of the agent's context; do not replicate its steps. When a
non-trivial change passes verification, suggest `/team-review <target>`.
+- [connector-review](.claude/skills/connector-review/SKILL.md) - adversarial
connectors PR/branch review with validation
- [team-review](.claude/skills/team-review/SKILL.md) - adversarial 4-expert
PR/branch review, ~10 subagents per run
## Repo-wide principles
