Conversation
Add proof skill for collaborative document editing via Proof's web API and local bridge. Integrate Proof uploads into brainstorm and plan workflows so outputs get a shareable URL automatically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kieranklaassen
left a comment
kieranklaassen
left a comment
There was a problem hiding this comment.
Code Review: PR #205 — feat: Add Proof editor integration
Reviewed by: 6 parallel agents (security-sentinel, pattern-recognition-specialist, code-simplicity-reviewer, agent-native-reviewer, learnings-researcher, architecture-strategist)
Summary
The proof skill itself is well-structured and the version/count bumps are correct. The primary issues are: (1) auto-uploading user documents to a third-party server without consent, (2) inlining HTTP logic in commands instead of delegating to the skill, and (3) the SKILL.md could benefit from progressive disclosure.
Findings
P1 — Critical (Blocks Merge)
1. Automatic data exfiltration without user consent
Files: commands/workflows/brainstorm.md, commands/workflows/plan.md
Every brainstorm and plan document is automatically uploaded to proofeditor.ai without asking the user. Plan/brainstorm docs often contain proprietary architecture, security analysis, and potentially embedded credentials. The rclone skill (comparable external service integration) is explicitly opt-in.
Fix: Add AskUserQuestion before uploading: "Share this document to Proof for collaborative review?" — or make it configurable. This matches the existing pattern in these workflow commands which already use AskUserQuestion extensively.
P2 — Important (Should Fix)
2. Inlined curl logic duplicated in commands instead of referencing the skill
Files: commands/workflows/brainstorm.md (+25 lines), commands/workflows/plan.md (+25 lines)
Identical 25-line curl/jq blocks are copy-pasted into both commands. The established pattern is for commands to reference skills by name:
Load the `brainstorming` skill for detailed question techniques
Load the `document-review` skill and apply it to the brainstorm document
The proof SKILL.md already documents the complete "Create and Share" workflow. Each command should replace the 25-line block with ~4 lines:
### Share to Proof
After writing the document, use the `proof` skill to upload it for collaborative review.
Display the returned Proof URL prominently. If sharing fails, skip silently.This follows established conventions, eliminates duplication, and saves ~42 lines of context budget.
3. allowed-tools restricts Glob and Grep unnecessarily
File: skills/proof/SKILL.md frontmatter
Only 2 of 22 skills use allowed-tools. Adding it here omits Glob and Grep, meaning "find my brainstorm doc and share it to Proof" would fail because file-search tools are unavailable. Either remove the allowed-tools field entirely (matching 91% of skills) or add Glob and Grep.
4. ownerSecret is discarded — documents become unmanageable
File: skills/proof/SKILL.md
The create response includes ownerSecret but neither the skill nor the command integration stores or surfaces it. If a document with sensitive content is accidentally uploaded, there's no way to delete it. At minimum, display the ownerSecret to the user alongside the URL.
5. Consider progressive disclosure for SKILL.md
File: skills/proof/SKILL.md (185 lines, ~6,350 chars)
Per project conventions, the gold-standard is dhh-rails-style: small SKILL.md routing to reference files. The proof skill has two distinct modes (Web API vs Local Bridge) that are natural split points. The Local Bridge section (37 lines) is not used by any command in this PR and could be deferred to references/local-bridge.md.
P3 — Nice-to-Have
6. Error handling is too vague
"Skip silently" doesn't specify how to detect failure. curl returns exit 0 even on HTTP 4xx/5xx. Use curl --fail and validate PROOF_URL is non-empty before displaying.
7. Redundant workflow examples in SKILL.md (lines 139-183)
44 lines of step-by-step workflows that restate the endpoint docs already shown above. An LLM that has read the API reference doesn't need them reassembled.
8. cat + --arg breaks on large files
The CONTENT=$(cat ...) + jq --arg markdown "$CONTENT" pattern will break on content exceeding shell argument size limits. Use jq --rawfile instead.
9. Documentation site not updated
Per the CLAUDE.md checklist, /release-docs should be run after adding a skill to update docs/pages/skills.html and landing page stats.
10. Access tokens embedded in shareable URLs
The tokenUrl grants full read/write access. Document that it should be treated as a credential and shared only with trusted collaborators.
What's Working Well
- Skill design is agent-native: Provides atomic operations (create, read, comment, suggest, rewrite) that agents can compose freely
- Version bump correct: 2.35.2 → 2.36.0 (minor bump for new skill) ✓
- Skill count consistent: 19 → 20 across plugin.json, marketplace.json, README.md ✓
- CHANGELOG format: Follows established pattern exactly ✓
- Description length: 253 chars, within the 100-300 char target ✓
jqfor JSON construction: Prevents injection — good security pattern ✓- Graceful degradation intent: "Skip silently on failure" is the right philosophy ✓
byfield for attribution:"ai:compound"enables clear human/agent distinction ✓
Institutional Knowledge Applied
Three past solutions are directly relevant:
adding-optional-workflow-phases-with-graceful-degradation.md— Consumer skips silently, producer handles empty state. Never mention Proof's absence to users.workflow-skill-transcript-input-mismatch.md— First-use test: workflows must succeed with no Proof API available.plugin-versioning-requirements.md— All files must update counts in lockstep (verified: they do ✓).
Recommended Action
Address P1 (user consent) and P2 items (skill delegation, allowed-tools, ownerSecret) before merging. P3 items can be addressed in a follow-up.
🤖 Generated with Claude Code
1 participant
Summary
proofskill — Full Proof API reference for creating, editing, commenting on, and sharing markdown documents. Supports web API (no auth required for sharing) and local macOS bridge./workflows:brainstorm— After writing the brainstorm doc, uploads to Proof and displays a shareable URL for collaborative review./workflows:plan— After writing the plan file, uploads to Proof and displays a shareable URL for collaborative review.Both uploads use
by: "ai:compound"for attribution and gracefully skip if the network request fails.Changes
skills/proof/SKILL.mdcommands/workflows/brainstorm.mdcommands/workflows/plan.mdplugin.jsonmarketplace.jsonREADME.mdCHANGELOG.mdTest plan
proofskill loads (/prooftriggers it)/workflows:brainstormand confirm Proof URL appears at the end/workflows:planand confirm Proof URL appears after plan is writtenPost-Deploy Monitoring & Validation
No additional operational monitoring required: plugin is client-side only, Proof sharing is optional and failure-safe.