feat(getting-started): rewrite as onboarding orchestrator (1/3)#26
Closed
sidneyswift wants to merge 2 commits into
Closed
feat(getting-started): rewrite as onboarding orchestrator (1/3)#26sidneyswift wants to merge 2 commits into
sidneyswift wants to merge 2 commits into
Conversation
…etection + skill routing
The skill now drives a 5-step flow:
Step 0 Detect environment + auth state (env vars + CLI + filesystem)
Step 1 Install CLI (BYOA only, skipped if already installed)
Step 2 Get an API key (BYOA only, skipped if RECOUP_API_KEY/ACCESS_TOKEN set)
Step 3 Verify auth + identify the account via /api/whoami
Step 4 Detect roster + filesystem state (orgs, artists, scaffold)
Step 5 Route to the right next skill based on the detected state
This handles both target user journeys end-to-end:
- BYOA from developers.recoupable.com: user pastes a getting-started prompt,
agent runs Step 0-5 in sequence — installs CLI, creates an account, detects
state, and routes to the right follow-up skill (create-artist for empty
rosters, artist-workspace for established ones, setup-sandbox when the
workspace tree is missing).
- Recoup-hosted on chat.recoupable.com: RECOUP_ACCESS_TOKEN is already
injected, so the skill skips install/auth and goes straight to state
detection + routing.
Also adds a throwaway-account warning (per the create-artist skill's caveat
about agent+ emails) and a routing table that picks between create-artist,
setup-sandbox, and artist-workspace based on the (orgs, artists, filesystem)
state vector.
No env vars or endpoints invented — uses the same `RECOUP_API_KEY` /
`RECOUP_ACCESS_TOKEN` / `RECOUP_ACCOUNT_ID` / `RECOUP_ORG_ID` already
documented in `recoup-api` and `setup-sandbox` skills, and the same
`/api/whoami`, `/api/organizations`, and `/api/organizations/{id}/artists`
endpoints used elsewhere.
Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThree skill documents are updated: chart-metric's README now reflects distribution via the recoup-skills plugin; create-artist consolidates API endpoints and authentication headers; getting-started evolves from a simple quickstart into a comprehensive multi-step onboarding orchestrator with conditional flows. ChangesSkills Documentation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/create-artist/SKILL.md">
<violation number="1" location="skills/create-artist/SKILL.md:18">
P1: The access-token auth branch initializes `AUTH_HEADER` incorrectly, so bearer-token flows can send requests without an Authorization header.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
sidneyswift
force-pushed
the
feat/orchestrator
branch
from
f72579f to
1a202b3
Compare
5 tasks
sidneyswift
changed the title
sidneyswift
changed the title
…json + cache validation
PR review surfaced four real issues that would break customers post-merge.
Three are fixed here. The fourth (setup-sandbox vs artist-workspace layout
inconsistency) is a deeper design decision documented as a follow-up.
1. getting-started — /whoami endpoint doesn't exist.
Step 3 originally called /api/whoami (returns 404 — the endpoint was
never built). Replaced with /api/organizations as the auth verification
call: 200 means the key works, response body is reused in Step 4 instead
of fetching orgs twice.
account_id + email come from two sources now:
- Fresh signup: capture from agents/signup or agents/verify response,
persist to ~/.config/recoup/account.json.
- Returning agent: read account.json from disk.
- Sandbox: RECOUP_ACCOUNT_ID injected — wins.
- None of the above: proceed with "identity unknown" — throwaway
warning silently skips, the rest of the flow still works.
Step 5 routing extended for the "0 orgs + email unknown" case.
2. Cache safety — validate cached account_id against current token.
~/.config/recoup/account.json may be stale (different token).
Validates with GET /api/accounts/{id}/subscription before trusting it;
discards on non-200 so onboarding can't be routed into the wrong workspace.
3. create-artist — Bearer-only auth + wrong base URL.
The skill required $RECOUP_ACCESS_TOKEN (sandbox-only) and used
api.recoupable.com (inconsistent with rest of marketplace). Now uses
the same AUTH_HEADER abstraction as getting-started; accepts either
RECOUP_API_KEY (BYOA) or RECOUP_ACCESS_TOKEN (sandbox).
All ~13 curl invocations updated to recoup-api.vercel.app + $AUTH_HEADER.
4. chart-metric/README.md — bogus install command.
Said `npx skills add recoupable/chartmetric` — no such repo.
Replaced with actual install commands across Claude Code, Codex,
and the Vercel CLI (with --skill chart-metric for cherry-pick).
Co-authored-by: Cursor <cursoragent@cursor.com>
sidneyswift
force-pushed
the
feat/orchestrator
branch
from
1a202b3 to
4fbb20f
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/getting-started/SKILL.md`:
- Around line 86-88: The current exports of RECOUP_API_KEY and RECOUP_ACCOUNT_ID
from SIGNUP_RESPONSE may produce empty/null values; update the script that sets
RECOUP_API_KEY and RECOUP_ACCOUNT_ID (and the similar assignments around lines
111-113) to validate the extracted values are non-empty before exporting/writing
them: extract the fields from SIGNUP_RESPONSE using jq into local variables,
check they are not null/empty (and optionally match expected formats), log a
clear error and exit non-zero if validation fails, and only export/persist the
credentials when both checks pass (use the same variable names RECOUP_API_KEY,
RECOUP_ACCOUNT_ID to locate the assignments).
- Line 121: The current one-line append (echo 'export
RECOUP_API_KEY="'$RECOUP_API_KEY'"' >> ~/.zshrc) is not idempotent; update the
setup to make the zshrc export idempotent by either replacing any existing
RECOUP_API_KEY export or only appending when none exists: locate the export line
pattern for RECOUP_API_KEY in ~/.zshrc (the same target used by the echo
command), and then use a replace-if-present (e.g., sed-style) operation to
update it with the new value, otherwise append the export; ensure the final
change affects the same export string referenced in SKILL.md so repeated runs
won't create duplicate entries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2516f975-9d3f-4070-b275-2684052f5085
📒 Files selected for processing (3)
skills/chart-metric/README.mdskills/create-artist/SKILL.mdskills/getting-started/SKILL.md
Comment on lines
+86
to
88
| export RECOUP_API_KEY=$(echo "$SIGNUP_RESPONSE" | jq -r .api_key) | ||
| RECOUP_ACCOUNT_ID=$(echo "$SIGNUP_RESPONSE" | jq -r .account_id) | ||
| ``` |
There was a problem hiding this comment.
Validate signup/verify outputs before exporting credentials.
If API responses are partial or error-shaped, jq -r can yield null/empty values and persist invalid auth state. Add non-empty guards for api_key and account_id before export/write.
Suggested fix
export RECOUP_API_KEY=$(echo "$SIGNUP_RESPONSE" | jq -r .api_key)
RECOUP_ACCOUNT_ID=$(echo "$SIGNUP_RESPONSE" | jq -r .account_id)
+[ -n "$RECOUP_API_KEY" ] && [ "$RECOUP_API_KEY" != "null" ] || { echo "Signup failed: missing api_key"; exit 1; }
+[ -n "$RECOUP_ACCOUNT_ID" ] && [ "$RECOUP_ACCOUNT_ID" != "null" ] || { echo "Signup failed: missing account_id"; exit 1; }
export RECOUP_API_KEY=$(echo "$VERIFY_RESPONSE" | jq -r .api_key)
RECOUP_ACCOUNT_ID=$(echo "$VERIFY_RESPONSE" | jq -r .account_id)
+[ -n "$RECOUP_API_KEY" ] && [ "$RECOUP_API_KEY" != "null" ] || { echo "Verify failed: missing api_key"; exit 1; }
+[ -n "$RECOUP_ACCOUNT_ID" ] && [ "$RECOUP_ACCOUNT_ID" != "null" ] || { echo "Verify failed: missing account_id"; exit 1; }Also applies to: 111-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/getting-started/SKILL.md` around lines 86 - 88, The current exports of
RECOUP_API_KEY and RECOUP_ACCOUNT_ID from SIGNUP_RESPONSE may produce empty/null
values; update the script that sets RECOUP_API_KEY and RECOUP_ACCOUNT_ID (and
the similar assignments around lines 111-113) to validate the extracted values
are non-empty before exporting/writing them: extract the fields from
SIGNUP_RESPONSE using jq into local variables, check they are not null/empty
(and optionally match expected formats), log a clear error and exit non-zero if
validation fails, and only export/persist the credentials when both checks pass
(use the same variable names RECOUP_API_KEY, RECOUP_ACCOUNT_ID to locate the
assignments).
| Add to shell profile so it persists: | ||
| ```bash | ||
| # Persist API key into the user's shell init | ||
| echo 'export RECOUP_API_KEY="'$RECOUP_API_KEY'"' >> ~/.zshrc |
There was a problem hiding this comment.
Make shell-init persistence idempotent to avoid duplicate/stale exports.
Appending a new export RECOUP_API_KEY=... on every run can accumulate conflicting entries in ~/.zshrc and cause hard-to-debug auth behavior across sessions. Replace existing entry (or append only if absent).
Suggested fix
-echo 'export RECOUP_API_KEY="'$RECOUP_API_KEY'"' >> ~/.zshrc
+grep -q '^export RECOUP_API_KEY=' ~/.zshrc \
+ && sed -i.bak 's|^export RECOUP_API_KEY=.*|export RECOUP_API_KEY="'"$RECOUP_API_KEY"'"|' ~/.zshrc \
+ || echo 'export RECOUP_API_KEY="'"$RECOUP_API_KEY"'"' >> ~/.zshrc📝 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.
Suggested change
| echo 'export RECOUP_API_KEY="'$RECOUP_API_KEY'"' >> ~/.zshrc | |
| grep -q '^export RECOUP_API_KEY=' ~/.zshrc \ | |
| && sed -i.bak 's|^export RECOUP_API_KEY=.*|export RECOUP_API_KEY="'"$RECOUP_API_KEY"'"|' ~/.zshrc \ | |
| || echo 'export RECOUP_API_KEY="'"$RECOUP_API_KEY"'"' >> ~/.zshrc |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/getting-started/SKILL.md` at line 121, The current one-line append
(echo 'export RECOUP_API_KEY="'$RECOUP_API_KEY'"' >> ~/.zshrc) is not
idempotent; update the setup to make the zshrc export idempotent by either
replacing any existing RECOUP_API_KEY export or only appending when none exists:
locate the export line pattern for RECOUP_API_KEY in ~/.zshrc (the same target
used by the echo command), and then use a replace-if-present (e.g., sed-style)
operation to update it with the new value, otherwise append the export; ensure
the final change affects the same export string referenced in SKILL.md so
repeated runs won't create duplicate entries.
Contributor
Author
|
Closing this — superseded by #29 (fix-only PR). Decision: this PR was doing too many things. The 3 bug fixes belong in a focused fix PR (#29). The orchestrator pattern (state detection, persona routing, cache safety) is real work but doesn't belong in a "fix the broken stuff" PR. The orchestrator branch ( Replacement: #29 |
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
1 participant
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.
The problem
A new developer or agent arrives at Recoup and doesn't know where to start. The old
getting-startedskill told them how to install the CLI but not what to do next — so agents either got stuck or did the wrong thing first.The fix
Rewrites
skills/getting-started/SKILL.mdinto a 5-step orchestrator that detects state and routes to the right next skill:create-artistartist-workspacesetup-sandboxThe orchestrator detects environment (env vars + CLI + filesystem state), installs the CLI + creates an account only if needed, then hands off.
Files (3)
skills/getting-started/SKILL.md— the orchestrator rewriteskills/create-artist/SKILL.md—AUTH_HEADERfix (works with both BYOA and sandbox auth)skills/chart-metric/README.md— fixes a broken install commandWhat you should focus on
getting-started/SKILL.md— does the (orgs, artists, filesystem) → next-skill mapping cover the cases?account.jsonwhen the current token can't read the account?Test plan
python3 scripts/validate-manifests.pyexits 0Merge order
PR 1 of 3. Independent of #25 and #27 — can merge anytime, in any order. Smallest PR — start here.
Merge preference
Please merge yourself when satisfied — I'd rather you push the button than approve.
🤖 Made with Cursor
Summary by CodeRabbit