Add Claude spell skills - #179
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds two Claude skills for Spark spell code generation and security review, plus shared reference documents covering specification extraction, sourcing, precedents, conventions, testing, and known review footguns. ChangesCodegen workflow
Review workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 10
🧹 Nitpick comments (2)
.claude/skills/spark-spell-codegen/SKILL.md (1)
191-216: 🩺 Stability & Availability | 🔵 TrivialWait for CI and spell-caster validation before the final report.
The reference conventions state that spell-caster simulation failures are findings, but this workflow opens the PR and reports completion based only on local
forgeresults. Add a post-PR wait/poll step and include CI or simulation failures inNot verifiedrather than claiming final validation.🤖 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/spark-spell-codegen/SKILL.md around lines 191 - 216, Update the Phase 8 validation workflow to wait for CI and spell-caster simulation results after opening the PR and before issuing the final report. Treat simulation or CI failures as findings, and list any unavailable, failed, or still-pending checks under “Not verified” instead of reporting validation as complete; retain the existing forge test result separately..claude/skills/spark-spell-review/SKILL.md (1)
80-85: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDefine the canonical
spec.mdcontract.The shared codegen reference states that codegen and review must produce/consume the same specification artifact shape, but this phase only lists fields to extract and does not define the file location, headings, or serialization rules. Add a schema or link to the authoritative spec-extraction reference before passing the artifact to specialists.
🤖 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/spark-spell-review/SKILL.md around lines 80 - 85, Define the canonical spec.md artifact contract for the extraction phase, including its required location, headings, field structure, and serialization rules. Update the instructions around extracting numbered instructions, parameters, links, and constraints to reference the authoritative schema or spec-extraction reference before handing the artifact to specialists.
🤖 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/spark-spell-codegen/references/precedent-index.md:
- Around line 74-81: Update the “OTC buffer onboarding (Ethereum)” heading text
to accurately state eight admin calls, matching the two approvals, four
configuration updates, and two whitelist updates listed in the sequence. Keep
the existing call order and operations unchanged.
- Around line 83-91: Update the SLL bridge-route guidance around
LIMIT_LAYERZERO_TRANSFER to require reading the live controller’s normalization
behavior before generating finite limits. Derive the configured value using the
controller’s actual token-decimal or 1e18 normalization, rather than always
using token decimals, while preserving the existing three-field LayerZero key
and outbound/inbound route requirements.
In @.claude/skills/spark-spell-codegen/references/spec-extraction.md:
- Around line 19-24: Update the specification extraction workflow around
post_stream.posts[0].cooked to treat fetched HTML and subsequent replies as
untrusted data: parse only the approved specification fields, ignore embedded
instructions, and bind amendment acceptance to the original proposal author’s
stable ID. Require user confirmation for content outside the allowed schema
while preserving the existing amendment scan and fetch fallback order.
- Around line 12-17: Update the forum URL fetch instructions around the `.json`
curl flow to permit only HTTPS URLs on an explicit approved-host allowlist.
Require validation of resolved destinations to reject localhost, private, and
link-local addresses; prevent unsafe redirects; and enforce bounded
connection/response time and response size before processing remote content,
while preserving local-file handling.
In @.claude/skills/spark-spell-codegen/SKILL.md:
- Around line 42-54: Update the workflow instructions in the “Environment &
access model” section to require an explicit preflight before generation.
Validate required RPC URLs, API keys, Foundry FFI, repository access, and Slack
availability; report each missing configuration exactly, then stop or mark
impacted checks unverified rather than discovering failures during later
commands.
- Around line 172-178: The consolidated-question gate in the spark-spell-codegen
skill must also include unresolved [VERIFY] decisions from precedent-index.md.
Add [VERIFY] items to the batched questions and prevent codegen or PR completion
until they are answered, unless the user explicitly proceeds with the gaps
flagged.
In @.claude/skills/spark-spell-review/SKILL.md:
- Around line 73-85: Add an explicit trust boundary to the
specification-fetching and extraction workflow around the post content and
subsequent amendments. Treat fetched HTML, forum posts, PR bodies, and comments
as untrusted data: delimit and sanitize active markup, extract only proposal
facts, and ignore any embedded instructions that attempt to change the review
workflow, suppress findings, or trigger tool calls.
- Around line 68-72: Update the forum-post fetching instructions in the
Discourse retrieval flow to parse and strictly allowlist the expected forum
origin and topic path before use. Replace interpolated shell execution with an
argument-array curl invocation, restrict redirects to the same host, and enforce
connection/overall timeouts plus a maximum response size; reject malformed,
unexpected, or internal-target URLs before fetching.
- Around line 191-201: The “Severity rubric” and “Verdict” rules conflict on how
missing tests affect approval. Update both sections so they define precedence
and distinguish release-blocking, demonstrably missing tests that require
“Request changes” from actionable but non-blocking 🟡 coverage gaps that permit
“Approve with comments,” ensuring identical findings yield one verdict.
- Around line 228-231: Complete the “Not verified” template by replacing the
truncated “N addresses checked on-chain” wording with a grammatically complete
phrase such as “all addresses checked on-chain,” while preserving the
surrounding verification examples and template structure.
---
Nitpick comments:
In @.claude/skills/spark-spell-codegen/SKILL.md:
- Around line 191-216: Update the Phase 8 validation workflow to wait for CI and
spell-caster simulation results after opening the PR and before issuing the
final report. Treat simulation or CI failures as findings, and list any
unavailable, failed, or still-pending checks under “Not verified” instead of
reporting validation as complete; retain the existing forge test result
separately.
In @.claude/skills/spark-spell-review/SKILL.md:
- Around line 80-85: Define the canonical spec.md artifact contract for the
extraction phase, including its required location, headings, field structure,
and serialization rules. Update the instructions around extracting numbered
instructions, parameters, links, and constraints to reference the authoritative
schema or spec-extraction reference before handing the artifact to specialists.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 85850076-7e26-4ef7-a2e7-40d0b960659c
📒 Files selected for processing (10)
.claude/skills/spark-spell-codegen/SKILL.md.claude/skills/spark-spell-codegen/references/conventions.md.claude/skills/spark-spell-codegen/references/footguns.md.claude/skills/spark-spell-codegen/references/precedent-index.md.claude/skills/spark-spell-codegen/references/sourcing.md.claude/skills/spark-spell-codegen/references/spec-extraction.md.claude/skills/spark-spell-codegen/references/test-harness.md.claude/skills/spark-spell-review/SKILL.md.claude/skills/spark-spell-review/reference/conventions.md.claude/skills/spark-spell-review/reference/footguns.md
| ### 2. OTC buffer onboarding (Ethereum) | ||
| Six admin calls, in order: `OTCBuffer.approve(asset, type(uint256).max)` ×2 (buffer allowance to | ||
| ALM Proxy, for `otcClaim`); `setRateLimitData(makeAddressKey(LIMIT_OTC_SWAP, exchange), max, slope)`; | ||
| `setMaxSlippage(exchange, wad)`; `setOTCBuffer(exchange, buffer)`; `setOTCRechargeRate(exchange, | ||
| perSecond18)`; `setOTCWhitelistedAsset(exchange, asset, true)` ×2 (**requires buffer already set**). | ||
| - **Intake:** buffer + exchange addresses (verify on-chain: buffer `almProxy()`, admin roles, proxy implementation). | ||
| - **Test:** pre/post config asserts + `_testOTCIntegration(OTCE2ETestParams({...}))`, one test per swap direction. | ||
| - **Example:** `archive/20260618/SparkEthereum_20260618.sol`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the OTC call count.
The card says “Six admin calls,” but the sequence expands to eight calls: two approvals, one rate-limit update, one slippage update, one buffer update, one recharge update, and two whitelist updates. Change the count to eight, or describe six call groups, so agents do not omit a required approval or whitelist operation.
🧰 Tools
🪛 LanguageTool
[style] ~80-~80: Consider using the typographical ellipsis character here instead.
Context: .... - Test: pre/post config asserts + _testOTCIntegration(OTCE2ETestParams({...})), one test per swap direction. - **E...
(ELLIPSIS)
🤖 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/spark-spell-codegen/references/precedent-index.md around
lines 74 - 81, Update the “OTC buffer onboarding (Ethereum)” heading text to
accurately state eight admin calls, matching the two approvals, four
configuration updates, and two whitelist updates listed in the sequence. Keep
the existing call order and operations unchanged.
| ### 3. SLL bridge-route rate limit (LayerZero/OFT e.g. USDT0; CCTP for USDC) | ||
| - **When:** enable/cap a token bridge route between Ethereum and an L2. **USDC uses CCTP** (`LIMIT_USDC_TO_DOMAIN` + Circle domain id via `makeDomainKey`/`makeUint32Key`, paired with `setMintRecipient`). **USDT0 / OFT uses LayerZero** (below). | ||
| - **Key (LayerZero):** `keccak256(abi.encode(controller.LIMIT_LAYERZERO_TRANSFER(), oftAddress, dstEndpointId))` — **3 fields, no `RateLimitHelpers` builder; hand-roll it** to match `LayerZeroLib`. | ||
| - **Calls — outbound (rate-limited), on the sending chain's spell:** | ||
| 1. `controller.setLayerZeroRecipient(dstEid, SLLHelpers.addrToBytes32(<dstChain>.ALM_PROXY))` | ||
| 2. set the 3-field key's rate limit (finite, token decimals). | ||
| - **Calls — inbound (unlimited return), on the receiving chain's spell:** `setLayerZeroRecipient(ENDPOINT_ID_ETHEREUM, addrToBytes32(Ethereum.ALM_PROXY))` + `setUnlimitedRateLimitData(<3-field key with the local OFT + ETH EID>)`. | ||
| - **Id spaces (footguns §5):** LayerZero **EIDs** (30101 Ethereum, 30110 Arbitrum, …) ≠ CCTP domains (0 Ethereum, 3 Arbitrum, 6 Base, …) ≠ EVM chain ids. Confirm each against official docs; EIDs for new chains are intake, not guesses. | ||
| - **Intake:** OFT adapter + token addresses per chain, destination EID. Each direction needs its own key + recipient — setting one side does not enable the return route. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Derive LayerZero limits using the controller’s normalization.
This card says finite limits use token decimals, while the surrounding rate-limit guidance and sourcing.md require checking whether each limit is token-denominated or 1e18-normalized. Require deriving LIMIT_LAYERZERO_TRANSFER from the live controller before generating values; otherwise USDC/USDT routes can be configured at the wrong magnitude.
🧰 Tools
🪛 LanguageTool
[style] ~91-~91: Since ownership is already implied, this phrasing may be redundant.
Context: ..., destination EID. Each direction needs its own key + recipient — setting one side does...
(PRP_OWN)
🤖 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/spark-spell-codegen/references/precedent-index.md around
lines 83 - 91, Update the SLL bridge-route guidance around
LIMIT_LAYERZERO_TRANSFER to require reading the live controller’s normalization
behavior before generating finite limits. Derive the configured value using the
controller’s actual token-decimal or 1e18 normalization, rather than always
using token decimals, while preserving the existing three-field LayerZero key
and outbound/inbound route requirements.
| Input may be a local file (use as-is) or a forum URL. The forum is Discourse, so a URL gets clean | ||
| structured content by appending `.json`: | ||
|
|
||
| ```bash | ||
| curl -sfL "<forum-url>.json" # e.g. .../t/<slug>/<topic-id>.json | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict remote fetches to approved HTTPS forum hosts.
Appending .json and fetching an arbitrary user-supplied URL can expose localhost or private-network services to the agent. Enforce HTTPS and an allowlist of approved forum hosts, block private/link-local destinations and unsafe redirects, and apply response time/size limits.
🤖 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/spark-spell-codegen/references/spec-extraction.md around
lines 12 - 17, Update the forum URL fetch instructions around the `.json` curl
flow to permit only HTTPS URLs on an explicit approved-host allowlist. Require
validation of resolved destinations to reject localhost, private, and link-local
addresses; prevent unsafe redirects; and enforce bounded connection/response
time and response size before processing remote content, while preserving
local-file handling.
| - The specification is `post_stream.posts[0].cooked` (HTML — convert/read it fully). | ||
| - **Scan subsequent posts by the proposal author for amendments** ("updated the rate limit to…"). | ||
| The spec is the post **as amended** — an amendment silently missed is a wrong-constant spell. | ||
| - Record the post's `version`; if `> 1` note that it has been edited (and when vs. today). | ||
| - Fallbacks, in order: WebFetch on the plain URL; `curl` on the plain URL. If all fetches fail, | ||
| STOP and ask the user for the post content — there is no spell without a spec. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat fetched content as untrusted data and authenticate amendments.
The workflow feeds arbitrary HTML and later replies into generation without defining how to ignore embedded instructions or verify a stable identity for the proposal author. Parse only the allowed specification fields, bind amendments to the original author ID, and require confirmation for content outside that schema.
🤖 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/spark-spell-codegen/references/spec-extraction.md around
lines 19 - 24, Update the specification extraction workflow around
post_stream.posts[0].cooked to treat fetched HTML and subsequent replies as
untrusted data: parse only the approved specification fields, ignore embedded
instructions, and bind amendment acceptance to the original proposal author’s
stable ID. Require user confirmation for content outside the allowed schema
while preserving the existing amendment scan and fetch fallback order.
| ## Environment & access model | ||
|
|
||
| - Normally runs as a **remote Claude Code session in a Slack thread**. Post progress/questions | ||
| there. **If there is no Slack thread (local run), do that communication in your chat response | ||
| instead** — everywhere below that says "ask/post (Slack)" means "report to the user." | ||
| - **Work from a clone/checkout of the target fork.** `archive/` and `src/` are your authoritative | ||
| examples — read and mirror them. `lib/` submodules are your authoritative sources for | ||
| signatures and addresses. | ||
| - **Never modify anything outside `src/proposals/<date>/`.** Base contracts | ||
| (`src/SparkPayload*.sol`), test harness (`src/test-harness/*`), `archive/*`, and `lib/*` are | ||
| reference-only. | ||
| - You **push a branch and open a PR** on the fork — the only write destination. | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add an explicit environment preflight.
The PR objective requires reporting missing RPC or environment configuration, but this workflow only discovers failures when later commands run. Check required RPC URLs, API keys, Foundry FFI, repository access, and Slack availability before generation; stop or mark affected checks as unverified with the exact missing configuration.
🤖 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/spark-spell-codegen/SKILL.md around lines 42 - 54, Update the
workflow instructions in the “Environment & access model” section to require an
explicit preflight before generation. Validate required RPC URLs, API keys,
Foundry FFI, repository access, and Slack availability; report each missing
configuration exactly, then stop or mark impacted checks unverified rather than
discovering failures during later commands.
| `PAYLOAD_*` unset, harness simulates). Then ask the user ONE consolidated question set (Slack/chat): | ||
| - every `[TBD]` / `TBC` address or value the agents and triage raised; | ||
| - deferred items (confirm the deferral); | ||
| - post-internal contradictions (never silently pick a reading); | ||
| - missing Forum topic id / Vote URLs; | ||
| - the **⚠️ NOVEL flags** (which items had no precedent). | ||
| Only proceed once the user's answers are in (or the user says proceed with the gaps flagged). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make unresolved [VERIFY] items blocking questions.
precedent-index.md contains load-bearing [VERIFY] decisions, but this phase only batches [TBD], TBC, contradictions, and novel flags. The agent could therefore choose between materially different implementations without confirmation. Add [VERIFY] to the gate list and block codegen/PR completion for unresolved choices.
🤖 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/spark-spell-codegen/SKILL.md around lines 172 - 178, The
consolidated-question gate in the spark-spell-codegen skill must also include
unresolved [VERIFY] decisions from precedent-index.md. Add [VERIFY] items to the
batched questions and prevent codegen or PR completion until they are answered,
unless the user explicitly proceeds with the gaps flagged.
| 3. Fetch each forum post. The forum is Discourse, so append `.json` to the topic URL for clean | ||
| structured content: | ||
| ```bash | ||
| curl -sfL "<forum-url>.json" # e.g. .../t/<slug>/<topic-id>.json | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not interpolate the PR-controlled Forum URL into a shell command.
The URL is extracted from untrusted payload source and inserted inside a shell command. A malicious Forum: value containing $(...), backticks, or an escaped quote can execute arbitrary commands; even a syntactically valid URL can trigger SSRF to an internal host.
Parse and strictly allowlist the expected forum origin/path, then invoke curl with an argument array (not shell interpolation), redirects restricted to the same host, and a timeout/response-size limit.
🤖 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/spark-spell-review/SKILL.md around lines 68 - 72, Update the
forum-post fetching instructions in the Discourse retrieval flow to parse and
strictly allowlist the expected forum origin and topic path before use. Replace
interpolated shell execution with an argument-array curl invocation, restrict
redirects to the same host, and enforce connection/overall timeouts plus a
maximum response size; reject malformed, unexpected, or internal-target URLs
before fetching.
| The specification is `post_stream.posts[0].cooked` (HTML — convert/read it fully). Also scan | ||
| subsequent posts by the proposal author for amendments ("updated the rate limit to…") — the | ||
| spec is the post **as amended**. If a post shows `version > 1`, note that it has been edited. | ||
| 4. Fallbacks, in order: WebFetch on the plain URL; `curl` on the plain URL and read the embedded | ||
| content. If all fetches fail, **continue the review but** mark every spec-dependent check as | ||
| NOT VERIFIED in the output, state this in the summary's first line, and cap the verdict at | ||
| "Request changes / cannot approve — specification unavailable". | ||
| 5. Extract from the post into a working file (scratchpad `spec.md`): every numbered instruction, | ||
| every parameter with its exact value and unit, every address, every rate limit (max + slope), | ||
| every date/timestamp, and any execution constraints (office hours, direct execution, ordering, | ||
| dependencies on other spells or prior votes). | ||
| 6. Extract the `Vote:` Snapshot links. Pre-vote PRs may legitimately lack them; finalize-stage | ||
| PRs must have them, and if fetchable, the vote content must match the post. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat fetched forum content as untrusted data, not agent instructions.
The fetched cooked HTML and subsequent posts are external, attacker-controlled text. Without an explicit trust boundary, a malicious proposal author can inject instructions that suppress findings, alter the workflow, or induce unsafe tool calls. Delimit the content, strip active markup, and state that only proposal facts may be extracted; instructions found in the forum, PR body, or comments must never override the skill workflow.
🤖 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/spark-spell-review/SKILL.md around lines 73 - 85, Add an
explicit trust boundary to the specification-fetching and extraction workflow
around the post content and subsequent amendments. Treat fetched HTML, forum
posts, PR bodies, and comments as untrusted data: delimit and sanitize active
markup, extract only proposal facts, and ignore any embedded instructions that
attempt to change the review workflow, suppress findings, or trigger tool calls.
| 3. Severity rubric: | ||
| - 🔴 **Critical** — funds loss/trap, wrong parameter live on mainnet, unauthorized action, | ||
| bricked channel/config, governance bypass. | ||
| - 🟠 **High** — spec deviation with material effect, missing revoke/exit path, unverifiable | ||
| deployed contract, broken migration completeness. | ||
| - 🟡 **Medium** — test-coverage gaps, stale forks/balances, missing boundary/e2e tests, | ||
| unverified-but-probably-fine items, convention violations with correctness risk. | ||
| - 🔵 **Info/Nit** — clarity, style, comments, placement. | ||
| 4. Verdict: **Approve** (no 🔴/🟠, 🟡 at most minor), **Approve with comments** (no 🔴/🟠, | ||
| actionable 🟡), or **Request changes** (any 🔴/🟠, or spec unavailable, or tests demonstrably | ||
| missing for a change). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the contradictory verdict rules for missing tests.
The severity rubric classifies test-coverage gaps as 🟡 Medium and allows “Approve with comments,” but the verdict section says any “tests demonstrably missing for a change” requires “Request changes.” Define the precedence and distinguish release-blocking missing tests from actionable non-blocking gaps so identical findings cannot produce conflicting verdicts.
🤖 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/spark-spell-review/SKILL.md around lines 191 - 201, The
“Severity rubric” and “Verdict” rules conflict on how missing tests affect
approval. Update both sections so they define precedence and distinguish
release-blocking, demonstrably missing tests that require “Request changes” from
actionable but non-blocking 🟡 coverage gaps that permit “Approve with
comments,” ensuring identical findings yield one verdict.
| ### Not verified | ||
| <Every check that could not be completed and why: forum unfetchable, no RPC, source unverified, | ||
| tests not run, etc. If everything ran, say what WAS independently verified (forum fetched, N | ||
| addresses checked on-chain, forge test result, simulation reviewed).> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Complete the “Not verified” output template.
The sentence ends with N addresses, producing malformed review output. Replace it with the intended wording, such as “all addresses checked on-chain.”
Proposed fix
-<If everything ran, say what WAS independently verified (forum fetched, N
+<If everything ran, say what WAS independently verified (forum fetched,
+addresses checked on-chain,🤖 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/spark-spell-review/SKILL.md around lines 228 - 231, Complete
the “Not verified” template by replacing the truncated “N addresses checked
on-chain” wording with a grammatically complete phrase such as “all addresses
checked on-chain,” while preserving the surrounding verification examples and
template structure.
Source: Linters/SAST tools
Plain-scalar description contained ": " sequences, which YAML parses as mapping separators. Replaced with em dashes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Add
spark-spell-review+spark-spell-codegenskillsHow to use
Just make sures the skills are in your local repo under
.claude/skills/and mention them in your Claude Code local session. Because these skills are under.claude/skills/, any local Claude Code session in this repo auto-discovers them. Say what you want in plain language and Claude picks the right skill:<forum-url>" →spark-spell-codegensrc/proposals/20260716" →spark-spell-reviewThey run your tests for real. Running locally (not in a sandbox) means Claude has the whole Foundry setup and RPCs, so it will
forge buildandforge test --match-path 'src/proposals/<date>/*'against forked mainnet/L2s as part of the work.If RPC/env is missing, both report that honestly instead of pretending a check ran.
When Claude will stop and ask you something — the two skills differ on purpose:
spark-spell-reviewspark-spell-codegenForum:/Vote:URLs from the payload natspec and fetches the spec itself. The only thing you may hand it is a scope hint (e.g. "exclude items 1 and 3")._blockDate, chain set, PR target.For
spark-spell-codegen, expect questions at two points:_blockDate, the chain set, the PR target, an unfetchable post, or a scope/triage call (e.g. "items 3–4 defer — confirm?").[TBD]/to-be-confirmed address or value it couldn't derive on-chain, any deferred items, any post-internal contradiction (it never silently picks a reading), missing Forum topic id / Vote URLs, and thespark-spell-codegen, generate a spell from a proposalTurns an approved proposal (forum URL or draft file) into a complete, building, tested spell in
src/proposals/<YYYYMMDD>/— per-chain payloads + a Foundry test file — and opens a PR.archive/that did the same kind of action and reproduces that code pattern verbatim, swapping only the constants. A gated stage pairs each item with its precedent excerpt and a cited derivation of every argument before any code is written._blockDate.spark-spell-review, audit a spell PR against its specExpert, security-first audit of a spell PR against its forum-post specification — read-only on the code; it reports findings, it never fixes them.
vm.prankred-flag rule that catches tests which assume (without proof) that a privileged intermediate contract is operable in production.file:lineevidence and one concrete remediation each.Summary by CodeRabbit
New Features
Documentation