Conversation
…back - Add resolvePath() helper for platform-specific Daintree config paths (macOS/Win/Linux) - Detect Daintree first, fall back to canopy-app paths for backwards compatibility - Surface productName, customPresets, globalEnv, providerTemplates in CANOPY_INFO - Update NOT-FOUND banner to mention both Daintree and legacy canopy-app paths - Update DISCOVERED banner with Product line and three new sections (custom presets, global env keys, provider templates) - mcp.json and plugins dir also try ~/.daintree/ first, fall back to ~/.canopy/ - mcpServers lookup tries 'daintree' key first, then legacy 'canopy' Issue 138 AC1: Discovery banner shows customPresets, globalEnv, providerTemplates
…ets export with brand colors
Step 2d - Import preset env into providers.json (issue 138 AC2 + AC5):
- Match Daintree presets to providers.json entries via exact/prefix/model heuristic
- Idempotent merge: never overwrites non-empty existing env values
- globalEnv allowlist: ^(ANTHROPIC_|OPENAI_|GOOGLE_|TOGETHER_|DEEPSEEK_|OLLAMA_|OPENROUTER_|XAI_|MODEL$|.*_BASE_URL$|.*_API_KEY$) covers all BRAND_COLORS providers
- Backup providers.json before write at every candidate path
- Pass CANOPY_INFO and SELECTED_MATCHES_JSON via env vars (no interpolation)
Step 3e - Export quorum slots as Daintree customPresets (issue 138 AC3 + AC4 + AC5):
- Implemented inside Step 3d atomic write to avoid double-backup
- BRAND_COLORS map: openai, google, anthropic, github, xai, opencode, together(.xyz), openrouter, deepseek, ollama; default #6366f1
- Derives env from providers.json: MODEL, ANTHROPIC_BASE_URL, *_API_KEY as ${VAR} placeholders
- Existing customPresets show 'unchanged' status - never overwritten by default
- Result block distinguishes userAgentRegistry vs customPresets entries (two columns)
Closing summary updated with: Product detected, Preset env imported, customPresets added/unchanged counts.
success_criteria section updated to cover all 5 issue 138 acceptance criteria.
…py-app fallback
- Step 1 NF_DETECT script: add Daintree config detection block before project state, populates result.daintree = { installed, product, config_path, custom_preset_count } with try/catch around config read so unreadable configs do not throw
- Step 2 dashboard: insert 'Daintree IDE' section between 'MCP Servers' and 'Project'
- Step 3 section C: add bidirectional bridge mention - if Daintree installed AND nForma installed, recommend /nf:link-canopy with current custom_preset_count from detection result
All edits are additive; zero deletions of existing CLI/MCP detection logic. Bullet idempotency note clarifies safe re-runs (issue 138 AC5).
…providers (issue 138)
Live-testing the previous implementation against an actual Daintree
v20 install (~/Library/Application Support/Daintree/config.json,
_schemaVersion=20) surfaced a schema mismatch and a design issue:
1. Schema correction
- customPresets is a per-agent ARRAY at agentSettings.agents.<a>.customPresets[]
(not an object map at agentSettings.customPresets)
- globalEnv → globalEnvironmentVariables, lives at config root
(not under agentSettings)
- providerTemplates does not exist in v20 — removed all references
- Step 1 reader now aggregates per-agent arrays + reads top-level
globalEnvironmentVariables; banner shows per-agent breakdown
2. Step 2d redesign — fan-out import
The previous "active-preset-only merge" semantics would silently
overwrite a vanilla provider's env from a single Daintree preset
AND fan out to all mainTool=claude providers (including
together-routed ccr-* slots that should NOT receive Anthropic env).
Tested locally: this gave ccr-1..6 wrong env values and would have
broken the quorum.
New design: each Daintree preset becomes its OWN nForma quorum slot
alongside the vanilla. Example with two claude presets:
claude-1 (vanilla, untouched)
claude-z-ai (clone of claude-1 + Z.AI env)
claude-minimax (clone of claude-1 + MiniMax env)
Slot name = {agentName}-{slug(preset.name)} — e.g. "claude-z-ai".
Slug rule: lowercase, non-[a-z0-9] → "-", collapse, trim.
Family gate: provider.mainTool === agentName AND
provider.provider === inferredFamily(preset.env). Family inferred
from env-key prefixes (ANTHROPIC_*, OPENAI_*, GOOGLE_/GEMINI_*,
etc.). Falls back to mainTool-only when no family signal.
3. Step 2d writes to all 3 live files (each backed up first):
- ~/.claude/nf/bin/providers.json — appends new provider entry per
preset, cloning the vanilla and overlaying allowlisted preset env.
- ~/.claude.json mcpServers — clones vanilla MCP entry with
PROVIDER_SLOT={newName} AND UNIFIED_PROVIDERS_CONFIG pointing at
the providers.json we wrote (otherwise unified-mcp-server.mjs
reads the repo source by default and exits "Unknown PROVIDER_SLOT"
for the new entries — found via runtime probe).
- ~/.claude/nf.json quorum_active — appends the new slot name.
4. Idempotency (AC5)
Each new provider carries daintree_preset_id = preset.id. On re-run:
- Existing entry with same id → updated in place (env, description,
family) but name stays stable.
- Preset that Daintree no longer has → preserved by default
(option b: explicit removal only). Avoids destroying slots the
user may have wired into prompts/scripts.
5. Step 3e (export) reworked for v20
Writes to agentSettings.agents.<targetAgent>.customPresets[] (array
push, not object key set). targetAgent = providerEntry.mainTool,
falling back to slot.cli, then 'claude'. Idempotency by id-in-array
lookup. Correct entry shape: {id, name, description, env, color,
fallbacks: [], dangerousEnabled: false}.
6. onboard.md detection updated
Counts customPresets across ALL agent buckets (per-agent arrays)
instead of treating agentSettings.customPresets as a flat object
map.
Allowlist regex unchanged:
^(ANTHROPIC_|OPENAI_|GOOGLE_|TOGETHER_|DEEPSEEK_|OLLAMA_|OPENROUTER_|XAI_|MODEL$|.*_BASE_URL$|.*_API_KEY$)
Verified end-to-end against a live Daintree install: 2 customPresets
(Z.AI, MiniMax) under the claude agent fan out cleanly to claude-z-ai
and claude-minimax slots. All 7 resulting slots respond to MCP
initialize handshake (~95-220ms each) per nf-slot-health-probe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Daintree-first discovery to /nf:link-canopy, parses per-agent Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "CLI / nForma"
participant Discovery as "Discovery Phase"
participant Daintree as "Daintree Config"
participant Import as "Import / Fan-out"
participant Providers as "providers.json"
participant Claude as "~/.claude.json"
participant Registration as "Registration / Export"
User->>CLI: Run /nf:link-canopy
CLI->>Discovery: Detect Daintree / legacy Canopy (platform paths)
Discovery->>Daintree: Read config.json (agentSettings, globalEnvironmentVariables)
Daintree-->>Discovery: Return agents + customPresets
Discovery-->>CLI: Show detection banner
CLI->>Import: Parse presets per agent
Import->>Import: Filter env via allowlist, infer provider family
Import->>Import: Create/clone provider slot per preset, tag daintree_preset_id
Import->>Providers: Write/update providers.json
Import->>Claude: Clone/append MCP entries (~/.claude.json)
Import-->>CLI: Report import results
CLI->>Registration: Map providers to Daintree customPresets
Registration->>Providers: Read providers.json entries
Registration->>Daintree: Write per-agent customPresets (placeholders, colors)
Registration-->>CLI: Report export results
CLI-->>User: Final summary (imported, exported, idempotent statuses)
sequenceDiagram
participant Daintree as "Daintree\n(agentSettings)"
participant nForma as "nForma\n(providers.json)"
participant MCP as "MCP Servers\n(~/.claude.json)"
rect rgba(100,150,200,0.5)
Note over Daintree,nForma: Import: Presets → Providers
Daintree->>nForma: customPresets[].env per agent
nForma->>nForma: Create/update provider slots (daintree_preset_id)
nForma->>MCP: Register MCP entries for daintree servers
end
rect rgba(200,150,100,0.5)
Note over nForma,Daintree: Export: Providers → Presets
nForma->>nForma: Read providers.json
nForma->>Daintree: Convert to customPresets format (placeholders, colors)
Daintree->>Daintree: Store per-agent presets (idempotent)
end
rect rgba(150,200,100,0.5)
Note over Daintree,nForma: Idempotency
Daintree->>nForma: Check daintree_preset_id / preset id
nForma->>nForma: Skip unchanged writes, report "unchanged"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds bidirectional syncing between a local Daintree (legacy Canopy) install and nForma quorum slots, and surfaces Daintree detection in onboarding so users discover /nf:link-canopy.
Changes:
- Extend
/nf:link-canopyto detect Daintree-first (with canopy-app fallback), import Daintree presets into new nForma slots (fan-out), and export nForma slots back into DaintreecustomPresetswith brand colors. - Update
onboard.mdto detect Daintree installs + count custom presets, and recommend/nf:link-canopyin the onboarding routing text. - Add/update planning artifacts for quick task 405 and update
.planning/STATE.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
onboard.md |
Adds Daintree detection + dashboard section + onboarding routing hint for /nf:link-canopy. |
commands/nf/link-canopy.md |
Implements Daintree-first discovery, preset fan-out import into nForma slots, and export of quorum slots as Daintree custom presets with brand colors. |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-PLAN.md |
Captures the intended implementation plan for task 405. |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-SUMMARY.md |
Summarizes the task outcome and manual verification checklist. |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.md |
Records verification claims/checklist for task 405. |
.planning/STATE.md |
Resolves conflict markers and records quick task 405 entry + last activity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 (4)
commands/nf/link-canopy.md (4)
930-943: ⚡ Quick winAdd language to the Register result output example block (MD040).
markdownlint flags around Line 932. Add a
textlanguage label to avoid the MD040 warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/nf/link-canopy.md` around lines 930 - 943, The markdown example for REGISTER_RESULT lacks a fenced code block language tag causing MD040; update the example code fence that shows the "Agents registered in Canopy" output to use a language label (e.g., add "text" after the opening ``` so it becomes ```text) to satisfy markdownlint, ensuring the block that demonstrates REGISTER_RESULT output remains unchanged except for the added language label.
616-633: ⚡ Quick winAdd language to the “Fan-out import complete” output example block (MD040).
markdownlint flags the fenced block around Line 618. Add
textfor the banner/output examples.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/nf/link-canopy.md` around lines 616 - 633, The fenced code block that begins with "✓ Fan-out import complete" is missing a language tag which triggers markdownlint MD040; update the opening fence from ``` to ```text so the block is explicitly marked as plain text (locate the block containing the "✓ Fan-out import complete" output example in the link-canopy documentation and change its opening fence accordingly).
366-381: ⚡ Quick winAdd language to the second allowlist regex block (MD040).
Also flagged by markdownlint at/around Line 368. Adding a language (e.g.,
text) should remove the warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/nf/link-canopy.md` around lines 366 - 381, Banner code block lacks a language tag which triggers markdownlint MD040; update the fenced code block shown in the diff (the triple-backtick block that begins the "Display the REVIEW FAN-OUT IMPORT banner" sample) to include an explicit language identifier (e.g., change the opening ``` to ```text) so the second allowlist regex block warning is suppressed; ensure only the opening fence is modified to include the language and leave the block content unchanged.
356-361: ⚡ Quick winAdd language to the allowlist regex fenced code block (MD040).
Your markdownlint output flags the fenced block at/around Line 358 for missing a language identifier. Add
text(orregex) for consistent rendering and lint cleanliness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/nf/link-canopy.md` around lines 356 - 361, The fenced code block showing the Allowlist regex under the "Allowlist (AC5 safety guardrail)" section is missing a language identifier; update the block that contains the regex ^(ANTHROPIC_|OPENAI_|GOOGLE_|TOGETHER_|DEEPSEEK_|OLLAMA_|OPENROUTER_|XAI_|MODEL$|.*_BASE_URL$|.*_API_KEY$) by adding a language tag such as text or regex immediately after the opening triple backticks (e.g., ```text) so markdownlint MD040 is satisfied and the block renders consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/nf/link-canopy.md`:
- Around line 856-895: The code skips adding customPresets when the computed
targetAgent (from targetAgentFor(slot, providerEntry)) has no agentBucket;
change this to attempt a safe fallback to 'claude' before skipping: after
computing targetAgent and agentBucket, if (!agentBucket) set targetAgent =
'claude' and re-resolve agentBucket = config.agentSettings.agents[targetAgent];
if that resolves, proceed to add the preset under that agentBucket using the
existing presetId logic and push a registered entry indicating status 'added
(fallback to claude)' or 'skipped (no agent: claude)' as appropriate; keep
existing idempotency/never-overwrite behavior for customPresets.
- Around line 901-909: The export currently only copies providerEntry.model into
env.MODEL and special-cases providerEntry.env.ANTHROPIC_BASE_URL into
env.ANTHROPIC_BASE_URL while only placeholdering keys that match /_API_KEY$/;
update the logic around providerEntry.env so that any key matching /_BASE_URL$/
is also copied into env (e.g., env[key] = providerEntry.env[key]) rather than
only handling ANTHROPIC_BASE_URL, keeping the existing API key placeholder
behavior and preserving MODEL; modify the block that constructs env (the const
env = {} / if (providerEntry.model) ... / if (providerEntry.env && ...) / for
(const k of Object.keys(providerEntry.env || {})) ...) to include exporting all
*_BASE_URL entries.
- Around line 517-523: The fallback slug construction for baseSlug can throw if
preset.id isn't iterable because it spreads preset.id; update the fallback to
coerce preset.id to a string before iterating so it's safe for numbers and other
non-iterables. Locate the expression that builds baseSlug (the line using
slugify(preset.name) || ('preset-' +
Math.abs([...preset.id].reduce(...)).toString(36))) and change it to use
String(preset.id) (e.g., iterate over String(preset.id) or otherwise call
String(preset.id) before the reduce) so candidate formation and subsequent
SLOT_NAME_RE checks remain safe; ensure variable names baseSlug, preset.id,
slugify, candidate, and SLOT_NAME_RE are preserved.
- Around line 542-559: The code is replacing provider.env wholesale with
item.env which contains only allowlisted keys and thus wipes out other necessary
env entries; instead, for the add branch (newProvider) start from the vanilla
env (v.env) and overlay the allowlisted keys from item.env onto that base, and
for the update branch merge item.env into existing.env (preserving existing
keys) rather than assigning it wholesale; update the logic around
newProvider.env and existing.env to perform a shallow merge of allowlisted keys
onto the base env (v.env or existing.env) so only allowlisted keys are changed
while other env entries remain intact.
- Around line 509-559: The import step populates provider.env but never updates
the provider.model from item.env.MODEL, so preset model overrides are lost;
update both the 'add' branch (where newProvider is created from v) and the
'update' branch (where existing is found) to set provider.model = item.env.MODEL
if present (falling back to the vanilla/model that was already there) so
providerEntry.model reflects the preset and preserves round-trip idempotency;
reference the plan handling for item.kind === 'add' and item.kind === 'update',
the newProvider/existing objects, and the item.env.MODEL and providerEntry.model
fields when making this change.
---
Nitpick comments:
In `@commands/nf/link-canopy.md`:
- Around line 930-943: The markdown example for REGISTER_RESULT lacks a fenced
code block language tag causing MD040; update the example code fence that shows
the "Agents registered in Canopy" output to use a language label (e.g., add
"text" after the opening ``` so it becomes ```text) to satisfy markdownlint,
ensuring the block that demonstrates REGISTER_RESULT output remains unchanged
except for the added language label.
- Around line 616-633: The fenced code block that begins with "✓ Fan-out import
complete" is missing a language tag which triggers markdownlint MD040; update
the opening fence from ``` to ```text so the block is explicitly marked as plain
text (locate the block containing the "✓ Fan-out import complete" output example
in the link-canopy documentation and change its opening fence accordingly).
- Around line 366-381: Banner code block lacks a language tag which triggers
markdownlint MD040; update the fenced code block shown in the diff (the
triple-backtick block that begins the "Display the REVIEW FAN-OUT IMPORT banner"
sample) to include an explicit language identifier (e.g., change the opening ```
to ```text) so the second allowlist regex block warning is suppressed; ensure
only the opening fence is modified to include the language and leave the block
content unchanged.
- Around line 356-361: The fenced code block showing the Allowlist regex under
the "Allowlist (AC5 safety guardrail)" section is missing a language identifier;
update the block that contains the regex
^(ANTHROPIC_|OPENAI_|GOOGLE_|TOGETHER_|DEEPSEEK_|OLLAMA_|OPENROUTER_|XAI_|MODEL$|.*_BASE_URL$|.*_API_KEY$)
by adding a language tag such as text or regex immediately after the opening
triple backticks (e.g., ```text) so markdownlint MD040 is satisfied and the
block renders consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 22dfc490-bd25-43dd-859f-4ea2d5f260d3
⛔ Files ignored due to path filters (4)
.planning/STATE.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-PLAN.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-SUMMARY.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.mdis excluded by!.planning/**
📒 Files selected for processing (2)
commands/nf/link-canopy.mdonboard.md
…irectional-fleet-sync # Conflicts: # .planning/STATE.md
Code fixes (commands/nf/link-canopy.md):
- Step 2d add: overlay allowlisted preset env onto vanilla provider env
instead of wholesale replacement (preserves runtime fields the user
may have hand-added on the slot)
- Step 2d update: same overlay rule, plus refresh `description` so a
renamed preset shows the new name (was documented but not implemented)
- Both branches now propagate `env.MODEL` to `provider.model` so Step 3d
export round-trips correctly when a preset overrides MODEL
- baseSlug fallback: String(preset.id) coercion so non-string ids
(numbers, etc.) don't throw during the spread
- userAgentRegistry command: treat slot.cli === 'unknown' as missing
(Step 3a sets that literal when provider.cli is null) — prefer
providerEntry.mainTool over the sentinel
- Step 3e targetAgentFor: same 'unknown' guard
- Step 3e: fall back to the 'claude' Daintree bucket when the resolved
targetAgent doesn't exist in agentSettings.agents (covers wrapper /
router slots like CCR that drive the claude CLI but route through a
different upstream provider). Status reports
'added (fallback to claude)' for visibility.
- Step 3e: export ALL *_BASE_URL keys verbatim, not just
ANTHROPIC_BASE_URL. Endpoints aren't secrets and per-provider
overrides need to round-trip back to Daintree.
Documentation fixes (commands/nf/link-canopy.md):
- Slot-naming description corrected to {agentName}-{slug} (was
{vanillaSlotName}-{slug}, didn't match the implementation)
- Step 2d idempotency block now explicitly distinguishes vanilla
(never touched) from preset-linked (replaced-in-place) slots, plus
the export side's no-overwrite rule
- Closing summary text rewritten to match actual semantics — drops the
blanket "NEVER overwritten" claim and lists the three idempotency
rules separately
- Step 3e implementation notes updated for *_BASE_URL exports + claude
fallback + 'unknown' cli handling
- success_criteria entry rewritten to describe fan-out, not merge
Documentation fixes (onboard.md):
- Daintree IDE bridge text now uses the v20 schema path
(agentSettings.agents.<agent>.customPresets[]) instead of the stale
agentSettings.customPresets reference, and clarifies the fan-out +
per-direction idempotency rules
Planning docs (405-PLAN/SUMMARY/VERIFICATION.md):
- Each frontmatter now has an `implementation_evolution` block that
flags the body as historical and points at the shipped design
changes. Body kept intact for record. Verification block adds a
runtime-verification subsection summarizing what was tested live.
Conflict resolution: merged origin/main (slot-statusline #141) into
the branch — single conflict in .planning/STATE.md (both sides added
a row 405) resolved by keeping both rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/nf/link-canopy.md`:
- Around line 103-113: The active preset check only accepts strings; change the
guard in the agent loop (where config.agentSettings.agents is iterated and
result.activePresetIdByAgent is set) to accept any non-null/undefined presetId
(e.g., use agentCfg.presetId != null) instead of typeof === 'string', then
assign agentCfg.presetId to result.activePresetIdByAgent[agentName]; ensure the
stored type remains consistent with customPresets[].id across the codebase.
- Around line 542-635: The final process.stdout.write currently includes the
full plan (variable plan) which may contain secret env values; remove plan from
the emitted object (or replace it with a redacted version) before writing to
stdout. Locate the process.stdout.write(...) call and change the payload to
exclude plan (e.g., only include written, providersPath, claudeJsonPath,
nfJsonPath, summary) or construct a sanitizedPlan that strips any env keys
matching /API_KEY$/ or other sensitive patterns and include that instead; ensure
no other references to plan are printed thereafter.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c5ad57e-2937-44c2-8697-f00b9396c44f
⛔ Files ignored due to path filters (4)
.planning/STATE.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-PLAN.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-SUMMARY.mdis excluded by!.planning/**.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.mdis excluded by!.planning/**
📒 Files selected for processing (2)
commands/nf/link-canopy.mdonboard.md
🚧 Files skipped from review as they are similar to previous changes (1)
- onboard.md
- CRITICAL (security): drop `plan` from the Step 2d stdout payload. plan[].env carries raw preset.env values including *_API_KEY tokens (the allowlist passes raw values, not redactions); emitting it would leak secrets to terminal/logs/CI artifacts. The `summary` field already carries only key NAMES (envKeys arrays), no values. - MAJOR: normalize daintree_preset_id to String() everywhere — plan match, lookup, and stored field. Without this, a Daintree schema change between runs (string id vs number id) would fail strict equality and produce duplicate preset-linked providers instead of updating in place. AC5 idempotency violation. - MINOR: drop `typeof === 'string'` guard on agentCfg.presetId. Use != null so non-string presetIds (numbers, etc.) still mark the active preset correctly in the discovery banner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/nf/link-canopy.md`:
- Line 364: Several fenced code blocks in the link-canopy markdown are missing
language identifiers (plain "```") which triggers markdownlint MD040; replace
those bare fences with language-tagged fences (e.g., "```text" or the
appropriate language like "```json"/"```bash") for each occurrence (the fences
currently shown as "```" at the flagged locations) so the linter recognizes the
block language and the files pass MD040. Ensure you update every similar bare
fence in the document (including the additional occurrences noted) to the
correct language identifier.
- Around line 524-525: The plan.push call currently sets vanilla: vanilla ||
existing which causes re-imports to repeatedly append the "— Daintree preset:
..." suffix because later code rebuilds existing.description from that vanilla;
fix by ensuring the vanilla used for updates is the original unsuffixed source
(e.g., clone or derive a vanillaWithoutSuffix from existing with the preset
suffix removed) or by changing the description-rebuild logic in the update path
to first strip any existing "— Daintree preset:" suffix before appending; target
the plan.push site (where vanilla is chosen) and the description-rebuild code
that mutates existing.description to prevent double-appending.
- Around line 452-456: The ALLOWLIST regex used by filterAllowlisted is missing
the GEMINI_ prefix so GEMINI_API_KEY and related keys get dropped; update the
ALLOWLIST constant (referenced by filterAllowlisted) to include GEMINI_ (e.g.,
add GEMINI_ to the alternation), then run a quick check against inferFamily
usage to confirm GEMINI_* env keys are preserved during import.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c36f460-5925-402d-89c1-7193a3c29b40
📒 Files selected for processing (1)
commands/nf/link-canopy.md
There was a problem hiding this comment.
Pull request overview
This PR extends the /nf:link-canopy workflow to support Daintree (formerly Canopy) v20 by detecting Daintree installs, reading/writing v20 preset schema, and performing bidirectional sync between Daintree presets and nForma quorum slots.
Changes:
- Add Daintree (and legacy canopy-app) install detection + preset counting to
onboard.md. - Update
/nf:link-canopydiscovery to read Daintree v20 per-agentcustomPresets[]plus top-levelglobalEnvironmentVariables. - Add a fan-out import step that creates/updates nForma provider slots per Daintree preset and exports nForma slots back into Daintree’s per-agent
customPresets[]with brand colors.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| onboard.md | Adds Daintree install detection and surfaces custom preset counts in the onboarding dashboard + routing guidance. |
| commands/nf/link-canopy.md | Implements Daintree-first path resolution, v20 schema parsing, fan-out preset→slot import, and slot→Daintree customPresets export. |
| .planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.md | Adds verification notes and human test checklist for the bidirectional sync work. |
| .planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-SUMMARY.md | Records implementation decisions and historical plan deltas for task 405. |
| .planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-PLAN.md | Captures the original plan and documents implementation-evolution deltas. |
| .planning/STATE.md | Adds task 405 entry for the bidirectional sync work and resolves prior conflict markers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ix dup
- BUG: GEMINI_ and GROK_ prefixes were matched by inferFamily but
missing from ALLOWLIST. Presets keying only on GEMINI_*/GROK_* would
have their env silently dropped before family inference could run on
them. Added both to the allowlist regex (updated in code, banner
text, and success_criteria).
- BUG: description suffix duplication on re-import. Plan stored
`vanilla: vanilla || existing`, then update branch rebuilt
description from item.vanilla.description — but when vanilla was
null, item.vanilla === existing (the cloned slot, already suffixed).
Each re-run would compound " — Daintree preset: X". Fix: pass
vanilla as-is (may be null), and only refresh description when a
fresh non-cloned vanilla (no daintree_preset_id) is available.
- BUG/doc-mismatch: env layering. Doc said globalEnv merges
non-overwrite, but code did `{...vanilla.env, ...item.env}` where
item.env was already globalEnv-overlaid-with-preset. Effective
precedence was preset > globalEnv > vanilla — i.e. globalEnv
overwrote vanilla. Restructured plan to carry presetEnv and
globalEnv separately; apply now composes as
`{...globalEnv, ...vanilla.env, ...presetEnv}` so the documented
precedence (vanilla wins over globalEnv, preset wins over both)
actually holds. Same fix on the update path.
- Stale comment: activePresetIdByAgent description now correctly
notes Step 2d uses fan-out (every preset → new slot), so the
active-preset id is informational/banner-only, not an import gate.
- Stale verification doc: human_verification.expected entries
rewritten to describe the fan-out + 3-layer env composition, with
the actual idempotency rules (vanilla untouched, preset-linked
in-place by daintree_preset_id String() match, description-suffix
guard via vanilla check).
Acknowledged but deferred: markdown lint MD040 (fenced blocks without
language identifiers) — pure lint, will land in a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
commands/nf/link-canopy.md (1)
364-400:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix markdownlint MD040: add language identifiers to fenced code blocks (allowlist + “REVIEW FAN-OUT IMPORT” banner).
Several fenced blocks are started with bare ``` which triggers MD040. Add a language (e.g.,
text) to the opening fence(s).🛠️ Proposed patch
-``` +```text ^(ANTHROPIC_|OPENAI_|GOOGLE_|GEMINI_|TOGETHER_|DEEPSEEK_|OLLAMA_|OPENROUTER_|XAI_|GROK_|MODEL$|.*_BASE_URL$|.*_API_KEY$)@@
-+text
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
nForma ► REVIEW FAN-OUT IMPORT
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
@@
Each new slot also gets:
- cloned MCP server entry in ~/.claude.json (PROVIDER_SLOT={newName})
- appended to quorum_active in ~/.claude/nf.json
- daintree_preset_id field for re-import idempotency
@@
Backups (timestamped) are written for all three files before any change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/nf/link-canopy.md` around lines 364 - 400, The markdown has fenced code blocks without language identifiers causing MD040; update the two blocks containing the allowlist regex (the block starting with ^(ANTHROPIC_|OPENAI_... ) and the "REVIEW FAN-OUT IMPORT" banner block that begins with the long line of ━ characters and the text "nForma ► REVIEW FAN-OUT IMPORT") to use a language tag (e.g., change ``` to ```text) on their opening fences so markdownlint MD040 is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/nf/link-canopy.md`:
- Around line 667-684: Change the opening code fence for the "Fan-out import
complete" rendered output block from an unannotated ``` to a language-tagged
fence (```text) so markdownlint MD040 is satisfied; locate the banner block
containing the line "✓ Fan-out import complete" and update its opening triple
backticks to ```text while leaving the block contents and closing fence
unchanged.
- Around line 999-1012: The fenced code block that begins with "✓ Agents
registered in Canopy" is missing a language identifier which triggers
markdownlint MD040; update the opening fence from ``` to ```text so the block is
declared as plain text (look for the code block containing the sample two-column
output and the line "⚠ Restart Daintree (or canopy-app) for changes to take
effect." and add the language identifier to the opening backticks).
- Around line 196-205: The active preset matching uses strict === between
preset.id and activeId which can fail if types differ; update the comparisons in
the Step 1 discovery banner so both sides are coerced to strings before
comparing (e.g., when computing presets.find(p => ...?.name) and when setting
marker = ... ? "★" : " "), using the existing symbols activePresetIdByAgent,
customPresets (or presets), preset.id, activeId, and the marker assignment so
the displayed active name and star correctly reflect matches regardless of id
type.
---
Duplicate comments:
In `@commands/nf/link-canopy.md`:
- Around line 364-400: The markdown has fenced code blocks without language
identifiers causing MD040; update the two blocks containing the allowlist regex
(the block starting with ^(ANTHROPIC_|OPENAI_... ) and the "REVIEW FAN-OUT
IMPORT" banner block that begins with the long line of ━ characters and the text
"nForma ► REVIEW FAN-OUT IMPORT") to use a language tag (e.g., change ``` to
```text) on their opening fences so markdownlint MD040 is satisfied.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7817950d-cf2a-4c19-a5f3-c2097b975059
⛔ Files ignored due to path filters (1)
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.mdis excluded by!.planning/**
📒 Files selected for processing (1)
commands/nf/link-canopy.md
- Discovery banner: use String() coercion on both sides of the active preset comparison (presets.find(p => String(p.id) === activeId) and the marker assignment). Mirrors the data-side normalization done for daintree_preset_id; without it the active marker silently shows "(none)" when Daintree emits id as a number while customPresets[].id is a string (or vice-versa). - MD040: added `text` language to the two banner code fences flagged by markdownlint (Fan-out import complete and Agents registered in Canopy result blocks). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the /nf:link-canopy command and onboarding flow to support Daintree (formerly Canopy) v20 by detecting Daintree installs first (with legacy canopy-app fallback) and enabling bidirectional sync between Daintree presets and nForma quorum slots/providers.
Changes:
- Add Daintree install + preset discovery to
onboard.mdand surface it in the onboarding dashboard/routing text. - Update
/nf:link-canopydiscovery to read Daintree v20 schema (agentSettings.agents.*.customPresets[],globalEnvironmentVariables) and add a new fan-out import step that creates/updates one nForma slot per Daintree preset. - Extend
/nf:link-canopyexport to write nForma slots back into Daintree per-agentcustomPresets[]with provider brand colors and idempotent “no-overwrite” semantics.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
onboard.md |
Adds Daintree detection (Daintree-first, canopy-app fallback), preset counting across per-agent customPresets[], and onboarding guidance to run /nf:link-canopy. |
commands/nf/link-canopy.md |
Implements Daintree-first discovery, fan-out preset→slot import (providers.json + ~/.claude.json + ~/.claude/nf.json), and slot→Daintree customPresets export with brand colors/idempotency. |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-VERIFICATION.md |
Adds verification artifact describing runtime checks and historical plan evolution notes (currently still mixes stale “truths” with “shipped” behavior). |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-SUMMARY.md |
Adds summary artifact with an implementation-evolution disclaimer, but still contains top-level text that describes the superseded design. |
.planning/quick/405-bidirectional-fleet-sync-canopy-presets-/405-PLAN.md |
Adds plan artifact with frontmatter “implementation_evolution” corrections documenting schema + design changes. |
.planning/STATE.md |
Records completion entry for the new quick task 405 (issue 138) and resolves prior conflict markers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… HISTORICAL banners
- BUG: customPreset export only emitted ${KEY} placeholders for keys
ending in _API_KEY, leaving ANTHROPIC_AUTH_TOKEN (and other
*_AUTH_TOKEN / *_TOKEN keys) unprotected. Result: those auth keys
never round-tripped to the exported Daintree preset, so a user who
imported a Z.AI preset (which uses ANTHROPIC_AUTH_TOKEN, not
ANTHROPIC_API_KEY) and then re-exported back would get a customPreset
missing its auth. Broadened the secret-bearing pattern to
/_(API_KEY|AUTH_TOKEN|TOKEN)$/. Also extended the verbatim-pass-
through to other allowlisted prefixes (ANTHROPIC_*/OPENAI_*/GOOGLE_*/
GEMINI_*/XAI_*/GROK_*) — provider-prefixed env keys round-trip even
when not endpoint or secret.
- BUG (edge): daintree_preset_family wasn't cleared on update when
inferFamily returned null. A preset whose env mutated to drop its
family signal would keep its old family field stale forever.
Explicitly delete existing.daintree_preset_family in that case.
Planning docs (405-PLAN/SUMMARY/VERIFICATION.md):
Frontmatter `implementation_evolution` blocks were already added in
earlier rounds, but bots kept reading the body as authoritative and
flagging stale assertions. Added prominent HISTORICAL banners at
the top of each body that explicitly: name the doc as historical,
point at the authoritative location (link-canopy.md Step 2d preamble
+ frontmatter), and call out the specific schema/design corrections.
SUMMARY also gets a "Shipped design (one-liner)" inline so the
current behavior is the FIRST thing readers see.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 1, 2026
3 tasks
jobordu
added a commit
that referenced
this pull request
May 1, 2026
…es (MD040) (#144) markdownlint-cli2 flagged 10 bare ```...``` fences in commands/nf/link-canopy.md. The two banner-output blocks were fixed during PR #142 review; this commit sweeps the remaining 10 (all rendered output / regex / banner text — `text` is the right tag). CodeRabbit previously deferred this as a separate PR to keep #142 focused on the bidirectional fleet sync redesign. Co-authored-by: jobordu <jonathan@jonathanborduas.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jobordu
added a commit
that referenced
this pull request
May 1, 2026
…#148) Before: install.js unconditionally copyFileSync'd bin/providers.json over the top of ~/.claude/nf/bin/providers.json. Any provider entries the user had added — fan-out preset slots from /nf:link-canopy (carrying daintree_preset_id), ccr-* slots manually re-added via /nf:mcp-setup after the default-fleet retirement, hand-rolled custom slots — were silently destroyed on every npm update / install.js run. This was observed in practice: PR #142's bidirectional-sync testing required restoring claude-z-ai and claude-minimax slots multiple times because intermediate install.js runs kept clobbering them. After: a new mergeProvidersJson() helper does name-keyed merging: - Repo-shipped slots (codex-1, gemini-1, etc.): REPLACED with the repo version so metadata bumps (description, mainTool, model defaults) propagate on update. - User-only slots (no name match in repo): PRESERVED verbatim, appended after the repo entries. This covers fan-out preset slots, manually-added ccr-* slots, and any user customization. - Top-level fields merged shallowly (repo wins; today the file is just `{providers: [...]}` so this is effectively a no-op). - Atomic write via .merge.tmp + rename so a power-cut mid-merge never leaves a half-written file. - Fail-open: corrupt user JSON → fall back to repo overwrite (so installs never wedge); corrupt repo JSON → leave user file untouched. The merge logic lives in a new module bin/install-helpers.cjs so it can be exercised by node --test without spawning a real install. 9 new tests cover: fresh install, single preserved fan-out slot, metadata bump on shared slot, multiple user extras, corrupt user JSON fallback, corrupt repo JSON bail, atomic-write cleanup, malformed user.providers handling, and ordering invariants. install.js wraps the helper with its color-coded log adapter so the install output reads the same as before for repo-only updates, plus a `✓ providers.json: merged repo defaults; preserved N user-added slot(s): <names>` line when there are extras. Co-authored-by: jobordu <jonathan@jonathanborduas.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #138.
Summary
/nf:link-canopynow bidirectionally syncs between a Daintree (formerly Canopy) IDE install and nForma's quorum slots:customPresets[]arrays so the user can launch them from Daintree's preset palette, with provider-specific brand colors.Example: with two Daintree
claudepresets (Z.AI, MiniMax), the import produces:Why the fan-out design
The original implementation in this branch used "active-preset-only merge" — it took whichever Daintree preset was selected and applied its env to every nForma provider with
mainTool === agentName. Live testing surfaced two problems:Schema mismatch. The code targeted
agentSettings.customPresets(object map) but Daintree v20 stores them atagentSettings.agents.<agent>.customPresets[](per-agent arrays). The discovery banner always reported0 customPresetsregardless of what was configured. Same forglobalEnv(real key:globalEnvironmentVariables, top-level) andproviderTemplates(does not exist in v20).Wrong fan-out target. With the schema fixed, the active-preset import gave Z.AI's
ANTHROPIC_AUTH_TOKENto everymainTool=claudeprovider — including theccr-*Together.xyz-routed slots, which expectTOGETHER_API_KEY. Verified by running it once and rolling back via the timestamped backup.The fan-out design solves both: each preset gets its own dedicated slot, family gate (
provider.mainTool === agentNameANDprovider.provider === inferredFamily(preset.env)) ensures Z.AI doesn't leak into Together-routed slots, and the user can switch between presets by picking which slot to invoke.Slot naming
{agentName}-{slug(preset.name)}— e.g.claude-z-ai,claude-minimax.[a-z0-9]→-→ collapse → trim. Matches nForma's canonical slot regex^[a-z][a-z0-9-]*$.-2,-3suffix.What gets written per new slot
~/.claude/nf/bin/providers.jsondaintree_preset_idfor re-import idempotency.~/.claude.jsonmcpServersPROVIDER_SLOT+UNIFIED_PROVIDERS_CONFIGpointing at the providers.json we wrote (otherwiseunified-mcp-server.mjsreads the repo source by default and exitsUnknown PROVIDER_SLOT).~/.claude/nf.jsonquorum_activeAll three files are backed up with a timestamped suffix before any change.
Idempotency (AC5)
daintree_preset_id→ updated in place, name stable, env replaced.Allowlist (AC5 safety guardrail)
Both preset env and
globalEnvironmentVariableskeys are filtered through:Generic free-form keys (e.g.
API_TIMEOUT_MS,CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC) are blocked.Onboard.md
Detection counts customPresets across ALL agent buckets in the v20 schema, not the (non-existent) flat
agentSettings.customPresetsobject.Test plan
claude-z-aiandclaude-minimaxslotsinitializehandshake (~95-220ms each)~/.claude/nf/slot-health.jsoncorrectly via the new statusline probe (feat(statusline): show quorum-slot responsiveness #141)daintree_preset_idlookup matches existing entries (defer until re-test on user's machine)Backwards compatibility
canopy-applegacy paths preserved as fallback in Step 1, banner labels, mcp.json fallback, plugins dir fallback, andmcpServers.canopyfallback key. Legacy installs without Daintree but with canopy-app paths still detect successfully.Related issues / follow-ups (not in this PR)
install.jsclobbers user-customizedproviders.json— observed during testing: runninginstall.js --claude --globaloverwrote the fan-out additions with the repo's vanilla providers list. User-specific entries should be preserved on install.pr-merge-autopilot.shbash bug —set -e + ((var++))exits silently on the first pending check. Patched locally to land this PR; needs its own fix-PR.bin/providers.json— user instruction during testing said they aren't supposed to exist anymore. Live install was cleaned but the repo source still has them.Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Documentation