Skip to content

Apply code review fixes: security, deduplication, CLI UX, and bug fixes#11

Merged
teallarson merged 13 commits intomainfrom
teallarson/code-review-fixes
Mar 5, 2026
Merged

Apply code review fixes: security, deduplication, CLI UX, and bug fixes#11
teallarson merged 13 commits intomainfrom
teallarson/code-review-fixes

Conversation

@teallarson
Copy link
Copy Markdown
Collaborator

Summary

Implements all findings from the code review audit across 5 phases (stacked naturally on #10, which is now merged):

  • CLI UX: --help / --version flags; name validation (rejects hyphen-prefixed names, shell metacharacters, path separators); bun pre-check before install; graceful install failure (warn + continue instead of process.exit(1)); stdout capture in runAsync
  • Auto-generated secrets: BETTER_AUTH_SECRET and APP_SECRET_KEY are now filled with randomBytes(32).toString("hex") when .env is created from .env.example — no more placeholder secrets in scaffolded projects
  • Template config: hint fields for interactive selector; synced lucide-react/radix-ui/tailwind-merge/tw-animate-css versions (mastra→ai-sdk); fixed hardcoded localhost:8000 in langchain README to localhost:{{port}}; added export const maxDuration = 60 to mastra chat route; cookie_secure now derives from app_url scheme; scoped ESLint global rules to src/**/*.ts to avoid overriding template-level warns; expanded .gitignore
  • Open redirect: next_uri validated same-origin before redirecting in both Next.js and Python verify routes
  • Deduplication: arcade.ts (~150 lines × 2) collapsed into one arcade-oauth-provider.hbs partial with a Handlebars conditional for the mastra-specific MCPClient export; mapToolToSource() and extractAuthUrlFromToolOutput() extracted into shared partials, removing inline duplicates from plan-route-body.hbs and sources-route-body.hbs; unified mapToolToSource regex uses [._] split across all templates
  • File permissions: .arcade-auth/ created with mode 0o700; all written token/client/verifier files use 0o600 (both TypeScript and Python)

Test plan

  • npm run build && npm run typecheck && npm run lint && npm run format:check — all pass
  • ruff check templates/langchain/ — all pass
  • node dist/index.js --help — prints usage
  • node dist/index.js --version — prints 0.1.0
  • node dist/index.js "../bad" --template ai-sdk — rejects with path separator error
  • Scaffold ai-sdk template, verify .env has non-empty BETTER_AUTH_SECRET
  • Scaffold mastra template, verify .env has non-empty BETTER_AUTH_SECRET
  • Scaffold langchain template, verify .env has non-empty APP_SECRET_KEY
  • CI smoke tests pass for all 3 templates

🤖 Generated with Claude Code

teallarson and others added 2 commits March 4, 2026 17:13
Phase 1 – CLI source:
- Add --help and --version flags to the CLI
- Extract validateProjectName() with shell metachar + hyphen rejection
- Auto-generate BETTER_AUTH_SECRET and APP_SECRET_KEY on scaffold
- Pre-check bun is installed before running bun install steps
- Gracefully warn (not exit) on install failures so project is still usable
- Capture stdout in runAsync alongside stderr

Phase 2 – Template/config:
- Fill in template.json hint fields for interactive selector UX
- Sync lucide-react, radix-ui, tailwind-merge, tw-animate-css versions (mastra→ai-sdk)
- Fix langchain README hardcoded localhost:8000 → localhost:{{port}}
- Add maxDuration = 60 to mastra chat route
- Fix cookie_secure to derive from app_url scheme instead of hardcoded False
- Scope ESLint global rules block to src/**/*.ts to avoid overriding template warns
- Update .gitignore to cover .arcade-auth/, .venv/, bun.lock, .serena/, .context/, test-proj*/

Phase 3 – Open redirect:
- Validate next_uri is same-origin before redirecting in Next.js verify route
- Validate next_uri netloc in langchain verify route with urlparse

Phase 4 – Deduplication:
- Extract arcade OAuth provider into arcade-oauth-provider.hbs partial
- Replace ai-sdk and mastra arcade.ts with .hbs stubs referencing the partial
- Extract mapToolToSource() and extractAuthUrlFromToolOutput() into shared partials
- Remove inline duplicates from plan-route-body.hbs and sources-route-body.hbs
- Unify mapToolToSource regex (split on [._]) across both partials

Phase 5 – Security hardening:
- Set mode 0o700 on .arcade-auth/ dir and 0o600 on all written token files (TS + Python)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@teallarson teallarson marked this pull request as ready for review March 4, 2026 22:19
teallarson and others added 11 commits March 4, 2026 17:37
Instead of hard-failing when bun is missing, detect the available
package manager and remap install/migrate/devCommand to use npm/npx.
This lets users run `npx @arcadeai/create-agent` without needing bun
pre-installed for the TypeScript templates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The "url" field on todo items was getting populated with URLs found
inside message content (links to docs, Jira, etc.) instead of
permalinks to the actual Slack message. Add explicit URL RULES to
both the TypeScript (Handlebars) and Python (LangChain) plan prompts
telling the model to always link to the item itself in its source app.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- installDeps returns boolean; runMigrations gated on success
- validateProjectName blocks colon (Windows reserved path char)
- eslint.config.mjs: explicit no-console:off for templates/**
- verify/route.ts: cache new URL(base) instead of constructing twice
- arcade-oauth-provider.hbs: break long setPendingAuthUrl line
- arcade_oauth.py: comment explaining mkdir+chmod pattern

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- useState(true) so first-time visitors land on "Create account"
- Replace the small bottom text link with a pill tab switcher
  (Create account / Sign in) so the current mode is always obvious

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, any failure in verifyExistingConnection() (including network
glitches or a briefly-down gateway) caused the code to fall through to
initiateOAuth(), which would call redirectToAuthorization() and return
a new auth URL — forcing users to re-authorize even though their tokens
were valid.

Fix: verifyExistingConnection() now returns "ok" | "needs_reauth" |
"unreachable" instead of boolean. Only a genuine 401/403 response falls
through to OAuth. Any other failure surfaces a "check your gateway URL
and retry" error instead of triggering a spurious re-auth prompt.

Same logic applied to both TypeScript templates and the Python template.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The strict "OMIT url if no deep link" rule was causing most items to
render without links. The new rules still prefer direct deep links
(Slack permalink, Linear issue URL, Gmail thread URL, etc.) but fall
back to any URL found in the tool response rather than omitting the
field entirely. Only omit url if no URL exists anywhere in the response.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to register

Replace red accent colors with neutral gray/dark palette matching the Next.js
design. Add a pill tab switcher (Create account / Sign in) replacing the old
bottom toggle link, and default to the register mode on first load.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Next.js template doesn't show this widget; drop it from the Python
template too so the dashboards stay consistent.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace all red accents (spinner, buttons, gate icons) with neutral gray/dark
- Remove emoji from source badges; match Next.js badge color tokens exactly
- Fix P2/FYI priority badge colors to match Next.js (gray/outline)
- Add dark mode to task and stats cards (bg-white dark:bg-gray-900, dark border)
- Align task card layout: badges in one row with effort ml-auto; next step as
  italic right-aligned text instead of a boxed "Next:" label
- Stats cards: text-3xl font-bold, label/value stacked to match StatsBar.tsx

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CLAUDE.md now points to AGENTS.md, which contains all project guidance
including CLI mechanics, package managers, template system, UI components,
linting details, CI jobs, E2E testing, and release process.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@teallarson teallarson merged commit 63dde2e into main Mar 5, 2026
@teallarson teallarson deleted the teallarson/code-review-fixes branch March 5, 2026 15:32
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.

1 participant