Skip to content

Add oddkit_write tool definition (stub)#56

Open
klappy wants to merge 6 commits intomainfrom
feature/oddkit-write
Open

Add oddkit_write tool definition (stub)#56
klappy wants to merge 6 commits intomainfrom
feature/oddkit-write

Conversation

@klappy
Copy link
Copy Markdown
Owner

@klappy klappy commented Feb 28, 2026

  • Add write action to tool-registry.js with files, message, branch, pr, repo params
  • Add case in actions.js switch (returns not_implemented)
  • Follows D0017: One action, progressive protection

Note

High Risk
Introduces a new destructive action that can commit to arbitrary GitHub repositories and optionally open PRs, which is security- and data-integrity-sensitive despite basic path-traversal blocking and validation.

Overview
Adds a new destructive write/oddkit_write tool that writes one or more {path, content} files to a GitHub repo via the GitHub Contents API, with optional branch creation and PR opening, and provenance appended to commit messages.

Wires the new parameters through both CLI and MCP (files, message, branch, pr, repo, surface), adds governance-oriented validation (and blocks unsafe traversal paths), and introduces utils/githubApi.js helpers for authenticated GitHub REST calls.

Written by Cursor Bugbot for commit 910df12. This will update automatically on new commits. Configure here.

- Add write action to tool-registry.js with files, message, branch, pr, repo params
- Add case in actions.js switch (returns not_implemented)
- Follows D0017: One action, progressive protection
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Feb 28, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
oddkit 910df12 Commit Preview URL

Branch Preview URL
Feb 28 2026, 07:47 PM

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Write action params never forwarded by callers
    • Added forwarding of write-specific args (files, message, branch, pr, repo) in MCP server Layer 1, Layer 2, and both CLI handleAction call sites.
  • ✅ Fixed: CLI short flags collide with existing global flags
    • Removed colliding short flags (-f, -b) from write cliFlags and renamed --message to --commit-message to avoid being consumed by the input resolution fallback.
Preview (e4cda98f99)
diff --git a/src/cli.js b/src/cli.js
--- a/src/cli.js
+++ b/src/cli.js
@@ -217,6 +217,10 @@
           mode: options.mode,
           baseline: options.baseline,
           repoRoot: options.repo,
+          files: options.files ? JSON.parse(options.files) : undefined,
+          message: options.commitMessage,
+          branch: options.branch,
+          pr: options.pr,
         });
 
         outputActionResult(tool.name, result, format, quiet);
@@ -543,6 +547,10 @@
           mode: options.mode,
           baseline: options.baseline,
           repoRoot: options.repo,
+          files: options.files ? JSON.parse(options.files) : undefined,
+          message: options.commitMessage,
+          branch: options.branch,
+          pr: options.pr,
         });
         const ok = !isActionError(result);
         console.log(JSON.stringify(wrapToolJson(tool.name, result, ok)));

diff --git a/src/core/actions.js b/src/core/actions.js
--- a/src/core/actions.js
+++ b/src/core/actions.js
@@ -478,6 +478,51 @@
         };
       }
 
+      case "write": {
+        // Write files to GitHub repo via Contents API
+        // Validates against governance constraints, commits to default branch or branch
+        const { files, message, branch, pr, repo } = params;
+        
+        // Validate input
+        if (!files || !Array.isArray(files) || files.length === 0) {
+          return {
+            action: "write",
+            result: { error: "No files provided. Expected array of {path, content} objects." },
+            assistant_text: "No files provided. Please provide an array of files with path and content.",
+            debug: makeDebug(),
+          };
+        }
+        
+        if (!message) {
+          return {
+            action: "write",
+            result: { error: "Commit message required." },
+            assistant_text: "Commit message is required.",
+            debug: makeDebug(),
+          };
+        }
+
+        // TODO: Implement actual GitHub API write
+        // - Parse repo from baseline or use provided repo
+        // - Get current file SHA if updating existing files (Contents API requires SHA for updates)
+        // - Write via GitHub REST API
+        // - Handle branch creation if branch param provided
+        // - Handle PR opening if pr param provided
+        // - Include inline validation against governance constraints
+        // - Add provenance metadata (surface, session, timestamp)
+        
+        return {
+          action: "write",
+          result: { 
+            status: "not_implemented", 
+            message: "Write action is planned but not yet implemented.",
+            files_written: [],
+          },
+          assistant_text: `Write action is planned. Files: ${files.length} file(s), Message: ${message}, Branch: ${branch || 'default'}, PR: ${pr || false}, Repo: ${repo || 'default'}. Implementation pending.`,
+          debug: makeDebug({ files_count: files.length }),
+        };
+      }
+
       default:
         return {
           action: "error",

diff --git a/src/core/tool-registry.js b/src/core/tool-registry.js
--- a/src/core/tool-registry.js
+++ b/src/core/tool-registry.js
@@ -265,6 +265,40 @@
     annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false },
     cliFlags: {},
   },
+  {
+    name: "write",
+    mcpName: "oddkit_write",
+    description: "Write files to the GitHub repo. Accepts file path(s), content, commit message. Validates against governance constraints. Supports branches and PRs optionally.",
+    inputSchema: {
+      type: "object",
+      properties: {
+        files: {
+          type: "array",
+          items: {
+            type: "object",
+            properties: {
+              path: { type: "string", description: "Repo-relative file path (e.g., docs/decisions/D0017.md)" },
+              content: { type: "string", description: "File content (UTF-8 text)" },
+            },
+            required: ["path", "content"],
+          },
+          description: "Array of files to write",
+        },
+        message: { type: "string", description: "Commit message" },
+        branch: { type: "string", description: "Optional: target branch. If omitted, writes to default branch." },
+        pr: { type: "boolean", description: "Optional: if true, opens a PR from branch to default branch." },
+        repo: { type: "string", description: "Optional: GitHub repo (owner/repo). Defaults to baseline repo." },
+      },
+      required: ["files", "message"],
+    },
+    annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false },
+    cliFlags: {
+      files: { flag: "--files <json>", description: "JSON array of {path, content} objects" },
+      commitMessage: { flag: "--commit-message <text>", description: "Commit message", required: true },
+      branch: { flag: "--branch <name>", description: "Optional branch name" },
+      pr: { flag: "--pr", description: "Open PR after commit" },
+    },
+  },
 ];
 
 // ──────────────────────────────────────────────────────────────────────────────

diff --git a/src/mcp/server.js b/src/mcp/server.js
--- a/src/mcp/server.js
+++ b/src/mcp/server.js
@@ -192,6 +192,11 @@
         state: args.state,
         baseline: args.canon_url,
         include_metadata: args.include_metadata,
+        files: args.files,
+        message: args.message,
+        branch: args.branch,
+        pr: args.pr,
+        repo: args.repo,
       });
       return {
         content: [{ type: "text", text: JSON.stringify(result, null, 2) }],
@@ -209,7 +214,11 @@
         canon_url: args.canon_url,
         baseline: args.canon_url,
         include_metadata: args.include_metadata,
-        // No state for individual tools
+        files: args.files,
+        message: args.message,
+        branch: args.branch,
+        pr: args.pr,
+        repo: args.repo,
       });
       return {
         content: [{ type: "text", text: JSON.stringify(result, null, 2) }],

Comment thread src/core/actions.js Outdated
Comment thread src/core/tool-registry.js
cursoragent and others added 2 commits February 28, 2026 19:07
- Forward write-specific args (files, message, branch, pr, repo) in MCP
  server Layer 1 and Layer 2 handleAction calls
- Forward write-specific options in both CLI handleAction call sites
- Remove colliding short flags from write cliFlags: -f (collides with
  --format), -b (collides with --baseline), -m (collides with --message
  alias consumed as input)
- Rename write --message to --commit-message to avoid being consumed by
  the input resolution fallback (options.message)
- Add src/utils/githubApi.js - GitHub REST API helper using native fetch
- Add src/utils/writeValidation.js - Governance validation for canon/odd/docs
- Update src/core/actions.js with full write implementation:
  - Parse baseline_url to get owner/repo
  - Validate files against governance constraints
  - Add provenance metadata in commit footer
  - Write via GitHub Contents API
  - Support branch and PR creation
- Follows IMPL-oddkit-write.md spec
Comment thread src/core/actions.js
Comment thread src/core/actions.js Outdated
Comment thread src/utils/githubApi.js
Comment thread src/utils/githubApi.js Outdated
…, and URL regex

- Actually call createBranch when target branch does not exist (was detected but never created)
- Pass actual default branch to createPR instead of hardcoded main
- Honor providedRepo param instead of silently ignoring it
- Fix github.com URL regex to allow dots in repo names (e.g. klappy.dev)
Comment thread src/utils/githubApi.js
Comment thread src/utils/githubApi.js
Comment thread src/core/actions.js Outdated
Comment thread src/core/actions.js
- Add Content-Type application/json header to githubRequest
- Use correct env var ODDKIT_BASELINE instead of ODDKIT_BASELINE_URL
- Auto-generate branch name when pr is true but branch is omitted
- URL-encode branch names in branchExists and getBranchSha
Comment thread src/core/actions.js
Comment thread src/core/actions.js Outdated
Comment thread src/core/actions.js
Comment thread src/core/actions.js Outdated
Comment thread src/core/actions.js
Comment thread src/core/tool-registry.js
…rsal, report last commit, pass surface from callers, update orchestrator annotations

- Remove hardcoded klappy.dev fallback for destructive write action
- Block writes when path traversal sequences detected in file paths
- Report last commit SHA/URL instead of first for multi-file writes
- Pass surface param (cli/mcp) from both callers for accurate provenance
- Update orchestrator annotations to destructiveHint:true, idempotentHint:false
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Comment thread src/core/tool-registry.js
destructiveHint: false,
idempotentHint: true,
destructiveHint: true,
idempotentHint: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orchestrator annotations mark all actions as destructive

High Severity

The orchestrator tool's annotations were changed from destructiveHint: false / idempotentHint: true to destructiveHint: true / idempotentHint: false. This affects ALL actions routed through the oddkit orchestrator tool (search, get, orient, catalog, etc.), not just the new write action. MCP hosts use these hints to decide whether to auto-approve tool calls or show confirmation dialogs. This change causes unnecessary user confirmation prompts for the ~11 read-only, idempotent actions that existed before this PR.

Fix in Cursor Fix in Web

Comment thread src/utils/githubApi.js
export async function getBranchSha(owner, repo, branch) {
const data = await githubRequest(`/repos/${owner}/${repo}/git/ref/heads/${encodeURIComponent(branch)}`);
return data.object.sha;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBranchSha encodes slashes breaking git refs API

Low Severity

getBranchSha uses encodeURIComponent(branch) in the GitHub git/ref/heads/ endpoint URL. The git refs API expects literal slashes in branch names (e.g., git/ref/heads/feature/test), not encoded ones (git/ref/heads/feature%2Ftest). This would cause a 404 for any branch containing /. Currently only called with defaultBranch (typically "main"), but the auto-generated branch format oddkit-write/{timestamp} contains a slash and would break if passed to this function.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants