Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/redis-wrapper-scan-cursor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'uptide': patch
---

The redis (node-redis) 4 to 5 and 5 to 6 pack is verified: 5 public upgrades, 20 real breaking sites, breaking precision 100% and recall 83%; its four rewrites no real site exercises (`keepAlive`, `quit`/`disconnect`, scan iterators, `unstableResp3`) run as assisted. It now reports numeric SCAN cursors in files that use the client through the repository's own module (`getRedisClient` from `../config/redis`, `redisClient` from `~/utils/redis.server`) without importing redis, including files the compile scope left out under the importer memory cap, and treats `scan(cursor)` after `let cursor = 0` as a numeric cursor. The resolver behind it, shared with the zod pack's form-error rule, moves to `check/importers.ts` as `packageImports`.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ The contract: [migration packs](docs/packs.md).
| `openai` | 4.x → 7.x | 100% | 15% | 20 | zeikar/charivo, openai/openai-guardrails-js, wardby/wardby, csnyder256/ux-struggle-detector, grantsingleton/batch-ai, digitalchokro/askchokro, microsoft/prompty | candidate |
| `react` | 18.x → 19.x | 100% | 69% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified |
| `react-router` | 6.x → 7.x | 100% | 88% | 18 | Arize-ai/phoenix, getlago/lago-front | verified |
| `redis` | >=4 <6 → >=5 <7 | n/a | n/a | — | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, deanmoses/tacocat-gallery-sam, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | candidate |
| `redis` | >=4 <6 → >=5 <7 | 100% | 83% | 20 | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, deanmoses/tacocat-gallery-sam, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | verified |
| `@sentry/browser` | 10.x → 11.x | 100% | 100% | 3 | Automancer-Ltd/automancer-site, sbomify/sbomify | candidate |
| `@sentry/nextjs` | 10.x → 11.x | 100% | 100% | 8 | cohstats/coh3-stats, xdoubleu/tools.xdoubleu.com, maximunited/yomu | verified |
| `@sentry/react` | 10.x → 11.x | 100% | 100% | 2 | owine/youth-activity-scheduler, successible/cleanslate | candidate |
Expand Down
105 changes: 103 additions & 2 deletions packages/core/src/check/importers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { readdirSync, readFileSync, statSync } from 'node:fs';
import { basename, dirname, join, resolve } from 'node:path';
import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs';
import { basename, dirname, join, relative, resolve } from 'node:path';
import { ts } from 'ts-morph';

const SOURCE = /\.(?:[cm]?[jt]sx?)$/;
Expand Down Expand Up @@ -299,3 +299,104 @@ export function importersOf(
return packages === undefined || takesDependent(file, hits);
});
}

/** The repository file a specifier names, the way the compiler resolves it; never a package. */
function localModule(
specifier: string,
from: string,
options: ts.CompilerOptions,
): string | undefined {
const target = ts.resolveModuleName(specifier, from, options, ts.sys).resolvedModule
?.resolvedFileName;
return target && !target.includes('/node_modules/') && !/\.d\.[cm]?ts$/.test(target)
? resolve(target)
: undefined;
}

/**
* What a repository module exports that depends on `packages` (`packageExports`), through its
* own re-exports too: `export * from './schema'` is how a `lib/zod/index.ts` hands on its schemas.
*/
function localExports(
file: string,
packages: ReadonlySet<string>,
options: ts.CompilerOptions,
seen: Map<string, Set<string>>,
): Set<string> {
const known = seen.get(file);
if (known) return known;
const exported = packageExports(file, packages);
seen.set(file, exported);
for (const statement of parseFile(file)?.statements ?? []) {
if (
!ts.isExportDeclaration(statement) ||
statement.isTypeOnly ||
!statement.moduleSpecifier ||
!ts.isStringLiteral(statement.moduleSpecifier)
)
continue;
const target = localModule(statement.moduleSpecifier.text, file, options);
if (!target) continue;
const inner = localExports(target, packages, options, seen);
const clause = statement.exportClause;
if (!clause) {
for (const name of inner) if (name !== 'default') exported.add(name);
} else if (ts.isNamespaceExport(clause)) {
if (inner.size) exported.add(clause.name.text);
} else
for (const e of clause.elements)
if (inner.has((e.propertyName ?? e.name).text)) exported.add(e.name.text);
}
return exported;
}

/**
* The names `file` imports from the repository's own modules that depend on one of `packages`
* there (a zod schema behind `@/lib/zod`, a redis client behind `../config/redis`), resolved the
* way the compiler resolves them. Type-only imports are left out: they hold no value.
*/
export function packageImports(
file: string,
packages: ReadonlySet<string>,
options: ts.CompilerOptions,
): Set<string> {
const names = new Set<string>();
const seen = new Map<string, Set<string>>();
for (const statement of parseFile(file)?.statements ?? []) {
if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier))
continue;
const clause = statement.importClause;
if (!clause || clause.isTypeOnly) continue;
const target = localModule(statement.moduleSpecifier.text, file, options);
if (!target) continue;
const exported = localExports(target, packages, options, seen);
if (exported.size === 0) continue;
if (clause.name && exported.has('default')) names.add(clause.name.text);
const bindings = clause.namedBindings;
if (bindings && ts.isNamespaceImport(bindings)) names.add(bindings.name.text);
else if (bindings)
for (const e of bindings.elements)
if (!e.isTypeOnly && (exported.has((e.propertyName ?? e.name).text) || exported.has('*')))
names.add(e.name.text);
}
return names;
}

/**
* A workspace's own TypeScript files (workspace-relative; no declarations, nothing under
* `node_modules` or outside it) and compiler options, from its `tsconfig.json`.
*/
export function projectFiles(dir: string): { files: string[]; options: ts.CompilerOptions } {
const config = join(dir, 'tsconfig.json');
const parsed = existsSync(config)
? ts.getParsedCommandLineOfConfigFile(
config,
{},
{ ...ts.sys, onUnRecoverableConfigFileDiagnostic: () => {} },
)
: undefined;
const files = (parsed?.fileNames ?? [])
.map((f) => relative(dir, resolve(f)))
.filter((f) => !f.startsWith('..') && !f.includes('node_modules/') && !/\.d\.[cm]?ts$/.test(f));
return { files, options: parsed?.options ?? {} };
}
2 changes: 1 addition & 1 deletion packages/core/src/packs/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ export function ruleFor<R extends MigrationRule & { message?: RegExp }>(
}

/** Rule and note sites found in one file's text, as `check` reports a pack's own finding. */
function detected(
export function detected(
pack: Pick<Pack, 'meta' | 'rules' | 'behavior'>,
input: { file: string; text: string; from: string; to: string },
): Finding[] {
Expand Down
61 changes: 47 additions & 14 deletions packages/core/src/packs/redis/RESEARCH.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,40 +98,72 @@ repository's own fix, and deprecations the code already had at the installed ver

## Measured pack test

`uptide pack test redis`, 2026-10-14:
Recorded **verified** on 2026-10-15 (UPT-66): `uptide pack test redis --write` under UPT-48's gate,
ground truth unchanged (the record's digest is that of the committed `ground-truth.json`):

```
rule precision recall tp fp fn
client-defaults 0% 0% 0 4 2
client-type 100% 33% 2 0 4
generic 50% 100% 2 2 0
renamed-exports 100% 100% 2 0 0
scan-cursor 100% 30% 3 0 7
breaking (sites) 100% 61% 11 0 7
scan-cursor 100% 70% 7 0 3
overall (sites) 89% 77% 17 2 5
behavior notes (sites) 50% 50% 2 2 2
breaking (sites) 100% 83% 15 0 3

gate 20 breaking sites, 2 behavior-note sites in public ground truth · verified
· no real site exercises keep-alive, quit-disconnect, scan-iterators, unstable-resp3: their rewrites run as assisted
```

Breaking misses, none of them a rule that did not match:
5 repositories, 20 real breaking sites, no breaking false positive. The four rewrites no real site
exercises (`keep-alive`, `quit-disconnect`, `scan-iterators`, `unstable-resp3`) are listed as
`unexercised` in `verification.json`, so `fix` hands those sites to the agent (assisted) instead of
applying the rewrite.

The three remaining breaking misses, all in tearingItUp786/blog:

- `app/inngest/refresh-cache.ts:204` and `:206`: TS2345 at `scanRedisKeys(redisClient, …)`, a
compile error only. `check` follows the file's import of `redisClient` from
`~/utils/redis.server` (`importersOf`), but the importer memory cap (`IMPORTER_GROWTH`, 0.2 of the
roots' own estimated heap) leaves it out of the program, and the run says so: "4 importing files
not analyzed (memory cap)". A growth of 0.5 would take it. The cap is engine-wide and stays as it
is here; raising it is a separate decision.
- `app/inngest/utils.ts:106`: `cursor !== 0` in a helper that takes a local structural
`RedisScanClient` and imports nothing from redis or from a module that depends on it. Following
imports cannot reach it; only a call-graph step (the caller passes the client in) could.

- blog (5 scan-cursor): `refresh-cache.ts` and `utils.ts` reach the client through a local module
and a local `RedisScanClient` type and never import `redis`; `check` reports no compile error
there, and runtime detection reads importing files only.
- FCM and tacocat 5 → 6 (4 client-type): the repositories' own `tsc` reports TS2322 at these lines,
`check` reports nothing there. Both lines already had a different TS2322 at the installed version
(`ReturnType<typeof createClient>` against a configured client) and `check` subtracts errors it
finds at the installed version, which is most likely why (not confirmed in the engine).
- FCM `cursor !== 0` (2): the files import a local wrapper, not `redis`.
Reported under another rule (they count as found for breaking, against `client-type` in the rule
rows): FCM `src/config/redis.ts:39`, `:46` (as `generic`) and tacocat 5 → 6
`redisClientUtils.ts:16`, `:21` (as `client-defaults`).

Behavior: the two `client-defaults` false positives are clients whose repositories accepted the
RESP3 default without pinning (mcp-coordinator, blog). The defaults did change for them; the ground
truth rule (only what the repository changed) counts them against the note. FCM pins RESP2 in an
options function the detector cannot read (it reads inline literals only).

### Clients reached through a local module (UPT-66)

The first score (2026-10-14) had breaking recall 61% (11/18): 7 `scan-cursor` misses, FCM's two
`cursor !== 0` and blog's five. The detector read only files that import redis, and those files
take the client from the repository's own module.

`scan-cursor` now also reads files that never import redis but import a value from a repository
module that depends on it (`getRedisClient` from `../config/redis`, `redisClient` from
`~/utils/redis.server`), resolved the way the compiler resolves it, `export *` chains included:
`packageImports` in `check/importers.ts`, the resolver the zod pack's `error-members` uses. Every
tsconfig file is a candidate, so a file the importer cap left out is read too. Such a file needs a
scan call; the other detectors still read importing files only (their sites are option names, not
client calls). `scan(cursor)` after `let cursor = 0` counts as a numeric cursor (fixture
`scan-cursor` line 9 is now marked). Found since: FCM `adminNukeController.ts:68`,
`roleVerificationService.ts:72`; blog `refresh-cache.ts:322`, `:326`. 61% → 83%, no new false
positive.

### Per-rule evidence (for the UPT-48 gate)

Exercised by a real site: `renamed-exports` (its rewrite too), `scan-cursor`, `client-type`.
Rewrites with no real site: `scan-iterators`, `keep-alive`, `quit-disconnect`, `unstable-resp3`;
under UPT-48 they run as assisted. Rules and notes with no real site: `legacy-mode`,
they run as assisted (`unexercised` in the record). Rules and notes with no real site: `legacy-mode`,
`command-options`, `number-replies`, `deep-imports`, `graph-removed`, `set-expiration`,
`sentinel-node-command-options`, `search-dialect-2` and the decision-only notes.

Expand All @@ -147,4 +179,5 @@ has `client-type` in four repositories, two of them scored.
gate in `transform` only refuses rewrites of a major the upgrade already passed.
- A site reported but deliberately not rewritten (a scan loop with `break`) cannot be a fixture
marker; it is a unit test.
- Compile errors in files that use the client without importing the package are not reported (blog).
- Compile errors in an importing file the importer memory cap leaves out are not reported (blog
`refresh-cache.ts:204`, `:206`); only the syntactic `scan-cursor` sites there are.
47 changes: 39 additions & 8 deletions packages/core/src/packs/redis/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,25 @@ function bindings(source: SourceFile): Bindings {
return { named, modules, uses };
}

function using(text: string, file: string): { source: SourceFile; b: Bindings } | undefined {
const MENTIONS = /['"](?:redis|@redis\/[\w-]+)(?:\/[^'"]*)?['"]/;

/** Whether a file names one of the packages at all: what `detect` reads. */
export const mentionsRedis = (text: string): boolean => MENTIONS.test(text);

/**
* A node-redis file, or, with `clients` (names it imports from a repository module that depends
* on redis: `getRedisClient` from `../config/redis`), a file that uses the client through it.
*/
function using(
text: string,
file: string,
clients?: ReadonlySet<string>,
): { source: SourceFile; b: Bindings } | undefined {
// Cheap gate before parsing: most files never mention the packages.
if (!/['"](?:redis|@redis\/[\w-]+)(?:\/[^'"]*)?['"]/.test(text)) return undefined;
if (!MENTIONS.test(text) && !clients?.size) return undefined;
const source = parse(text, file);
const b = bindings(source);
return b.uses ? { source, b } : undefined;
return b.uses || clients?.size ? { source, b } : undefined;
}

const lineOf = (source: SourceFile, node: Node) =>
Expand Down Expand Up @@ -267,12 +280,30 @@ const EQUALITY = new Set([
SyntaxKind.ExclamationEqualsToken,
]);

const SCANS = /\.(?:scan|hScan|sScan|zScan)\s*\(/;

/** A numeric literal, or a variable declared with one (`let cursor = 0`). */
function numeric(node: Node): boolean {
if (Node.isNumericLiteral(node)) return true;
if (!Node.isIdentifier(node)) return false;
return (node.getSymbol()?.getDeclarations() ?? []).some((d) => {
const value = Node.isVariableDeclaration(d) ? d.getInitializer() : undefined;
return value !== undefined && Node.isNumericLiteral(value);
});
}

/**
* A numeric SCAN cursor: `scan(0)`, `hScan(key, 0)`, or a cursor compared with a number. 5 takes
* and returns the cursor as a string, so `cursor !== 0` never ends the loop in JavaScript.
* A numeric SCAN cursor: `scan(0)`, `hScan(key, 0)`, `scan(cursor)` after `let cursor = 0`, or a
* cursor compared with a number. 5 takes and returns the cursor as a string, so `cursor !== 0`
* never ends the loop in JavaScript. With `clients`, a file that never imports redis is read too
* when it makes a scan call (see `using`).
*/
export function scanCursorSites(text: string, file: string): SourceSite[] {
const found = using(text, file);
export function scanCursorSites(
text: string,
file: string,
clients?: ReadonlySet<string>,
): SourceSite[] {
const found = using(text, file, SCANS.test(text) ? clients : undefined);
if (!found) return [];
const { source } = found;
const sites: SourceSite[] = [];
Expand All @@ -282,7 +313,7 @@ export function scanCursorSites(text: string, file: string): SourceSite[] {
const name = callee.getName();
const index = name === 'scan' ? 0 : /^(?:hScan|sScan|zScan)$/.test(name) ? 1 : -1;
const cursor = index >= 0 ? call.getArguments()[index] : undefined;
if (cursor && Node.isNumericLiteral(cursor)) sites.push(site(source, call));
if (cursor && numeric(cursor)) sites.push(site(source, call));
}
for (const binary of source.getDescendantsOfKind(SyntaxKind.BinaryExpression)) {
if (!EQUALITY.has(binary.getOperatorToken().getKind())) continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export async function keys(pattern: string) {
const found: string[] = [];
let cursor = 0;
do {
const reply = await client.scan(cursor, { MATCH: pattern, COUNT: 100 });
const reply = await client.scan(cursor, { MATCH: pattern, COUNT: 100 }); // @uptide scan-cursor
cursor = reply.cursor;
found.push(...reply.keys);
} while (cursor !== 0); // @uptide scan-cursor
Expand Down
43 changes: 41 additions & 2 deletions packages/core/src/packs/redis/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { join, resolve } from 'node:path';
import { packageImports, projectFiles } from '../../check/importers.js';
import type { Finding } from '../../domain/report.js';
import { satisfies } from '../../fetch/range.js';
import { definePack, type Pack, type PackRule, replaceAtSite } from '../contract.js';
import { definePack, detected, type Pack, type PackRule, replaceAtSite } from '../contract.js';
import type { TransformResult } from '../types.js';
import { crosses, NOTE_MAJOR, redisBehavior } from './behavior.js';
import {
commandOptionsSites,
graphSites,
keepAliveSites,
legacyModeSites,
mentionsRedis,
rewriteKeepAlive,
rewriteScanIterator,
rewriteUnstableResp3,
Expand Down Expand Up @@ -270,6 +274,39 @@ const base = definePack({
"Sources read 2026-10-14 at node-redis b64bacb; see RESEARCH.md. Migrate only the reported site. An upgrade from 4 crosses both majors: apply the 5 changes, then the 6 ones. Keep 5's client behavior unless the decision for that client says otherwise: where a client is reported under client-defaults, pin the 5 defaults on it (`RESP: 2`, `commandOptions: { timeout: undefined }`, `socket: { keepAliveInitialDelay: 5000 }`) rather than adapting callers to RESP3 replies. Never cast a client or a reply to any, never convert a SCAN cursor to a number, and never suppress a diagnostic to get a clean compile.",
});

const PACKAGES = new Set([
'redis',
'@redis/client',
'@redis/bloom',
'@redis/json',
'@redis/search',
'@redis/time-series',
'@redis/graph',
]);
const SCAN_CURSOR = V5.find((r) => r.id === 'scan-cursor') as PackRule;

/**
* SCAN cursors in files that use the client through the repository's own module and never import
* redis (`getRedisClient` from `../config/redis`, `redisClient` from `~/utils/redis.server`):
* `detect` reads only files that import it, and the compile scope can leave such a file out (the
* importer cap). Every project file is a candidate; the text tests are the cheap part.
*/
function wrapperFindings(input: Parameters<NonNullable<Pack['runtimeFindings']>>[0]): Finding[] {
const dir = resolve(input.root, input.workspace);
const { files, options } = projectFiles(dir);
return [...new Set([...input.usages.map((u) => u.file), ...files])].sort().flatMap((file) => {
const text = input.read(join(input.workspace, file));
if (text === undefined || mentionsRedis(text) || !/\bcursor\b/i.test(text)) return [];
const clients = packageImports(join(dir, file), PACKAGES, options);
if (clients.size === 0) return [];
const rule = {
...SCAN_CURSOR,
detect: (t: string, f: string) => scanCursorSites(t, f, clients),
};
return detected({ meta: base.meta, rules: [rule], behavior: [] }, { ...input, file, text });
});
}

/**
* redis >=4 <6 → >=5 <7. One package, two majors: 4 → 5 and 5 → 6 are separate rule sets, and
* an upgrade gets the sets of the majors it crosses (4 → 6 gets both). Pack lookup stays one pack
Expand Down Expand Up @@ -299,5 +336,7 @@ export const redisPack: Pack = {
return base.transform(text, finding, context);
},
runtimeFindings: (input) =>
(base.runtimeFindings?.(input) ?? []).filter((f) => appliesTo(f.rule, input.from, input.to)),
[...(base.runtimeFindings?.(input) ?? []), ...wrapperFindings(input)].filter((f) =>
appliesTo(f.rule, input.from, input.to),
),
};
Loading
Loading