Claude/spec driven plan system wx oxs#6
Conversation
This change implements a spec-driven workflow where approved plans are saved as specs rather than immediately implemented. Key features: - Add InstructionsPrompt component with collapsible UI for user instructions - Auto-generate default instructions based on plan content (infers path from title/content, defaults to ./docs/specs/<namespace>/<filename>.md) - Include instructions with both approval and denial responses - Update hook output to pass instructions to Claude with clear formatting - Update sharing system to serialize instructions in share URLs The instructions guide Claude to: - Save approved plans to a spec file (not implement automatically) - Handle feedback as expected when changes are requested
- Update install scripts (sh, ps1, cmd) to download from new repo - Update marketplace.json and plugin.json with new owner - Update package.json with new repo URL and homepage (plannotator.ai) - Bump version to 0.3.0 for spec-driven release
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates repository ownership from "backnotprop" to "bradennapier" and introduces a new instructions feature that allows users to provide custom guidance to Claude on how to handle plan approvals and denials. The version is bumped from 0.2.1 to 0.3.0 to reflect these changes.
- Repository references updated across install scripts and metadata files
- New instructions prompt component that persists user preferences and can be shared via URL
- Instructions are sent to Claude with plan approval/denial decisions
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install.sh | Updated repository references from backnotprop to bradennapier |
| scripts/install.ps1 | Updated repository references from backnotprop to bradennapier |
| scripts/install.cmd | Updated repository references from backnotprop to bradennapier |
| package.json | Updated version to 0.3.0, author and repository URLs to bradennapier, homepage to plannotator.ai |
| .claude-plugin/marketplace.json | Updated owner name to bradennapier |
| apps/hook/.claude-plugin/plugin.json | Updated version to 0.3.0, author and repository to bradennapier, added spec-driven keyword |
| packages/ui/utils/sharing.ts | Added optional instructions field to SharePayload, includes instructions in share URL generation |
| packages/ui/hooks/useSharing.ts | Added instructions parameter, loads and returns instructions from shared URLs |
| packages/ui/components/InstructionsPrompt.tsx | New component providing collapsible UI for editing instructions with default generation logic |
| packages/editor/App.tsx | Integrated instructions feature with cookie persistence, share URL loading, auto-generation, and API submission |
| apps/hook/server/index.ts | Modified approval/denial endpoints to accept and process instructions, formats them into structured messages for Claude |
| bun.lock | Updated opencode-plugin version, changed dependencies from dev to peer, added bun package entries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Persist instructions to cookies when they change | ||
| useEffect(() => { | ||
| if (instructions.trim()) { | ||
| storage.setItem('plannotator-instructions', instructions); |
There was a problem hiding this comment.
The instructions are only persisted to storage when they have content (instructions.trim() is truthy), but there's no mechanism to remove them from storage when the user clears the instructions field. This means once instructions are set, they can never be fully cleared from storage even if the user deletes all text. Consider also calling storage.removeItem('plannotator-instructions') when instructions.trim() is empty.
| storage.setItem('plannotator-instructions', instructions); | |
| storage.setItem('plannotator-instructions', instructions); | |
| } else { | |
| storage.removeItem('plannotator-instructions'); |
| // Apply loaded instructions from shared URL | ||
| useEffect(() => { | ||
| if (loadedInstructions && !instructions.trim()) { | ||
| setInstructions(loadedInstructions); | ||
| } | ||
| }, [loadedInstructions, instructions]); |
There was a problem hiding this comment.
This effect will continuously overwrite user-set instructions with loaded instructions whenever the instructions state changes to an empty/whitespace string. If a user intentionally clears instructions that were previously loaded from a shared URL, this effect will restore them. The dependency array includes 'instructions' which means this will trigger on every instruction change, potentially creating unexpected behavior. Consider removing the 'instructions' dependency or using a ref/flag to track whether instructions have been loaded once.
| if (isApiMode && markdown && !instructions.trim()) { | ||
| setInstructions(generateDefaultInstructions(markdown)); | ||
| } | ||
| }, [isApiMode, markdown, instructions]); | ||
|
|
There was a problem hiding this comment.
This effect generates default instructions every time the markdown changes if instructions are empty. This could overwrite instructions that a user intentionally cleared. Additionally, the dependency array includes 'instructions' which means this effect runs whenever instructions change, potentially resetting them when the user clears the field. Consider using a ref or flag to track whether default instructions have been generated once for a given plan load.
| if (isApiMode && markdown && !instructions.trim()) { | |
| setInstructions(generateDefaultInstructions(markdown)); | |
| } | |
| }, [isApiMode, markdown, instructions]); | |
| if (!isApiMode || !markdown) { | |
| return; | |
| } | |
| setInstructions((prev) => { | |
| // If the user or other logic has already provided instructions, keep them. | |
| if (prev.trim()) { | |
| return prev; | |
| } | |
| return generateDefaultInstructions(markdown); | |
| }); | |
| }, [isApiMode, markdown]); |
| <span className="text-sm font-medium text-foreground">Instructions for Claude</span> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {value.trim() ? 'Configured' : 'Default'} |
There was a problem hiding this comment.
The status badge displays "Default" when the instructions field is empty, but this is misleading. In API mode, default instructions are automatically generated via generateDefaultInstructions(), so an empty field doesn't mean default instructions are being used - it means NO instructions are set. The badge should either say "None" or be updated to check if the current value matches the generated default.
| {value.trim() ? 'Configured' : 'Default'} | |
| {value.trim() ? 'Configured' : 'None'} |
| const fileName = planTitle | ||
| ? planTitle | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9\s-]/g, '') | ||
| .replace(/\s+/g, '-') | ||
| .replace(/-+/g, '-') | ||
| .slice(0, 50) | ||
| : 'plan'; | ||
|
|
There was a problem hiding this comment.
The fileName generation doesn't handle the edge case where after removing all non-alphanumeric characters and replacing spaces with dashes, the result could be an empty string or only dashes. For example, a title like "!!!" would result in an empty string, and titles like "- - -" would result in empty strings after the replacements. Consider adding validation to ensure fileName is never empty before slicing.
| const fileName = planTitle | |
| ? planTitle | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s-]/g, '') | |
| .replace(/\s+/g, '-') | |
| .replace(/-+/g, '-') | |
| .slice(0, 50) | |
| : 'plan'; | |
| const rawFileName = planTitle | |
| ? planTitle | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s-]/g, '') | |
| .replace(/\s+/g, '-') | |
| .replace(/-+/g, '-') | |
| .slice(0, 50) | |
| : ''; | |
| const fileName = | |
| rawFileName && rawFileName.replace(/-/g, '').length > 0 ? rawFileName : 'plan'; |
| <div className="px-3 py-2 border-b border-border/30 bg-muted/30"> | ||
| <p className="text-xs text-muted-foreground"> | ||
| These instructions are sent to Claude with your approval or feedback. | ||
| They guide how Claude handles the plan but are NOT saved with it. |
There was a problem hiding this comment.
The description states "They guide how Claude handles the plan but are NOT saved with it," which is misleading. The instructions ARE included in share URLs (via the SharePayload.i field) and persisted in browser cookies via storage. What's true is that they're not saved as part of the plan markdown itself. Consider clarifying this to say "not saved with the plan markdown" or "not part of the plan document."
| They guide how Claude handles the plan but are NOT saved with it. | |
| They guide how Claude handles the plan but are not saved as part of the plan markdown. |
* Start daemon on install so hooks work immediately Install scripts now start the daemon in the background after placing the binary. This ensures improve-context and future pre-session hooks have a daemon to talk to from the first invocation. - install.sh: background start with >/dev/null 2>&1 & - install.ps1: Start-Process with -WindowStyle Hidden -ErrorAction SilentlyContinue - Mark backlog items #6 (smart session opening) and #14 (daemon on install) as DONE * Stop old daemon before replacing binary on upgrade On Windows the running exe is file-locked; on Unix the old daemon keeps serving stale code from memory. Now both installers stop any existing daemon before replacing the binary, then start a fresh one after. * Add 10s timeout to Windows daemon stop during install Start-Process -Wait has no timeout. If the stop command hangs (antivirus, debugger), the installer freezes. Now uses -PassThru with WaitForExit(10s) and Kill() fallback so the installer always continues.
No description provided.