This repository was archived by the owner on Mar 30, 2026. It is now read-only.
feat: generalize hooks to swarm, identity, and plugin levels#175
Merged
feat: generalize hooks to swarm, identity, and plugin levels#175
Conversation
Extract HooksSchema as a shared shape reused at all three levels: swarm (clawup.yaml), identity (identity.yaml), and plugin manifests. Execution order is broadest-first (swarm → identity → plugin) for lifecycle and onboard hooks, with most-specific-wins for resolve hooks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Schema tests: HooksSchema standalone, hooks on ClawupManifestSchema and IdentityManifestSchema (validation, backward compat, empty scripts) - Cloud-init pipeline tests: extraHooks merging order (swarm → identity → plugin), extraHooks-only, mixed with plugins, no hooks, multiple plugins, phase ordering in generated script - Onboard hooks unit tests: execution order across all three levels, multi-agent scenarios, skip behavior, error propagation (swarm failure prevents identity/plugin), instructions labeling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds multi-level lifecycle hooks (swarm and identity) to schemas, types, CLI onboarding/resolution orchestration, and Pulumi cloud-init generation; threads swarm/identity hooks before plugin hooks with labeling, env resolution, and extensive tests validating ordering and behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Manifest as Manifest (swarm + identity)
participant CLI as CLI Setup
participant Resolve as Resolve Orchestrator
participant Onboard as Onboard Orchestrator
participant Pulumi as Pulumi Provisioner
participant CloudInit as Cloud-Init Generator
Manifest->>CLI: load manifest.hooks + identity.manifest.hooks
CLI->>Resolve: run resolve hooks (swarm → identity)
Resolve->>Resolve: execute swarm resolve
Resolve->>Resolve: execute identity resolve (prefer identity)
Resolve-->>CLI: resolved env vars
CLI->>Onboard: run onboard hooks (swarm → identity → plugins)
Onboard->>Onboard: run swarm onboard
Onboard->>Onboard: run identity onboard
Onboard->>Onboard: run plugin onboard hooks
Onboard-->>CLI: combined instructions / results
CLI->>Pulumi: pass extraHooks (swarm + identity) + plugin metadata
Pulumi->>CloudInit: build cloud-init config (extraHooks + plugin hooks)
CloudInit->>CloudInit: merge hooks: swarm → identity → plugin
CloudInit-->>Pulumi: generated cloud-init script with ordered hooks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/cli/lib/setup.ts (1)
421-427: Handle duplicate identity resolve keys explicitly.Right now, if two identities define the same resolve env var, the later one silently overwrites the earlier one. That makes behavior order-dependent and hard to diagnose.
💡 Proposed conflict handling
for (const fi of fetchedIdentities) { if (fi.manifest.hooks?.resolve) { for (const [envVar, script] of Object.entries(fi.manifest.hooks.resolve)) { - nonPluginResolve[envVar] = { script, label: `identity:${fi.manifest.name}` }; + const next = { script, label: `identity:${fi.manifest.name}` }; + const prev = nonPluginResolve[envVar]; + if (prev && prev.script !== next.script) { + return { + ok: false, + error: `Conflicting identity resolve hooks for ${envVar}: ${prev.label} vs ${next.label}`, + }; + } + nonPluginResolve[envVar] = next; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/lib/setup.ts` around lines 421 - 427, The loop that builds nonPluginResolve from fetchedIdentities currently lets later identities overwrite earlier keys (see fetchedIdentities, fi.manifest.hooks.resolve, nonPluginResolve); change it to detect duplicates and handle them explicitly by checking if nonPluginResolve[envVar] already exists and then either (a) throw an informative error (including both labels / identity names and the conflicting envVar) or (b) log a clear conflict and skip/abort as your policy requires; update the code that assigns nonPluginResolve[envVar] to perform this existence check and produce the conflict message referencing fi.manifest.name and the existing entry's label so conflicts are not silently overwritten.packages/cli/lib/onboard-hooks.ts (1)
69-89: Extract shared swarm/identity hook execution flow into a helper.The two blocks duplicate the same orchestration (run, print instructions, fail). A small helper would reduce drift risk and keep future changes safer.
Also applies to: 92-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/lib/onboard-hooks.ts` around lines 69 - 89, The swarm and identity onboard blocks duplicate orchestration logic (call runOnboardHook, redact and print instructions, handle failure via exitWithError), so extract that flow into a helper like runAndReportOnboardHook(hook: OnboardHook | undefined, label: string, env: Record<string,string>) which calls runOnboardHook({ script: hook.script, env }), checks result.ok, on success redacts result.instructions with redactSecretsFromString and logs via p.log.info/console, and on failure logs via p.log.error and calls exitWithError; replace the duplicated blocks that reference swarmOnboard and identityOnboard to call this helper with appropriate label strings and env (e.g., "swarm onboard" / "identity onboard").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/lib/onboard-hooks.ts`:
- Line 84: The code is logging raw hook errors (p.log.error(`Swarm onboard hook
failed: ${result.error}`) and the similar call at the other occurrence), which
may contain secrets; replace direct logging of result.error with a
sanitized/redacted message. Implement or call a sanitizer (e.g., redactError or
maskSecrets) to strip tokens/URLs/keys from result.error before passing it to
p.log.error, or log a generic message ("Swarm onboard hook failed: <redacted
error>") plus a non-sensitive error type/ID if needed; update both places that
reference result.error to use the sanitizer and avoid printing raw error
details.
In `@packages/cli/lib/setup.ts`:
- Around line 440-442: The warning currently logs raw resolve-hook output
(progress.log.warn(`Non-plugin resolve hook failed for ${envVar}:
${result.error}`)), which may include sensitive stdout/stderr; change it to a
sanitized/generic message instead. Update the block around s.stop and
progress.log.warn so you still call s.stop(`Failed to resolve ${envVar}
(${label})`) but replace logging of result.error with a non-sensitive message
like `progress.log.warn(\`Non-plugin resolve hook failed for ${envVar}: hook
returned an error\`)` and, if you must capture metadata, only log non-secret
fields (e.g. an error code or truncated/hash of the output) from result (not raw
stderr/stdout). Ensure references: s.stop, progress.log.warn, result.error,
envVar, label are updated accordingly.
---
Nitpick comments:
In `@packages/cli/lib/onboard-hooks.ts`:
- Around line 69-89: The swarm and identity onboard blocks duplicate
orchestration logic (call runOnboardHook, redact and print instructions, handle
failure via exitWithError), so extract that flow into a helper like
runAndReportOnboardHook(hook: OnboardHook | undefined, label: string, env:
Record<string,string>) which calls runOnboardHook({ script: hook.script, env }),
checks result.ok, on success redacts result.instructions with
redactSecretsFromString and logs via p.log.info/console, and on failure logs via
p.log.error and calls exitWithError; replace the duplicated blocks that
reference swarmOnboard and identityOnboard to call this helper with appropriate
label strings and env (e.g., "swarm onboard" / "identity onboard").
In `@packages/cli/lib/setup.ts`:
- Around line 421-427: The loop that builds nonPluginResolve from
fetchedIdentities currently lets later identities overwrite earlier keys (see
fetchedIdentities, fi.manifest.hooks.resolve, nonPluginResolve); change it to
detect duplicates and handle them explicitly by checking if
nonPluginResolve[envVar] already exists and then either (a) throw an informative
error (including both labels / identity names and the conflicting envVar) or (b)
log a clear conflict and skip/abort as your policy requires; update the code
that assigns nonPluginResolve[envVar] to perform this existence check and
produce the conflict message referencing fi.manifest.name and the existing
entry's label so conflicts are not silently overwritten.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/cli/commands/onboard.tspackages/cli/lib/__tests__/onboard-hooks.test.tspackages/cli/lib/onboard-hooks.tspackages/cli/lib/setup.tspackages/core/src/__tests__/schemas.test.tspackages/core/src/index.tspackages/core/src/schemas/identity.tspackages/core/src/schemas/index.tspackages/core/src/schemas/manifest.tspackages/core/src/schemas/plugin-manifest.tspackages/core/src/types.tspackages/pulumi/src/components/__tests__/cloud-init-pipeline.test.tspackages/pulumi/src/components/cloud-init.tspackages/pulumi/src/components/provisioner-config.tspackages/pulumi/src/components/shared.tspackages/pulumi/src/components/types.tspackages/pulumi/src/index.ts
Hook stderr output may contain sensitive values. Apply redactSecretsFromString to all error/warning log messages from swarm, identity, and non-plugin resolve hooks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/lib/onboard-hooks.ts`:
- Around line 91-115: The identity-level onboard hook in the loop over
fetchedIdentities runs for every agent entry causing duplicates; modify the loop
to only run each identity's onboard once by tracking processed identity names
(use identityResult.manifest.name) in a Set and skip if already seen before
calling runOnboardHook; keep existing behavior for logging and error handling
(p.log.info/p.log.error, redactSecretsFromString, exitWithError) but only invoke
it when the identity name is not yet in the Set, then add the name to the Set
after running the hook.
In `@packages/cli/lib/setup.ts`:
- Around line 421-428: The current loop over fetchedIdentities writes
identity-level resolve hooks into nonPluginResolve and silently overwrites
existing keys; update the loop that processes fi.manifest.hooks.resolve so that
before assigning nonPluginResolve[envVar] you check whether that envVar already
exists and if so throw or return a clear error including the conflicting envVar
and both identity names (use fi.manifest.name and the existing entry's label) to
fail-fast on collisions; apply the same conflict check logic to the analogous
swarm-level resolve handling (the block around lines 430-444) so neither
identity- nor swarm-level duplicate resolve keys are silently overwritten.
Comment on lines
91
to
+115
| for (const fi of fetchedIdentities) { | ||
| // --- Identity-level onboard hook (before plugin hooks) --- | ||
| const identityOnboard = fi.identityResult.manifest.hooks?.onboard; | ||
| if (identityOnboard) { | ||
| p.log.info( | ||
| `Running identity onboard hook for ${fi.agent.displayName}: ${identityOnboard.description}` | ||
| ); | ||
|
|
||
| const hookEnv: Record<string, string> = { ...envDict }; | ||
| const result = await runOnboardHook({ script: identityOnboard.script, env: hookEnv }); | ||
| if (result.ok) { | ||
| if (result.instructions) { | ||
| const redacted = redactSecretsFromString(result.instructions); | ||
| console.log(); | ||
| p.log.info(`Follow-up instructions (identity:${fi.identityResult.manifest.name}):`); | ||
| console.log(redacted); | ||
| console.log(); | ||
| } | ||
| } else { | ||
| p.log.error(`Identity onboard hook for ${fi.agent.displayName} failed: ${redactSecretsFromString(result.error)}`); | ||
| exitWithError( | ||
| "Identity onboard hook failed. Fix the issue and run `clawup setup --onboard` again, or run `clawup onboard` separately." | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Run identity-level onboard once per identity, not once per agent entry.
If multiple agents share the same identity manifest, this block executes the same identity onboard hook repeatedly, which can duplicate prompts and side effects.
🔧 Suggested fix (dedupe by identity name)
export async function runOnboardHooks(args: RunOnboardHooksArgs): Promise<void> {
@@
if (swarmOnboard) {
@@
}
+ const executedIdentityOnboard = new Set<string>();
for (const fi of fetchedIdentities) {
// --- Identity-level onboard hook (before plugin hooks) ---
+ const identityName = fi.identityResult.manifest.name;
+ if (executedIdentityOnboard.has(identityName)) {
+ continue;
+ }
const identityOnboard = fi.identityResult.manifest.hooks?.onboard;
if (identityOnboard) {
@@
} else {
p.log.error(`Identity onboard hook for ${fi.agent.displayName} failed: ${redactSecretsFromString(result.error)}`);
exitWithError(
"Identity onboard hook failed. Fix the issue and run `clawup setup --onboard` again, or run `clawup onboard` separately."
);
}
+ executedIdentityOnboard.add(identityName);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/lib/onboard-hooks.ts` around lines 91 - 115, The identity-level
onboard hook in the loop over fetchedIdentities runs for every agent entry
causing duplicates; modify the loop to only run each identity's onboard once by
tracking processed identity names (use identityResult.manifest.name) in a Set
and skip if already seen before calling runOnboardHook; keep existing behavior
for logging and error handling (p.log.info/p.log.error, redactSecretsFromString,
exitWithError) but only invoke it when the identity name is not yet in the Set,
then add the name to the Set after running the hook.
Comment on lines
+421
to
+428
| // Identity-level resolve hooks (override swarm for same key) | ||
| for (const fi of fetchedIdentities) { | ||
| if (fi.manifest.hooks?.resolve) { | ||
| for (const [envVar, script] of Object.entries(fi.manifest.hooks.resolve)) { | ||
| nonPluginResolve[envVar] = { script, label: `identity:${fi.manifest.name}` }; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Handle identity-level resolve key collisions explicitly.
When two identities define hooks.resolve for the same env var, the later one silently overwrites the earlier one. This makes outcomes depend on agent ordering and can resolve to the wrong value without any signal.
🔧 Suggested fix (fail fast on conflicting identity hooks for same key)
// Collect resolve scripts: most-specific-wins on key conflicts (identity > swarm)
const nonPluginResolve: Record<string, { script: string; label: string }> = {};
// Swarm-level resolve hooks (lowest priority)
if (manifest.hooks?.resolve) {
for (const [envVar, script] of Object.entries(manifest.hooks.resolve)) {
nonPluginResolve[envVar] = { script, label: "swarm" };
}
}
// Identity-level resolve hooks (override swarm for same key)
for (const fi of fetchedIdentities) {
if (fi.manifest.hooks?.resolve) {
for (const [envVar, script] of Object.entries(fi.manifest.hooks.resolve)) {
+ const existing = nonPluginResolve[envVar];
+ if (existing && existing.label.startsWith("identity:") && existing.script !== script) {
+ return {
+ ok: false,
+ error: `Conflicting identity resolve hooks for ${envVar}: ${existing.label} vs identity:${fi.manifest.name}`,
+ };
+ }
nonPluginResolve[envVar] = { script, label: `identity:${fi.manifest.name}` };
}
}
}Also applies to: 430-444
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/lib/setup.ts` around lines 421 - 428, The current loop over
fetchedIdentities writes identity-level resolve hooks into nonPluginResolve and
silently overwrites existing keys; update the loop that processes
fi.manifest.hooks.resolve so that before assigning nonPluginResolve[envVar] you
check whether that envVar already exists and if so throw or return a clear error
including the conflicting envVar and both identity names (use fi.manifest.name
and the existing entry's label) to fail-fast on collisions; apply the same
conflict check logic to the analogous swarm-level resolve handling (the block
around lines 430-444) so neither identity- nor swarm-level duplicate resolve
keys are silently overwritten.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Summary
clawup.yaml), identity (identity.yaml), and plugin (plugins/<name>.yaml) — sameHooksSchemashape (resolve, postProvision, preStart, onboard) at all levelsTest plan
pnpm build— all 4 packages compile (core, cli, pulumi, web)pnpm test— 341 tests pass across 19 test files (37 new)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests