Skip to content

Commit f1c8d73

Browse files
mariojgtclaude
andauthored
Collapse duplicated helpers and drop dead code (#281)
Ten cleanups across the CLI, config, parsers and the protect scaffolder. None of them change behaviour: no printed string, doc or guide-checklist output is touched. The largest group is reuse. Four adapters in the protect scaffolder had each inlined a character-for-character copy of hasDependency() from the util module sitting next to them, and the CLI had four copies of the same resolveConfig() call, two of the same CI test, and two of the same gitignore-outcome line. The rest is dead code and naming: two exported functions with no callers anywhere, a pair of destructured locals whose underscore prefix said "unused" while both were read two lines later, and a bare 191 that now has the same name its twin already carries in site-name.ts. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 62dcf95 commit f1c8d73

10 files changed

Lines changed: 60 additions & 114 deletions

File tree

‎src/cli.ts‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
secretFileIgnored,
3939
persistSiteUuid,
4040
resolveConfig,
41+
type ResolveConfigOptions,
4142
writeConfigFile,
4243
persistTimeout,
4344
} from './config.js';
@@ -76,6 +77,7 @@ import { runProtect, runVerify } from './protect/install/index.js';
7677
import { formatRuntimeCheck, runRuntimeCheck, runtimeExitCode } from './protect/install/runtime/check.js';
7778
import { runMap } from './map-command.js';
7879
import { getStringFlag } from './flags.js';
80+
import { isCanonicalUuid } from './endpoint-policy.js';
7981
import { setupProtection, wireBuildScripts } from './setup.js';
8082
import type { SetupProtectionResult, WireBuildScriptsResult } from './setup.js';
8183
import { isInstallOrBuildHook, isPreBundleBuildHook, undeliveredReportLines } from './build-hook.js';
@@ -296,7 +298,38 @@ function parseArgs(argv: string[]): ParsedArgs {
296298
};
297299
}
298300

301+
/** The site/endpoint overrides every command accepts, resolved against the current directory. */
302+
function resolveCliConfig(
303+
args: ParsedArgs,
304+
extra: Omit<ResolveConfigOptions, 'cwd' | 'cliSiteUuid' | 'cliEndpoint'> = {},
305+
): Promise<Config> {
306+
return resolveConfig({
307+
cwd: process.cwd(),
308+
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
309+
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
310+
...extra,
311+
});
312+
}
313+
314+
/**
315+
* Whether a build agent is running this. `CI=false` is how a platform says "not a CI build", so it
316+
* counts as absent — the interactive commands refuse only where there is really no one to answer.
317+
*/
318+
function runningInCi(): boolean {
319+
return process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false';
320+
}
299321

322+
/**
323+
* What became of the credential file's ignore entry, for the person reading the output.
324+
*
325+
* Only claimed when `.gitignore` was read back and really covers it: an assurance that turns out to be
326+
* false is worse than none, because it is the reason somebody stops checking.
327+
*/
328+
function gitignoreOutcomeLine(ignore: { ignored: boolean; reason?: string }): string {
329+
return ignore.ignored
330+
? ' Added to .gitignore.'
331+
: ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`;
332+
}
300333

301334
async function runInit(args: ParsedArgs): Promise<number> {
302335
const uuid = args.positional[0];
@@ -305,7 +338,7 @@ async function runInit(args: ParsedArgs): Promise<number> {
305338
console.error('Usage: patchstack-connect init <site-uuid>');
306339
return 1;
307340
}
308-
if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(uuid)) {
341+
if (!isCanonicalUuid(uuid)) {
309342
console.error(`Error: "${uuid}" does not look like a valid UUID.`);
310343
return 1;
311344
}
@@ -319,16 +352,12 @@ async function runInit(args: ParsedArgs): Promise<number> {
319352

320353
async function runClaim(args: ParsedArgs): Promise<number> {
321354
// No browser and no human to sign in. A deploy inherits an already-claimed site; it never claims.
322-
if (process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false') {
355+
if (runningInCi()) {
323356
console.error('`claim` is interactive and cannot run in CI. Claim the site from a developer machine.');
324357
return 1;
325358
}
326359

327-
const config = await resolveConfig({
328-
cwd: process.cwd(),
329-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
330-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
331-
});
360+
const config = await resolveCliConfig(args);
332361

333362
const siteUuid = config.siteUuid;
334363
if (siteUuid === null) {
@@ -345,11 +374,7 @@ async function runClaim(args: ParsedArgs): Promise<number> {
345374
const ignore = await secretFileIgnored(process.cwd());
346375
// The value itself is never printed — only that it landed, and only that it is ignored when it is.
347376
console.log(` A credential for this site was issued and saved to ${SECRET_CONFIG_FILENAME}.`);
348-
console.log(
349-
ignore.ignored
350-
? ' Added to .gitignore.'
351-
: ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`,
352-
);
377+
console.log(gitignoreOutcomeLine(ignore));
353378
}
354379
console.log('');
355380
return 0;
@@ -438,28 +463,20 @@ async function runClaim(args: ParsedArgs): Promise<number> {
438463
async function runLogin(args: ParsedArgs): Promise<number> {
439464
// CI has no browser and no human; build agents must not print credentials
440465
// into logs. Deploys use PATCHSTACK_PULSE_AUTH from the platform's secrets.
441-
if (process.env.CI !== undefined && process.env.CI !== '' && process.env.CI !== 'false') {
466+
if (runningInCi()) {
442467
console.error('`login` is interactive and cannot run in CI. Set PATCHSTACK_PULSE_AUTH instead.');
443468
return 1;
444469
}
445470

446-
const config = await resolveConfig({
447-
cwd: process.cwd(),
448-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
449-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
450-
});
471+
const config = await resolveCliConfig(args);
451472

452473
// Checked here rather than carried up from the write: the rotation happens several layers down, and the
453474
// claim belongs to the line that prints it.
454475
const approved = async () => {
455476
const ignore = await secretFileIgnored(process.cwd());
456477
// The value itself is never printed — only that it landed, and only that it is ignored when it is.
457478
console.log(`\n ✓ Credential restored and saved to ${SECRET_CONFIG_FILENAME}.`);
458-
console.log(
459-
ignore.ignored
460-
? ' Added to .gitignore.'
461-
: ` NOT ignored by git — ${ignore.reason ?? 'unknown reason'}. Add it to .gitignore yourself before committing.`,
462-
);
479+
console.log(gitignoreOutcomeLine(ignore));
463480
console.log(' The previous credential no longer works. Update it anywhere else it was set:');
464481
console.log(' CI secrets, hosting env vars, preview environments, other checkouts.\n');
465482
return 0;
@@ -619,10 +636,7 @@ async function runScan(
619636
options: { showRemainingSetup?: boolean } = {},
620637
): Promise<number> {
621638
const dryRun = args.flags.get('dry-run') === true;
622-
const config = await resolveConfig({
623-
cwd: process.cwd(),
624-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
625-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
639+
const config = await resolveCliConfig(args, {
626640
cliClaimToken: getStringFlag(args.flags, 'claim-token'),
627641
// The one command that reports them, so the one command that resolves them.
628642
detectSiteIdentity: true,
@@ -1009,12 +1023,7 @@ async function runDemoCommand(args: ParsedArgs): Promise<number> {
10091023
try {
10101024
const scenario = resolveDemoScenario(args.positional[0]);
10111025
const cwd = process.cwd();
1012-
const config = await resolveConfig({
1013-
cwd,
1014-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
1015-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
1016-
requireSiteUuid: true,
1017-
});
1026+
const config = await resolveCliConfig(args, { requireSiteUuid: true });
10181027
if (config.environment !== 'production') {
10191028
throw new DemoError(
10201029
'The production-backed demo requires PATCHSTACK_ENVIRONMENT=production. Unset the sandbox override and try again.',
@@ -1269,11 +1278,7 @@ function setupOutcome(
12691278
}
12701279

12711280
async function runStatus(args: ParsedArgs): Promise<number> {
1272-
const config = await resolveConfig({
1273-
cwd: process.cwd(),
1274-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
1275-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
1276-
});
1281+
const config = await resolveCliConfig(args);
12771282
console.log(`Site UUID: ${config.siteUuid ?? '(none yet — the next `scan` will provision one)'}`);
12781283
console.log(
12791284
`Endpoint: ${config.endpoint}${config.endpoint === DEFAULT_ENDPOINT ? '' : ' (override)'}`,
@@ -1311,11 +1316,7 @@ async function runStatus(args: ParsedArgs): Promise<number> {
13111316
}
13121317

13131318
async function runUninstall(args: ParsedArgs): Promise<number> {
1314-
const config = await resolveConfig({
1315-
cwd: process.cwd(),
1316-
cliSiteUuid: getStringFlag(args.flags, 'site-uuid'),
1317-
cliEndpoint: getStringFlag(args.flags, 'endpoint'),
1318-
});
1319+
const config = await resolveCliConfig(args);
13191320

13201321
if (config.siteUuid === null) {
13211322
console.log('No site UUID configured — there is no site record to signal about.');

‎src/config.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,13 +448,13 @@ export async function persistTimeout(cwd: string, timeoutMs: number): Promise<st
448448
* credential the server has replaced.
449449
*/
450450
export async function persistApiKey(cwd: string, apiKey: string): Promise<SecretFileResult> {
451-
const { apiKey: _movedKey, pulseAuth: _movedPulse, ...publicConfig } = await readConfigFile(cwd);
451+
const { apiKey: movedKey, pulseAuth: movedPulse, ...publicConfig } = await readConfigFile(cwd);
452452
const existingSecrets = await readSecretFile(cwd);
453453

454454
const target = await writeSecretFile(cwd, { ...existingSecrets, apiKey, pulseAuth: undefined });
455455

456456
// Only rewritten when it actually held a credential, so a normal provision does not touch it.
457-
if (_movedKey !== undefined || _movedPulse !== undefined) {
457+
if (movedKey !== undefined || movedPulse !== undefined) {
458458
await writeConfigFile(cwd, publicConfig);
459459
}
460460

‎src/login.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
import { mkdtempSync } from 'node:fs';
2-
import { tmpdir } from 'node:os';
3-
import path from 'node:path';
41
import { persistApiKey } from './config.js';
52
import {
63
clearPending,
@@ -127,8 +124,3 @@ export async function login(
127124

128125
return waitForApproval(config, started.pending, deps);
129126
}
130-
131-
/** Exported for tests that need a scratch temp dir. */
132-
export function makeTempDir(prefix = 'patchstack-'): string {
133-
return mkdtempSync(path.join(tmpdir(), prefix));
134-
}

‎src/map/coordinates.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,6 @@ export function inputIdOf(source: InputSource | undefined, path: string): string
7676
return `${addressSpaceOf(source)}:${path}`;
7777
}
7878

79-
/**
80-
* The rule-engine NAMESPACE an input lands in (`post`, `get`, `cookie`, `files`, `server`), or null when
81-
* it has no address. Derived from `runtimeCoordinate` on purpose: comparing raw source labels would call
82-
* `json-body` and an Express `req.body` read different places when both resolve to `post.*`, and would
83-
* miss that `post.id` and `get.id` are genuinely different places.
84-
*/
85-
export function namespaceOf(source: InputSource | undefined, path: string): string | null {
86-
const { runtimeParameter } = runtimeCoordinate(source, path);
87-
if (!runtimeParameter) return null;
88-
const dot = runtimeParameter.indexOf('.');
89-
return dot === -1 ? runtimeParameter : runtimeParameter.slice(0, dot);
90-
}
91-
9279
/** Place extracted fields in a request region: attach `source`, the runtime coordinate, and the id. */
9380
export function withCoordinates(fields: FieldShape[], source: InputSource): InputField[] {
9481
return fields.map((f) => {

‎src/normalize.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Manifest, PackageEntry } from './types.js';
1+
import type { Manifest } from './types.js';
22

33
export interface WirePackage {
44
name: string;
@@ -210,10 +210,3 @@ function compareSegments(a: string[], b: string[]): number {
210210
}
211211
return 0;
212212
}
213-
214-
export function findPackageInManifest(
215-
manifest: Manifest,
216-
name: string,
217-
): PackageEntry[] {
218-
return manifest.packages.filter((p) => p.name === name);
219-
}

‎src/protect/install/adapters/astro.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Adapter: Astro. Wires the guard as middleware (`src/middleware.ts` → `onRequest`).
2-
import { join } from 'node:path';
3-
import { read } from '../util.js';
2+
import { hasDependency } from '../util.js';
43
import { wireSeam, verifySeam, type SeamSpec } from '../seam.js';
54
import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js';
65

@@ -14,12 +13,7 @@ const SPEC: SeamSpec = {
1413
};
1514

1615
function detect(cwd: string): boolean {
17-
try {
18-
const pkg = JSON.parse(read(join(cwd, 'package.json')));
19-
return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.astro);
20-
} catch {
21-
return false;
22-
}
16+
return hasDependency(cwd, 'astro');
2317
}
2418

2519
function wire(cwd: string, opts: WireOptions): WireResult {

‎src/protect/install/adapters/next.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,10 @@
55

66
import { existsSync } from 'node:fs';
77
import { join } from 'node:path';
8-
import { bakeSiteUuid, read, log, templatesDir } from '../util.js';
8+
import { bakeSiteUuid, hasDependency, read, log, templatesDir } from '../util.js';
99
import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js';
1010
import { copyProjectFileSync, ensureProjectDirectorySync } from '../../../safe-file.js';
1111

12-
function hasNextDep(cwd: string): boolean {
13-
try {
14-
const pkg = JSON.parse(read(join(cwd, 'package.json')));
15-
return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.next);
16-
} catch {
17-
return false;
18-
}
19-
}
20-
2112
// Next reads middleware from `middleware.ts` at the project root, or `src/middleware.ts` when the
2213
// app uses a `src/` dir. Return the existing one if present, else the conventional target.
2314
function middlewareInfo(cwd: string): { relDir: string; relFile: string; exists: boolean } {
@@ -30,7 +21,7 @@ function middlewareInfo(cwd: string): { relDir: string; relFile: string; exists:
3021
}
3122

3223
function detect(cwd: string): boolean {
33-
return hasNextDep(cwd);
24+
return hasDependency(cwd, 'next');
3425
}
3526

3627
function rulesFile(relDir: string): string {

‎src/protect/install/adapters/sveltekit.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Adapter: SvelteKit. Wires the guard as a server hook (`src/hooks.server.ts` → `handle`).
2-
import { join } from 'node:path';
3-
import { read } from '../util.js';
2+
import { hasDependency } from '../util.js';
43
import { wireSeam, verifySeam, type SeamSpec } from '../seam.js';
54
import type { Adapter, WireOptions, WireResult, VerifyResult } from '../types.js';
65

@@ -14,12 +13,7 @@ const SPEC: SeamSpec = {
1413
};
1514

1615
function detect(cwd: string): boolean {
17-
try {
18-
const pkg = JSON.parse(read(join(cwd, 'package.json')));
19-
return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }['@sveltejs/kit']);
20-
} catch {
21-
return false;
22-
}
16+
return hasDependency(cwd, '@sveltejs/kit');
2317
}
2418

2519
function wire(cwd: string, opts: WireOptions): WireResult {

‎src/protect/install/generic.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { readFileSync, existsSync, readdirSync, lstatSync } from 'node:fs';
77
import { join, dirname } from 'node:path';
8-
import { bakeSiteUuid, read, templatesDir } from './util.js';
8+
import { bakeSiteUuid, hasDependency, read, templatesDir } from './util.js';
99
import type { WireOptions, VerifyResult } from './types.js';
1010
import type { GuardModuleQuery } from './source-scope.js';
1111
import { copyProjectFileSync, ensureProjectDirectorySync } from '../../safe-file.js';
@@ -132,18 +132,9 @@ function candidateEntries(cwd: string): string[] {
132132
return [...new Set(hits)];
133133
}
134134

135-
function usesExpress(cwd: string): boolean {
136-
try {
137-
const pkg = JSON.parse(read(join(cwd, 'package.json')));
138-
return Boolean({ ...pkg.dependencies, ...pkg.devDependencies }.express);
139-
} catch {
140-
return false;
141-
}
142-
}
143-
144135
export function wiringPlan(cwd: string, dir: string): string {
145136
const entries = candidateEntries(cwd);
146-
const express = usesExpress(cwd);
137+
const express = hasDependency(cwd, 'express');
147138
const target = genericGuardTarget(cwd);
148139
const lines = [
149140
`no built-in adapter matched this stack — scaffolded a generic guard at ${dir}/${target.file} + ${dir}/rules.json.`,

‎src/site-url.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ function isUnpublishableHost(hostname: string): boolean {
6767
return RESERVED_SUFFIXES.some((suffix) => host.endsWith(suffix));
6868
}
6969

70+
/** The most Patchstack accepts for an address. */
71+
const URL_MAX_LENGTH = 191;
72+
7073
/**
7174
* Reduce a reported address to the `scheme://host[:port]` Patchstack stores, or null when it is not one.
7275
*
@@ -92,8 +95,8 @@ export function normaliseSiteUrl(value: string | undefined | null): string | nul
9295

9396
const origin = `${parsed.protocol}//${parsed.host}`;
9497

95-
// 191 characters is the most Patchstack accepts for an address; an origin near that length is not one.
96-
return origin.length <= 191 ? origin : null;
98+
// An origin near the limit is not one.
99+
return origin.length <= URL_MAX_LENGTH ? origin : null;
97100
}
98101

99102
/**

0 commit comments

Comments
 (0)