Skip to content

Polish npx shotfix for first-run experience #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JasperNoBoxDev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Polish npx shotfix for first-run experience #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@
"shotfix-mcp": "dist/index.js"
},
"files": [
"dist"
"dist",
"widget/dist/shotfix.min.js"
],
"scripts": {
"build": "tsc",
"start": "node dist/index.js",
"prepublishOnly": "npm run build"
"build:widget": "cd ../widget && npm run build && mkdir -p ../cli/widget/dist && cp dist/shotfix.min.js ../cli/widget/dist/",
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 15, 2026

Choose a reason for hiding this comment

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

medium

The build:widget script relies on shell commands (cd, mkdir -p, cp) that may not be available or behave consistently across all operating systems, particularly on standard Windows command prompts. This can hinder cross-platform development and usage. For better portability, consider using Node.js-based utilities for file system operations, either through a library like fs-extra or by creating a small Node script that uses the built-in fs/promises module.

"prepublishOnly": "npm run build:widget && npm run build",
"start": "node dist/index.js"
},
"keywords": [
"mcp",
"shotfix",
"claude",
"screenshot",
"visual-ai",
"auto-fix",
"developer-tools",
"ui-debug",
"mcp",
"model-context-protocol"
],
"repository": {
Expand Down
108 changes: 92 additions & 16 deletions cli/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,38 @@ import { createServer } from 'node:http';
import { request as httpsRequest } from 'node:https';
import { mkdir, writeFile, readdir, unlink, copyFile, stat, readFile } from 'node:fs/promises';
import { join, relative, resolve } from 'node:path';
import { execSync, spawnSync } from 'node:child_process';
import { spawnSync } from 'node:child_process';
import { createInterface } from 'node:readline';

const PORT = 2847;
const VERSION = '0.1.0';

async function main() {
const args = process.argv.slice(2);

// --help
if (args.includes('--help') || args.includes('-h')) {
console.log(`
shotfix v${VERSION} — Visual AI input for developers

Usage:
shotfix [options]

Options:
--port <n> Server port (default: 2847)
--dir <path> Captures directory (default: .shotfix/captures)
Comment on lines +24 to +25
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove --port from user-facing CLI behavior to keep endpoint contract fixed.

Line 24 advertises a configurable port, but this service contract is required to stay on 2847 for health/capture integration.

🔧 Proposed fix 
-    --port <n>      Server port (default: 2847)
     --dir <path>    Captures directory (default: .shotfix/captures)

And remove --port parsing so the server always listens on PORT = 2847.

As per coding guidelines: "cli/src/**/*.{js,ts}: Fixed port 2847 must be used for the local dev server health check and capture endpoint".

📝 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
--port <n> Server port (default: 2847)
--dir <path> Captures directory (default: .shotfix/captures)
--dir <path> Captures directory (default: .shotfix/captures)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cli/src/cli.ts` around lines 24 - 25, Remove the user-configurable port
option from the CLI: delete the `--port` entry from the help/usage text and
remove any parsing/handling of a `port` flag/option in cli.ts (e.g., where CLI
args are parsed or a `port` variable is set). Ensure the server always uses a
fixed constant PORT = 2847 (replace any reference to parsed port with the
constant) so the health/capture endpoints remain on 2847; update any variables
or function calls that used `port` to reference the fixed `PORT` instead and
remove unused imports/variables related to the removed option.

--no-watch Disable auto-fix watch mode
--help, -h Show this help

Environment:
GEMINI_API_KEY Gemini API key for auto-fix (prompted on first run)
`);
process.exit(0);
}

const portArg = args.find((_, i) => args[i - 1] === '--port');
const dirArg = args.find((_, i) => args[i - 1] === '--dir');
const watchMode = args.includes('--watch');
const watchMode = !args.includes('--no-watch');

const port = portArg ? parseInt(portArg, 10) : PORT;
const capturesDir = join(process.cwd(), dirArg || '.shotfix/captures');
Expand All @@ -22,9 +45,15 @@ async function main() {

// Write .gitignore for .shotfix/
try {
await writeFile(join(shotfixDir, '.gitignore'), 'captures/\n', { flag: 'wx' });
await writeFile(join(shotfixDir, '.gitignore'), 'captures/\n.env\n', { flag: 'wx' });
} catch {
// Already exists, fine
// Already exists — make sure .env is in it
try {
const existing = await readFile(join(shotfixDir, '.gitignore'), 'utf-8');
if (!existing.includes('.env')) {
await writeFile(join(shotfixDir, '.gitignore'), existing.trimEnd() + '\n.env\n');
}
Comment on lines +50 to +55
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Backfill both .gitignore entries when the file already exists.

Lines 53-55 only append .env. If an existing .shotfix/.gitignore is missing captures/, screenshots/JSON can still be committed accidentally.

🛠️ Proposed fix 
-      const existing = await readFile(join(shotfixDir, '.gitignore'), 'utf-8');
-      if (!existing.includes('.env')) {
-        await writeFile(join(shotfixDir, '.gitignore'), existing.trimEnd() + '\n.env\n');
-      }
+      const gitignorePath = join(shotfixDir, '.gitignore');
+      const existing = await readFile(gitignorePath, 'utf-8');
+      const lines = new Set(existing.split(/\r?\n/).filter(Boolean));
+      lines.add('captures/');
+      lines.add('.env');
+      await writeFile(gitignorePath, `${Array.from(lines).join('\n')}\n`);
📝 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
// Already exists — make sure .env is in it
try {
const existing = await readFile(join(shotfixDir, '.gitignore'), 'utf-8');
if (!existing.includes('.env')) {
await writeFile(join(shotfixDir, '.gitignore'), existing.trimEnd() + '\n.env\n');
}
// Already exists — make sure .env is in it
try {
const gitignorePath = join(shotfixDir, '.gitignore');
const existing = await readFile(gitignorePath, 'utf-8');
const lines = new Set(existing.split(/\r?\n/).filter(Boolean));
lines.add('captures/');
lines.add('.env');
await writeFile(gitignorePath, `${Array.from(lines).join('\n')}\n`);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cli/src/cli.ts` around lines 50 - 55, The existing .gitignore backfill only
appends ".env"; update the logic in the try block that reads/writes
join(shotfixDir, '.gitignore') to check for and append any missing entries from
the list [".env", "captures/", "screenshots.json"] (or whichever specific
screenshot JSON filename you use) instead of only ".env"; use the existing
variable names (readFile, writeFile, shotfixDir, existing) to build a
newContents string that adds only the missing lines and then
writeFile(join(shotfixDir, '.gitignore'), newContents).

} catch {}
Comment on lines +51 to +56
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 15, 2026

Choose a reason for hiding this comment

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

high

The nested catch block is empty, which silences any errors that might occur when trying to read or update the .gitignore file. If an error other than the file already existing occurs (e.g., a permissions issue), it will fail silently, and the .env file might not be ignored. This increases the risk of accidentally committing secrets. It's crucial to log potential errors here to alert the user.

    try {
      const existing = await readFile(join(shotfixDir, '.gitignore'), 'utf-8');
      if (!existing.includes('.env')) {
        await writeFile(join(shotfixDir, '.gitignore'), existing.trimEnd() + '\n.env\n');
      }
    } catch (e) {
      console.warn(`[shotfix] Failed to update .shotfix/.gitignore: ${(e as Error).message}`);
    }

}

// Cleanup old captures (>1 hour)
Expand All @@ -51,13 +80,38 @@ async function main() {
}
}

// Resolve API key from env or noxkey
// Resolve API key: env var → .shotfix/.env → interactive prompt
let apiKey = process.env.GEMINI_API_KEY || '';
if (!apiKey && watchMode) {

if (!apiKey) {
const envPath = join(shotfixDir, '.env');
try {
apiKey = execSync('noxkey get shared/GEMINI_API_KEY --raw', { encoding: 'utf-8', stdio: 'pipe' }).trim();
const envContent = await readFile(envPath, 'utf-8');
const match = envContent.match(/^GEMINI_API_KEY=(.+)$/m);
if (match) apiKey = match[1].trim();
Comment on lines +90 to +91
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 15, 2026

Choose a reason for hiding this comment

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

medium

The regular expression for parsing the .env file is not robust enough to handle common formats, such as values enclosed in quotes or lines with trailing comments. This could lead to parsing an incorrect API key. To improve reliability, the regex should be updated to accommodate these variations.

      const match = envContent.match(/^\s*GEMINI_API_KEY=(.*?)(\s*#.*)?$/m);
      if (match) apiKey = match[1].trim().replace(/^['"]|['"]$/g, '');

} catch {
// Will warn at startup
// No .env file yet
}
}

if (!apiKey && watchMode && process.stdin.isTTY) {
console.log('');
console.log(' 🔑 Shotfix needs a Gemini API key for auto-fix.');
console.log(' Get one free at: https://aistudio.google.com/apikey');
console.log('');
Comment on lines +97 to +101
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Prefix new console logs with [Shotfix].

Lines 99-101, 114, 483, and 507-520 use unprefixed console output; changed logs should follow the project logging prefix rule.

As per coding guidelines: "Applies to /{widget,cli}//*.{js,ts} : Use [Shotfix] prefix for all console logging".

Also applies to: 114-114, 483-483, 507-520

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cli/src/cli.ts` around lines 97 - 101, Several console.log calls in cli.ts
(notably the branch that checks apiKey and watchMode using variables apiKey and
watchMode, plus the other occurrences around the blocks referenced) are missing
the required "[Shotfix]" prefix; update each plain console.log invocation
(including the messages at the apiKey/watchMode check and the other occurrences
flagged) to prepend "[Shotfix] " to the message string so all CLI output follows
the project logging prefix rule. Locate the console.log calls in the top-level
CLI flow (the apiKey/watchMode branch) and the other unprefixed console.log
blocks and change their messages to start with "[Shotfix]" while preserving the
original message text and spacing.


const rl = createInterface({ input: process.stdin, output: process.stdout });
apiKey = await new Promise<string>((resolve) => {
rl.question(' Enter your API key: ', (answer) => {
rl.close();
resolve(answer.trim());
});
});

if (apiKey) {
await mkdir(shotfixDir, { recursive: true });
await writeFile(join(shotfixDir, '.env'), `GEMINI_API_KEY=${apiKey}\n`);
console.log(' ✅ Saved to .shotfix/.env\n');
Comment on lines +111 to +114
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify .env write call and whether explicit restrictive mode is set.
rg -n "writeFile\\(\\s*join\\(shotfixDir, '\\.env'\\)" cli/src/cli.ts -C4

Repository: No-Box-Dev/shotfix

Length of output: 317

🏁 Script executed:

# Check if there are other instances of writeFile in this file that might have the same issue
rg -n "writeFile" cli/src/cli.ts

Repository: No-Box-Dev/shotfix

Length of output: 772

Write .shotfix/.env with restrictive permissions.

Line 113 saves the API key without explicit file mode; default permissions (typically 0o644 with common umask settings) make the file readable by other users on the system.

Fix 
-      await writeFile(join(shotfixDir, '.env'), `GEMINI_API_KEY=${apiKey}\n`);
+      await writeFile(
+        join(shotfixDir, '.env'),
+        `GEMINI_API_KEY=${apiKey}\n`,
+        { mode: 0o600 }
+      );
📝 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
if (apiKey) {
await mkdir(shotfixDir, { recursive: true });
await writeFile(join(shotfixDir, '.env'), `GEMINI_API_KEY=${apiKey}\n`);
console.log(' ✅ Saved to .shotfix/.env\n');
if (apiKey) {
await mkdir(shotfixDir, { recursive: true });
await writeFile(
join(shotfixDir, '.env'),
`GEMINI_API_KEY=${apiKey}\n`,
{ mode: 0o600 }
);
console.log(' ✅ Saved to .shotfix/.env\n');
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cli/src/cli.ts` around lines 111 - 114, The .shotfix/.env is being written
with default permissions; update the mkdir and writeFile calls so the directory
is created with restrictive mode and the env file is written with mode 0o600:
when calling mkdir(shotfixDir, { recursive: true }) include a mode of 0o700 and
when calling writeFile(join(shotfixDir, '.env'), ...) pass options with mode:
0o600 (or alternatively call fs.chmod after writing) so the API key file is only
readable/writable by the owner; locate the mkdir and writeFile usages in cli.ts
to apply this change.

}
}

Expand Down Expand Up @@ -323,9 +377,25 @@ async function main() {
return;
}

// Serve the widget JS
if (url.pathname === '/widget.js' && req.method === 'GET') {
try {
const widgetPath = new URL('../widget/dist/shotfix.min.js', import.meta.url);
const widgetContent = await readFile(widgetPath);
res.writeHead(200, {
'Content-Type': 'application/javascript',
'Cache-Control': 'no-cache',
});
res.end(widgetContent);
} catch {
res.writeHead(404, { 'Content-Type': 'text/plain' });
res.end('Widget not found');
}
Comment on lines +390 to +393
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 15, 2026

Choose a reason for hiding this comment

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

medium

The catch block for serving /widget.js sends a 404 response but doesn't log the error on the server. If reading the file fails for any reason (e.g., permissions, build errors), the issue is not visible on the server, making debugging difficult. Logging the error here would provide valuable diagnostic information.

      } catch (err) {
        console.error(`[shotfix] Error serving widget.js: ${(err as Error).message}`);
        res.writeHead(404, { 'Content-Type': 'text/plain' });
        res.end('Widget not found');
      }

return;
}

if (url.pathname === '/cancel' && req.method === 'POST') {
cancelRequested = true;
// Clear the queue too
const cleared = fixQueue.length;
fixQueue.length = 0;
console.log(` 🚫 Cancel requested (${cleared} queued items cleared)`);
Expand Down Expand Up @@ -410,7 +480,7 @@ async function main() {
await cleanOldCaptures();

const title = captureJson.title;
console.log(`[${timeLabel}] ⚡ Captured: ${title}`);
console.log(` ⚡ Captured: ${title}`);
broadcast('capture', { title, timestamp: timeLabel });

res.writeHead(200, { 'Content-Type': 'application/json' });
Expand All @@ -434,16 +504,22 @@ async function main() {
});

server.listen(port, () => {
console.log(`⚡ Shotfix dev server running on port ${port}`);
console.log(` Captures saved to ${capturesDir}`);
console.log('');
console.log(` ⚡ Shotfix v${VERSION}`);
console.log('');
console.log(' Add to your HTML:');
console.log(` <script src="http://localhost:${port}/widget.js"><\/script>`);
console.log(' <script>Shotfix.init();<\/script>');
console.log('');
console.log(` Server: http://localhost:${port}`);
if (watchMode && apiKey) {
console.log(` 🤖 Watch mode: captures will auto-fix via Gemini Flash`);
console.log(' Watch: ON (Gemini Flash)');
} else if (watchMode) {
console.log(` ⚠️ Watch mode enabled but no GEMINI_API_KEY found`);
console.log(` Set env var or store with: noxkey set shared/GEMINI_API_KEY`);
console.log(' Watch: ON (no API key — captures only)');
} else {
console.log(' Watch: OFF');
}
console.log('');
console.log('Tip: Add to CLAUDE.md: "Check .shotfix/captures/latest.json and latest.png for bug captures"');
});
}

Expand Down