feat: switch MCP tool responses to TOON output - #490
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR introduces TOON (Token-Oriented Object Notation) encoding for databricks-devtools MCP tool responses and significantly restructures the create-pr skill with a simplified workflow. Changes include a new sync-with-base.sh script, rewritten conflict checking and PR status verification logic, and corresponding test updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@plugins/me/skills/create-pr/scripts/verify-pr-status.sh`:
- Around line 23-26: The script currently calls gh pr view to populate PR_URL
and PR_STATUS (used to derive MERGEABLE and STATE) without handling gh failing
(no open PR), so wrap the gh pr view calls with a guard: run gh pr view --json
url -q .url and gh pr view --json mergeable,mergeStateStatus in a conditional
(or capture their exit codes), and if either fails or returns empty set
PR_URL/PR_STATUS, print a clear friendly error like "No open PR for current
branch" and exit with non-zero status; update references to PR_URL, PR_STATUS,
MERGEABLE, and STATE accordingly so downstream logic only runs when these
variables are valid.
In `@plugins/me/skills/create-pr/SKILL.md`:
- Around line 19-42: The heading "## Minimal Workflow (/writing-skills)" is
using the wrong path; update the heading string in SKILL.md by replacing
"(/writing-skills)" with the correct skill path such as "(/create-pr)" or remove
the path entirely so it reads "## Minimal Workflow"; locate and edit the heading
line containing "## Minimal Workflow (/writing-skills)" to apply the fix.
🧹 Nitpick comments (6)
plugins/databricks-devtools/src/mcp/server.ts (1)
42-52: Clean refactoring from JSON to TOON encoding.The payload construction and
encode()call are straightforward. One consideration: this is a breaking change for any downstream consumers that previously parsed JSON from these tool responses. If there are external integrations or clients that call these tools andJSON.parse()the result, they'll break silently.Ensure any consumers outside of this plugin (e.g., other plugins, scripts, or agents) are aware of the format change, or consider a transitional approach (e.g., a query parameter or header to opt into TOON).
plugins/databricks-devtools/tests/mcp/server.test.ts (1)
69-82: Solid round-trip semantic validation.This test verifies that
decode(encode(payload))preserves all fields, which is the critical correctness guarantee for the format migration.One minor note: the tests only cover
listCatalogsTool. Consider adding at least one TOON-decode assertion for a tool with a multi-row result (e.g.,listTablesToolwith multiple tables) to exercise TOON's tabular encoding path more thoroughly.plugins/me/skills/create-pr/scripts/sync-with-base.sh (2)
12-26: Consider reordering: git-repo check beforeghCLI call.The
gh repo viewcall on line 14 happens before verifying we're inside a git repository (line 23). If a user omits the base-branch argument and isn't in a git repo,gh repo viewwill likely fail (caught by|| true), then the "Cannot determine default branch" error fires — which is a misleading message when the real problem is "not in a git repo." Moving the git-repo guard above theghfallback would give a clearer error.Proposed reorder
BASE="${1:-}" + +if ! git rev-parse --git-dir >/dev/null 2>&1; then + echo "ERROR: Not in a git repository" >&2 + exit 2 +fi + if [[ -z "$BASE" ]]; then BASE=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null || echo "") - if [[ -z "$BASE" ]]; then - echo "ERROR: Cannot determine default branch" >&2 - echo " - Pass base branch explicitly: $0 <base-branch>" >&2 - echo " - Or ensure 'gh' CLI is authenticated" >&2 - exit 2 - fi fi - -if ! git rev-parse --git-dir >/dev/null 2>&1; then - echo "ERROR: Not in a git repository" >&2 +if [[ -z "$BASE" ]]; then + echo "ERROR: Cannot determine default branch" >&2 + echo " - Pass base branch explicitly: $0 <base-branch>" >&2 + echo " - Or ensure 'gh' CLI is authenticated" >&2 exit 2 fi
34-46: Conflict/failure messages go to stdout — intentional?Lines 37–45 (conflict summary and resolution steps) are printed to stdout, while similar guidance in
check-conflicts.shgoes to stderr. If these scripts are composed in pipelines or their stdout is captured, the conflict detail will mix into the data stream. Consider sending at least the "✗ Merge failed" line to stderr for consistency.plugins/me/skills/create-pr/scripts/check-conflicts.sh (1)
12-26: Same reorder opportunity assync-with-base.sh: git-repo check beforeghcall.This is the same pattern —
gh repo viewruns before verifying we're in a git repository. The same reordering suggestion fromsync-with-base.shapplies here for clearer error messages.plugins/me/skills/create-pr/scripts/verify-pr-status.sh (1)
23-30: Three separategh pr viewcalls could be consolidated.Lines 23, 24, and 30 each make a separate GitHub API call. The first two could be combined into one (
--json url,mergeable,mergeStateStatus), and the third (statusCheckRollup) could also be folded in if the slight over-fetch for non-CLEAN states is acceptable. This reduces API round-trips and avoids potential rate-limiting in CI.Possible consolidation
-PR_URL=$(gh pr view --json url -q .url) -PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus) -MERGEABLE=$(echo "$PR_STATUS" | jq -r .mergeable) -STATE=$(echo "$PR_STATUS" | jq -r .mergeStateStatus) +PR_JSON=$(gh pr view --json url,mergeable,mergeStateStatus,statusCheckRollup) +PR_URL=$(echo "$PR_JSON" | jq -r .url) +MERGEABLE=$(echo "$PR_JSON" | jq -r .mergeable) +STATE=$(echo "$PR_JSON" | jq -r .mergeStateStatus)Then inside the
CLEAN)branch, extract checks from the already-fetched data:- CHECKS=$(gh pr view --json statusCheckRollup -q '.statusCheckRollup') + CHECKS=$(echo "$PR_JSON" | jq '.statusCheckRollup')
| PR_URL=$(gh pr view --json url -q .url) | ||
| PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus) | ||
| MERGEABLE=$(echo "$PR_STATUS" | jq -r .mergeable) | ||
| STATE=$(echo "$PR_STATUS" | jq -r .mergeStateStatus) |
There was a problem hiding this comment.
No guard if gh pr view fails (e.g., no open PR).
Under set -euo pipefail, if there's no PR associated with the current branch, gh pr view on line 23 will fail and the script will exit with gh's raw error — no friendly message. A brief guard would improve UX.
Proposed guard
+if ! PR_URL=$(gh pr view --json url -q .url 2>/dev/null); then
+ echo "ERROR: No open PR found for this branch" >&2
+ echo " - Create a PR first: gh pr create" >&2
+ exit 1
+fi
-PR_URL=$(gh pr view --json url -q .url)
-PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus)
+PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PR_URL=$(gh pr view --json url -q .url) | |
| PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus) | |
| MERGEABLE=$(echo "$PR_STATUS" | jq -r .mergeable) | |
| STATE=$(echo "$PR_STATUS" | jq -r .mergeStateStatus) | |
| if ! PR_URL=$(gh pr view --json url -q .url 2>/dev/null); then | |
| echo "ERROR: No open PR found for this branch" >&2 | |
| echo " - Create a PR first: gh pr create" >&2 | |
| exit 1 | |
| fi | |
| PR_STATUS=$(gh pr view --json mergeable,mergeStateStatus) | |
| MERGEABLE=$(echo "$PR_STATUS" | jq -r .mergeable) | |
| STATE=$(echo "$PR_STATUS" | jq -r .mergeStateStatus) |
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/scripts/verify-pr-status.sh` around lines 23 -
26, The script currently calls gh pr view to populate PR_URL and PR_STATUS (used
to derive MERGEABLE and STATE) without handling gh failing (no open PR), so wrap
the gh pr view calls with a guard: run gh pr view --json url -q .url and gh pr
view --json mergeable,mergeStateStatus in a conditional (or capture their exit
codes), and if either fails or returns empty set PR_URL/PR_STATUS, print a clear
friendly error like "No open PR for current branch" and exit with non-zero
status; update references to PR_URL, PR_STATUS, MERGEABLE, and STATE accordingly
so downstream logic only runs when these variables are valid.
| ## Minimal Workflow (/writing-skills) | ||
|
|
||
| ```bash | ||
| # Check current state | ||
| # 1) pre-flight | ||
| git status | ||
| git log --oneline -5 | ||
| git branch --show-current | ||
| git log --oneline -5 | ||
|
|
||
| # Block if on main/master | ||
| # Block if no changes (working tree clean) | ||
| ``` | ||
|
|
||
| ### Commit | ||
|
|
||
| ```bash | ||
| # Stage specific files only (never -A) | ||
| # 2) commit | ||
| git add <specific-files> | ||
| git commit -m "type(scope): description" | ||
| ``` | ||
| git commit -m "type(scope): summary" | ||
|
|
||
| ### Conflict Check (REQUIRED) | ||
|
|
||
| ```bash | ||
| # 3) conflict check (read-only) | ||
| "${CLAUDE_PLUGIN_ROOT}/skills/create-pr/scripts/check-conflicts.sh" | ||
| ``` | ||
|
|
||
| Exits with: | ||
| - `0` - No conflicts, safe to proceed | ||
| - `1` - Conflicts detected, manual resolution required | ||
| - `2` - Error (missing branch, git errors) | ||
|
|
||
| ### Push | ||
|
|
||
| ```bash | ||
| # 4) push | ||
| git push -u origin HEAD | ||
| ``` | ||
|
|
||
| ### Create PR | ||
| # 5) create PR (do not hardcode --base main) | ||
| gh pr create --title "$(git log -1 --pretty=%s)" --body "<summary, details, tests>" | ||
|
|
||
| ```bash | ||
| gh pr create --title "$(git log -1 --pretty=%s)" --body "<comprehensive description>" | ||
| # 6) verify (read-only) | ||
| "${CLAUDE_PLUGIN_ROOT}/skills/create-pr/scripts/verify-pr-status.sh" | ||
| ``` |
There was a problem hiding this comment.
Fix the “/writing-skills” path in the heading.
This looks like a stray or incorrect path reference for this skill doc.
✏️ Suggested tweak
-## Minimal Workflow (/writing-skills)
+## Minimal Workflow🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/SKILL.md` around lines 19 - 42, The heading "##
Minimal Workflow (/writing-skills)" is using the wrong path; update the heading
string in SKILL.md by replacing "(/writing-skills)" with the correct skill path
such as "(/create-pr)" or remove the path entirely so it reads "## Minimal
Workflow"; locate and edit the heading line containing "## Minimal Workflow
(/writing-skills)" to apply the fix.
# [5.23.0](v5.22.0...v5.23.0) (2026-02-13) ### Features * switch MCP tool responses to TOON output ([#490](#490)) ([c463c8b](c463c8b))
# [5.23.0](baleen37/bstack@v5.22.0...v5.23.0) (2026-02-13) ### Features * switch MCP tool responses to TOON output ([#490](baleen37/bstack#490)) ([c463c8b](baleen37/bstack@c463c8b))
1 participant
Summary
feat(create-pr): split verify and sync workflows)Test plan
cd plugins/databricks-devtools && bun run test -- tests/mcp/server.test.tscd plugins/databricks-devtools && bun run testcd plugins/databricks-devtools && bun run typecheckcd plugins/databricks-devtools && bun run build"/Users/jito.hello/.claude/plugins/cache/baleen-plugins/me/5.21.2/skills/create-pr/scripts/check-conflicts.sh" mainNotes
feat(create-pr): split verify and sync workflows🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests