mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-15 20:53:23 +02:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The web UI embeds CodeMirror editors, and CodeMirror validates
extensions with `instanceof`
> - Different CodeMirror packages pin different transitive minors of
`@codemirror/state` (6.7.1 / 6.7.2) and `@codemirror/view` (6.43.9 /
6.43.11), so the bundle ships two module instances
> - The second instance makes valid extensions fail the `instanceof`
check and crashes the editor
> - This pull request adds a shared caret override to `pnpm.overrides`,
the same mechanism the existing `react` and `rollup` overrides use, so
every consumer resolves one copy of each package
> - The lockfile is not edited by hand; the lockfile automation
regenerates it from the manifests
> - The benefit is that the editor stops crashing with "Unrecognized
extension value in extension set"
## Linked Issues or Issue Description
**What happened?**
The production UI throws `Error: Unrecognized extension value in
extension set ([object Object]). This sometimes happens because multiple
instances of @codemirror/state are loaded, breaking instanceof checks.`
Observed 43 times in one week.
**Expected behavior**
The editor loads its extension set without errors. One instance of
`@codemirror/state` and `@codemirror/view` serves every CodeMirror
package.
**Steps to reproduce**
1. Run `grep "'@codemirror/state@" pnpm-lock.yaml` on master: two
versions resolve (6.7.1 and 6.7.2).
2. Build `ui/` and search the output for `Unrecognized extension value`,
a string unique to `@codemirror/state`: two chunks each carry a full
copy, one with a 6.7.1-only code pattern and one without it.
3. Open a view that composes extensions from packages on different
copies: the extension set rejects the foreign-instance extension.
**Paperclip version or commit**
master (0e14c61da)
## What Changed
- `package.json` (`pnpm.overrides`): added `"@codemirror/state":
"^6.7.2"` and `"@codemirror/view": "^6.43.11"`. A shared range forces
every consumer onto one resolution of each package.
- `pnpm-workspace.yaml`: the mirror overrides block gets the same two
entries, kept in sync with `package.json`.
- `ui/src/lib/codemirror-single-instance.test.ts`: regression pin that
fails when the lockfile resolves more than one version of either
package.
- No lockfile change in this PR. The `policy` job regenerates
`pnpm-lock.yaml` from the manifests for downstream jobs; CI owns
lockfile updates.
## Verification
- With the override, `pnpm install` resolves a single
`@codemirror/state@6.7.2` and a single `@codemirror/view@6.43.11`.
- Built `ui/` before and after. Before: two chunks each carried a full
copy of `@codemirror/state` (four total occurrences of its unique error
string; one chunk fingerprints as 6.7.1, the other as 6.7.2). After: one
chunk carries one copy (two occurrences, no 6.7.1 fingerprint).
- `pnpm vitest run src/components/IssuesList.test.tsx` in `ui/` — 46/46
pass.
- New test `ui/src/lib/codemirror-single-instance.test.ts` pins the
invariant: the lockfile must resolve exactly one version of
`@codemirror/state` and `@codemirror/view`. The PR CI policy job
regenerates the lockfile from the manifests, so the test evaluates this
PR's real resolution — verified locally against a lockfile regenerated
the same way (`pnpm install --resolution-only`): 2/2 pass, and the same
test fails against the current master lockfile with its two resolved
copies.
## Risks
- Low risk. The override stays inside the caret ranges every consumer
already declares, so no package receives a version outside its stated
compatibility. Rollback is removing the two override lines.
- A future CodeMirror consumer that needs a major bump of these packages
must update the override; the override comment states why it exists.
## Model Used
Claude (Anthropic) — claude-fable-5 (Claude Fable 5), Claude Code
harness, extended thinking with tool use.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
130 lines
5.1 KiB
TypeScript
130 lines
5.1 KiB
TypeScript
import { existsSync, readdirSync, readFileSync, realpathSync } from "node:fs";
|
|
import path from "node:path";
|
|
import { describe, expect, it } from "vitest";
|
|
|
|
// CodeMirror validates extensions with instanceof, so the installed graph
|
|
// must contain exactly one physical copy of these packages. Two resolved
|
|
// versions ship two module instances and crash the editor at runtime with
|
|
// "Unrecognized extension value in extension set" — a failure no unit test
|
|
// of the editor itself catches, because each test file sees only one copy.
|
|
// The pnpm.overrides entries in the root package.json hold the graph to a
|
|
// single resolution; this file pins that invariant against the graph the
|
|
// current install actually resolved, so it holds wherever the tests run —
|
|
// CI (which installs from the lockfile it regenerates for the PR) and
|
|
// local checkouts alike.
|
|
const SINGLE_INSTANCE_PACKAGES = ["@codemirror/state", "@codemirror/view"];
|
|
|
|
const repoRoot = path.resolve(__dirname, "../../..");
|
|
const uiRoot = path.resolve(__dirname, "../..");
|
|
const workspaceManifest = readFileSync(
|
|
path.join(repoRoot, "pnpm-workspace.yaml"),
|
|
"utf8",
|
|
);
|
|
const rootManifest = JSON.parse(
|
|
readFileSync(path.join(repoRoot, "package.json"), "utf8"),
|
|
) as { pnpm?: { overrides?: Record<string, string> } };
|
|
|
|
/**
|
|
* Every physical copy of `target` reachable from the ui package's
|
|
* installed graph. Walks the graph the way pnpm lays it out — each
|
|
* package's dependencies are linked either in its own node_modules
|
|
* (workspace packages) or beside it in its .pnpm bucket — and realpaths
|
|
* every link, so each resolved version collapses to one path. Walking
|
|
* from the ui roots keeps orphaned .pnpm buckets from older installs out
|
|
* of the census.
|
|
*/
|
|
function reachableCopies(target: string): string[] {
|
|
const copies = new Set<string>();
|
|
const visited = new Set<string>();
|
|
const queue: string[] = [uiRoot];
|
|
|
|
const packageEntries = (nodeModulesDir: string): string[] => {
|
|
const entries: string[] = [];
|
|
for (const name of readdirSync(nodeModulesDir)) {
|
|
if (name.startsWith(".")) continue;
|
|
if (name.startsWith("@")) {
|
|
const scopeDir = path.join(nodeModulesDir, name);
|
|
for (const child of readdirSync(scopeDir)) {
|
|
if (!child.startsWith(".")) entries.push(`${name}/${child}`);
|
|
}
|
|
} else {
|
|
entries.push(name);
|
|
}
|
|
}
|
|
return entries;
|
|
};
|
|
|
|
while (queue.length > 0) {
|
|
const packageDir = queue.shift()!;
|
|
if (visited.has(packageDir)) continue;
|
|
visited.add(packageDir);
|
|
if (visited.size > 20_000) {
|
|
throw new Error("dependency walk exceeded its safety bound");
|
|
}
|
|
|
|
// A package's dependencies live in its own node_modules (workspace
|
|
// packages) and, for store-installed packages, beside it in the
|
|
// .pnpm bucket's shared node_modules.
|
|
const dependencyDirs = [path.join(packageDir, "node_modules")];
|
|
const parent = path.dirname(packageDir);
|
|
const grandparent = path.dirname(parent);
|
|
if (path.basename(parent) === "node_modules") {
|
|
dependencyDirs.push(parent);
|
|
} else if (path.basename(grandparent) === "node_modules") {
|
|
dependencyDirs.push(grandparent); // scoped package
|
|
}
|
|
|
|
for (const dependencyDir of dependencyDirs) {
|
|
if (!existsSync(dependencyDir)) continue;
|
|
for (const entry of packageEntries(dependencyDir)) {
|
|
let entryDir: string;
|
|
try {
|
|
entryDir = realpathSync(path.join(dependencyDir, entry));
|
|
} catch {
|
|
continue; // dangling symlink
|
|
}
|
|
if (entry === target) copies.add(entryDir);
|
|
queue.push(entryDir);
|
|
}
|
|
}
|
|
}
|
|
return [...copies];
|
|
}
|
|
|
|
describe("codemirror single-instance invariant", () => {
|
|
for (const pkg of SINGLE_INSTANCE_PACKAGES) {
|
|
it(`keeps the ${pkg} override in the root manifest and its workspace mirror`, () => {
|
|
// Removing the override is the only way a second copy can come
|
|
// back (an override rewrites every dependent's range), so the
|
|
// override's presence is the other half of the invariant.
|
|
expect(
|
|
rootManifest.pnpm?.overrides?.[pkg],
|
|
`${pkg} must stay in pnpm.overrides (root package.json); without ` +
|
|
"it the graph can resolve two copies and instanceof checks " +
|
|
"inside the editor break.",
|
|
).toMatch(/^\^6\./);
|
|
expect(
|
|
workspaceManifest,
|
|
`pnpm-workspace.yaml mirrors the pnpm.overrides block and must ` +
|
|
`carry the same ${pkg} entry.`,
|
|
).toMatch(new RegExp(`^\\s+"${pkg}":`, "m"));
|
|
});
|
|
|
|
it(`installs exactly one physical copy of ${pkg}`, () => {
|
|
const copies = reachableCopies(pkg);
|
|
expect(
|
|
copies.length,
|
|
`${pkg} is not installed anywhere in the ui graph`,
|
|
).toBeGreaterThan(0);
|
|
expect(
|
|
copies,
|
|
`the installed graph carries multiple physical copies of ${pkg}, ` +
|
|
"which break instanceof checks inside the editor. Reinstall " +
|
|
"against the current manifests; if the copies persist, fix the " +
|
|
"pnpm.overrides entry in the root package.json instead of " +
|
|
"allowing a second copy.",
|
|
).toHaveLength(1);
|
|
});
|
|
}
|
|
});
|