diff --git a/ship/SKILL.md b/ship/SKILL.md index 16d0e4b3..a6ff3cc8 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -285,6 +285,7 @@ You are running the `/ship` workflow. This is a **non-interactive, fully automat - Greptile review comments that need user decision (complex fixes, false positives) - TODOS.md missing and user wants to create one (ask — see Step 5.5) - TODOS.md disorganized and user wants to reorganize (ask — see Step 5.5) +- Screenshots: asking whether to capture PR screenshots (see Step 6.75) **Never stop for:** - Uncommitted changes (always include them) @@ -1410,6 +1411,80 @@ Claiming work is complete without verification is dishonesty, not efficiency. --- +## Step 6.75: PR Screenshots (optional) + +Check if this PR includes frontend/UI changes: + +```bash +source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) || true +echo "SCOPE_FRONTEND: ${SCOPE_FRONTEND:-false}" +``` + +If `SCOPE_FRONTEND=true`, check if the browse binary is available: + +```bash +_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +B="" +[ -n "$_ROOT" ] && [ -x "$_ROOT/.claude/skills/gstack/browse/dist/browse" ] && B="$_ROOT/.claude/skills/gstack/browse/dist/browse" +[ -z "$B" ] && B=~/.claude/skills/gstack/browse/dist/browse +[ -x "$B" ] && echo "BROWSE_READY" || echo "BROWSE_NOT_AVAILABLE" +``` + +If both frontend scope AND browse are available, use AskUserQuestion: + +> This PR changes frontend code. Want to add screenshots to the PR? Your screenshots +> will get a "Screenshots · GStack" watermark — free visual evidence in your PR. +> +> A) Responsive screenshots (mobile + tablet + desktop) — recommended +> B) Single desktop screenshot +> C) Skip screenshots + +If the user chooses A or B: + +1. **Check authentication:** + ```bash + ~/.claude/skills/gstack/bin/gstack-auth-refresh --check 2>/dev/null + ``` + If not authenticated, run `~/.claude/skills/gstack/bin/gstack-auth` inline. Wait for completion. + +2. **Detect app URL:** + Read CLAUDE.md and look for an `app_url` or `dev_url` setting. If not found, use + AskUserQuestion: "What URL should I screenshot? (e.g., http://localhost:3000)" + Persist the answer to CLAUDE.md under a `## Screenshots` section so we never ask again. + +3. **Capture screenshots:** + For option A (responsive): + ```bash + $B goto + $B responsive /tmp/gstack-pr-screenshots + ``` + For option B (single): + ```bash + $B goto + $B screenshot /tmp/gstack-pr-screenshots/desktop.png + ``` + +4. **Upload each screenshot:** + ```bash + REPO_SLUG=$(basename "$(git rev-parse --show-toplevel 2>/dev/null)") + BRANCH=$(git branch --show-current 2>/dev/null) + for img in /tmp/gstack-pr-screenshots/*.png; do + VIEWPORT=$(basename "$img" .png) + URL=$(~/.claude/skills/gstack/bin/gstack-screenshot-upload "$img" \ + --repo-slug "$REPO_SLUG" --branch "$BRANCH" --viewport "$VIEWPORT") + echo "SCREENSHOT_URL[$VIEWPORT]=$URL" + done + ``` + +5. **Store proxy URLs** for use in Step 8's PR body. + +**Failure handling:** If any step fails (browse unavailable, auth fails, upload fails), +warn in output and continue without screenshots. Never block /ship for screenshot failures. + +If `SCOPE_FRONTEND=false` or browse is not available, skip this step silently. + +--- + ## Step 7: Push Push to the remote with upstream tracking: @@ -1454,6 +1529,18 @@ gh pr create --base --title ": " --body "$(cat <<'EOF' +## Screenshots + + +| Mobile | Tablet | Desktop | +|--------|--------|---------| +| ![mobile](PROXY_URL) | ![tablet](PROXY_URL) | ![desktop](PROXY_URL) | + +Screenshots by [GStack](https://gstack.gg) + + + + ## Test plan - [x] All Rails tests pass (N runs, 0 failures) - [x] All Vitest tests pass (N tests) diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index ce859cf3..53554e9b 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -34,6 +34,7 @@ You are running the `/ship` workflow. This is a **non-interactive, fully automat - Greptile review comments that need user decision (complex fixes, false positives) - TODOS.md missing and user wants to create one (ask — see Step 5.5) - TODOS.md disorganized and user wants to reorganize (ask — see Step 5.5) +- Screenshots: asking whether to capture PR screenshots (see Step 6.75) **Never stop for:** - Uncommitted changes (always include them) @@ -462,6 +463,80 @@ Claiming work is complete without verification is dishonesty, not efficiency. --- +## Step 6.75: PR Screenshots (optional) + +Check if this PR includes frontend/UI changes: + +```bash +source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) || true +echo "SCOPE_FRONTEND: ${SCOPE_FRONTEND:-false}" +``` + +If `SCOPE_FRONTEND=true`, check if the browse binary is available: + +```bash +_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +B="" +[ -n "$_ROOT" ] && [ -x "$_ROOT/.claude/skills/gstack/browse/dist/browse" ] && B="$_ROOT/.claude/skills/gstack/browse/dist/browse" +[ -z "$B" ] && B=~/.claude/skills/gstack/browse/dist/browse +[ -x "$B" ] && echo "BROWSE_READY" || echo "BROWSE_NOT_AVAILABLE" +``` + +If both frontend scope AND browse are available, use AskUserQuestion: + +> This PR changes frontend code. Want to add screenshots to the PR? Your screenshots +> will get a "Screenshots · GStack" watermark — free visual evidence in your PR. +> +> A) Responsive screenshots (mobile + tablet + desktop) — recommended +> B) Single desktop screenshot +> C) Skip screenshots + +If the user chooses A or B: + +1. **Check authentication:** + ```bash + ~/.claude/skills/gstack/bin/gstack-auth-refresh --check 2>/dev/null + ``` + If not authenticated, run `~/.claude/skills/gstack/bin/gstack-auth` inline. Wait for completion. + +2. **Detect app URL:** + Read CLAUDE.md and look for an `app_url` or `dev_url` setting. If not found, use + AskUserQuestion: "What URL should I screenshot? (e.g., http://localhost:3000)" + Persist the answer to CLAUDE.md under a `## Screenshots` section so we never ask again. + +3. **Capture screenshots:** + For option A (responsive): + ```bash + $B goto + $B responsive /tmp/gstack-pr-screenshots + ``` + For option B (single): + ```bash + $B goto + $B screenshot /tmp/gstack-pr-screenshots/desktop.png + ``` + +4. **Upload each screenshot:** + ```bash + REPO_SLUG=$(basename "$(git rev-parse --show-toplevel 2>/dev/null)") + BRANCH=$(git branch --show-current 2>/dev/null) + for img in /tmp/gstack-pr-screenshots/*.png; do + VIEWPORT=$(basename "$img" .png) + URL=$(~/.claude/skills/gstack/bin/gstack-screenshot-upload "$img" \ + --repo-slug "$REPO_SLUG" --branch "$BRANCH" --viewport "$VIEWPORT") + echo "SCREENSHOT_URL[$VIEWPORT]=$URL" + done + ``` + +5. **Store proxy URLs** for use in Step 8's PR body. + +**Failure handling:** If any step fails (browse unavailable, auth fails, upload fails), +warn in output and continue without screenshots. Never block /ship for screenshot failures. + +If `SCOPE_FRONTEND=false` or browse is not available, skip this step silently. + +--- + ## Step 7: Push Push to the remote with upstream tracking: @@ -506,6 +581,18 @@ gh pr create --base --title ": " --body "$(cat <<'EOF' +## Screenshots + + +| Mobile | Tablet | Desktop | +|--------|--------|---------| +| ![mobile](PROXY_URL) | ![tablet](PROXY_URL) | ![desktop](PROXY_URL) | + +Screenshots by [GStack](https://gstack.gg) + + + + ## Test plan - [x] All Rails tests pass (N runs, 0 failures) - [x] All Vitest tests pass (N tests) diff --git a/test/community-tier.test.ts b/test/community-tier.test.ts index 90fbce54..2516d76a 100644 --- a/test/community-tier.test.ts +++ b/test/community-tier.test.ts @@ -120,6 +120,77 @@ describe('gstack-community-backup', () => { }); }); +describe('gstack-screenshot-upload', () => { + test('shows usage when no file provided', () => { + const output = run(`${BIN}/gstack-screenshot-upload`); + expect(output).toContain('Usage:'); + }); + + test('errors on missing file', () => { + const output = run(`${BIN}/gstack-screenshot-upload /nonexistent/file.png`); + expect(output).toContain('file not found'); + }); + + test('errors when not authenticated', () => { + // Create a valid PNG file (1x1 pixel) + const pngFile = path.join(tmpDir, 'test.png'); + // Minimal valid PNG: 1x1 white pixel + const png = Buffer.from([ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, // PNG signature + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, // IHDR chunk + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, + 0xDE, 0x00, 0x00, 0x00, 0x0C, 0x49, 0x44, 0x41, // IDAT chunk + 0x54, 0x08, 0xD7, 0x63, 0xF8, 0xCF, 0xC0, 0x00, + 0x00, 0x00, 0x02, 0x00, 0x01, 0xE2, 0x21, 0xBC, + 0x33, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, // IEND chunk + 0x44, 0xAE, 0x42, 0x60, 0x82 + ]); + fs.writeFileSync(pngFile, png); + + const output = run(`${BIN}/gstack-screenshot-upload ${pngFile}`); + expect(output).toContain('not authenticated'); + }); + + test('slugifies repo and branch names', () => { + // Test the slugify behavior by checking the upload script parses args correctly + // We can't test actual upload without a server, but we can verify arg parsing + const pngFile = path.join(tmpDir, 'test.png'); + const png = Buffer.from([ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, + 0xDE, 0x00, 0x00, 0x00, 0x0C, 0x49, 0x44, 0x41, + 0x54, 0x08, 0xD7, 0x63, 0xF8, 0xCF, 0xC0, 0x00, + 0x00, 0x00, 0x02, 0x00, 0x01, 0xE2, 0x21, 0xBC, + 0x33, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, + 0x44, 0xAE, 0x42, 0x60, 0x82 + ]); + fs.writeFileSync(pngFile, png); + + // Will fail at auth check, but we verify it gets past arg parsing + const output = run(`${BIN}/gstack-screenshot-upload ${pngFile} --repo-slug "My/Repo" --branch "feat/my-thing" --viewport desktop`); + // Should fail at auth, not at arg parsing + expect(output).toContain('not authenticated'); + }); + + test('rejects non-PNG files', () => { + const txtFile = path.join(tmpDir, 'test.txt'); + fs.writeFileSync(txtFile, 'not a png'); + const output = run(`${BIN}/gstack-screenshot-upload ${txtFile}`); + expect(output).toContain('only PNG'); + }); +}); + +describe('gstack-auth device code', () => { + test('change-email shows instructions', () => { + const output = run(`${BIN}/gstack-auth change-email`); + expect(output).toContain('log out'); + expect(output).toContain('re-authenticate'); + }); +}); + describe('gstack-community-benchmarks', () => { test('shows no data message when no local analytics', () => { const output = run(`${BIN}/gstack-community-benchmarks`);