diff --git a/.dex/tasks.jsonl b/.dex/tasks.jsonl index 8121b6e9..b21326f8 100644 --- a/.dex/tasks.jsonl +++ b/.dex/tasks.jsonl @@ -8,3 +8,4 @@ {"id":"k746m3ok","parent_id":"bpb4f298","name":"Bun compile and CI","description":"Set up Bun binary compilation and GitHub Actions CI.\n\nBuild:\n- bun build src/cli/index.ts --compile --outfile dist/dotagents\n- Per-platform binaries: darwin-arm64, darwin-x64, linux-x64, linux-arm64\n\nGitHub Actions:\n- CI workflow: lint, typecheck, test on push/PR\n- Release workflow: build per-platform binaries, attach to GitHub Release\n- Install script (optional): curl-pipe-sh that detects platform and downloads binary\n\nnpm:\n- Package published with bin field pointing to dist/cli/index.js (JS fallback)\n- npx dotagents works for Node.js users\n\nAcceptance criteria:\n- bun build produces a working standalone binary\n- Binary works without Bun/Node.js installed\n- CI runs lint + typecheck + test on every push","priority":1,"completed":false,"result":null,"metadata":null,"created_at":"2026-02-07T18:00:02.720Z","updated_at":"2026-02-07T18:00:14.303Z","started_at":null,"completed_at":null,"blockedBy":["srnjavhs"],"blocks":[],"children":[]} {"id":"qze9byy6","parent_id":"bpb4f298","name":"Skill resolution and git source","description":"Implement skill specifier parsing and git-based skill fetching.\n\nFiles to create:\n- src/skills/resolver.ts -- Parse source specifiers into ResolvedSource objects\n- src/sources/git.ts -- Clone/fetch git repos via git CLI\n- src/sources/local.ts -- Resolve path: sources relative to project root\n- src/sources/cache.ts -- Global cache at ~/.local/dotagents/cache/\n- src/utils/exec.ts -- Child process execution helper for non-interactive git commands\n- src/skills/loader.ts -- Parse SKILL.md files (YAML frontmatter + markdown body)\n\nResolution rules:\n- github:owner/repo/path -> https://github.com/owner/repo.git, subdirectory path/\n- git:url + path field -> full URL clone, use path for subdirectory\n- path:relative -> resolve relative to project root\n- @scope/name -> try github:scope/skills/name, fallback to github:scope/name\n\nCache structure: ~/.local/dotagents/cache/github.com/owner/repo/ (bare git repos)\n\nAcceptance criteria:\n- All 4 source types resolve correctly\n- Git clone works with --depth=1 for fresh clones\n- Cache is checked before network fetch\n- SKILL.md is validated (name + description required in frontmatter)","priority":1,"completed":false,"result":null,"metadata":null,"created_at":"2026-02-07T17:59:17.801Z","updated_at":"2026-02-07T18:00:08.470Z","started_at":null,"completed_at":null,"blockedBy":["iashsswc"],"blocks":["g6gl9kkc"],"children":[]} {"id":"srnjavhs","parent_id":"bpb4f298","name":"Update, sync, and list commands","description":"Implement the remaining CLI commands: update, sync, list.\n\nFiles: src/cli/commands/update.ts, src/cli/commands/sync.ts, src/cli/commands/list.ts\n\ndotagents update []:\n- Branch refs: fetch latest commit\n- Tag refs: check for newer semver tags\n- Commit SHA refs: skip (immutable)\n- Update lockfile, re-install, print changelog\n\ndotagents sync:\n- Regenerate .agents/.gitignore\n- Verify/repair symlinks\n- Warn on managed skills accidentally in git\n- Warn on orphaned skills (installed but not in agents.toml)\n- Verify integrity hashes\n\ndotagents list [--json]:\n- Show each skill: name, source, installed commit (short SHA), status\n- Status: up to date, outdated, modified (integrity mismatch), custom (unmanaged)\n- --json for machine consumption\n\nAcceptance criteria:\n- update fetches new commits for branch-pinned skills\n- sync detects and reports drift\n- list shows accurate status for all skills","priority":1,"completed":false,"result":null,"metadata":null,"created_at":"2026-02-07T17:59:57.259Z","updated_at":"2026-02-07T18:00:13.465Z","started_at":null,"completed_at":null,"blockedBy":["ib51q4f3"],"blocks":["k746m3ok"],"children":[]} +{"id":"z7bt1ifx","parent_id":null,"name":"Install anti-slop Oxlint rules","description":"Vendor Dillon Mulroy's dmmulroy/anti-slop generic Oxlint plugin into tools/oxlint/anti-slop, upgrade and align oxlint plus @oxlint/plugins to the current matching versions, register every generic rule at error severity, and merge repository-local agent/tooling ignores into the existing Oxlint configuration. Bring all owned TypeScript/JavaScript source into compliance without weakening rules, suppressing findings, or changing intended behavior. Effect rules are out of scope because no package manifest directly depends on Effect. Done when the normal pnpm lint/typecheck/test checks pass, the docs workspace remains valid as applicable, the vendored plugin is excluded from self-linting, and no anti-slop findings remain.","priority":1,"completed":true,"result":"Vendored dmmulroy/anti-slop generic Oxlint plugin sources at upstream commit 6d538555cb151d4121ed51a27db81890eacf8ae9 with MIT provenance, pinned oxlint and @oxlint/plugins 1.79.0, registered all 15 generic rules at error severity, expanded root lint to all owned files, and resolved all 274 initial findings without rule suppressions or unsafe casts. Replaced module mocks with typed dependency seams and boundary parsing, including deterministic CLI scope resolution after clean-container QA exposed a checkout-dependent test. Exported the new dotagents-lib public signature types. Verification: local frozen install and pnpm check pass; 0 Oxlint findings across 154 files, 6 AST policy tests pass, TypeScript declarations pass, and 1,138 Vitest tests pass. Docker dotagents-qa:local sha256:19e019f ran as non-root Node v24.19.0/pnpm 10.28.2 with a read-only host checkout and isolated state; frozen install, pnpm check, pnpm qa:example install/sync repair, and pnpm qa:plugins Claude validation/marketplace install plus Codex discovery all passed. Evidence retained at /var/folders/ml/rhcrtjss3glfydv5v9rryhlm0000gn/T/dotagents-qa.tpSADp.","metadata":null,"created_at":"2026-08-22T17:14:58.639Z","updated_at":"2026-08-22T17:47:17.511Z","started_at":"2026-08-22T17:47:17.509Z","completed_at":"2026-08-22T17:47:17.511Z","blockedBy":[],"blocks":[],"children":[]} diff --git a/.oxlintrc.json b/.oxlintrc.json index 7d550de7..cb5d69d1 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1,5 +1,30 @@ { "$schema": "https://raw.githubusercontent.com/oxc-project/oxc/main/npm/oxlint/configuration_schema.json", + "ignorePatterns": [ + ".agent/**", + ".agents/**", + ".claude/**", + ".claude-plugin/**", + ".codex/**", + ".continue/**", + ".cursor/**", + ".cursor-plugin/**", + ".dex/**", + ".gemini/**", + ".grok/**", + ".opencode/**", + ".pi/**", + ".roo/**", + ".windsurf/**", + "docs/src/env.d.ts", + "tools/oxlint/anti-slop/**" + ], + "jsPlugins": [ + { + "name": "anti-slop", + "specifier": "./tools/oxlint/anti-slop/index.ts" + } + ], "categories": { "suspicious": "error", "perf": "error" @@ -21,6 +46,27 @@ "no-duplicate-imports": "error", "prefer-template": "error", - "curly": "error" + "curly": "error", + + "anti-slop/no-chained-type-assertions": "error", + "anti-slop/no-conditional-empty-object-spread": "error", + "anti-slop/no-known-value-widening": "error", + "anti-slop/no-module-mocking": "error", + "anti-slop/no-object-parameters": "error", + "anti-slop/no-reflect-apply": "error", + "anti-slop/no-reflect-get": "error", + "anti-slop/no-runtime-typeof": [ + "error", + { + "allowInTypeGuards": true + } + ], + "anti-slop/no-shape-in-symbol-names": "error", + "anti-slop/no-unknown-parameters": "error", + "anti-slop/no-unknown-returns": "error", + "anti-slop/no-unknown-type-aliases": "error", + "anti-slop/no-unsafe-dictionary-type": "error", + "anti-slop/no-widen-then-assert": "error", + "anti-slop/require-safety-comment-for-type-assertion": "error" } } diff --git a/package.json b/package.json index 6df70921..60eb0f8f 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "scripts": { "build": "pnpm -r build", "test": "pnpm -r test", - "lint": "pnpm -r lint && pnpm lint:ast-grep", + "lint": "node --import tsx ./node_modules/oxlint/bin/oxlint . --deny-warnings && pnpm lint:ast-grep", "lint:ast-grep": "ast-grep scan && ast-grep test --skip-snapshot-tests", "typecheck": "pnpm -r typecheck", "check": "pnpm lint && pnpm typecheck && pnpm test", @@ -21,7 +21,7 @@ }, "lint-staged": { "packages/*/src/**/*.ts": [ - "oxlint --deny-warnings" + "node --import tsx ./node_modules/oxlint/bin/oxlint --deny-warnings" ] }, "packageManager": "pnpm@10.28.2", @@ -31,9 +31,10 @@ }, "devDependencies": { "@ast-grep/cli": "^0.44.1", + "@oxlint/plugins": "1.79.0", "@types/node": "^25.2.1", "lint-staged": "^16.2.7", - "oxlint": "^1.43.0", + "oxlint": "1.79.0", "simple-git-hooks": "^2.13.1", "tsx": "^4.21.0", "typescript": "^5.9.3", diff --git a/packages/dotagents-lib/package.json b/packages/dotagents-lib/package.json index e91cf518..c654b84d 100644 --- a/packages/dotagents-lib/package.json +++ b/packages/dotagents-lib/package.json @@ -15,7 +15,7 @@ "build": "tsc -b", "test": "vitest run", "test:watch": "vitest", - "lint": "oxlint src --deny-warnings", + "lint": "node --import tsx ../../node_modules/oxlint/bin/oxlint src --deny-warnings", "typecheck": "tsc -b --emitDeclarationOnly" }, "files": [ diff --git a/packages/dotagents-lib/src/index.ts b/packages/dotagents-lib/src/index.ts index 84447631..8a7a1c18 100644 --- a/packages/dotagents-lib/src/index.ts +++ b/packages/dotagents-lib/src/index.ts @@ -38,6 +38,7 @@ export { } from "./skills/resolver.js"; export type { ParseSourceErrorKind } from "./skills/resolver.js"; export type { + ParsedSource, ResolvedSkill, ResolvedGitSkill, ResolvedLocalSkill, @@ -45,6 +46,7 @@ export type { NamedResolvedSkill, WildcardDependencyInput, ResolveOpts, + ResolverServices, } from "./skills/resolver.js"; // Sources / cache @@ -56,7 +58,7 @@ export { isGitRepo, GitError, } from "./sources/git.js"; -export type { GitErrorDetails } from "./sources/git.js"; +export type { GitErrorDetails, GitExecutor } from "./sources/git.js"; export { ensureCached, CacheError, diff --git a/packages/dotagents-lib/src/skills/loader.ts b/packages/dotagents-lib/src/skills/loader.ts index 7cc59c55..99f366e5 100644 --- a/packages/dotagents-lib/src/skills/loader.ts +++ b/packages/dotagents-lib/src/skills/loader.ts @@ -47,10 +47,10 @@ export async function loadSkillMd( fileDescription: "SKILL.md", }); - if (typeof meta["name"] !== "string" || !meta["name"]) { + if (!isNonEmptyString(meta["name"])) { throw new SkillLoadError(`Missing 'name' in SKILL.md frontmatter: ${filePath}`); } - if (typeof meta["description"] !== "string" || !meta["description"]) { + if (!isNonEmptyString(meta["description"])) { throw new SkillLoadError(`Missing 'description' in SKILL.md frontmatter: ${filePath}`); } @@ -59,6 +59,8 @@ export async function loadSkillMd( meta["allowedTools"] = allowedTools; } + // SAFETY: name and description were validated above; the remaining fields + // retain the SerializedObject contract established by the frontmatter parser. return meta as SkillMeta; } @@ -152,7 +154,7 @@ function extractFrontmatter(content: string): { yaml: string; end: number } | nu * a way the lib can't interpret (e.g. an object). */ function parseAllowedTools( - raw: unknown, + raw: SerializedObject[string], onWarning?: (message: string) => void, ): ToolName[] | undefined { if (raw === undefined || raw === null) {return undefined;} @@ -162,20 +164,20 @@ function parseAllowedTools( // likely a typo and we treat it as absent. let tokens: string[]; let isArrayForm = false; - if (typeof raw === "string") { + if (isString(raw)) { tokens = raw.split(/\s+/).filter(Boolean); } else if (Array.isArray(raw)) { isArrayForm = true; tokens = []; for (const entry of raw) { - if (typeof entry === "string" && entry.trim().length > 0) { + if (isNonEmptyString(entry)) { tokens.push(entry.trim()); } else { onWarning?.(`allowed-tools: skipping non-string entry ${JSON.stringify(entry)}`); } } } else { - onWarning?.(`allowed-tools must be a string or array, got ${typeof raw}; ignoring`); + onWarning?.(`allowed-tools must be a string or array; ignoring ${JSON.stringify(raw)}`); return undefined; } @@ -196,3 +198,11 @@ function parseAllowedTools( // Declared-but-no-recognized-tokens stays distinct from field-absent. return accepted; } + +function isString(value: Value): value is Value & string { + return typeof value === "string"; +} + +function isNonEmptyString(value: Value): value is Value & string { + return typeof value === "string" && value.length > 0; +} diff --git a/packages/dotagents-lib/src/skills/resolver.test.ts b/packages/dotagents-lib/src/skills/resolver.test.ts index 7b3feaee..72fc64d7 100644 --- a/packages/dotagents-lib/src/skills/resolver.test.ts +++ b/packages/dotagents-lib/src/skills/resolver.test.ts @@ -9,6 +9,16 @@ import { ParseSourceError, } from "./resolver.js"; +function captureParseSourceError(source: string): ParseSourceError { + try { + parseSource(source); + } catch (err) { + if (err instanceof ParseSourceError) {return err;} + throw err; + } + throw new Error("expected parseSource to throw"); +} + describe("parseOwnerRepoShorthand", () => { it("parses owner/repo", () => { expect(parseOwnerRepoShorthand("getsentry/skills")).toEqual({ @@ -472,32 +482,18 @@ describe("isSourceExcluded", () => { describe("parseSource strict shorthand", () => { it("throws ParseSourceError on empty SHA after @", () => { expect(() => parseSource("owner/repo@")).toThrow(ParseSourceError); - try { - parseSource("owner/repo@"); - } catch (err) { - expect(err).toBeInstanceOf(ParseSourceError); - expect((err as ParseSourceError).kind).toBe("empty-sha"); - } + const err = captureParseSourceError("owner/repo@"); + expect(err.kind).toBe("empty-sha"); }); it("throws ParseSourceError on empty SHA in scoped shorthand", () => { - try { - parseSource("@owner/repo@"); - throw new Error("expected to throw"); - } catch (err) { - expect(err).toBeInstanceOf(ParseSourceError); - expect((err as ParseSourceError).kind).toBe("empty-sha"); - } + expect(captureParseSourceError("@owner/repo@").kind).toBe("empty-sha"); }); it("throws ParseSourceError on multi-segment shorthand", () => { - try { - parseSource("owner/repo/nested"); - throw new Error("expected to throw"); - } catch (err) { - expect(err).toBeInstanceOf(ParseSourceError); - expect((err as ParseSourceError).kind).toBe("multi-segment-shorthand"); - } + expect(captureParseSourceError("owner/repo/nested").kind).toBe( + "multi-segment-shorthand", + ); }); it("still parses valid github shorthand without throwing", () => { diff --git a/packages/dotagents-lib/src/skills/resolver.trust.test.ts b/packages/dotagents-lib/src/skills/resolver.trust.test.ts index 96964563..01e1d2ec 100644 --- a/packages/dotagents-lib/src/skills/resolver.trust.test.ts +++ b/packages/dotagents-lib/src/skills/resolver.trust.test.ts @@ -1,29 +1,19 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { resolveSkill, resolveWildcardSkills } from "./resolver.js"; +import { resolveSkill, resolveWildcardSkills, type ResolverServices } from "./resolver.js"; import { TrustError } from "../trust/validator.js"; import type { TrustPolicy } from "../trust/policy.js"; -vi.mock("../sources/cache.js", () => ({ +const services = { ensureCached: vi.fn(async () => { throw new Error("ensureCached should not be called when trust rejects"); }), -})); - -vi.mock("../sources/wellknown.js", () => ({ ensureWellKnownCached: vi.fn(async () => { throw new Error("ensureWellKnownCached should not be called when trust rejects"); }), -})); - -vi.mock("../sources/local.js", () => ({ resolveLocalSource: vi.fn(async () => { throw new Error("resolveLocalSource should not be called when trust rejects"); }), - LocalSourceError: class extends Error {}, -})); - -import { ensureCached } from "../sources/cache.js"; -import { ensureWellKnownCached } from "../sources/wellknown.js"; +} satisfies ResolverServices; const allowOnlyAnthropics: TrustPolicy = { allow_all: false, @@ -44,12 +34,12 @@ describe("resolveSkill with trust opt", () => { resolveSkill( "foo", { source: "evil-org/evil-skills" }, - { stateDir: STATE_DIR, trust: allowOnlyAnthropics }, + { stateDir: STATE_DIR, trust: allowOnlyAnthropics, services }, ), ).rejects.toBeInstanceOf(TrustError); - expect(ensureCached).not.toHaveBeenCalled(); - expect(ensureWellKnownCached).not.toHaveBeenCalled(); + expect(services.ensureCached).not.toHaveBeenCalled(); + expect(services.ensureWellKnownCached).not.toHaveBeenCalled(); }); it("blocks a disallowed well-known source BEFORE any network access", async () => { @@ -57,12 +47,12 @@ describe("resolveSkill with trust opt", () => { resolveSkill( "foo", { source: "https://untrusted.example.com/skills" }, - { stateDir: STATE_DIR, trust: allowOnlyAnthropics }, + { stateDir: STATE_DIR, trust: allowOnlyAnthropics, services }, ), ).rejects.toBeInstanceOf(TrustError); - expect(ensureCached).not.toHaveBeenCalled(); - expect(ensureWellKnownCached).not.toHaveBeenCalled(); + expect(services.ensureCached).not.toHaveBeenCalled(); + expect(services.ensureWellKnownCached).not.toHaveBeenCalled(); }); it("validates the expanded source under a non-default host (regression: shorthand+gitlab bypass)", async () => { @@ -73,21 +63,21 @@ describe("resolveSkill with trust opt", () => { resolveSkill( "foo", { source: "anthropics/skills" }, - { stateDir: STATE_DIR, trust: allowOnlyAnthropics, defaultRepositorySource: "gitlab" }, + { stateDir: STATE_DIR, trust: allowOnlyAnthropics, defaultRepositorySource: "gitlab", services }, ), ).rejects.toBeInstanceOf(TrustError); - expect(ensureCached).not.toHaveBeenCalled(); + expect(services.ensureCached).not.toHaveBeenCalled(); }); it("does NOT enforce trust when the opt is omitted (today's behavior)", async () => { // No trust opt → resolver should proceed to network. Mock throws a sentinel // so we can verify network was attempted (rather than trust short-circuiting). await expect( - resolveSkill("foo", { source: "evil-org/evil-skills" }, { stateDir: STATE_DIR }), + resolveSkill("foo", { source: "evil-org/evil-skills" }, { stateDir: STATE_DIR, services }), ).rejects.toThrowError(/ensureCached should not be called/); - expect(ensureCached).toHaveBeenCalledTimes(1); + expect(services.ensureCached).toHaveBeenCalledTimes(1); }); }); @@ -100,19 +90,19 @@ describe("resolveWildcardSkills with trust opt", () => { await expect( resolveWildcardSkills( { source: "evil-org/evil-skills" }, - { stateDir: STATE_DIR, trust: allowOnlyAnthropics }, + { stateDir: STATE_DIR, trust: allowOnlyAnthropics, services }, ), ).rejects.toBeInstanceOf(TrustError); - expect(ensureCached).not.toHaveBeenCalled(); - expect(ensureWellKnownCached).not.toHaveBeenCalled(); + expect(services.ensureCached).not.toHaveBeenCalled(); + expect(services.ensureWellKnownCached).not.toHaveBeenCalled(); }); it("does NOT enforce trust when the opt is omitted", async () => { await expect( - resolveWildcardSkills({ source: "evil-org/evil-skills" }, { stateDir: STATE_DIR }), + resolveWildcardSkills({ source: "evil-org/evil-skills" }, { stateDir: STATE_DIR, services }), ).rejects.toThrowError(/ensureCached should not be called/); - expect(ensureCached).toHaveBeenCalledTimes(1); + expect(services.ensureCached).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/dotagents-lib/src/skills/resolver.ts b/packages/dotagents-lib/src/skills/resolver.ts index dcda1bed..53bc65a7 100644 --- a/packages/dotagents-lib/src/skills/resolver.ts +++ b/packages/dotagents-lib/src/skills/resolver.ts @@ -89,6 +89,17 @@ export interface ResolvedWellKnownSkill { export type ResolvedSkill = ResolvedGitSkill | ResolvedLocalSkill | ResolvedWellKnownSkill; +export interface ParsedSource { + type: "github" | "git" | "local" | "well-known"; + url?: string; + /** Original URL to use for cloning (preserves SSH/HTTPS protocol). Undefined for owner/repo shorthand. */ + cloneUrl?: string; + owner?: string; + repo?: string; + ref?: string; + path?: string; +} + export function isExplicitSourceSpecifier(specifier: string): boolean { return ( specifier.startsWith("path:") || @@ -168,16 +179,7 @@ export function applyDefaultRepositorySource( /** * Parse a source string into its components. */ -export function parseSource(source: string): { - type: "github" | "git" | "local" | "well-known"; - url?: string; - /** Original URL to use for cloning (preserves SSH/HTTPS protocol). Undefined for owner/repo shorthand. */ - cloneUrl?: string; - owner?: string; - repo?: string; - ref?: string; - path?: string; -} { +export function parseSource(source: string): ParsedSource { if (source.startsWith("path:")) { return { type: "local", path: source.slice(5) }; } @@ -329,8 +331,22 @@ export interface ResolveOpts { trust?: TrustPolicy; /** Exact git checkout acquired earlier in the current operation. */ reuse?: CacheReuse; + /** Dependency overrides for tests and embedded hosts. */ + services?: Partial; } +export interface ResolverServices { + ensureCached: typeof ensureCached; + ensureWellKnownCached: typeof ensureWellKnownCached; + resolveLocalSource: typeof resolveLocalSource; +} + +const DEFAULT_RESOLVER_SERVICES: ResolverServices = { + ensureCached, + ensureWellKnownCached, + resolveLocalSource, +}; + type AcquiredSkillSource = | { type: "local"; skillDir: string } | { type: "well-known"; cacheDir: string | null; resolvedUrl: string } @@ -346,6 +362,7 @@ async function acquireSkillSource( dep: { source: string; ref?: string }, opts: ResolveOpts, ): Promise { + const services = { ...DEFAULT_RESOLVER_SERVICES, ...opts.services }; const sourceForResolve = applyDefaultRepositorySource( dep.source, opts.defaultRepositorySource, @@ -356,13 +373,13 @@ async function acquireSkillSource( const parsed = parseSource(sourceForResolve); if (parsed.type === "local") { const projectRoot = opts.projectRoot || process.cwd(); - const skillDir = await resolveLocalSource(projectRoot, parsed.path!); + const skillDir = await services.resolveLocalSource(projectRoot, parsed.path!); return { type: "local", skillDir }; } if (parsed.type === "well-known") { const resolvedUrl = parsed.url!; - const cached = await ensureWellKnownCached({ + const cached = await services.ensureWellKnownCached({ stateDir: opts.stateDir, url: resolvedUrl, cacheKey: wellKnownCacheKey(resolvedUrl), @@ -382,7 +399,7 @@ async function acquireSkillSource( ? `${parsed.owner}/${parsed.repo}` : sanitizeCacheKey(url); const excluded = isSourceExcluded(dep.source, opts.minimumReleaseAgeExclude); - const cached = await ensureCached({ + const cached = await services.ensureCached({ stateDir: opts.stateDir, url: resolvedUrl, cacheKey, @@ -502,7 +519,7 @@ async function discoverWildcardScope( try { scopeStat = await stat(scopeDir); } catch (err) { - if (!(err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT")) { + if (!(err instanceof Error && "code" in err && err.code === "ENOENT")) { throw err; } throw new ResolveError(`Wildcard path "${path}" does not exist in source`); diff --git a/packages/dotagents-lib/src/skills/resolver.wellknown.test.ts b/packages/dotagents-lib/src/skills/resolver.wellknown.test.ts index 3650c847..057d8fe3 100644 --- a/packages/dotagents-lib/src/skills/resolver.wellknown.test.ts +++ b/packages/dotagents-lib/src/skills/resolver.wellknown.test.ts @@ -1,9 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { ResolveError, resolveSkill, resolveWildcardSkills } from "./resolver.js"; -vi.mock("../sources/wellknown.js", () => ({ - ensureWellKnownCached: vi.fn().mockResolvedValue(null), -})); +const ensureWellKnownCached = vi.fn(async () => null); const STATE_DIR = "/tmp/dotagents-well-known-test-cache"; const SOURCE = "https://skills.example.com"; @@ -11,7 +9,10 @@ const SOURCE = "https://skills.example.com"; describe("missing well-known index", () => { it("rejects single-skill resolution", async () => { await expect( - resolveSkill("review", { source: SOURCE }, { stateDir: STATE_DIR }), + resolveSkill("review", { source: SOURCE }, { + stateDir: STATE_DIR, + services: { ensureWellKnownCached }, + }), ).rejects.toThrow(ResolveError); }); @@ -19,7 +20,7 @@ describe("missing well-known index", () => { await expect( resolveWildcardSkills( { source: SOURCE, exclude: [] }, - { stateDir: STATE_DIR }, + { stateDir: STATE_DIR, services: { ensureWellKnownCached } }, ), ).resolves.toEqual([]); }); diff --git a/packages/dotagents-lib/src/skills/tool-name.test.ts b/packages/dotagents-lib/src/skills/tool-name.test.ts index d922d251..00805b51 100644 --- a/packages/dotagents-lib/src/skills/tool-name.test.ts +++ b/packages/dotagents-lib/src/skills/tool-name.test.ts @@ -1,16 +1,19 @@ import { describe, it, expect, expectTypeOf } from "vitest"; import { TOOL_NAMES, isToolName, type ToolName } from "./tool-name.js"; +function verifyCandidate(candidate: string | number): void { + if (isToolName(candidate)) { + expectTypeOf(candidate).toEqualTypeOf(); + } +} + describe("isToolName", () => { it("accepts every name in TOOL_NAMES", () => { for (const name of TOOL_NAMES) { expect(isToolName(name)).toBe(true); } - const candidate: unknown = "Read"; - if (isToolName(candidate)) { - expectTypeOf(candidate).toEqualTypeOf(); - } + verifyCandidate("Read"); }); it("rejects unknown names", () => { diff --git a/packages/dotagents-lib/src/skills/tool-name.ts b/packages/dotagents-lib/src/skills/tool-name.ts index b27b3c84..b6d317c8 100644 --- a/packages/dotagents-lib/src/skills/tool-name.ts +++ b/packages/dotagents-lib/src/skills/tool-name.ts @@ -21,6 +21,6 @@ export type ToolName = (typeof TOOL_NAMES)[number]; const TOOL_NAME_SET: ReadonlySet = new Set(TOOL_NAMES); -export function isToolName(value: unknown): value is ToolName { +export function isToolName(value: Value): value is Value & ToolName { return typeof value === "string" && TOOL_NAME_SET.has(value); } diff --git a/packages/dotagents-lib/src/sources/cache.ts b/packages/dotagents-lib/src/sources/cache.ts index b14920f2..f4f48f35 100644 --- a/packages/dotagents-lib/src/sources/cache.ts +++ b/packages/dotagents-lib/src/sources/cache.ts @@ -75,8 +75,8 @@ const cacheLocks = new Map>(); async function withCacheLock(key: string, fn: () => Promise): Promise { const previous = cacheLocks.get(key) ?? Promise.resolve(); let release!: () => void; - const current = new Promise((resolve) => { - release = resolve; + const current = new Promise((releaseLock) => { + release = releaseLock; }); const next = previous.catch(() => {}).then(() => current); diff --git a/packages/dotagents-lib/src/sources/git.test.ts b/packages/dotagents-lib/src/sources/git.test.ts index 49cc91f0..0d8508a9 100644 --- a/packages/dotagents-lib/src/sources/git.test.ts +++ b/packages/dotagents-lib/src/sources/git.test.ts @@ -1,20 +1,14 @@ import { describe, it, expect, vi, afterEach } from "vitest"; +import { clone, fetchAndReset, fetchRef, headCommitDate, findCommitOlderThan, type GitExecutor } from "./git.js"; +import { ExecError } from "../utils/exec.js"; -vi.mock("../utils/exec.js", () => ({ - exec: vi.fn(async () => ({ stdout: "", stderr: "" })), - ExecError: Error, -})); - -import { clone, fetchAndReset, fetchRef, headCommitDate, findCommitOlderThan } from "./git.js"; -import { exec } from "../utils/exec.js"; - -const mockExec = vi.mocked(exec); +const mockExec = vi.fn(async () => ({ stdout: "", stderr: "" })); afterEach(() => vi.clearAllMocks()); describe("clone", () => { it("uses --branch for tag refs", async () => { - await clone("https://github.com/owner/repo.git", "/tmp/dest", "v1.0.0"); + await clone("https://github.com/owner/repo.git", "/tmp/dest", "v1.0.0", mockExec); expect(mockExec).toHaveBeenCalledTimes(1); expect(mockExec).toHaveBeenCalledWith("git", [ @@ -29,7 +23,7 @@ describe("clone", () => { }); it("uses --branch for branch refs", async () => { - await clone("https://github.com/owner/repo.git", "/tmp/dest", "main"); + await clone("https://github.com/owner/repo.git", "/tmp/dest", "main", mockExec); expect(mockExec).toHaveBeenCalledTimes(1); expect(mockExec).toHaveBeenCalledWith("git", [ @@ -45,7 +39,7 @@ describe("clone", () => { it("skips --branch for full SHA refs and fetches after clone", async () => { const sha = "405638a2ee3f131b910be238af499eac5c86e92c"; - await clone("https://github.com/owner/repo.git", "/tmp/dest", sha); + await clone("https://github.com/owner/repo.git", "/tmp/dest", sha, mockExec); expect(mockExec).toHaveBeenCalledTimes(3); // First call: clone without --branch @@ -74,7 +68,7 @@ describe("clone", () => { it("skips --branch for short SHA refs (7+ hex chars)", async () => { const shortSha = "405638a"; - await clone("https://github.com/owner/repo.git", "/tmp/dest", shortSha); + await clone("https://github.com/owner/repo.git", "/tmp/dest", shortSha, mockExec); // Clone without --branch, then fetch+checkout expect(mockExec).toHaveBeenCalledTimes(3); @@ -88,7 +82,7 @@ describe("clone", () => { }); it("does not treat 6-char hex strings as SHAs", async () => { - await clone("https://github.com/owner/repo.git", "/tmp/dest", "abcdef"); + await clone("https://github.com/owner/repo.git", "/tmp/dest", "abcdef", mockExec); // Too short — treated as a branch name expect(mockExec).toHaveBeenCalledTimes(1); @@ -105,7 +99,7 @@ describe("clone", () => { it("handles uppercase SHA refs", async () => { const sha = "405638A2EE3F131B910BE238AF499EAC5C86E92C"; - await clone("https://github.com/owner/repo.git", "/tmp/dest", sha); + await clone("https://github.com/owner/repo.git", "/tmp/dest", sha, mockExec); expect(mockExec).toHaveBeenCalledTimes(3); expect(mockExec).toHaveBeenNthCalledWith(1, "git", [ @@ -119,7 +113,7 @@ describe("clone", () => { it("does not treat refs with non-hex chars as SHAs", async () => { // "release-v1" has non-hex chars, even though length > 7 - await clone("https://github.com/owner/repo.git", "/tmp/dest", "release-v1"); + await clone("https://github.com/owner/repo.git", "/tmp/dest", "release-v1", mockExec); expect(mockExec).toHaveBeenCalledTimes(1); expect(mockExec).toHaveBeenCalledWith("git", [ @@ -134,7 +128,7 @@ describe("clone", () => { }); it("clones without ref when none provided", async () => { - await clone("https://github.com/owner/repo.git", "/tmp/dest"); + await clone("https://github.com/owner/repo.git", "/tmp/dest", undefined, mockExec); expect(mockExec).toHaveBeenCalledTimes(1); expect(mockExec).toHaveBeenCalledWith("git", [ @@ -149,7 +143,7 @@ describe("clone", () => { describe("fetchAndReset", () => { it("force-fetches origin before resetting to FETCH_HEAD", async () => { - await fetchAndReset("/tmp/repo"); + await fetchAndReset("/tmp/repo", mockExec); expect(mockExec).toHaveBeenNthCalledWith(1, "git", [ "fetch", @@ -168,7 +162,7 @@ describe("fetchAndReset", () => { describe("fetchRef", () => { it("force-fetches the requested ref before checkout", async () => { - await fetchRef("/tmp/repo", "v0"); + await fetchRef("/tmp/repo", "v0", mockExec); expect(mockExec).toHaveBeenNthCalledWith(1, "git", [ "fetch", @@ -189,7 +183,7 @@ describe("headCommitDate", () => { it("returns the committer date of HEAD", async () => { mockExec.mockResolvedValueOnce({ stdout: "2026-03-15T10:30:00+00:00\n", stderr: "" }); - const date = await headCommitDate("/tmp/repo"); + const date = await headCommitDate("/tmp/repo", mockExec); expect(mockExec).toHaveBeenCalledWith( "git", @@ -208,7 +202,7 @@ describe("findCommitOlderThan", () => { // Second call: git log --before mockExec.mockResolvedValueOnce({ stdout: `${sha}\n`, stderr: "" }); - const result = await findCommitOlderThan("/tmp/repo", 3); + const result = await findCommitOlderThan("/tmp/repo", 3, mockExec); expect(result).toBe(sha); expect(mockExec).toHaveBeenNthCalledWith( @@ -231,7 +225,7 @@ describe("findCommitOlderThan", () => { // git log returns empty (repo younger than threshold) mockExec.mockResolvedValueOnce({ stdout: "", stderr: "" }); - const result = await findCommitOlderThan("/tmp/repo", 30); + const result = await findCommitOlderThan("/tmp/repo", 30, mockExec); expect(result).toBeNull(); }); @@ -239,13 +233,13 @@ describe("findCommitOlderThan", () => { it("tolerates already-unshallowed repos", async () => { const sha = "abc123def456789012345678901234567890abcd"; // fetch --unshallow fails because repo is already complete - const err = new Error("fatal: --unshallow on a complete repository does not make sense") as Error & { stderr: string }; - err.stderr = "fatal: --unshallow on a complete repository does not make sense"; + const stderr = "fatal: --unshallow on a complete repository does not make sense"; + const err = new ExecError(stderr, 128, stderr); mockExec.mockRejectedValueOnce(err); // git log --before mockExec.mockResolvedValueOnce({ stdout: `${sha}\n`, stderr: "" }); - const result = await findCommitOlderThan("/tmp/repo", 3); + const result = await findCommitOlderThan("/tmp/repo", 3, mockExec); expect(result).toBe(sha); }); diff --git a/packages/dotagents-lib/src/sources/git.ts b/packages/dotagents-lib/src/sources/git.ts index 645a3c7f..f93262dc 100644 --- a/packages/dotagents-lib/src/sources/git.ts +++ b/packages/dotagents-lib/src/sources/git.ts @@ -1,6 +1,8 @@ import { exec, ExecError } from "../utils/exec.js"; import { existsSync } from "node:fs"; +export type GitExecutor = typeof exec; + function toSshCloneUrl(url: string): string | undefined { const hostedMatch = url.match( /^https?:\/\/(github\.com|gitlab\.com)\/(.+)$/i, @@ -53,6 +55,7 @@ export async function clone( url: string, dest: string, ref?: string, + run: GitExecutor = exec, ): Promise { const isSha = ref && SHA_LIKE.test(ref); const args = ["clone", "--depth=1"]; @@ -62,7 +65,7 @@ export async function clone( args.push("--", url, dest); try { - await exec("git", args); + await run("git", args); } catch (err) { if (err instanceof ExecError) { const stderr = err.stderr; @@ -88,17 +91,17 @@ export async function clone( // For SHA refs, fetch the specific commit after the initial clone if (isSha) { - await fetchRef(dest, ref); + await fetchRef(dest, ref, run); } } /** * Fetch latest and reset to origin's HEAD. For updating unpinned repos. */ -export async function fetchAndReset(repoDir: string): Promise { +export async function fetchAndReset(repoDir: string, run: GitExecutor = exec): Promise { try { - await exec("git", ["fetch", "--force", "--depth=1", "--", "origin"], { cwd: repoDir }); - await exec("git", ["reset", "--hard", "FETCH_HEAD"], { cwd: repoDir }); + await run("git", ["fetch", "--force", "--depth=1", "--", "origin"], { cwd: repoDir }); + await run("git", ["reset", "--hard", "FETCH_HEAD"], { cwd: repoDir }); } catch (err) { if (err instanceof ExecError) { throw new GitError(`Failed to update ${repoDir}: ${err.stderr}`); @@ -110,12 +113,12 @@ export async function fetchAndReset(repoDir: string): Promise { /** * Fetch a specific ref and checkout. */ -export async function fetchRef(repoDir: string, ref: string): Promise { +export async function fetchRef(repoDir: string, ref: string, run: GitExecutor = exec): Promise { try { - await exec("git", ["fetch", "--force", "--depth=1", "--", "origin", ref], { + await run("git", ["fetch", "--force", "--depth=1", "--", "origin", ref], { cwd: repoDir, }); - await exec("git", ["checkout", "FETCH_HEAD"], { cwd: repoDir }); + await run("git", ["checkout", "FETCH_HEAD"], { cwd: repoDir }); } catch (err) { if (err instanceof ExecError) { throw new GitError( @@ -129,8 +132,8 @@ export async function fetchRef(repoDir: string, ref: string): Promise { /** * Get the current HEAD commit SHA (full 40 chars). */ -export async function headCommit(repoDir: string): Promise { - const { stdout } = await exec("git", ["rev-parse", "HEAD"], { cwd: repoDir }); +export async function headCommit(repoDir: string, run: GitExecutor = exec): Promise { + const { stdout } = await run("git", ["rev-parse", "HEAD"], { cwd: repoDir }); return stdout.trim(); } @@ -139,8 +142,8 @@ export async function headCommit(repoDir: string): Promise { * Uses committer date (not author date) to reflect when the commit landed on the branch, * which aligns with "release age" semantics (survives cherry-picks and merges). */ -export async function headCommitDate(repoDir: string): Promise { - const { stdout } = await exec("git", ["log", "-1", "--format=%cI", "HEAD"], { cwd: repoDir }); +export async function headCommitDate(repoDir: string, run: GitExecutor = exec): Promise { + const { stdout } = await run("git", ["log", "-1", "--format=%cI", "HEAD"], { cwd: repoDir }); return new Date(stdout.trim()); } @@ -154,6 +157,7 @@ export async function headCommitDate(repoDir: string): Promise { export async function findCommitOlderThan( repoDir: string, minAgeMinutes: number, + run: GitExecutor = exec, ): Promise { const cutoff = new Date(Date.now() - minAgeMinutes * 60 * 1000); const iso = cutoff.toISOString(); @@ -161,7 +165,7 @@ export async function findCommitOlderThan( // Unshallow to get full history — needed to find commits older than the cutoff. // Only called when HEAD is too new, so the extra fetch is acceptable. try { - await exec("git", ["fetch", "--force", "--unshallow", "--", "origin"], { cwd: repoDir }); + await run("git", ["fetch", "--force", "--unshallow", "--", "origin"], { cwd: repoDir }); } catch (err) { if (!(err instanceof ExecError)) {throw err;} // --unshallow fails on a complete (non-shallow) repository — that's fine @@ -173,7 +177,7 @@ export async function findCommitOlderThan( // Find the newest commit at or before the cutoff. // Use HEAD (not FETCH_HEAD) — after the prior fetchAndReset, HEAD is at the // latest commit and --unshallow made full history available behind it. - const { stdout } = await exec( + const { stdout } = await run( "git", ["log", "--format=%H", "--before", iso, "-1", "HEAD"], { cwd: repoDir }, @@ -185,9 +189,9 @@ export async function findCommitOlderThan( /** * Checkout a local ref (commit, branch, tag). No fetch — the ref must already exist locally. */ -export async function checkout(repoDir: string, ref: string): Promise { +export async function checkout(repoDir: string, ref: string, run: GitExecutor = exec): Promise { try { - await exec("git", ["checkout", ref], { cwd: repoDir }); + await run("git", ["checkout", ref], { cwd: repoDir }); } catch (err) { if (err instanceof ExecError) { throw new GitError(`Failed to checkout ${ref} in ${repoDir}: ${err.stderr}`); diff --git a/packages/dotagents-lib/src/sources/name-safety.test.ts b/packages/dotagents-lib/src/sources/name-safety.test.ts index 65437268..eec53f57 100644 --- a/packages/dotagents-lib/src/sources/name-safety.test.ts +++ b/packages/dotagents-lib/src/sources/name-safety.test.ts @@ -1,6 +1,16 @@ import { describe, it, expect } from "vitest"; import { validateGitNameSafety, GitNameSafetyError } from "./name-safety.js"; +function captureGitNameSafetyError(run: () => void): GitNameSafetyError { + try { + run(); + } catch (err) { + if (err instanceof GitNameSafetyError) {return err;} + throw err; + } + throw new Error("expected validateGitNameSafety to throw"); +} + describe("validateGitNameSafety", () => { it("accepts safe owner/repo/ref", () => { expect(() => @@ -29,80 +39,60 @@ describe("validateGitNameSafety", () => { expect(() => validateGitNameSafety({ owner: "-malicious" })).toThrow( GitNameSafetyError, ); - try { - validateGitNameSafety({ owner: "-malicious" }); - } catch (err) { - expect(err).toBeInstanceOf(GitNameSafetyError); - expect((err as GitNameSafetyError).field).toBe("owner"); - expect((err as GitNameSafetyError).reason).toBe("leading-dash"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "-malicious" }), + ); + expect(err.field).toBe("owner"); + expect(err.reason).toBe("leading-dash"); }); it("rejects leading-dash repo", () => { - try { - validateGitNameSafety({ repo: "-evil" }); - throw new Error("expected to throw"); - } catch (err) { - expect(err).toBeInstanceOf(GitNameSafetyError); - expect((err as GitNameSafetyError).field).toBe("repo"); - expect((err as GitNameSafetyError).reason).toBe("leading-dash"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ repo: "-evil" }), + ); + expect(err.field).toBe("repo"); + expect(err.reason).toBe("leading-dash"); }); it("rejects leading-dash ref", () => { - try { - validateGitNameSafety({ ref: "--upload-pack=evil" }); - throw new Error("expected to throw"); - } catch (err) { - expect(err).toBeInstanceOf(GitNameSafetyError); - expect((err as GitNameSafetyError).field).toBe("ref"); - expect((err as GitNameSafetyError).reason).toBe("leading-dash"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ ref: "--upload-pack=evil" }), + ); + expect(err.field).toBe("ref"); + expect(err.reason).toBe("leading-dash"); }); it("rejects '..' as repo", () => { - try { - validateGitNameSafety({ owner: "safe", repo: ".." }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).reason).toBe("traversal"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "safe", repo: ".." }), + ); + expect(err.reason).toBe("traversal"); }); it("rejects '.' as repo", () => { - try { - validateGitNameSafety({ repo: "." }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).reason).toBe("traversal"); - } + const err = captureGitNameSafetyError(() => validateGitNameSafety({ repo: "." })); + expect(err.reason).toBe("traversal"); }); it("rejects embedded '..' in a segment", () => { - try { - validateGitNameSafety({ repo: "foo..bar" }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).reason).toBe("traversal"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ repo: "foo..bar" }), + ); + expect(err.reason).toBe("traversal"); }); it("rejects embedded '..' in owner segment", () => { - try { - validateGitNameSafety({ owner: "..foo" }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).reason).toBe("traversal"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "..foo" }), + ); + expect(err.reason).toBe("traversal"); }); it("rejects invalid characters", () => { - try { - validateGitNameSafety({ owner: "weird name" }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).reason).toBe("invalid-characters"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "weird name" }), + ); + expect(err.reason).toBe("invalid-characters"); }); it("accepts GitLab nested group with safe segments", () => { @@ -112,22 +102,18 @@ describe("validateGitNameSafety", () => { }); it("rejects GitLab nested group when one segment is unsafe", () => { - try { - validateGitNameSafety({ owner: "group/-evil", repo: "repo" }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).field).toBe("owner"); - expect((err as GitNameSafetyError).reason).toBe("leading-dash"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "group/-evil", repo: "repo" }), + ); + expect(err.field).toBe("owner"); + expect(err.reason).toBe("leading-dash"); }); it("rejects GitLab nested group with traversal segment", () => { - try { - validateGitNameSafety({ owner: "group/../escape", repo: "repo" }); - throw new Error("expected to throw"); - } catch (err) { - expect((err as GitNameSafetyError).field).toBe("owner"); - expect((err as GitNameSafetyError).reason).toBe("traversal"); - } + const err = captureGitNameSafetyError(() => + validateGitNameSafety({ owner: "group/../escape", repo: "repo" }), + ); + expect(err.field).toBe("owner"); + expect(err.reason).toBe("traversal"); }); }); diff --git a/packages/dotagents-lib/src/sources/wellknown.ts b/packages/dotagents-lib/src/sources/wellknown.ts index 51045834..caf0fad5 100644 --- a/packages/dotagents-lib/src/sources/wellknown.ts +++ b/packages/dotagents-lib/src/sources/wellknown.ts @@ -42,7 +42,7 @@ export async function fetchWellKnownIndex( }); if (!response.ok) {return null;} - const data = (await response.json()) as unknown; + const data = await response.json(); if (!isValidIndex(data)) {return null;} return data; } catch { @@ -50,18 +50,22 @@ export async function fetchWellKnownIndex( } } -function isValidIndex(data: unknown): data is WellKnownIndex { +function isValidIndex(data: Value): data is Value & WellKnownIndex { if (!isSerializedObject(data)) {return false;} const skills = data["skills"]; if (!Array.isArray(skills)) {return false;} return skills.every((entry) => ( isSerializedObject(entry) && - typeof entry["name"] === "string" && + isString(entry["name"]) && Array.isArray(entry["files"]) && - entry["files"].every((file) => typeof file === "string") + entry["files"].every(isString) )); } +function isString(value: Value): value is Value & string { + return typeof value === "string"; +} + export interface WellKnownCacheResult { /** Path to the cached skills directory (contains one subdirectory per skill) */ cacheDir: string; diff --git a/packages/dotagents-lib/src/trust/validator.test.ts b/packages/dotagents-lib/src/trust/validator.test.ts index e06bcbd5..62cb155c 100644 --- a/packages/dotagents-lib/src/trust/validator.test.ts +++ b/packages/dotagents-lib/src/trust/validator.test.ts @@ -17,6 +17,16 @@ function makeTrust(overrides: Partial = {}): TrustPolicy { }; } +function captureTrustError(source: string, trust: TrustPolicy): TrustError { + try { + validateTrustedSource(source, trust); + } catch (err) { + if (err instanceof TrustError) {return err;} + throw err; + } + throw new Error("expected validateTrustedSource to throw"); +} + describe("validateTrustedSource", () => { it("allows everything when trust config is undefined", () => { expect(() => validateTrustedSource("evil/repo")).not.toThrow(); @@ -250,15 +260,9 @@ describe("validateTrustedSource", () => { it("exposes domain details on rejected well-known source", () => { const trust = makeTrust({ git_domains: ["other.example.com"] }); - try { - validateTrustedSource("https://cli.sentry.dev", trust); - throw new Error("expected TrustError"); - } catch (err) { - expect(err).toBeInstanceOf(TrustError); - const e = err as TrustError; - expect(e.details.kind).toBe("well-known"); - expect(e.details.domain).toBe("cli.sentry.dev"); - } + const err = captureTrustError("https://cli.sentry.dev", trust); + expect(err.details.kind).toBe("well-known"); + expect(err.details.domain).toBe("cli.sentry.dev"); }); it("matches well-known domains case-insensitively", () => { @@ -361,29 +365,17 @@ describe("validateTrustedSource", () => { it("exposes owner+repo details on rejected GitHub sources", () => { const trust = makeTrust({ github_orgs: ["getsentry"] }); - try { - validateTrustedSource("evil/repo", trust); - throw new Error("expected TrustError"); - } catch (err) { - expect(err).toBeInstanceOf(TrustError); - const e = err as TrustError; - expect(e.details.kind).toBe("github"); - expect(e.details.owner).toBe("evil"); - expect(e.details.repo).toBe("repo"); - } + const err = captureTrustError("evil/repo", trust); + expect(err.details.kind).toBe("github"); + expect(err.details.owner).toBe("evil"); + expect(err.details.repo).toBe("repo"); }); it("exposes domain details on rejected git domain sources", () => { const trust = makeTrust({ git_domains: ["git.corp.com"] }); - try { - validateTrustedSource("git:https://evil.com/repo.git", trust); - throw new Error("expected TrustError"); - } catch (err) { - expect(err).toBeInstanceOf(TrustError); - const e = err as TrustError; - expect(e.details.kind).toBe("git"); - expect(e.details.domain).toBe("evil.com"); - } + const err = captureTrustError("git:https://evil.com/repo.git", trust); + expect(err.details.kind).toBe("git"); + expect(err.details.domain).toBe("evil.com"); }); }); }); diff --git a/packages/dotagents-lib/src/utils/exec.ts b/packages/dotagents-lib/src/utils/exec.ts index 8125e293..46e0e798 100644 --- a/packages/dotagents-lib/src/utils/exec.ts +++ b/packages/dotagents-lib/src/utils/exec.ts @@ -39,7 +39,7 @@ export function exec( execFile(cmd, args, { cwd: opts?.cwd, env, maxBuffer: 50 * 1024 * 1024, timeout: opts?.timeoutMs ?? DEFAULT_TIMEOUT_MS }, (err, stdout, stderr) => { if (err) { - const code = "code" in err ? (err.code as number | null) : null; + const code = "code" in err && isNumber(err.code) ? err.code : null; reject( new ExecError( `${cmd} ${args.join(" ")} failed: ${stderr.trim() || err.message}`, @@ -53,3 +53,7 @@ export function exec( }); }); } + +function isNumber(value: Value): value is Value & number { + return typeof value === "number"; +} diff --git a/packages/dotagents-lib/src/utils/fs.ts b/packages/dotagents-lib/src/utils/fs.ts index ab768d3b..ce5c49aa 100644 --- a/packages/dotagents-lib/src/utils/fs.ts +++ b/packages/dotagents-lib/src/utils/fs.ts @@ -11,11 +11,14 @@ export async function copyDir( opts: { verbatimSymlinks?: boolean } = {}, ): Promise { await rm(dest, { recursive: true, force: true }); - await cp(src, dest, { + const copyOptions: Parameters[2] = { recursive: true, - ...(opts.verbatimSymlinks === undefined ? {} : { verbatimSymlinks: opts.verbatimSymlinks }), filter: (source) => basename(source) !== ".git", - }); + }; + if (opts.verbatimSymlinks !== undefined) { + copyOptions.verbatimSymlinks = opts.verbatimSymlinks; + } + await cp(src, dest, copyOptions); } /** Strip trailing `/` characters from a string. */ diff --git a/packages/dotagents-lib/src/utils/serialized.test.ts b/packages/dotagents-lib/src/utils/serialized.test.ts index c0195754..40a237e1 100644 --- a/packages/dotagents-lib/src/utils/serialized.test.ts +++ b/packages/dotagents-lib/src/utils/serialized.test.ts @@ -24,7 +24,7 @@ describe("serialized values", () => { expect(isSerializedValue(hiddenCallback)).toBe(false); const accessor = Object.defineProperty({}, "enabled", { get: () => true, enumerable: true }); expect(isSerializedValue(accessor)).toBe(false); - expect(isSerializedValue(JSON.parse('{"__proto__":{"polluted":true}}') as unknown)).toBe(false); + expect(isSerializedValue(JSON.parse('{"__proto__":{"polluted":true}}'))).toBe(false); expect(isSerializedValue(new Map([["enabled", true]]))).toBe(false); expect(isSerializedValue(new (class Metadata { enabled = true; })())).toBe(false); const sparse: unknown[] = []; @@ -33,7 +33,8 @@ describe("serialized values", () => { }); it("rejects cyclic objects without overflowing", () => { - const value: { self?: object } = {}; + interface CyclicValue { self?: object } + const value: CyclicValue = {}; value.self = value; expect(isSerializedObject(value)).toBe(false); }); diff --git a/packages/dotagents-lib/src/utils/serialized.ts b/packages/dotagents-lib/src/utils/serialized.ts index e7253a00..13b4920e 100644 --- a/packages/dotagents-lib/src/utils/serialized.ts +++ b/packages/dotagents-lib/src/utils/serialized.ts @@ -13,7 +13,7 @@ export interface SerializedObject { const MAX_SERIALIZED_DEPTH = 1_000; -export function isSerializedObject(value: unknown): value is SerializedObject { +export function isSerializedObject(value: Value): value is Value & SerializedObject { try { return isPlainObject(value) && validateSerializedValue(value); } catch { @@ -21,7 +21,7 @@ export function isSerializedObject(value: unknown): value is SerializedObject { } } -export function isSerializedValue(value: unknown): value is SerializedValue { +export function isSerializedValue(value: Value): value is Value & SerializedValue { try { return validateSerializedValue(value); } catch { @@ -35,14 +35,16 @@ interface VisitFrame { depth: number; } -function validateSerializedValue(root: unknown): boolean { +function validateSerializedValue(root: Value): root is Value & SerializedValue { const ancestors = new Set(); const stack: VisitFrame[] = [{ value: root, exiting: false, depth: 0 }]; while (stack.length > 0) { - const frame = stack.pop()!; + const frame = stack.pop(); + if (frame === undefined) {break;} const value = frame.value; if (frame.exiting) { + // SAFETY: exiting frames are only pushed for arrays and plain objects. ancestors.delete(value as object); continue; } @@ -89,7 +91,7 @@ function validateSerializedValue(root: unknown): boolean { return true; } -function isPlainObject(value: unknown): value is object { +function isPlainObject(value: Value): value is Value & object { if (typeof value !== "object" || value === null || Array.isArray(value) || value instanceof Date) { return false; } diff --git a/packages/dotagents/package.json b/packages/dotagents/package.json index 5a98b52f..7980a03d 100644 --- a/packages/dotagents/package.json +++ b/packages/dotagents/package.json @@ -13,7 +13,7 @@ "build": "tsc -b", "test": "vitest run", "test:watch": "vitest", - "lint": "oxlint src --deny-warnings", + "lint": "node --import tsx ../../node_modules/oxlint/bin/oxlint src --deny-warnings", "typecheck": "tsc -b --emitDeclarationOnly" }, "files": [ diff --git a/packages/dotagents/src/cli/commands/add.test.ts b/packages/dotagents/src/cli/commands/add.test.ts index 3d90c971..02c09c4e 100644 --- a/packages/dotagents/src/cli/commands/add.test.ts +++ b/packages/dotagents/src/cli/commands/add.test.ts @@ -4,21 +4,38 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import * as clack from "@clack/prompts"; -import add, { runAdd, AddCancelledError, AddError } from "./add.js"; +import addCommand, { + runAdd as runAddCommand, + AddCancelledError, + AddError, + type AddOptions, + type AddPrompts, +} from "./add.js"; import * as installModule from "./install.js"; import { TrustError, exec } from "@sentry/dotagents-lib"; import { resolveScope } from "../../scope.js"; +import type { CommandContext } from "../context.js"; +import { z } from "zod/v4"; + +const prompts = { + select: vi.fn(), + multiselect: vi.fn(), + isCancel: vi.fn(clack.isCancel), + spinner: vi.fn(), +} satisfies AddPrompts; + +const gitTraceEventSchema = z.object({ + event: z.string(), + argv: z.array(z.string()).optional(), +}).passthrough(); + +function runAdd(opts: AddOptions): ReturnType { + return runAddCommand({ ...opts, prompts }); +} -vi.mock("@clack/prompts", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - select: vi.fn(), - multiselect: vi.fn(), - isCancel: vi.fn(actual.isCancel), - spinner: vi.fn(), - }; -}); +function add(args: string[], context: CommandContext): Promise { + return addCommand(args, context, prompts); +} const SKILL_MD = (name: string) => `--- name: ${name} @@ -52,7 +69,7 @@ function countGitFetches(tracePath: string): number { .trim() .split("\n") .filter(Boolean) - .map((line) => JSON.parse(line) as { event: string; argv?: string[] }) + .map((line) => gitTraceEventSchema.parse(JSON.parse(line))) .filter((event) => event.event === "start" && event.argv?.[1] === "fetch") .length; } @@ -146,7 +163,7 @@ describe("runAdd", () => { const installEvents = (await readFile(installTracePath, "utf-8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as { event: string; argv?: string[] }); + .map((line) => gitTraceEventSchema.parse(JSON.parse(line))); const installFetches = installEvents.filter( (event) => event.event === "start" && event.argv?.[1] === "fetch", ); @@ -342,7 +359,8 @@ describe("runAdd", () => { }).catch((cause: unknown) => cause); expect(error).toBeInstanceOf(AddError); - expect((error as Error).message).toBe( + if (!(error instanceof Error)) {throw new Error("expected add to reject");} + expect(error.message).toBe( "Skills already exist in agents.toml with a different source or ref: review.", ); }); @@ -525,8 +543,8 @@ describe("runAdd", () => { }); it("cancels interactive catalog selection before writing", async () => { - vi.mocked(clack.select).mockResolvedValue("pick"); - vi.mocked(clack.isCancel).mockReturnValue(true); + prompts.select.mockResolvedValue("pick"); + prompts.isCancel.mockReturnValue(true); await expect( runAdd({ @@ -542,8 +560,8 @@ describe("runAdd", () => { }); it("persists an interactive all selection and installs once", async () => { - vi.mocked(clack.select).mockResolvedValue("all"); - vi.mocked(clack.isCancel).mockReturnValue(false); + prompts.select.mockResolvedValue("all"); + prompts.isCancel.mockReturnValue(false); const install = vi.spyOn(installModule, "runInstall").mockResolvedValue({ installed: [], installedPlugins: [], @@ -569,9 +587,9 @@ describe("runAdd", () => { }); it("persists interactive skill selections and installs once", async () => { - vi.mocked(clack.select).mockResolvedValue("pick"); - vi.mocked(clack.multiselect).mockResolvedValue(["pdf", "review"]); - vi.mocked(clack.isCancel).mockReturnValue(false); + prompts.select.mockResolvedValue("pick"); + prompts.multiselect.mockResolvedValue(["pdf", "review"]); + prompts.isCancel.mockReturnValue(false); const install = vi.spyOn(installModule, "runInstall").mockResolvedValue({ installed: [], installedPlugins: [], @@ -763,9 +781,9 @@ describe("runAdd (local sources)", () => { interactive: false, })).rejects.toThrow(/Multiple plugins found.*alpha, beta.*--name/); - vi.mocked(clack.select).mockResolvedValue("pick"); - vi.mocked(clack.multiselect).mockResolvedValue([1]); - vi.mocked(clack.isCancel).mockReturnValue(false); + prompts.select.mockResolvedValue("pick"); + prompts.multiselect.mockResolvedValue([1]); + prompts.isCancel.mockReturnValue(false); mockRunInstall(); const result = await runAdd({ scope: resolveScope("project", projectRoot), @@ -783,9 +801,9 @@ describe("runAdd (local sources)", () => { const sourceDir = join(projectRoot, "root-plugin-picker"); await writePlugin(sourceDir, "shared"); await writePlugin(join(sourceDir, ".agents", "plugins", "shared"), "shared"); - vi.mocked(clack.select).mockResolvedValue("pick"); - vi.mocked(clack.multiselect).mockResolvedValue([0]); - vi.mocked(clack.isCancel).mockReturnValue(false); + prompts.select.mockResolvedValue("pick"); + prompts.multiselect.mockResolvedValue([0]); + prompts.isCancel.mockReturnValue(false); mockRunInstall(); const result = await runAdd({ @@ -872,9 +890,9 @@ describe("runAdd (local sources)", () => { { name: "beta", source: "./shared-plugin" }, ], })); - vi.mocked(clack.select).mockResolvedValue("pick"); - vi.mocked(clack.multiselect).mockResolvedValue([0, 1]); - vi.mocked(clack.isCancel).mockReturnValue(false); + prompts.select.mockResolvedValue("pick"); + prompts.multiselect.mockResolvedValue([0, 1]); + prompts.isCancel.mockReturnValue(false); mockRunInstall(); const result = await runAdd({ @@ -965,7 +983,8 @@ describe("runAdd (local sources)", () => { }).catch((cause: unknown) => cause); expect(error).toBeInstanceOf(AddError); - expect((error as Error).message).toBe( + if (!(error instanceof Error)) {throw new Error("expected add to reject");} + expect(error.message).toBe( "Plugins already exist in agents.toml with a different source, ref, or path: beta.", ); }); @@ -1203,7 +1222,7 @@ describe("add() CLI parsing", () => { clear: vi.fn(), isCancelled: false, }; - vi.mocked(clack.spinner).mockReturnValue(spinner); + prompts.spinner.mockReturnValue(spinner); const originalIsTTY = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: true }); const log = vi.spyOn(console, "log").mockImplementation(() => {}); @@ -1212,7 +1231,7 @@ describe("add() CLI parsing", () => { try { await add(["path:plugin-source", "--skill", "cli-plugin"], { scope: resolveScope("project", projectRoot) }); - expect(clack.spinner).toHaveBeenCalledWith({ indicator: "timer" }); + expect(prompts.spinner).toHaveBeenCalledWith({ indicator: "timer" }); expect(spinner.start.mock.calls).toEqual([ ["Resolving path:plugin-source"], ["Installing components"], @@ -1229,7 +1248,7 @@ describe("add() CLI parsing", () => { if (originalIsTTY) { Object.defineProperty(process.stdout, "isTTY", originalIsTTY); } else { - delete (process.stdout as { isTTY?: boolean }).isTTY; + Reflect.deleteProperty(process.stdout, "isTTY"); } } }); diff --git a/packages/dotagents/src/cli/commands/add.ts b/packages/dotagents/src/cli/commands/add.ts index 08dab04a..4c0faf4b 100644 --- a/packages/dotagents/src/cli/commands/add.ts +++ b/packages/dotagents/src/cli/commands/add.ts @@ -3,6 +3,7 @@ import { readFile, realpath, writeFile } from "node:fs/promises"; import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { parseArgs } from "node:util"; import * as clack from "@clack/prompts"; +import type { MultiSelectOptions, SelectOptions } from "@clack/prompts"; import chalk from "chalk"; import { loadConfig } from "../../config/loader.js"; import { @@ -35,6 +36,7 @@ import { validateTrustedSource, TrustError, GitError, + type CacheReuse, } from "@sentry/dotagents-lib"; import { getCacheStateDir, HOST_SCAN_DIRS } from "../cache.js"; import { formatGitError, formatTrustError } from "../errors.js"; @@ -46,6 +48,7 @@ import { discoverPlugins, type PluginCandidate, } from "../../plugins/store.js"; +import { isNumber, isString } from "../../utils/type-guards.js"; /** Parent paths that are standard/expected — no need to show them in the picker */ const STANDARD_PARENTS = new Set(["", "skills", ".agents/skills", ".claude/skills"]); @@ -72,8 +75,26 @@ export interface AddOptions { all?: boolean; interactive?: boolean; progress?: AddProgress; + prompts?: AddPrompts; } +type AddPromptValue = string | number; +type AddPromptResult = AddPromptValue | AddPromptValue[] | symbol; + +export interface AddPrompts { + select(options: SelectOptions): Promise; + multiselect(options: MultiSelectOptions): Promise; + isCancel(value: AddPromptResult): boolean; + spinner: typeof clack.spinner; +} + +const DEFAULT_ADD_PROMPTS: AddPrompts = { + select: clack.select, + multiselect: clack.multiselect, + isCancel: clack.isCancel, + spinner: clack.spinner, +}; + export interface AddProgress { start(message: string): void; message(message: string): void; @@ -201,6 +222,7 @@ async function selectSkills( source: string, interactive: boolean | undefined, command: string, + prompts: AddPrompts, ): Promise { if (acquired.local && !names?.length) { const meta = await loadSkillMd(join(acquired.rootDir, "SKILL.md")); @@ -241,7 +263,7 @@ async function selectSkills( ); } - const mode = await clack.select({ + const mode = await prompts.select({ message: `Multiple skills found in ${source}. How would you like to add them?`, options: [ { @@ -256,10 +278,11 @@ async function selectSkills( }, ], }); - if (clack.isCancel(mode)) {throw new AddCancelledError();} + if (prompts.isCancel(mode)) {throw new AddCancelledError();} if (mode === "all") {return { type: "wildcard" };} + if (mode !== "pick") {throw new AddError("Invalid skill selection mode.");} - const selected = await clack.multiselect({ + const selected = await prompts.multiselect({ message: "Select which skills to add:", options: skills .toSorted((a, b) => a.meta.name.localeCompare(b.meta.name)) @@ -279,11 +302,14 @@ async function selectSkills( }), required: true, }); - if (clack.isCancel(selected)) {throw new AddCancelledError();} + if (prompts.isCancel(selected)) {throw new AddCancelledError();} + if (!Array.isArray(selected)) {throw new AddError("Invalid skill selection.");} + const selectedNames = selected.filter(isString); + if (selectedNames.length !== selected.length) {throw new AddError("Invalid skill selection.");} return { type: "skills", - names: selected, - duplicatePolicy: selected.length === 1 ? "single" : "selected", + names: selectedNames, + duplicatePolicy: selectedNames.length === 1 ? "single" : "selected", }; } @@ -334,6 +360,7 @@ async function selectPlugins( interactive: boolean | undefined, all: boolean | undefined, command: string, + prompts: AddPrompts, ): Promise { if (all) { assertUniquePluginNames(candidates, source); @@ -356,7 +383,7 @@ async function selectPlugins( ); } - const mode = await clack.select({ + const mode = await prompts.select({ message: `Multiple plugins found in ${source}. How would you like to add them?`, options: [ { @@ -371,16 +398,17 @@ async function selectPlugins( }, ], }); - if (clack.isCancel(mode)) {throw new AddCancelledError();} + if (prompts.isCancel(mode)) {throw new AddCancelledError();} if (mode === "all") { assertUniquePluginNames(candidates, source); return { candidates, duplicatePolicy: "selected" }; } + if (mode !== "pick") {throw new AddError("Invalid plugin selection mode.");} const pickerCandidates = candidates.toSorted((a, b) => a.name.localeCompare(b.name) || a.path.localeCompare(b.path) ); - const selectedIndices = await clack.multiselect({ + const selectedIndices = await prompts.multiselect({ message: "Select which plugins to add:", options: pickerCandidates .map((candidate, index) => ({ @@ -390,8 +418,11 @@ async function selectPlugins( })), required: true, }); - if (clack.isCancel(selectedIndices)) {throw new AddCancelledError();} - const selected = selectedIndices.map((index) => { + if (prompts.isCancel(selectedIndices)) {throw new AddCancelledError();} + if (!Array.isArray(selectedIndices)) {throw new AddError("Invalid plugin selection.");} + const indices = selectedIndices.filter(isNumber); + if (indices.length !== selectedIndices.length) {throw new AddError("Invalid plugin selection.");} + const selected = indices.map((index) => { const candidate = pickerCandidates[index]; if (!candidate) {throw new AddError("Invalid plugin selection.");} return candidate; @@ -412,6 +443,7 @@ async function executeAdd(opts: AddOptions): Promise { all, interactive, progress, + prompts = DEFAULT_ADD_PROMPTS, } = opts; const command = commandPrefix(scope); const specifier = stripLeadingAt(rawSpecifier); @@ -461,10 +493,11 @@ async function executeAdd(opts: AddOptions): Promise { progress?.start("Installing components"); try { await mutateConfig(); - await runInstall({ - scope, - ...(reuse ? { reuse } : {}), - }); + if (reuse) { + await runInstall({ scope, reuse }); + } else { + await runInstall({ scope }); + } progress?.stop("Installation complete"); } catch (err) { progress?.error("Installation failed"); @@ -669,13 +702,11 @@ async function executeAdd(opts: AddOptions): Promise { ? err : new AddError(message); } - const reuse = acquired.git && acquired.commit - ? { - repoDir: acquired.rootDir, - commit: acquired.commit, - ...(effectiveRef ? { ref: effectiveRef } : {}), - } - : undefined; + let reuse: CacheReuse | undefined; + if (acquired.git && acquired.commit) { + reuse = { repoDir: acquired.rootDir, commit: acquired.commit }; + if (effectiveRef) {reuse.ref = effectiveRef;} + } // Plugin presence classifies the entire source; bundled and standalone skills stay hidden. if (plugins.length > 0) { @@ -695,6 +726,7 @@ async function executeAdd(opts: AddOptions): Promise { interactive, all, command, + prompts, ); if (acquired.local) { const managedPluginsDir = await physicalPath(scope.pluginsDir); @@ -735,6 +767,7 @@ async function executeAdd(opts: AddOptions): Promise { sourceForStorage, interactive, command, + prompts, ); return persistSkills(selection); } @@ -746,6 +779,7 @@ export async function runAdd(opts: AddOptions): Promise { export default async function add( args: string[], context: CommandContext, + prompts: AddPrompts = DEFAULT_ADD_PROMPTS, ): Promise { const { positionals, values } = parseArgs({ args, @@ -791,7 +825,7 @@ export default async function add( const interactive = process.stdout.isTTY === true && !names && !values["all"]; const progress = process.stdout.isTTY === true - ? clack.spinner({ indicator: "timer" }) + ? prompts.spinner({ indicator: "timer" }) : undefined; const { kind, result, action } = await executeAdd({ scope, @@ -801,6 +835,7 @@ export default async function add( all: values["all"], interactive, progress, + prompts, }); if (action === "refreshed") { if (Array.isArray(result)) { diff --git a/packages/dotagents/src/cli/commands/doctor.ts b/packages/dotagents/src/cli/commands/doctor.ts index 3385af0d..71e6e92d 100644 --- a/packages/dotagents/src/cli/commands/doctor.ts +++ b/packages/dotagents/src/cli/commands/doctor.ts @@ -1,6 +1,7 @@ import { existsSync } from "node:fs"; import { readFile, writeFile } from "node:fs/promises"; import { parse as parseTOML } from "smol-toml"; +import { isObject } from "../../utils/type-guards.js"; import { parseArgs } from "node:util"; import chalk from "chalk"; import { filterManagedPluginSkillNames } from "../../gitignore/skills.js"; @@ -100,9 +101,9 @@ export async function runDoctor(opts: DoctorOptions): Promise { if (lockfile && existsSync(scope.lockPath)) { const rawLock = parseTOML(await readFile(scope.lockPath, "utf-8")); const rawSkills = rawLock["skills"]; - const hasLegacyLockFields = rawSkills !== null && typeof rawSkills === "object" && + const hasLegacyLockFields = isObject(rawSkills) && !Array.isArray(rawSkills) && !(rawSkills instanceof Date) && Object.values(rawSkills).some( - (skill) => skill !== null && typeof skill === "object" && + (skill) => isObject(skill) && !Array.isArray(skill) && !(skill instanceof Date) && ("commit" in skill || "integrity" in skill), ); diff --git a/packages/dotagents/src/cli/commands/init.test.ts b/packages/dotagents/src/cli/commands/init.test.ts index d5817fe9..3adf4c73 100644 --- a/packages/dotagents/src/cli/commands/init.test.ts +++ b/packages/dotagents/src/cli/commands/init.test.ts @@ -4,11 +4,17 @@ import { mkdtemp, mkdir, rm, writeFile, readFile, lstat, readdir } from "node:fs import { join } from "node:path"; import { tmpdir } from "node:os"; import { existsSync } from "node:fs"; -import init, { runInit, InitError, installPostMergeHook } from "./init.js"; +import init, { + runInit as runInitCommand, + InitError, + installPostMergeHook, + type InitOptions, + type InitServices, +} from "./init.js"; import { loadConfig } from "../../config/loader.js"; -vi.mock("./install.js", () => ({ - runInstall: vi.fn().mockResolvedValue({ +const services = { + runInstall: vi.fn(async () => ({ installed: [], installedPlugins: [], pruned: [], @@ -17,8 +23,12 @@ vi.mock("./install.js", () => ({ hookWarnings: [], subagentWarnings: [], pluginWarnings: [], - }), -})); + })), +} satisfies InitServices; + +function runInit(opts: Omit): Promise { + return runInitCommand({ ...opts, services }); +} describe("runInit", () => { let dir: string; @@ -53,7 +63,7 @@ describe("runInit", () => { try { process.chdir(child); - await init(["--agents", "claude"], { scope: resolveScope("project", dir) }); + await init(["--agents", "claude"], { scope: resolveScope("project", dir) }, services); } finally { process.chdir(cwd); } @@ -331,7 +341,7 @@ describe("init hook migration", () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); const log = vi.spyOn(console, "log").mockImplementation(() => {}); - await init([], { scope: resolveScope("project", dir) }); + await init([], { scope: resolveScope("project", dir) }, services); const hook = await readFile(hookPath, "utf-8"); expect(hook).toContain("dotagents --project install"); @@ -348,7 +358,7 @@ describe("init hook migration", () => { await mkdir(join(dir, ".git", "hooks", "post-merge"), { recursive: true }); const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - await init(["--agents", "claude"], { scope: resolveScope("project", dir) }); + await init(["--agents", "claude"], { scope: resolveScope("project", dir) }, services); expect(existsSync(join(dir, "agents.toml"))).toBe(true); expect(warn).toHaveBeenCalledWith(expect.stringContaining("--project doctor --fix")); diff --git a/packages/dotagents/src/cli/commands/init.ts b/packages/dotagents/src/cli/commands/init.ts index 6a7ca53b..c5b9b1fd 100644 --- a/packages/dotagents/src/cli/commands/init.ts +++ b/packages/dotagents/src/cli/commands/init.ts @@ -32,8 +32,15 @@ export interface InitOptions { trust?: TrustConfig; skills?: Array<{ name: string; source: string }>; scope: ScopeRoot; + services?: InitServices; } +export interface InitServices { + runInstall: typeof runInstall; +} + +const DEFAULT_INIT_SERVICES: InitServices = { runInstall }; + export async function runInit(opts: InitOptions): Promise { const { scope, force, agents } = opts; let { trust } = opts; @@ -96,7 +103,7 @@ export async function runInit(opts: InitOptions): Promise { // Auto-install declared skills (best-effort — may fail offline) if (config.skills.length > 0) { try { - await runInstall({ scope }); + await (opts.services ?? DEFAULT_INIT_SERVICES).runInstall({ scope }); } catch (err) { // Re-throw structured errors — TrustError is a policy violation, GitError // carries the auth-required SSH hint. Both deserve a hard fail with the @@ -159,7 +166,11 @@ const BANNER = ` |___/ `; -async function runInteractiveInit(scope: ScopeRoot, force?: boolean): Promise { +async function runInteractiveInit( + scope: ScopeRoot, + force?: boolean, + services: InitServices = DEFAULT_INIT_SERVICES, +): Promise { function cancelled(): never { clack.outro("Setup cancelled."); // eslint-disable-next-line no-restricted-syntax @@ -219,6 +230,7 @@ async function runInteractiveInit(scope: ScopeRoot, force?: boolean): Promise { +export default async function init( + args: string[], + context: CommandContext, + services: InitServices = DEFAULT_INIT_SERVICES, +): Promise { const { values } = parseArgs({ args, options: { @@ -297,7 +313,7 @@ export default async function init(args: string[], context: CommandContext): Pro // Interactive mode: TTY with no --agents flag if (process.stdout.isTTY && values["agents"] === undefined) { - await runInteractiveInit(scope, values["force"]); + await runInteractiveInit(scope, values["force"], services); return; } @@ -305,7 +321,7 @@ export default async function init(args: string[], context: CommandContext): Pro ? values["agents"].split(",").map((s) => s.trim()).filter(Boolean) : undefined; - await runInit({ scope, force: values["force"], agents }); + await runInit({ scope, force: values["force"], agents, services }); } catch (err) { if (err instanceof CancelledError) {return;} if (err instanceof TrustError) { diff --git a/packages/dotagents/src/cli/commands/install.test.ts b/packages/dotagents/src/cli/commands/install.test.ts index 86f94823..6601ec46 100644 --- a/packages/dotagents/src/cli/commands/install.test.ts +++ b/packages/dotagents/src/cli/commands/install.test.ts @@ -5,7 +5,7 @@ import { dirname, join, relative, resolve } from "node:path"; import { tmpdir } from "node:os"; import install, { runInstall as runInstallCommand, InstallError, type InstallOptions, type InstallResult } from "./install.js"; import { runSync } from "./sync.js"; -import { exec, type SerializedObject } from "@sentry/dotagents-lib"; +import { exec, isSerializedObject, type SerializedObject, type SerializedValue } from "@sentry/dotagents-lib"; import { loadLockfile } from "../../lockfile/loader.js"; import { writeLockfile } from "../../lockfile/writer.js"; import type { Lockfile } from "../../lockfile/schema.js"; @@ -31,10 +31,29 @@ Review the current diff. `; type HarnessEntry = - | { json: unknown } + | { json: SerializedValue } | { text: string } | { symlink: string }; +interface HarnessFiles extends Record {} + +function parseJsonObject(content: string): SerializedObject { + const parsed = JSON.parse(content); + if (!isSerializedObject(parsed)) {throw new Error("expected a serialized JSON object");} + return parsed; +} + +function objectValue(value: SerializedValue | undefined): SerializedObject { + if (!isSerializedObject(value)) {throw new Error("expected a serialized object value");} + return value; +} + +function arrayField(document: SerializedObject, key: string): SerializedValue[] { + const value = document[key]; + if (!Array.isArray(value)) {throw new Error(`expected array field ${key}`);} + return value; +} + function componentMarkerContent(linkPath: string, targetPath: string): string { return `managedBy=dotagents\ntarget=${JSON.stringify(relative(dirname(linkPath), targetPath))}\n`; } @@ -204,7 +223,9 @@ source = "path:plugin-source/review-tools" source: "path:plugin-source/review-tools", }); - const codexMarketplace = JSON.parse(await readFile(join(projectRoot, ".agents", "plugins", "marketplace.json"), "utf-8")) as SerializedObject; + const codexMarketplace = parseJsonObject( + await readFile(join(projectRoot, ".agents", "plugins", "marketplace.json"), "utf-8"), + ); expect(codexMarketplace).toEqual({ interface: { displayName: "Dotagents Plugins", @@ -230,8 +251,8 @@ source = "path:plugin-source/review-tools" }, ], }); - const codexPlugin = (codexMarketplace["plugins"] as Array<{ source: { path: string } }>)[0]!; - expect(resolve(projectRoot, codexPlugin["source"].path)).toBe( + const codexPlugin = objectValue(arrayField(codexMarketplace, "plugins")[0]); + expect(resolve(projectRoot, String(objectValue(codexPlugin["source"])["path"]))).toBe( join(projectRoot, ".agents", "plugins", "review-tools"), ); @@ -252,13 +273,17 @@ source = "path:plugin-source/review-tools" ] } `); - const claudeMarketplace = JSON.parse(claudeMarketplaceJson) as { plugins: Array<{ source: string }> }; - expect(resolve(projectRoot, claudeMarketplace["plugins"][0]!["source"])).toBe( + const claudeMarketplace = parseJsonObject(claudeMarketplaceJson); + const claudePlugin = objectValue(arrayField(claudeMarketplace, "plugins")[0]); + expect(resolve(projectRoot, String(claudePlugin["source"]))).toBe( join(projectRoot, ".agents", "plugins", "review-tools"), ); expect(await readFile(join(projectRoot, ".cursor-plugin", "marketplace.json"), "utf-8")).toBe(claudeMarketplaceJson); - const claudeManifest = JSON.parse(await readFile(join(projectRoot, ".agents", "plugins", "review-tools", ".claude-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const claudeManifest = parseJsonObject(await readFile( + join(projectRoot, ".agents", "plugins", "review-tools", ".claude-plugin", "plugin.json"), + "utf-8", + )); expect(claudeManifest["name"]).toBe("review-tools"); expect(claudeManifest["skills"]).toBe("./skills"); expect(claudeManifest["commands"]).toBe("./commands"); @@ -266,7 +291,10 @@ source = "path:plugin-source/review-tools" expect(claudeManifest["category"]).toBeUndefined(); expect(claudeManifest["metadata"]).toBeUndefined(); - const codexManifest = JSON.parse(await readFile(join(projectRoot, ".agents", "plugins", "review-tools", ".codex-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const codexManifest = parseJsonObject(await readFile( + join(projectRoot, ".agents", "plugins", "review-tools", ".codex-plugin", "plugin.json"), + "utf-8", + )); expect(codexManifest["name"]).toBe("review-tools"); expect(codexManifest["skills"]).toBe("./skills"); expect(codexManifest["commands"]).toBe("./commands"); @@ -358,7 +386,7 @@ source = "path:plugin-source/portable-tools" skills: "./skills", version: "1.0.0", }; - const canonicalFiles: Record = { + const canonicalFiles: HarnessFiles = { ".agents/plugins/portable-tools/.dotagents-managed": { text: "managedBy=dotagents\n" }, ".agents/plugins/portable-tools/com.example.client/agents/should-not-project.md": { text: extensionAgent }, ".agents/plugins/portable-tools/com.example.client/commands/should-not-project.md": { text: extensionCommand }, @@ -368,7 +396,7 @@ source = "path:plugin-source/portable-tools" ".agents/plugins/portable-tools/server.mjs": { text: serverSource }, ".agents/plugins/portable-tools/skills": { symlink: join(installedDir, "linked-skills") }, }; - const addNativeManifest = (dir: string, manifest: unknown): void => { + const addNativeManifest = (dir: string, manifest: SerializedValue): void => { canonicalFiles[`.agents/plugins/portable-tools/${dir}/plugin.json`] = { json: manifest }; canonicalFiles[`.agents/plugins/portable-tools/${dir}/plugin.json.dotagents-managed`] = { text: "managedBy=dotagents\n" }; }; @@ -435,7 +463,7 @@ source = "path:plugin-source/portable-tools" }); } - const targetOutputPaths: Record = { + const targetOutputPaths = { claude: [".claude-plugin"], cursor: [".cursor-plugin"], codex: [".agents/plugins/marketplace.json"], @@ -446,7 +474,7 @@ source = "path:plugin-source/portable-tools" ".opencode/agents", ], pi: [".agents/skills/portable-qa", ".agents/skills/.dotagents-managed/portable-qa"], - }; + } satisfies Record; await runSync({ scope }); for (const [otherTarget, paths] of Object.entries(targetOutputPaths)) { if (otherTarget === target) {continue;} @@ -560,7 +588,7 @@ source = "path:plugin-source/review-tools" it.each(["directory", "wrong-content file"] as const)( "does not overwrite an existing plugin destination with an unmanaged marker %s", - async (markerShape) => { + async (ownershipMarkerKind) => { const sourceDir = join(projectRoot, "plugin-source", "review-tools"); await mkdir(join(sourceDir, "skills", "review"), { recursive: true }); await writeFile(join(sourceDir, "plugin.json"), JSON.stringify({ name: "review-tools" }, null, 2)); @@ -568,7 +596,7 @@ source = "path:plugin-source/review-tools" const existingDir = join(projectRoot, ".agents", "plugins", "review-tools"); await mkdir(existingDir, { recursive: true }); - if (markerShape === "directory") { + if (ownershipMarkerKind === "directory") { await mkdir(join(existingDir, DOTAGENTS_MANAGED_PLUGIN_MARKER)); } else { await writeFile(join(existingDir, DOTAGENTS_MANAGED_PLUGIN_MARKER), "owned-by=user\n"); @@ -597,7 +625,7 @@ source = "path:plugin-source/review-tools" const scope = resolveScope("project", projectRoot); await expect(runInstall({ scope })).rejects.toThrow(/install destination already exists and is not managed/); - const existingManifest = JSON.parse(await readFile(join(existingDir, "plugin.json"), "utf-8")) as SerializedObject; + const existingManifest = parseJsonObject(await readFile(join(existingDir, "plugin.json"), "utf-8")); expect(existingManifest["description"]).toBe("Hand written"); }, ); @@ -715,7 +743,7 @@ source = "path:plugin-source/review-tools" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse(await readFile(join(existingDir, "plugin.json"), "utf-8")) as SerializedObject; + const installedManifest = parseJsonObject(await readFile(join(existingDir, "plugin.json"), "utf-8")); expect(installedManifest["description"]).toBe("Updated plugin"); expect(existsSync(join(existingDir, "skills", "review", "SKILL.md"))).toBe(true); expect(existsSync(join(existingDir, "skills", "old-review", "SKILL.md"))).toBe(false); @@ -749,7 +777,7 @@ source = "path:plugin-source/review-tools" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse(await readFile(join(existingDir, "plugin.json"), "utf-8")) as SerializedObject; + const installedManifest = parseJsonObject(await readFile(join(existingDir, "plugin.json"), "utf-8")); expect(installedManifest["description"]).toBe("Recovered plugin"); expect(existsSync(join(existingDir, "skills", "review", "SKILL.md"))).toBe(true); expect(existsSync(join(existingDir, "skills", "partial", "SKILL.md"))).toBe(false); @@ -790,7 +818,7 @@ source = "path:plugin-source/review-tools" const scope = resolveScope("project", projectRoot); await expect(runInstall({ scope })).rejects.toThrow(/install destination already exists and is not managed/); - const installedManifest = JSON.parse(await readFile(join(existingDir, "plugin.json"), "utf-8")) as SerializedObject; + const installedManifest = parseJsonObject(await readFile(join(existingDir, "plugin.json"), "utf-8")); expect(installedManifest["description"]).toBe("In-place plugin"); const lockfile = await loadLockfile(join(projectRoot, "agents.lock")); expect(lockfile!.plugins["review-tools"]).toEqual({ @@ -1204,9 +1232,9 @@ source = "path:plugin-source" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installed = JSON.parse( + const installed = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(installed["description"]).toBe("Canonical plugin"); }); @@ -1253,12 +1281,12 @@ source = "path:plugin-source" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse( + const installedManifest = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", "plugin.json"), "utf-8"), - ) as SerializedObject; - const codexManifest = JSON.parse( + ); + const codexManifest = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", ".codex-plugin", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(installedManifest).toEqual(sourceManifest); expect(codexManifest["description"]).toBe("Source description"); @@ -1307,9 +1335,9 @@ source = "path:plugin-source" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse( + const installedManifest = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(installedManifest["description"]).toBe("Canonical marketplace plugin"); }); @@ -1364,9 +1392,9 @@ source = "path:plugin-source" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse( + const installedManifest = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(installedManifest["description"]).toBe( "Fallback local plugin", ); @@ -1410,9 +1438,9 @@ source = "path:plugin-source" const scope = resolveScope("project", projectRoot); await runInstall({ scope }); - const installedManifest = JSON.parse( + const installedManifest = parseJsonObject( await readFile(join(projectRoot, ".agents", "plugins", "review-tools", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(installedManifest["description"]).toBe("Fallback local plugin"); }); @@ -1741,7 +1769,8 @@ path = "code-reviewer.md" } expect(error).toBeInstanceOf(InstallError); - expect((error as Error).message).toContain( + if (!(error instanceof Error)) {throw new Error("expected install to reject");} + expect(error.message).toContain( "Subagent file exists and is not managed by dotagents", ); expect(await readFile(installedPath, "utf-8")).toBe("hand-written subagent\n"); @@ -2530,7 +2559,8 @@ path = "reviewer.md" expect(lockfile).not.toBeNull(); expect(lockfile!.skills["pdf"]!).toBeDefined(); // Should have resolved to the old commit, not HEAD - const locked = lockfile!.skills["pdf"]! as { resolved_commit?: string }; + const locked = lockfile!.skills["pdf"]!; + if (!("resolved_commit" in locked)) {throw new Error("expected a git lock entry");} expect(locked.resolved_commit).toBe(oldSha.trim()); }); diff --git a/packages/dotagents/src/cli/commands/install/skills.ts b/packages/dotagents/src/cli/commands/install/skills.ts index d0531486..fa606327 100644 --- a/packages/dotagents/src/cli/commands/install/skills.ts +++ b/packages/dotagents/src/cli/commands/install/skills.ts @@ -102,13 +102,14 @@ async function expandSkills( function lockEntryForSkill(dep: SkillDependency, resolved: ResolvedSkill): LockedSkill { if (resolved.type === "git") { - return { + const entry: LockedSkill = { source: dep.source, resolved_url: resolved.resolvedUrl, resolved_path: resolved.resolvedPath, - ...(resolved.resolvedRef ? { resolved_ref: resolved.resolvedRef } : {}), resolved_commit: resolved.commit, }; + if (resolved.resolvedRef) {entry.resolved_ref = resolved.resolvedRef;} + return entry; } if (resolved.type === "well-known") { return { @@ -116,10 +117,11 @@ function lockEntryForSkill(dep: SkillDependency, resolved: ResolvedSkill): Locke resolved_url: resolved.resolvedUrl, }; } - return { + const entry: LockedSkill = { source: dep.source, - ...(resolved.resolvedPath ? { resolved_path: resolved.resolvedPath } : {}), }; + if (resolved.resolvedPath) {entry.resolved_path = resolved.resolvedPath;} + return entry; } /** Resolves, copies, and prunes canonical skill directories for install. */ diff --git a/packages/dotagents/src/cli/commands/mcp.ts b/packages/dotagents/src/cli/commands/mcp.ts index 5efeb604..c434f20f 100644 --- a/packages/dotagents/src/cli/commands/mcp.ts +++ b/packages/dotagents/src/cli/commands/mcp.ts @@ -71,10 +71,16 @@ export async function runMcpAdd(opts: McpAddOptions): Promise { const entry: McpConfig = { name, - ...(command ? { command, args: opts.args } : {}), - ...(url ? { url, headers: buildHeaders(opts.headers) } : {}), env: opts.env ?? [], }; + if (command) { + entry.command = command; + entry.args = opts.args; + } + if (url) { + entry.url = url; + entry.headers = buildHeaders(opts.headers); + } await addMcpToConfig(scope.configPath, entry); await runInstall({ scope }); diff --git a/packages/dotagents/src/cli/commands/remove.test.ts b/packages/dotagents/src/cli/commands/remove.test.ts index f3e75c88..7a988fcd 100644 --- a/packages/dotagents/src/cli/commands/remove.test.ts +++ b/packages/dotagents/src/cli/commands/remove.test.ts @@ -640,7 +640,8 @@ source = "path:plugins/review-tools" expect.unreachable("should have thrown"); } catch (err) { expect(err).toBeInstanceOf(WildcardSkillRemoveError); - expect((err as WildcardSkillRemoveError).source).toBe(`git:${repoDir}`); + if (!(err instanceof WildcardSkillRemoveError)) {throw err;} + expect(err.source).toBe(`git:${repoDir}`); } }); diff --git a/packages/dotagents/src/cli/commands/remove.ts b/packages/dotagents/src/cli/commands/remove.ts index 1646b91b..d1cfdab7 100644 --- a/packages/dotagents/src/cli/commands/remove.ts +++ b/packages/dotagents/src/cli/commands/remove.ts @@ -378,7 +378,7 @@ export default async function remove(args: string[], context: CommandContext): P return; } - const skipConfirm = values.yes as boolean; + const skipConfirm = values.yes; try { const { scope } = context; diff --git a/packages/dotagents/src/cli/commands/trust.ts b/packages/dotagents/src/cli/commands/trust.ts index 57ce08f5..9ff689ff 100644 --- a/packages/dotagents/src/cli/commands/trust.ts +++ b/packages/dotagents/src/cli/commands/trust.ts @@ -16,10 +16,15 @@ export class TrustCommandError extends Error { type TrustField = "github_orgs" | "github_repos" | "git_domains"; +export interface ClassifiedTrustSource { + field: TrustField; + value: string; +} + export function classifyTrustSource( source: string, defaultRepositorySource?: RepositorySource, -): { field: TrustField; value: string } { +): ClassifiedTrustSource { // When configured for GitLab, expand shorthands (no dots) to gitlab.com/... domain paths if (defaultRepositorySource === "gitlab" && !source.includes(".")) { return { field: "git_domains", value: `gitlab.com/${source}` }; diff --git a/packages/dotagents/src/cli/ensure-user-scope.test.ts b/packages/dotagents/src/cli/ensure-user-scope.test.ts index 769bf29f..b1b944e6 100644 --- a/packages/dotagents/src/cli/ensure-user-scope.test.ts +++ b/packages/dotagents/src/cli/ensure-user-scope.test.ts @@ -7,6 +7,18 @@ import { ensureUserScopeBootstrapped } from "./ensure-user-scope.js"; import { allAgentIds } from "../targets/registry.js"; import type { ScopeRoot } from "../scope.js"; +function userScope(home: string): ScopeRoot { + return { + scope: "user", + root: home, + agentsDir: home, + configPath: join(home, "agents.toml"), + lockPath: join(home, "agents.lock"), + skillsDir: join(home, "skills"), + pluginsDir: join(home, "plugins"), + }; +} + describe("ensureUserScopeBootstrapped", () => { let tmpDir: string; @@ -20,18 +32,6 @@ describe("ensureUserScopeBootstrapped", () => { await rm(tmpDir, { recursive: true, force: true }); }); - function userScope(home: string): ScopeRoot { - return { - scope: "user", - root: home, - agentsDir: home, - configPath: join(home, "agents.toml"), - lockPath: join(home, "agents.lock"), - skillsDir: join(home, "skills"), - pluginsDir: join(home, "plugins"), - }; - } - it("creates agents.toml and skills/ when user scope is uninitialized", async () => { const scope = userScope(tmpDir); await ensureUserScopeBootstrapped(scope); diff --git a/packages/dotagents/src/cli/help.ts b/packages/dotagents/src/cli/help.ts index 16ffe606..2e773dec 100644 --- a/packages/dotagents/src/cli/help.ts +++ b/packages/dotagents/src/cli/help.ts @@ -1,4 +1,4 @@ -const COMMAND_HELP: Record = { +const COMMAND_HELP = { init: `Usage: npx @sentry/dotagents [--project|--global|--user] init [options] Initialize agents.toml and the selected scope's managed directories. @@ -125,6 +125,15 @@ const SCOPE_HELP = `Scope: export function getCommandHelp(command: string, args: string[]): string | undefined { if (!args.some((arg) => arg === "--help" || arg === "-h")) {return undefined;} const subcommand = args[0]; - const help = COMMAND_HELP[subcommand ? `${command} ${subcommand}` : command] ?? COMMAND_HELP[command]; + const fullCommand = subcommand ? `${command} ${subcommand}` : command; + const help = commandHelp(fullCommand) ?? commandHelp(command); return help ? `${help}\n\n${SCOPE_HELP}` : undefined; } + +function commandHelp(command: string): string | undefined { + return isCommandHelp(command) ? COMMAND_HELP[command] : undefined; +} + +function isCommandHelp(command: string): command is keyof typeof COMMAND_HELP { + return Object.hasOwn(COMMAND_HELP, command); +} diff --git a/packages/dotagents/src/cli/index.test.ts b/packages/dotagents/src/cli/index.test.ts index 6aa1e059..c0ef66d2 100644 --- a/packages/dotagents/src/cli/index.test.ts +++ b/packages/dotagents/src/cli/index.test.ts @@ -2,28 +2,35 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { mkdtemp, mkdir, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; - -const init = vi.fn(); -const install = vi.fn(); -const add = vi.fn(); -const remove = vi.fn(); -const sync = vi.fn(); -const list = vi.fn(); -const mcp = vi.fn(); -const trust = vi.fn(); -const doctor = vi.fn(); +import { resolveProjectScope, resolveScope } from "../scope.js"; +import { main, type CliServices, type CommandHandler } from "./main.js"; + +const init = vi.fn(async () => {}); +const install = vi.fn(async () => {}); +const add = vi.fn(async () => {}); +const remove = vi.fn(async () => {}); +const sync = vi.fn(async () => {}); +const list = vi.fn(async () => {}); +const mcp = vi.fn(async () => {}); +const trust = vi.fn(async () => {}); +const doctor = vi.fn(async () => {}); const checkForUpdate = vi.fn(() => Promise.resolve(null)); -vi.mock("./commands/init.js", () => ({ default: init })); -vi.mock("./commands/install.js", () => ({ default: install })); -vi.mock("./commands/add.js", () => ({ default: add })); -vi.mock("./commands/remove.js", () => ({ default: remove })); -vi.mock("./commands/sync.js", () => ({ default: sync })); -vi.mock("./commands/list.js", () => ({ default: list })); -vi.mock("./commands/mcp.js", () => ({ default: mcp })); -vi.mock("./commands/trust.js", () => ({ default: trust })); -vi.mock("./commands/doctor.js", () => ({ default: doctor })); -vi.mock("./update-notifier.js", () => ({ checkForUpdate })); +const services = { + commands: { init, install, add, remove, sync, list, mcp, trust, doctor }, + checkForUpdate, + resolveProjectScope: (projectRoot: string) => resolveScope("project", projectRoot), + resolveScope, +} satisfies CliServices; + +const scopeServices = { + ...services, + resolveProjectScope, +} satisfies CliServices; + +function runMain(argv: string[], cliServices: CliServices = services): Promise { + return main(argv, cliServices); +} const COMMAND_CASES = [ ["init", init], @@ -41,7 +48,9 @@ describe("CLI help dispatch", () => { const originalExitCode = process.exitCode; beforeEach(() => { - for (const [, handler] of COMMAND_CASES) {handler.mockReset();} + for (const [, handler] of COMMAND_CASES) { + handler.mockReset(); + } checkForUpdate.mockClear(); process.exitCode = originalExitCode; }); @@ -52,9 +61,7 @@ describe("CLI help dispatch", () => { it("prints command help without running the command", async () => { const log = vi.spyOn(console, "log").mockImplementation(() => {}); - const { main } = await import("./main.js"); - - await main(["sync", "--help"]); + await runMain(["sync", "--help"]); expect(sync).not.toHaveBeenCalled(); expect(checkForUpdate).not.toHaveBeenCalled(); @@ -64,9 +71,7 @@ describe("CLI help dispatch", () => { }); it("defaults to global scope", async () => { - const { main } = await import("./main.js"); - - await main(["sync"]); + await runMain(["sync"]); expect(sync).toHaveBeenCalledWith([], { scope: expect.objectContaining({ scope: "user" }), @@ -74,9 +79,7 @@ describe("CLI help dispatch", () => { }); it.each(["--user", "--global"])("passes %s as explicit global scope", async (scopeFlag) => { - const { main } = await import("./main.js"); - - await main(["sync", scopeFlag]); + await runMain(["sync", scopeFlag]); expect(sync).toHaveBeenCalledWith([], { scope: expect.objectContaining({ scope: "user" }), @@ -84,9 +87,7 @@ describe("CLI help dispatch", () => { }); it("accepts both user scope aliases together", async () => { - const { main } = await import("./main.js"); - - await main(["--user", "--global", "sync"]); + await runMain(["--user", "--global", "sync"]); expect(sync).toHaveBeenCalledWith([], { scope: expect.objectContaining({ scope: "user" }), @@ -97,9 +98,7 @@ describe("CLI help dispatch", () => { ["before", ["--project", "sync"]], ["after", ["sync", "--project"]], ])("passes project scope with the flag %s the command", async (_placement, argv) => { - const { main } = await import("./main.js"); - - await main(argv); + await runMain(argv); expect(sync).toHaveBeenCalledWith([], { scope: expect.objectContaining({ scope: "project" }), @@ -108,9 +107,7 @@ describe("CLI help dispatch", () => { it("rejects contradictory scope flags without running the command", async () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); - const { main } = await import("./main.js"); - - await main(["--project", "sync", "--global"]); + await runMain(["--project", "sync", "--global"]); expect(sync).not.toHaveBeenCalled(); expect(process.exitCode).toBe(1); @@ -120,9 +117,7 @@ describe("CLI help dispatch", () => { it("rejects a project conflict with the legacy alias", async () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); - const { main } = await import("./main.js"); - - await main(["sync", "--project", "--user"]); + await runMain(["sync", "--project", "--user"]); expect(sync).not.toHaveBeenCalled(); expect(process.exitCode).toBe(1); @@ -132,9 +127,7 @@ describe("CLI help dispatch", () => { it("documents the default and compatibility flags in top-level help", async () => { const log = vi.spyOn(console, "log").mockImplementation(() => {}); - const { main } = await import("./main.js"); - - await main([]); + await runMain([]); expect(log).toHaveBeenCalledWith(expect.stringContaining("--project Operate on the current project instead of global scope")); expect(log).toHaveBeenCalledWith(expect.stringContaining("--global Explicitly operate on global scope (~/.agents/, the default)")); @@ -151,7 +144,9 @@ describe("scope isolation for all commands", () => { let originalCwd: string; beforeEach(async () => { - for (const [, handler] of COMMAND_CASES) {handler.mockReset();} + for (const [, handler] of COMMAND_CASES) { + handler.mockReset(); + } checkForUpdate.mockClear(); process.exitCode = undefined; originalCwd = process.cwd(); @@ -174,9 +169,7 @@ describe("scope isolation for all commands", () => { }); it.each(COMMAND_CASES)("uses only global state for unqualified %s", async (command, handler) => { - const { main } = await import("./main.js"); - - await main([command]); + await runMain([command], scopeServices); expect(handler).toHaveBeenCalledOnce(); expect(handler).toHaveBeenCalledWith([], { @@ -185,9 +178,7 @@ describe("scope isolation for all commands", () => { }); it.each(COMMAND_CASES)("uses only project state for explicit-project %s", async (command, handler) => { - const { main } = await import("./main.js"); - - await main(["--project", command]); + await runMain(["--project", command], scopeServices); expect(handler).toHaveBeenCalledOnce(); expect(handler).toHaveBeenCalledWith([], { @@ -198,9 +189,7 @@ describe("scope isolation for all commands", () => { it("fails a project command with no config instead of falling back globally", async () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); await rm(join(projectRoot, "agents.toml")); - const { main } = await import("./main.js"); - - await main(["--project", "sync"]); + await runMain(["--project", "sync"], scopeServices); expect(sync).not.toHaveBeenCalled(); expect(error).toHaveBeenCalledWith(expect.stringContaining("--project init")); @@ -210,9 +199,7 @@ describe("scope isolation for all commands", () => { it("allows project init in the current directory outside Git", async () => { await rm(join(projectRoot, ".git"), { recursive: true }); await rm(join(projectRoot, "agents.toml")); - const { main } = await import("./main.js"); - - await main(["--project", "init"]); + await runMain(["--project", "init"], scopeServices); expect(init).toHaveBeenCalledWith([], { scope: expect.objectContaining({ scope: "project", root: canonicalProjectRoot }), diff --git a/packages/dotagents/src/cli/index.ts b/packages/dotagents/src/cli/index.ts index d3351601..4132fca3 100644 --- a/packages/dotagents/src/cli/index.ts +++ b/packages/dotagents/src/cli/index.ts @@ -3,7 +3,7 @@ import { main } from "./main.js"; export { version } from "./main.js"; -main().catch((err: unknown) => { +main().catch((err) => { console.error(err instanceof Error ? err.message : String(err)); process.exitCode = 1; }); diff --git a/packages/dotagents/src/cli/main.ts b/packages/dotagents/src/cli/main.ts index a5089974..e6184472 100644 --- a/packages/dotagents/src/cli/main.ts +++ b/packages/dotagents/src/cli/main.ts @@ -12,16 +12,50 @@ import trust from "./commands/trust.js"; import doctor from "./commands/doctor.js"; import { getCommandHelp } from "./help.js"; import { resolveProjectScope, resolveScope, ScopeError, type Scope } from "../scope.js"; +import type { CommandContext } from "./context.js"; +import { z } from "zod/v4"; const require = createRequire(import.meta.url); -const { version } = require("../../package.json") as { version: string }; +const { version } = z.object({ version: z.string() }).parse(require("../../package.json")); export { version }; -const COMMANDS = { +export type CommandHandler = (args: string[], context: CommandContext) => Promise; + +export interface CliCommands { + init: CommandHandler; + install: CommandHandler; + add: CommandHandler; + remove: CommandHandler; + sync: CommandHandler; + list: CommandHandler; + mcp: CommandHandler; + trust: CommandHandler; + doctor: CommandHandler; +} + +export interface CliServices { + commands: CliCommands; + checkForUpdate: typeof checkForUpdate; + resolveProjectScope: typeof resolveProjectScope; + resolveScope: typeof resolveScope; +} + +const COMMANDS: CliCommands = { init, install, add, remove, sync, list, mcp, trust, doctor, -} as const; +}; type Command = keyof typeof COMMANDS; +const DEFAULT_CLI_SERVICES: CliServices = { + commands: COMMANDS, + checkForUpdate, + resolveProjectScope, + resolveScope, +}; + +function isCommand(value: string, commands: CliCommands): value is Command { + return Object.hasOwn(commands, value); +} + export interface ParsedScopeArgs { args: string[]; scope: Scope; @@ -63,7 +97,10 @@ Options: --version Show version`); } -export async function main(argv = process.argv.slice(2)): Promise { +export async function main( + argv = process.argv.slice(2), + services: CliServices = DEFAULT_CLI_SERVICES, +): Promise { let parsed: ParsedScopeArgs; try { parsed = parseScopeArgs(argv); @@ -87,14 +124,14 @@ export async function main(argv = process.argv.slice(2)): Promise { return; } - if (!Object.hasOwn(COMMANDS, first)) { + if (!isCommand(first, services.commands)) { console.error(`Unknown command: ${first}`); printUsage(); process.exitCode = 1; return; } - const command = COMMANDS[first as Command]; + const command = services.commands[first]; const commandArgs = args.slice(1); const commandHelp = getCommandHelp(first, commandArgs); if (commandHelp) { @@ -105,8 +142,8 @@ export async function main(argv = process.argv.slice(2)): Promise { let scope; try { scope = parsed.scope === "project" - ? resolveProjectScope(resolve("."), { requireConfig: first !== "init" }) - : resolveScope("user"); + ? services.resolveProjectScope(resolve("."), { requireConfig: first !== "init" }) + : services.resolveScope("user"); } catch (err) { if (err instanceof ScopeError) { console.error(err.message); @@ -116,7 +153,7 @@ export async function main(argv = process.argv.slice(2)): Promise { throw err; } - const updateMessage = checkForUpdate(version); + const updateMessage = services.checkForUpdate(version); await command(commandArgs, { scope }); const message = await updateMessage; diff --git a/packages/dotagents/src/cli/update-notifier.test.ts b/packages/dotagents/src/cli/update-notifier.test.ts index 6e2b10ad..562e7963 100644 --- a/packages/dotagents/src/cli/update-notifier.test.ts +++ b/packages/dotagents/src/cli/update-notifier.test.ts @@ -58,37 +58,25 @@ describe("checkForUpdate", () => { }); it("returns null when fetch returns non-ok status", async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: false, - json: async () => ({}), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(null, { status: 500 })); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).toBeNull(); }); it("returns null when already on the latest version", async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ version: "0.7.0" }), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ version: "0.7.0" }))); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).toBeNull(); }); it("returns null when on a newer version than registry", async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ version: "0.6.0" }), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ version: "0.6.0" }))); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).toBeNull(); }); it("returns a message when a newer version exists", async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ version: "0.8.0" }), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ version: "0.8.0" }))); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).not.toBeNull(); expect(result).toContain("0.7.0"); @@ -96,10 +84,7 @@ describe("checkForUpdate", () => { }); it("returns null when registry returns invalid json shape", async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ name: "no-version-field" }), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ name: "no-version-field" }))); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).toBeNull(); }); @@ -121,10 +106,7 @@ describe("checkForUpdate", () => { join(cacheDir, "update-check.json"), JSON.stringify({ lastCheck: Date.now() - 25 * 60 * 60 * 1000, latestVersion: "0.8.0" }), ); - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ version: "0.9.0" }), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ version: "0.9.0" }))); const result = await checkForUpdate("0.7.0", { cacheDir }); expect(result).toContain("0.9.0"); expect(fetch).toHaveBeenCalled(); diff --git a/packages/dotagents/src/cli/update-notifier.ts b/packages/dotagents/src/cli/update-notifier.ts index e7a9fad2..66a26196 100644 --- a/packages/dotagents/src/cli/update-notifier.ts +++ b/packages/dotagents/src/cli/update-notifier.ts @@ -1,6 +1,7 @@ import { readFile, writeFile, mkdir } from "node:fs/promises"; import { join } from "node:path"; import { homedir } from "node:os"; +import { z } from "zod/v4"; const DEFAULT_CACHE_DIR = join(homedir(), ".dotagents"); const ONE_DAY_MS = 24 * 60 * 60 * 1000; @@ -12,6 +13,13 @@ interface CacheData { latestVersion: string; } +const cacheDataSchema = z.object({ + lastCheck: z.number(), + latestVersion: z.string(), +}); + +const registryResponseSchema = z.object({ version: z.string() }); + /** * Compare two semver strings (x.y.z only). * Returns positive if b > a, negative if a > b, 0 if equal. @@ -28,11 +36,8 @@ export function compareSemver(a: string, b: string): number { async function readCache(cacheFile: string): Promise { try { - const data = JSON.parse(await readFile(cacheFile, "utf-8")); - if (typeof data?.lastCheck === "number" && typeof data?.latestVersion === "string") { - return { lastCheck: data.lastCheck, latestVersion: data.latestVersion }; - } - return null; + const parsed = cacheDataSchema.safeParse(JSON.parse(await readFile(cacheFile, "utf-8"))); + return parsed.success ? parsed.data : null; } catch { return null; } @@ -53,8 +58,8 @@ async function fetchLatestVersion(): Promise { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) {return null;} - const data = (await response.json()) as { version?: unknown }; - return typeof data.version === "string" ? data.version : null; + const parsed = registryResponseSchema.safeParse(await response.json()); + return parsed.success ? parsed.data.version : null; } catch { return null; } diff --git a/packages/dotagents/src/config/schema.test.ts b/packages/dotagents/src/config/schema.test.ts index 2a4f0b6c..1cfabe78 100644 --- a/packages/dotagents/src/config/schema.test.ts +++ b/packages/dotagents/src/config/schema.test.ts @@ -1,6 +1,20 @@ import { describe, it, expect } from "vitest"; import { agentsConfigSchema } from "./schema.js"; +function parseSkill(source: string) { + return agentsConfigSchema.safeParse({ + version: 1, + skills: [{ name: "test", source }], + }); +} + +function parseWithName(name: string) { + return agentsConfigSchema.safeParse({ + version: 1, + skills: [{ name, source: "owner/repo" }], + }); +} + describe("agentsConfigSchema", () => { it("parses a minimal valid config", () => { const result = agentsConfigSchema.safeParse({ version: 1 }); @@ -125,12 +139,6 @@ describe("agentsConfigSchema", () => { }); describe("source specifiers", () => { - const parseSkill = (source: string) => - agentsConfigSchema.safeParse({ - version: 1, - skills: [{ name: "test", source }], - }); - it("accepts owner/repo", () => { expect(parseSkill("anthropics/skills").success).toBe(true); }); @@ -238,12 +246,6 @@ describe("agentsConfigSchema", () => { }); describe("skill name validation", () => { - const parseWithName = (name: string) => - agentsConfigSchema.safeParse({ - version: 1, - skills: [{ name, source: "owner/repo" }], - }); - it("accepts valid skill names", () => { expect(parseWithName("pdf-processing").success).toBe(true); expect(parseWithName("my_skill").success).toBe(true); diff --git a/packages/dotagents/src/config/trust-policy-conformance.ts b/packages/dotagents/src/config/trust-policy-conformance.ts index 3d3d1b12..a8e0db1b 100644 --- a/packages/dotagents/src/config/trust-policy-conformance.ts +++ b/packages/dotagents/src/config/trust-policy-conformance.ts @@ -12,5 +12,4 @@ import type { TrustPolicy } from "@sentry/dotagents-lib"; * This file emits a tiny no-op assignment at runtime; it is here for the type * check, not the runtime behavior. */ -const _trustPolicyConformance: (t: TrustConfig) => TrustPolicy = (t) => t; -export const __trustPolicyConformance = _trustPolicyConformance; +export const trustPolicyConformance: (trust: TrustConfig) => TrustPolicy = (trust) => trust; diff --git a/packages/dotagents/src/config/writer.ts b/packages/dotagents/src/config/writer.ts index 8a0489fe..d4e9a6b8 100644 --- a/packages/dotagents/src/config/writer.ts +++ b/packages/dotagents/src/config/writer.ts @@ -14,6 +14,11 @@ export interface DefaultConfigOptions { skills?: Array<{ name: string; source: string; ref?: string; path?: string }>; } +interface TomlDependencyEntry extends Record { + name: string; + source: string; +} + /** * Add a skill entry to agents.toml. * Appends a [[skills]] block at the end of the file. @@ -27,7 +32,7 @@ export async function addSkillToConfig( const content = await readFile(filePath, "utf-8"); // Build a partial TOML object and stringify it for proper escaping - const entry: Record = { name, source: dep.source }; + const entry: TomlDependencyEntry = { name, source: dep.source }; if (dep.ref) {entry["ref"] = dep.ref;} if (dep.path) {entry["path"] = dep.path;} @@ -47,7 +52,7 @@ export async function addPluginsToConfig( } const content = await readFile(filePath, "utf-8"); const entries = plugins.map((plugin) => { - const entry: Record = { + const entry: TomlDependencyEntry = { name: plugin.name, source: plugin.source, }; @@ -289,7 +294,7 @@ function removeBlockByHeader(content: string, header: string, name: string): str ); } -function tomlValue(value: unknown): string { +function tomlValue(value: Value): string { return stringify({ v: value }).replace("v = ", "").trimEnd(); } @@ -475,7 +480,7 @@ export function generateDefaultConfig(opts?: DefaultConfigOptions | string[]): s if (options.skills && options.skills.length > 0) { for (const skill of options.skills) { - const entry: Record = { name: skill.name, source: skill.source }; + const entry: TomlDependencyEntry = { name: skill.name, source: skill.source }; if (skill.ref) {entry["ref"] = skill.ref;} if (skill.path) {entry["path"] = skill.path;} config += `\n${stringify({ skills: [entry] }).trimEnd()}\n`; diff --git a/packages/dotagents/src/gitignore/skills.ts b/packages/dotagents/src/gitignore/skills.ts index 88f2583f..799b9ed6 100644 --- a/packages/dotagents/src/gitignore/skills.ts +++ b/packages/dotagents/src/gitignore/skills.ts @@ -51,6 +51,6 @@ function isInside(path: string, root: string): boolean { return relPath === "" || (!relPath.startsWith("..") && !isAbsolute(relPath)); } -function isNotFoundError(err: unknown): boolean { +function isNotFoundError(err: ErrorValue): boolean { return err instanceof Error && "code" in err && err.code === "ENOENT"; } diff --git a/packages/dotagents/src/plugins/managed-files.ts b/packages/dotagents/src/plugins/managed-files.ts index b504e5f4..b37cec0b 100644 --- a/packages/dotagents/src/plugins/managed-files.ts +++ b/packages/dotagents/src/plugins/managed-files.ts @@ -1,11 +1,12 @@ import { lstat, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; import { dirname } from "node:path"; import { isSerializedObject, isSerializedValue, type SerializedValue } from "@sentry/dotagents-lib"; +import { hasErrorCode, isObject } from "../utils/type-guards.js"; const MANAGED_MARKER_SUFFIX = ".dotagents-managed"; let markerCounter = 0; -export function stableJson(value: unknown): string { +export function stableJson(value: Value): string { if (!isSerializedValue(value)) {throw new TypeError("Managed JSON must be serializable");} return `${JSON.stringify(sortJson(value), null, 2)}\n`; } @@ -13,7 +14,7 @@ export function stableJson(value: unknown): string { function sortJson(value: SerializedValue): SerializedValue { if (Array.isArray(value)) {return value.map(sortJson);} if (value instanceof Date) {return value;} - if (!value || typeof value !== "object") {return value;} + if (!value || !isObject(value)) {return value;} return Object.fromEntries( Object.entries(value) .toSorted(([a], [b]) => a < b ? -1 : a > b ? 1 : 0) @@ -59,7 +60,7 @@ export async function isManagedJsonFile(filePath: string): Promise { if (!isNotFoundError(err)) {throw err;} } try { - const parsed: unknown = JSON.parse(await readFile(filePath, "utf-8")); + const parsed = JSON.parse(await readFile(filePath, "utf-8")); if (!isSerializedObject(parsed)) {return false;} const metadata = parsed["metadata"]; return isSerializedObject(metadata) && metadata["managedBy"] === "dotagents"; @@ -85,7 +86,7 @@ async function writeManagedMarker(filePath: string): Promise { try { await rename(tempPath, filePath); } catch (err) { - if (!(err instanceof Error && "code" in err && ["EEXIST", "EPERM"].includes(String((err as NodeJS.ErrnoException).code)))) { + if (!(hasErrorCode(err, "EEXIST") || hasErrorCode(err, "EPERM"))) { throw err; } await rm(filePath, { force: true }); @@ -101,6 +102,6 @@ export function managedJsonMarkerPath(filePath: string): string { return `${filePath}${MANAGED_MARKER_SUFFIX}`; } -export function isNotFoundError(err: unknown): boolean { - return err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT"; +export function isNotFoundError(err: Value): boolean { + return hasErrorCode(err, "ENOENT"); } diff --git a/packages/dotagents/src/plugins/runtime/layout.ts b/packages/dotagents/src/plugins/runtime/layout.ts index e14ea45f..4087f7b0 100644 --- a/packages/dotagents/src/plugins/runtime/layout.ts +++ b/packages/dotagents/src/plugins/runtime/layout.ts @@ -2,6 +2,7 @@ import { existsSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; import type { ScopeRoot } from "../../scope.js"; +import { isString } from "../../utils/type-guards.js"; export interface PluginRuntimeLayout { claudeMarketplaceRoot: string; @@ -78,5 +79,5 @@ export function pluginRuntimeLayout(scope: ScopeRoot): PluginRuntimeLayout { } export function normalizePluginRuntimeLayout(root: PluginRuntimeRoot): PluginRuntimeLayout { - return typeof root === "string" ? projectPluginRuntimeLayout(root) : root; + return isString(root) ? projectPluginRuntimeLayout(root) : root; } diff --git a/packages/dotagents/src/plugins/runtime/manifest-values.ts b/packages/dotagents/src/plugins/runtime/manifest-values.ts index d8786db3..bb7c3686 100644 --- a/packages/dotagents/src/plugins/runtime/manifest-values.ts +++ b/packages/dotagents/src/plugins/runtime/manifest-values.ts @@ -1,11 +1,12 @@ import { isStandardPluginManifest, type LegacyPluginManifest, type PluginManifest } from "../schema.js"; +import { isString } from "../../utils/type-guards.js"; export function manifestString( manifest: PluginManifest, key: "description" | "version", ): string | undefined { const value = manifest[key]; - return typeof value === "string" ? value : undefined; + return isString(value) ? value : undefined; } export function legacyManifestString( @@ -13,8 +14,9 @@ export function legacyManifestString( key: "category", ): string | undefined { if (isStandardPluginManifest(manifest)) {return undefined;} + // SAFETY: the standard-manifest guard above leaves the legacy manifest variant. const value = (manifest as LegacyPluginManifest)[key]; - return typeof value === "string" && value.length > 0 ? value : undefined; + return isString(value) && value.length > 0 ? value : undefined; } /** Formats an already validated plugin-relative component path. */ diff --git a/packages/dotagents/src/plugins/runtime/manifests.ts b/packages/dotagents/src/plugins/runtime/manifests.ts index 671b05b2..f55580cd 100644 --- a/packages/dotagents/src/plugins/runtime/manifests.ts +++ b/packages/dotagents/src/plugins/runtime/manifests.ts @@ -14,6 +14,7 @@ import { import type { PluginWriteWarning } from "./types.js"; import { isSafeComponentPath } from "./component-paths.js"; import type { SerializedObject } from "@sentry/dotagents-lib"; +import { isString, isStringArray } from "../../utils/type-guards.js"; type ComponentManifestKey = | "skills" @@ -189,6 +190,7 @@ function cursorRuntimeManifest(plugin: PluginDeclaration, warnings: PluginWriteW function codexRuntimeManifest(plugin: PluginDeclaration, warnings: PluginWriteWarning[], standardMcpPath: string | undefined, portableSkills: boolean): SerializedObject { const standard = isStandardPluginManifest(plugin.manifest); const legacyComponents = usesLegacyPluginComponents(plugin, "codex"); + // SAFETY: legacy components are only selected for non-standard manifest fields. const legacy = legacyComponents ? plugin.manifest as LegacyPluginManifest : undefined; const manifest: SerializedObject = { name: plugin.name }; if (!standard && legacyComponents) { @@ -286,7 +288,7 @@ function codexInterface(plugin: PluginDeclaration): SerializedObject { function developerName(manifest: PluginManifest): string { const author = manifest.author; - if (author && typeof author.name === "string") {return author.name;} + if (author && isString(author.name)) {return author.name;} return "Unknown"; } @@ -307,8 +309,9 @@ function copyRuntimeComponentField( key: ComponentManifestKey, warnings: PluginWriteWarning[], ): boolean { + // SAFETY: this function is called only when legacy component fields are enabled. const value = (plugin.manifest as LegacyPluginManifest)[key]; - if (typeof value === "string") { + if (isString(value)) { if (isSafeComponentPath(value)) { dest[key] = runtimePath(value); } else { @@ -316,7 +319,7 @@ function copyRuntimeComponentField( } return true; } - if (Array.isArray(value) && value.every((item) => typeof item === "string")) { + if (isStringArray(value)) { const paths = value.flatMap((item) => { if (isSafeComponentPath(item)) {return [runtimePath(item)];} warnUnsafeComponentPath(plugin, key, item, warnings); diff --git a/packages/dotagents/src/plugins/runtime/opencode-mcp.ts b/packages/dotagents/src/plugins/runtime/opencode-mcp.ts index 5ad7bc36..c875f3be 100644 --- a/packages/dotagents/src/plugins/runtime/opencode-mcp.ts +++ b/packages/dotagents/src/plugins/runtime/opencode-mcp.ts @@ -109,13 +109,14 @@ async function openCodeDeclarations( const cwd = server.cwd?.startsWith("./") ? join(pluginRoot, server.cwd.slice(2)) : expand(server.cwd ?? "${PLUGIN_ROOT}"); - const envValues: Record = { - PLUGIN_ROOT: pluginRoot, - PLUGIN_DATA: dataDir, - }; - for (const [key, value] of Object.entries(server.env ?? {})) { - envValues[key] = expand(value); - } + const baseEnvironment = [ + ["PLUGIN_ROOT", pluginRoot], + ["PLUGIN_DATA", dataDir], + ] as const; + const pluginEnvironment = Object.entries(server.env ?? {}).map( + ([key, value]) => [key, expand(value)] as const, + ); + const envValues = Object.fromEntries([...baseEnvironment, ...pluginEnvironment]); declarations.push({ name, command, diff --git a/packages/dotagents/src/plugins/runtime/writer.test.ts b/packages/dotagents/src/plugins/runtime/writer.test.ts index dc4f826f..4618af99 100644 --- a/packages/dotagents/src/plugins/runtime/writer.test.ts +++ b/packages/dotagents/src/plugins/runtime/writer.test.ts @@ -5,14 +5,55 @@ import { dirname, join, relative, resolve } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { parse as parseJSONC } from "jsonc-parser"; import type { PluginDeclaration } from "../types.js"; -import { AGENT_PLUGIN_SCHEMA, isStandardPluginManifest } from "../schema.js"; +import { AGENT_PLUGIN_SCHEMA, isStandardPluginManifest, type LegacyPluginManifest } from "../schema.js"; import { prunePluginOutputs, projectedPiSkillNames, verifyPluginOutputs, writePluginOutputs, } from "./writer.js"; -import type { SerializedObject } from "@sentry/dotagents-lib"; +import { isSerializedObject, type SerializedObject, type SerializedValue } from "@sentry/dotagents-lib"; + +function parseJsonObject(content: string): SerializedObject { + const parsed = JSON.parse(content); + if (!isSerializedObject(parsed)) {throw new Error("expected a serialized JSON object");} + return parsed; +} + +function parseJsoncObject(content: string): SerializedObject { + const parsed = parseJSONC(content); + if (!isSerializedObject(parsed)) {throw new Error("expected a serialized JSONC object");} + return parsed; +} + +function objectValue(value: SerializedValue | undefined): SerializedObject { + if (!isSerializedObject(value)) {throw new Error("expected a serialized object value");} + return value; +} + +function arrayField(document: SerializedObject, key: string): SerializedValue[] { + const value = document[key]; + if (!Array.isArray(value)) {throw new Error(`expected array field ${key}`);} + return value; +} + +async function expectSymlinkTarget(linkPath: string, expectedTarget: string): Promise { + expect((await lstat(linkPath)).isSymbolicLink()).toBe(true); + expect(resolve(dirname(linkPath), await readlink(linkPath))).toBe(resolve(expectedTarget)); +} + +function componentMarkerContent(linkPath: string, targetPath: string): string { + return `managedBy=dotagents\ntarget=${JSON.stringify(relative(dirname(linkPath), targetPath))}\n`; +} + +async function writePluginSkill(pluginDir: string, name: string): Promise { + await mkdir(join(pluginDir, "skills", name), { recursive: true }); + await writeFile( + join(pluginDir, "skills", name, "SKILL.md"), + `---\nname: ${name}\ndescription: Plugin QA\n---\n`, + "utf-8", + ); +} describe("plugin writer", () => { let root: string; @@ -33,16 +74,21 @@ describe("plugin writer", () => { await mkdir(join(pluginDir, "skills"), { recursive: true }); await mkdir(join(pluginDir, "commands"), { recursive: true }); await mkdir(join(pluginDir, "agents"), { recursive: true }); - const manifest = (overrides.manifest && isStandardPluginManifest(overrides.manifest) - ? overrides.manifest - : { - name, - version: "1.0.0", - description: `Tools for ${name}`, - category: "Coding", - author: { name: "Sentry" }, - ...overrides.manifest, - }) as PluginDeclaration["manifest"]; + let manifest: PluginDeclaration["manifest"]; + if (overrides.manifest && isStandardPluginManifest(overrides.manifest)) { + manifest = overrides.manifest; + } else { + const legacyOverrides = overrides.manifest; + const legacyManifest: LegacyPluginManifest = { + name, + version: "1.0.0", + description: `Tools for ${name}`, + category: "Coding", + author: { name: "Sentry" }, + }; + Object.assign(legacyManifest, legacyOverrides); + manifest = legacyManifest; + } return { name, source: `path:.agents/plugins/${name}`, @@ -53,24 +99,6 @@ describe("plugin writer", () => { }; } - async function expectSymlinkTarget(linkPath: string, expectedTarget: string): Promise { - expect((await lstat(linkPath)).isSymbolicLink()).toBe(true); - expect(resolve(dirname(linkPath), await readlink(linkPath))).toBe(resolve(expectedTarget)); - } - - function componentMarkerContent(linkPath: string, targetPath: string): string { - return `managedBy=dotagents\ntarget=${JSON.stringify(relative(dirname(linkPath), targetPath))}\n`; - } - - async function writePluginSkill(pluginDir: string, name: string): Promise { - await mkdir(join(pluginDir, "skills", name), { recursive: true }); - await writeFile( - join(pluginDir, "skills", name, "SKILL.md"), - `---\nname: ${name}\ndescription: Plugin QA\n---\n`, - "utf-8", - ); - } - it("writes deterministic marketplace outputs for runtimes that need projections", async () => { const alpha = await plugin("alpha-tools"); const beta = await plugin("beta-tools"); @@ -83,7 +111,7 @@ describe("plugin writer", () => { expect(result.warnings).toEqual([]); expect(result.written).toBe(9); - const codexMarketplace = JSON.parse(await readFile(join(root, ".agents", "plugins", "marketplace.json"), "utf-8")) as SerializedObject; + const codexMarketplace = parseJsonObject(await readFile(join(root, ".agents", "plugins", "marketplace.json"), "utf-8")); expect(codexMarketplace).toEqual({ interface: { displayName: "Dotagents Plugins", @@ -123,8 +151,8 @@ describe("plugin writer", () => { }, ], }); - const codexPlugin = (codexMarketplace["plugins"] as Array<{ source: { path: string } }>)[0]!; - expect(resolve(root, codexPlugin["source"].path)).toBe(alpha.pluginDir); + const codexPlugin = objectValue(arrayField(codexMarketplace, "plugins")[0]); + expect(resolve(root, String(objectValue(codexPlugin["source"])["path"]))).toBe(alpha.pluginDir); const claudeMarketplaceJson = await readFile(join(root, ".claude-plugin", "marketplace.json"), "utf-8"); expect(claudeMarketplaceJson).toBe(`{ @@ -149,24 +177,25 @@ describe("plugin writer", () => { ] } `); - const claudeMarketplace = JSON.parse(claudeMarketplaceJson) as { plugins: Array<{ source: string }> }; - expect(resolve(root, claudeMarketplace["plugins"][0]!["source"])).toBe(alpha.pluginDir); + const claudeMarketplace = parseJsonObject(claudeMarketplaceJson); + const claudePlugin = objectValue(arrayField(claudeMarketplace, "plugins")[0]); + expect(resolve(root, String(claudePlugin["source"]))).toBe(alpha.pluginDir); expect(await readFile(join(root, ".cursor-plugin", "marketplace.json"), "utf-8")).toBe(claudeMarketplaceJson); - const claudeManifest = JSON.parse(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".claude-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const claudeManifest = parseJsonObject(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".claude-plugin", "plugin.json"), "utf-8")); expect(claudeManifest["skills"]).toBe("./skills"); expect(claudeManifest["commands"]).toBe("./commands"); expect(claudeManifest["agents"]).toBeUndefined(); expect(claudeManifest["category"]).toBeUndefined(); expect(claudeManifest["metadata"]).toBeUndefined(); - const cursorManifest = JSON.parse(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".cursor-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const cursorManifest = parseJsonObject(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".cursor-plugin", "plugin.json"), "utf-8")); expect(cursorManifest["skills"]).toBe("./skills"); expect(cursorManifest["commands"]).toBe("./commands"); expect(cursorManifest["agents"]).toBe("./agents"); expect(cursorManifest["metadata"]).toBeUndefined(); - const codexManifest = JSON.parse(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".codex-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const codexManifest = parseJsonObject(await readFile(join(root, ".agents", "plugins", "alpha-tools", ".codex-plugin", "plugin.json"), "utf-8")); expect(codexManifest["skills"]).toBe("./skills"); expect(codexManifest["commands"]).toBe("./commands"); expect(codexManifest["agents"]).toBe("./agents"); @@ -191,15 +220,15 @@ describe("plugin writer", () => { await writePluginOutputs(["codex"], [alpha], root); - const marketplace = JSON.parse( + const marketplace = parseJsonObject( await readFile(join(root, ".agents", "plugins", "marketplace.json"), "utf-8"), - ) as { plugins: Array<{ category: string }> }; - expect(marketplace.plugins[0]!.category).toBe("Productivity"); + ); + expect(objectValue(arrayField(marketplace, "plugins")[0])["category"]).toBe("Productivity"); - const manifest = JSON.parse( + const manifest = parseJsonObject( await readFile(join(alpha.pluginDir, ".codex-plugin", "plugin.json"), "utf-8"), - ) as { interface: { category: string } }; - expect(manifest.interface.category).toBe("Coding"); + ); + expect(objectValue(manifest["interface"])["category"]).toBe("Coding"); }); it("preserves explicit skills paths when repairing native manifests", async () => { @@ -215,10 +244,10 @@ describe("plugin writer", () => { await writePluginOutputs([nativeSource], [nativePlugin], root); - const manifest = JSON.parse( + const manifest = parseJsonObject( await readFile(join(nativePlugin.pluginDir, `.${nativeSource}-plugin`, "plugin.json"), "utf-8"), - ) as { skills: string }; - expect(manifest.skills).toBe("./custom-skills"); + ); + expect(manifest["skills"]).toBe("./custom-skills"); } }); @@ -290,9 +319,9 @@ describe("plugin writer", () => { expect(await readFile(adapterMcpPath, "utf-8")).toBe('{"user":"owned"}\n'); expect(existsSync(`${adapterMcpPath}.dotagents-managed`)).toBe(false); - const manifest = JSON.parse( + const manifest = parseJsonObject( await readFile(join(alpha.pluginDir, ".claude-plugin", "plugin.json"), "utf-8"), - ) as SerializedObject; + ); expect(manifest["mcpServers"]).toBeUndefined(); expect(result.warnings.some((warning) => warning.message.includes("MCP adapter exists and is not managed"))).toBe(true); }); @@ -359,14 +388,14 @@ describe("plugin writer", () => { expect(result.warnings).toEqual([]); expect(result.written).toBe(4); - const claudeManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".claude-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const claudeManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".claude-plugin", "plugin.json"), "utf-8")); expect(claudeManifest["agents"]).toBeUndefined(); expect(claudeManifest["commands"]).toEqual(["./cmds/review.md"]); expect(claudeManifest["hooks"]).toBe("./config/hooks.json"); expect(claudeManifest["mcpServers"]).toBe("./config/mcp.json"); expect(claudeManifest["skills"]).toBe("./plugin-skills"); - const cursorManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".cursor-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const cursorManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".cursor-plugin", "plugin.json"), "utf-8")); expect(cursorManifest["agents"]).toBe("./custom-agents"); expect(cursorManifest["commands"]).toEqual(["./cmds/review.md"]); expect(cursorManifest["hooks"]).toBe("./config/hooks.json"); @@ -402,9 +431,9 @@ describe("plugin writer", () => { message: 'Plugin component path "../outside" for "skills" is not a safe relative path and was skipped.', }, ]); - const claudeManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".claude-plugin", "plugin.json"), "utf-8")) as SerializedObject; - const cursorManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".cursor-plugin", "plugin.json"), "utf-8")) as SerializedObject; - const codexManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".codex-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const claudeManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".claude-plugin", "plugin.json"), "utf-8")); + const cursorManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".cursor-plugin", "plugin.json"), "utf-8")); + const codexManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".codex-plugin", "plugin.json"), "utf-8")); expect(claudeManifest["skills"]).toBeUndefined(); expect(cursorManifest["skills"]).toBeUndefined(); expect(codexManifest["skills"]).toBeUndefined(); @@ -426,7 +455,7 @@ describe("plugin writer", () => { message: 'Plugin component path "\\outside" for "skills" is not a safe relative path and was skipped.', }, ]); - const codexManifest = JSON.parse(await readFile(join(alpha.pluginDir, ".codex-plugin", "plugin.json"), "utf-8")) as SerializedObject; + const codexManifest = parseJsonObject(await readFile(join(alpha.pluginDir, ".codex-plugin", "plugin.json"), "utf-8")); expect(codexManifest["skills"]).toBeUndefined(); }); @@ -518,11 +547,11 @@ describe("plugin writer", () => { it.each(["directory", "wrong-content file"] as const)( "does not treat a Grok marker %s as projection ownership", - async (markerShape) => { + async (ownershipMarkerKind) => { const alpha = await plugin("alpha-tools"); const dest = join(root, ".grok", "plugins", "alpha-tools"); await mkdir(dest, { recursive: true }); - if (markerShape === "directory") { + if (ownershipMarkerKind === "directory") { await mkdir(join(dest, ".dotagents-managed")); } else { await writeFile(join(dest, ".dotagents-managed"), "owned-by=user\n"); @@ -612,15 +641,15 @@ describe("plugin writer", () => { expect(result.warnings).toEqual([]); const raw = await readFile(opencodePath, "utf-8"); expect(raw).toContain("// Preserve user config"); - const config = parseJSONC(raw) as { mcp: SerializedObject }; - expect(config.mcp["manual"]).toEqual({ type: "local", command: ["manual"] }); - expect(config.mcp["plugin.alpha-tools.remote"]).toEqual({ + const config = parseJsoncObject(raw); + expect(objectValue(config["mcp"])["manual"]).toEqual({ type: "local", command: ["manual"] }); + expect(objectValue(config["mcp"])["plugin.alpha-tools.remote"]).toEqual({ type: "remote", url: "https://example.com/${DEPLOYMENT}/mcp", headers: { "X-Fixture": "Bearer ${TOKEN}" }, }); const dataDir = join(root, ".agents", "plugin-data", "alpha-tools"); - expect(config.mcp["plugin.alpha-tools.local"]).toEqual({ + expect(objectValue(config["mcp"])["plugin.alpha-tools.local"]).toEqual({ type: "local", command: ["node", join(alpha.pluginDir, "server.mjs"), "${OTHER}"], cwd: join(alpha.pluginDir, "runtime"), @@ -630,7 +659,7 @@ describe("plugin writer", () => { PLUGIN_ROOT: alpha.pluginDir, }, }); - expect(config.mcp["plugin.alpha-tools.default-cwd"]).toEqual({ + expect(objectValue(config["mcp"])["plugin.alpha-tools.default-cwd"]).toEqual({ type: "local", command: ["node"], cwd: alpha.pluginDir, @@ -657,10 +686,8 @@ describe("plugin writer", () => { const prunedRaw = await readFile(opencodePath, "utf-8"); expect(prunedRaw).toContain("// Preserve user config"); - const prunedConfig = parseJSONC(prunedRaw) as { - mcp: SerializedObject; - }; - expect(prunedConfig.mcp).toEqual({ manual: { type: "local", command: ["manual"] } }); + const prunedConfig = parseJsoncObject(prunedRaw); + expect(prunedConfig["mcp"]).toEqual({ manual: { type: "local", command: ["manual"] } }); expect(existsSync(dataDir)).toBe(false); expect(existsSync(join(root, ".agents", "plugin-mcp", "opencode.json"))).toBe(false); }); diff --git a/packages/dotagents/src/plugins/runtime/writer.ts b/packages/dotagents/src/plugins/runtime/writer.ts index b5d333f4..281dc2ac 100644 --- a/packages/dotagents/src/plugins/runtime/writer.ts +++ b/packages/dotagents/src/plugins/runtime/writer.ts @@ -27,6 +27,7 @@ import { type PluginRuntimeLayout, type PluginRuntimeRoot, } from "./layout.js"; +import { hasErrorCode, isNonEmptyString, isString, isStringArray } from "../../utils/type-guards.js"; // Owns deterministic runtime plugin projections. Existing runtime artifacts are // overwritten only when they carry dotagents managed metadata or a managed marker. @@ -344,19 +345,18 @@ function portableCoreManifest(plugin: PluginDeclaration): SerializedObject { }; for (const key of ["description", "version", "homepage", "license"] as const) { const value = plugin.manifest[key]; - if (typeof value === "string" && value.length > 0) {manifest[key] = value;} + if (isNonEmptyString(value)) {manifest[key] = value;} } - if (typeof plugin.manifest.repository === "string") { + if (isString(plugin.manifest.repository)) { manifest["repository"] = plugin.manifest.repository; } if (plugin.manifest.author?.name) { - manifest["author"] = { - name: plugin.manifest.author.name, - ...(plugin.manifest.author.email ? { email: plugin.manifest.author.email } : {}), - ...(plugin.manifest.author.url ? { url: plugin.manifest.author.url } : {}), - }; + const author: SerializedObject = { name: plugin.manifest.author.name }; + if (plugin.manifest.author.email) {author["email"] = plugin.manifest.author.email;} + if (plugin.manifest.author.url) {author["url"] = plugin.manifest.author.url;} + manifest["author"] = author; } - if (plugin.manifest.keywords?.every((keyword) => typeof keyword === "string")) { + if (isStringArray(plugin.manifest.keywords)) { manifest["keywords"] = plugin.manifest.keywords; } return manifest; @@ -365,10 +365,11 @@ function portableCoreManifest(plugin: PluginDeclaration): SerializedObject { function nativeComponentRoots(plugin: PluginDeclaration): string[] { if (!plugin.nativeSource || isStandardPluginManifest(plugin.manifest)) {return [];} const roots = new Set(); + // SAFETY: standard manifests returned above; remaining native manifests use legacy fields. const manifest = plugin.manifest as LegacyPluginManifest; for (const key of ["skills", "agents", "commands", "rules", "hooks", "mcpServers", "lspServers", "apps", "monitors", "bin"] as const) { const value = manifest[key]; - const paths = typeof value === "string" ? [value] : Array.isArray(value) ? value : []; + const paths = isString(value) ? [value] : isStringArray(value) ? value : []; for (const path of paths) { if (!isSafeComponentPath(path)) {continue;} const root = path.replace(/^\.\//, "").split("/")[0]; @@ -683,13 +684,18 @@ function componentDirs( return paths.map((path) => join(plugin.pluginDir, path)); } -function manifestPaths( - value: unknown, +interface ManifestPaths { + present: boolean; + paths: string[]; +} + +function manifestPaths( + value: Value, plugin: PluginDeclaration, manifestKey: keyof Pick, agent?: ComponentProjectionAgent, warnings: PluginWriteWarning[] = [], -): { present: boolean; paths: string[] } { +): ManifestPaths { const collect = (values: string[]): string[] => { const paths: string[] = []; for (const item of values) { @@ -708,13 +714,13 @@ function manifestPaths( return paths; }; - if (typeof value === "string") { + if (isString(value)) { return { present: true, paths: collect([value]), }; } - if (Array.isArray(value) && value.every((item) => typeof item === "string")) { + if (isStringArray(value)) { return { present: true, paths: collect(value), @@ -754,8 +760,8 @@ function isOutsideRelativePath(path: string): boolean { return path === ".." || path.startsWith(`..${sep}`) || isAbsolute(path); } -function isNotDirectoryError(err: unknown): boolean { - return err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOTDIR"; +function isNotDirectoryError(err: ErrorValue): boolean { + return hasErrorCode(err, "ENOTDIR"); } /** Prunes stale component symlinks carrying a marker in the reserved ownership directory. */ @@ -781,8 +787,8 @@ async function pruneManagedComponentLinks( async function symlinkPointsTo(filePath: string, expectedTarget: string): Promise { try { - const stat = await lstat(filePath); - if (!stat.isSymbolicLink()) {return false;} + const linkStat = await lstat(filePath); + if (!linkStat.isSymbolicLink()) {return false;} const target = await readlink(filePath); return resolve(dirname(filePath), target) === resolve(expectedTarget); } catch { @@ -811,8 +817,8 @@ async function isDirectoryPath(filePath: string): Promise { async function isManagedComponentLink(filePath: string): Promise { try { - const stat = await lstat(filePath); - if (!stat.isSymbolicLink()) {return false;} + const linkStat = await lstat(filePath); + if (!linkStat.isSymbolicLink()) {return false;} const markerPath = await safeComponentLinkMarkerPath(filePath, false); if (!markerPath || !(await lstat(markerPath)).isFile()) {return false;} const target = await readlink(filePath); @@ -834,7 +840,7 @@ async function claimComponentLinkMarker(filePath: string, target: string): Promi await writeFile(markerPath, content, { encoding: "utf-8", flag: "wx" }); return true; } catch (err) { - if (!(err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "EEXIST")) { + if (!hasErrorCode(err, "EEXIST")) { throw err; } } @@ -856,7 +862,7 @@ function isComponentLinkMarkerContent(content: string): boolean { if (!content.startsWith(prefix) || !content.endsWith("\n")) {return false;} try { const target = JSON.parse(content.slice(prefix.length, -1)); - return typeof target === "string" && content === componentLinkMarkerContent(target); + return isString(target) && content === componentLinkMarkerContent(target); } catch { return false; } @@ -873,7 +879,7 @@ async function safeComponentLinkMarkerPath(filePath: string, createDir: boolean) try { await mkdir(markerDir); } catch (mkdirErr) { - if (!(mkdirErr instanceof Error && "code" in mkdirErr && (mkdirErr as NodeJS.ErrnoException).code === "EEXIST")) { + if (!hasErrorCode(mkdirErr, "EEXIST")) { throw mkdirErr; } } @@ -933,7 +939,7 @@ async function rmdirIfEmpty(dir: string): Promise { try { await rmdir(dir); } catch (err) { - if (!isNotFoundError(err) && !(err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOTEMPTY")) { + if (!isNotFoundError(err) && !hasErrorCode(err, "ENOTEMPTY")) { throw err; } } diff --git a/packages/dotagents/src/plugins/schema.test.ts b/packages/dotagents/src/plugins/schema.test.ts index 4933e844..279458e2 100644 --- a/packages/dotagents/src/plugins/schema.test.ts +++ b/packages/dotagents/src/plugins/schema.test.ts @@ -10,7 +10,6 @@ import { pluginManifestSchema, pluginMarketplaceSchema, } from "./schema.js"; -import type { SerializedObject } from "@sentry/dotagents-lib"; describe("plugin manifest schema", () => { it("accepts Agent Plugins v1 manifests", () => { @@ -134,7 +133,7 @@ describe("plugin manifest schema", () => { metadata: { count: Number.POSITIVE_INFINITY }, }, "plugin.json")).toThrow("expected a serializable object"); expect(() => parsePluginManifest( - JSON.parse('{"name":"review-tools","__proto__":{"polluted":true}}') as unknown, + JSON.parse('{"name":"review-tools","__proto__":{"polluted":true}}'), "plugin.json", )).toThrow("expected a serializable object"); }); @@ -170,11 +169,11 @@ describe("plugin manifest schema", () => { ); expect(manifest.name).toBe("review-tools"); - expect((manifest as SerializedObject)["x-runtime"]).toEqual({ + expect(manifest["x-runtime"]).toEqual({ plugins: ["runtime/plugin.ts"], runtime: "bun", }); - expect((manifest as SerializedObject)["x-dotagents"]).toEqual({ stable: true }); + expect(manifest["x-dotagents"]).toEqual({ stable: true }); }); it("rejects absolute and traversing component paths", () => { @@ -317,7 +316,7 @@ describe("plugin marketplace schema", () => { plugins: [], }, "marketplace.json")).toThrow("expected a serializable object"); expect(() => parsePluginMarketplace( - JSON.parse('{"name":"dotagents","plugins":[],"__proto__":{"polluted":true}}') as unknown, + JSON.parse('{"name":"dotagents","plugins":[],"__proto__":{"polluted":true}}'), "marketplace.json", )).toThrow("expected a serializable object"); }); diff --git a/packages/dotagents/src/plugins/schema.ts b/packages/dotagents/src/plugins/schema.ts index 3076fbc9..5113699f 100644 --- a/packages/dotagents/src/plugins/schema.ts +++ b/packages/dotagents/src/plugins/schema.ts @@ -2,6 +2,7 @@ import { z } from "zod/v4"; import { validateHeaderName, validateHeaderValue } from "node:http"; import { isIP } from "node:net"; import { isSerializedObject, type SerializedObject } from "@sentry/dotagents-lib"; +import { isObject, isString } from "../utils/type-guards.js"; export const AGENT_PLUGIN_SCHEMA = "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json"; export const AGENT_PLUGIN_MCP_SCHEMA = "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json"; @@ -182,10 +183,15 @@ export interface PluginMcpConfig { mcpServers: Record>; } -export function parsePluginMcpBestEffort( - value: unknown, +export interface PluginMcpParseResult { + config: PluginMcpConfig; + issues: string[]; +} + +export function parsePluginMcpBestEffort( + value: Value, filePath: string, -): { config: PluginMcpConfig; issues: string[] } { +): PluginMcpParseResult { const envelope = pluginMcpEnvelopeSchema.safeParse(value); if (!envelope.success) { throw new Error(`Invalid plugin MCP config ${filePath}: ${envelope.error.message}`); @@ -210,7 +216,7 @@ export function parsePluginMcpBestEffort( }; } -export function parsePluginMcp(value: unknown, filePath: string): PluginMcpConfig { +export function parsePluginMcp(value: Value, filePath: string): PluginMcpConfig { const parsed = parsePluginMcpBestEffort(value, filePath); if (parsed.issues.length > 0) { throw new Error(parsed.issues.join("\n")); @@ -268,8 +274,8 @@ export const pluginMarketplaceSchema = z.object({ export type PluginMarketplace = z.infer & SerializedObject; /** Parses an external plugin manifest and annotates schema errors with its file path. */ -export function parsePluginManifest( - value: unknown, +export function parsePluginManifest( + value: Value, filePath: string, ): PluginManifest { const recordValue = isSerializedObject(value) ? value : null; @@ -277,7 +283,7 @@ export function parsePluginManifest( throw new Error(`Invalid plugin manifest ${filePath}: expected a serializable object`); } const schemaValue = recordValue["$schema"]; - const normalizedSchemaValue = typeof schemaValue === "string" + const normalizedSchemaValue = isString(schemaValue) ? normalizeAgentPluginSchemaUrl(schemaValue) : null; const isStandard = normalizedSchemaValue === AGENT_PLUGIN_SCHEMA; @@ -287,12 +293,12 @@ export function parsePluginManifest( ) { throw new Error(`Invalid plugin manifest ${filePath}: unsupported Agent Plugins schema ${schemaValue}`); } - let input = value; + let input: SerializedObject = recordValue; if (isStandard) { - const record = { ...recordValue }; + const record: SerializedObject = { ...recordValue }; record["$schema"] = AGENT_PLUGIN_SCHEMA; const extensions = record["extensions"]; - if (extensions !== undefined && (extensions === null || typeof extensions !== "object" || Array.isArray(extensions))) { + if (extensions !== undefined && (!isObject(extensions) || Array.isArray(extensions))) { delete record["extensions"]; } else if (isSerializedObject(extensions)) { record["extensions"] = Object.fromEntries( @@ -301,7 +307,7 @@ export function parsePluginManifest( } input = record; } else if ("$schema" in recordValue) { - const record = { ...recordValue }; + const record: SerializedObject = { ...recordValue }; delete record["$schema"]; input = record; } @@ -334,8 +340,8 @@ function normalizeAgentPluginSchemaUrl(value: string): string | null { } /** Parses an external plugin marketplace and annotates schema errors with its file path. */ -export function parsePluginMarketplace( - value: unknown, +export function parsePluginMarketplace( + value: Value, filePath: string, ): PluginMarketplace { if (!isSerializedObject(value)) { diff --git a/packages/dotagents/src/plugins/store.test.ts b/packages/dotagents/src/plugins/store.test.ts index 93e2562a..0f823bb7 100644 --- a/packages/dotagents/src/plugins/store.test.ts +++ b/packages/dotagents/src/plugins/store.test.ts @@ -2,7 +2,7 @@ import { existsSync } from "node:fs"; import { lstat, mkdtemp, mkdir, readdir, readFile, realpath, rename, rm, symlink, writeFile } from "node:fs/promises"; import { dirname, join, relative } from "node:path"; import { tmpdir } from "node:os"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { discoverPlugins, installPluginBundle, @@ -13,24 +13,15 @@ import { type ResolvedPlugin, } from "./store.js"; -vi.mock("node:fs/promises", async () => { - const actual = await vi.importActual("node:fs/promises"); - return { - ...actual, - realpath: vi.fn(actual.realpath), - rename: vi.fn(actual.rename), - rm: vi.fn(actual.rm), - }; -}); +async function removeWithBackupFailure( + path: Parameters[0], + options: Parameters[1], +): Promise { + if (String(path).includes(".backup-")) {throw new Error("backup cleanup failed");} + await rm(path, options); +} describe("plugin store", () => { - afterEach(async () => { - const actual = await vi.importActual("node:fs/promises"); - vi.mocked(realpath).mockImplementation(actual.realpath); - vi.mocked(rename).mockImplementation(actual.rename); - vi.mocked(rm).mockImplementation(actual.rm); - }); - it("preserves an empty resolved path for root git plugins", () => { const resolved = { type: "git", @@ -174,8 +165,7 @@ describe("plugin store", () => { "utf-8", ); - const actual = await vi.importActual("node:fs/promises"); - vi.mocked(rename).mockImplementation(async (oldPath, newPath) => { + const renameWithFailures: typeof rename = async (oldPath, newPath) => { const oldString = String(oldPath); const newString = String(newPath); if (oldString.includes(".tmp-") && newString === destDir) { @@ -184,8 +174,8 @@ describe("plugin store", () => { if (oldString.includes(".backup-") && newString === destDir) { throw new Error("rollback failed"); } - await actual.rename(oldPath, newPath); - }); + await rename(oldPath, newPath); + }; await expect(installPluginBundle(pluginsDir, { type: "local", @@ -195,7 +185,7 @@ describe("plugin store", () => { pluginDir: sourceDir, manifest: { name: "review-tools", description: "New plugin" }, }, - })).rejects.toThrow("destination rename failed"); + }, { rename: renameWithFailures })).rejects.toThrow("destination rename failed"); expect(existsSync(destDir)).toBe(false); const backupName = (await readdir(pluginsDir)).find((name) => name.startsWith(".review-tools.backup-")); @@ -226,12 +216,6 @@ describe("plugin store", () => { "utf-8", ); - const actual = await vi.importActual("node:fs/promises"); - vi.mocked(rm).mockImplementation(async (path, options) => { - if (String(path).includes(".backup-")) {throw new Error("backup cleanup failed");} - await actual.rm(path, options); - }); - await expect(installPluginBundle(pluginsDir, { type: "local", plugin: { @@ -240,7 +224,7 @@ describe("plugin store", () => { pluginDir: sourceDir, manifest: { name: "review-tools", description: "New plugin" }, }, - })).resolves.toMatchObject({ pluginDir: destDir }); + }, { rm: removeWithBackupFailure })).resolves.toMatchObject({ pluginDir: destDir }); const installedManifest = JSON.parse(await readFile(join(destDir, "plugin.json"), "utf-8")); expect(installedManifest.description).toBe("New plugin"); @@ -349,29 +333,34 @@ describe("plugin store", () => { "utf-8", ); - const actual = await vi.importActual("node:fs/promises"); + // SAFETY: the fixture adds the Node error code required by the boundary check. const missingRoot = new Error("missing source root") as NodeJS.ErrnoException; missingRoot.code = "ENOENT"; - vi.mocked(realpath).mockImplementation(async (path, options) => { + const realpathWithMissingRoot = async (path: string): Promise => { if (String(path) === sourceRoot) { throw missingRoot; } - return actual.realpath(path, options); - }); + return realpath(path); + }; let error: unknown; try { await resolvePlugin( { name: "review-tools", source: "path:source", path: "plugins/review-tools" }, - { stateDir: join(projectRoot, "state"), projectRoot }, + { + stateDir: join(projectRoot, "state"), + projectRoot, + services: { realpath: realpathWithMissingRoot }, + }, ); } catch (err) { error = err; } expect(error).toBeInstanceOf(Error); - expect((error as Error).message).toBe("Plugin path source root does not exist: plugins/review-tools"); - expect((error as Error).cause).toBe(missingRoot); + if (!(error instanceof Error)) {throw new Error("expected resolvePlugin to reject");} + expect(error.message).toBe("Plugin path source root does not exist: plugins/review-tools"); + expect(error.cause).toBe(missingRoot); } finally { await rm(projectRoot, { recursive: true, force: true }); } @@ -385,27 +374,32 @@ describe("plugin store", () => { await mkdir(pluginDir, { recursive: true }); await writeFile(join(pluginDir, "plugin.json"), JSON.stringify({ name: "review-tools" }), "utf-8"); - const actual = await vi.importActual("node:fs/promises"); + // SAFETY: the fixture adds the Node error code required by the boundary check. const missingPath = new Error("missing plugin path") as NodeJS.ErrnoException; missingPath.code = "ENOENT"; - vi.mocked(realpath).mockImplementation(async (path, options) => { + const realpathWithMissingPath = async (path: string): Promise => { if (String(path) === pluginDir) {throw missingPath;} - return actual.realpath(path, options); - }); + return realpath(path); + }; let error: unknown; try { await resolvePlugin( { name: "review-tools", source: "path:source", path: "plugins/review-tools" }, - { stateDir: join(projectRoot, "state"), projectRoot }, + { + stateDir: join(projectRoot, "state"), + projectRoot, + services: { realpath: realpathWithMissingPath }, + }, ); } catch (err) { error = err; } expect(error).toBeInstanceOf(Error); - expect((error as Error).message).toBe("Plugin path source path does not exist: plugins/review-tools"); - expect((error as Error).cause).toBe(missingPath); + if (!(error instanceof Error)) {throw new Error("expected resolvePlugin to reject");} + expect(error.message).toBe("Plugin path source path does not exist: plugins/review-tools"); + expect(error.cause).toBe(missingPath); } finally { await rm(projectRoot, { recursive: true, force: true }); } diff --git a/packages/dotagents/src/plugins/store.ts b/packages/dotagents/src/plugins/store.ts index a9ff120a..a9debc90 100644 --- a/packages/dotagents/src/plugins/store.ts +++ b/packages/dotagents/src/plugins/store.ts @@ -13,6 +13,8 @@ import { type RepositorySource, type TrustPolicy, type CacheReuse, + isSerializedValue, + type SerializedValue, } from "@sentry/dotagents-lib"; import { PLUGIN_NAME_PATTERN, type PluginConfig } from "../config/schema.js"; import type { LockedPlugin } from "../lockfile/schema.js"; @@ -26,6 +28,7 @@ import { } from "./schema.js"; import { isManagedJsonFile } from "./managed-files.js"; import type { NativePluginSource, PluginDeclaration } from "./types.js"; +import { hasErrorCode, isString } from "../utils/type-guards.js"; // Owns plugin source discovery and installation into the canonical project tree. // Resolved sources are never allowed to live inside the same project's @@ -39,8 +42,17 @@ export interface PluginResolveOptions { trust?: TrustPolicy; /** Exact git checkout acquired earlier in the current operation. */ reuse?: CacheReuse; + services?: Partial; } +export interface PluginStoreServices { + realpath(path: string): Promise; + rename: typeof rename; + rm: typeof rm; +} + +const DEFAULT_PLUGIN_STORE_SERVICES: PluginStoreServices = { realpath, rename, rm }; + interface ResolvedLocalPlugin { type: "local"; plugin: PluginDeclaration; @@ -111,6 +123,7 @@ export async function resolvePlugin( config: PluginConfig, opts: PluginResolveOptions, ): Promise { + const services = { ...DEFAULT_PLUGIN_STORE_SERVICES, ...opts.services }; const sourceForResolve = applyDefaultRepositorySource( config.source, opts.defaultRepositorySource, @@ -121,7 +134,7 @@ export async function resolvePlugin( if (parsed.type === "local") { const sourceDir = await resolveLocalSource(opts.projectRoot, parsed.path!); - const discovered = await resolvePluginCandidate(sourceDir, config); + const discovered = await resolvePluginCandidate(sourceDir, config, services.realpath); if (!discovered) { throw new Error(`Plugin "${config.name}" not found in ${config.source}.`); } @@ -153,7 +166,7 @@ export async function resolvePlugin( reuse: opts.reuse, }); - const discovered = await resolvePluginCandidate(cached.repoDir, config); + const discovered = await resolvePluginCandidate(cached.repoDir, config, services.realpath); if (!discovered) { throw new Error(`Plugin "${config.name}" not found in ${config.source}.`); } @@ -172,7 +185,9 @@ export async function resolvePlugin( export async function installPluginBundle( pluginsDir: string, resolved: ResolvedPlugin, + overrides: Partial = {}, ): Promise { + const services = { ...DEFAULT_PLUGIN_STORE_SERVICES, ...overrides }; const destDir = join(pluginsDir, resolved.plugin.name); if (isProjectPluginSource(resolved.plugin.pluginDir, pluginsDir)) { throw new Error( @@ -190,7 +205,7 @@ export async function installPluginBundle( ); try { - const sourceDir = await realpath(resolved.plugin.pluginDir); + const sourceDir = await services.realpath(resolved.plugin.pluginDir); await copyDir(sourceDir, tempDir, { verbatimSymlinks: true }); await removeSourceOwnershipMarkers(tempDir); await assertPluginBundleSymlinksContained(tempDir); @@ -200,21 +215,21 @@ export async function installPluginBundle( await writeManagedMarker(tempDir); if (existsSync(destDir)) { - await rename(destDir, backupDir); + await services.rename(destDir, backupDir); try { - await rename(tempDir, destDir); + await services.rename(tempDir, destDir); } catch (err) { - await rename(backupDir, destDir).catch(() => {}); + await services.rename(backupDir, destDir).catch(() => {}); throw err; } // The new destination is committed; backup cleanup must not turn a // successful install into a failure that prevents lockfile updates. - await rm(backupDir, { recursive: true, force: true }).catch(() => {}); + await services.rm(backupDir, { recursive: true, force: true }).catch(() => {}); } else { - await rename(tempDir, destDir); + await services.rename(tempDir, destDir); } } finally { - await rm(tempDir, { recursive: true, force: true }); + await services.rm(tempDir, { recursive: true, force: true }); } return installed; @@ -333,13 +348,14 @@ export function lockEntryForPlugin(resolved: ResolvedPlugin): LockedPlugin { if (resolved.type === "local") { return { source: resolved.plugin.source }; } - return { + const entry: LockedPlugin = { source: resolved.plugin.source, resolved_url: resolved.resolvedUrl, resolved_path: resolved.resolvedPath, - ...(resolved.resolvedRef === undefined ? {} : { resolved_ref: resolved.resolvedRef }), resolved_commit: resolved.commit, }; + if (resolved.resolvedRef !== undefined) {entry.resolved_ref = resolved.resolvedRef;} + return entry; } /** Returns true for direct `path:.agents/plugins/...` plugin sources. */ @@ -393,9 +409,10 @@ export function isSameProjectPluginConfig( async function resolvePluginCandidate( sourceDir: string, config: PluginConfig, + resolveRealpath: PluginStoreServices["realpath"] = realpath, ): Promise { if (config.path) { - const dir = await resolveInside(sourceDir, config.path, "Plugin path"); + const dir = await resolveInside(sourceDir, config.path, "Plugin path", resolveRealpath); return loadPluginCandidate(sourceDir, dir, { name: config.name }, "Plugin source", "explicit"); } @@ -557,7 +574,7 @@ async function discoverFromMarketplaces( }); continue; } - const root = typeof marketplace.metadata?.pluginRoot === "string" + const root = isString(marketplace.metadata?.pluginRoot) ? marketplace.metadata.pluginRoot : "."; for (const entry of marketplace.plugins) { @@ -712,11 +729,12 @@ async function loadPluginCandidate( const manifest = loaded.manifest; await validateStandardBundleLayout(pluginDir, manifest, false); - const name = manifest && typeof manifest["name"] === "string" + const name = manifest && isString(manifest["name"]) ? String(manifest["name"]) - : typeof overlay["name"] === "string" + : isString(overlay["name"]) ? String(overlay["name"]) : basename(pluginDir); + // SAFETY: the merged fallback only combines fields from two legacy manifests. const combined = manifest && isStandardPluginManifest(manifest) ? manifest : normalizeManifest(name, { ...overlay, ...manifest } as LegacyPluginManifest); @@ -856,7 +874,7 @@ function assertPluginName( context: string, ): void { const actual = manifest["name"]; - if (typeof actual === "string" && actual !== expected) { + if (isString(actual) && actual !== expected) { throw new Error(`Plugin manifest name "${actual}" does not match configured name "${expected}" in ${context}.`); } } @@ -865,6 +883,7 @@ function normalizeManifest( name: string, manifest: PluginManifest, ): PluginManifest { + // SAFETY: replacing the required name field preserves every PluginManifest variant. return { ...manifest, name } as PluginManifest; } @@ -895,9 +914,9 @@ async function dedupeCandidates(candidates: PluginCandidate[]): Promise { +async function resolveInside( + root: string, + childPath: string, + label: string, + resolveRealpath: PluginStoreServices["realpath"] = realpath, +): Promise { const rootPath = resolve(root); const filePath = resolve(rootPath, childPath); const relPath = relative(rootPath, filePath); @@ -922,7 +946,7 @@ async function resolveInside(root: string, childPath: string, label: string): Pr throw new Error(`${label} resolves outside source: ${childPath}`); } if (existsSync(filePath)) { - await assertInsideSourceRoot(rootPath, filePath, label, childPath); + await assertInsideSourceRoot(rootPath, filePath, label, childPath, resolveRealpath); } return filePath; } @@ -950,10 +974,11 @@ async function assertInsideSourceRoot( filePath: string, label: string, displayPath = relativePath(root, filePath), + resolveRealpath: PluginStoreServices["realpath"] = realpath, ): Promise { let rootRealPath: string; try { - rootRealPath = await realpath(root); + rootRealPath = await resolveRealpath(root); } catch (err) { if (isNotFoundError(err)) { throw new Error(`${label} source root does not exist: ${displayPath}`, { cause: err }); @@ -962,7 +987,7 @@ async function assertInsideSourceRoot( } let fileRealPath: string; try { - fileRealPath = await realpath(filePath); + fileRealPath = await resolveRealpath(filePath); } catch (err) { if (isNotFoundError(err)) { throw new Error(`${label} source path does not exist: ${displayPath}`, { cause: err }); @@ -975,8 +1000,8 @@ async function assertInsideSourceRoot( } } -function isNotFoundError(err: unknown): boolean { - return err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT"; +function isNotFoundError(err: ErrorValue): boolean { + return hasErrorCode(err, "ENOENT"); } function managedPluginPath(pluginsDir: string, name: string): string | null { @@ -996,18 +1021,21 @@ function isOutsideRelativePath(path: string): boolean { return path === ".." || path.startsWith(`..${sep}`) || isAbsolute(path); } -function isNotDirectoryError(err: unknown): boolean { - return err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOTDIR"; +function isNotDirectoryError(err: ErrorValue): boolean { + return hasErrorCode(err, "ENOTDIR"); } -async function readJson(filePath: string): Promise { +async function readJson(filePath: string): Promise { const raw = await readFile(filePath, "utf-8"); - let parsed: unknown; + let parsed; try { - parsed = JSON.parse(raw) as unknown; + parsed = JSON.parse(raw); } catch (err) { const message = err instanceof Error ? err.message : String(err); throw new Error(`Invalid JSON ${filePath}: ${message}`, { cause: err }); } + if (!isSerializedValue(parsed)) { + throw new Error(`Invalid JSON ${filePath}: value is not safely serializable`); + } return parsed; } diff --git a/packages/dotagents/src/subagents/format.ts b/packages/dotagents/src/subagents/format.ts index 93f0f666..5676b379 100644 --- a/packages/dotagents/src/subagents/format.ts +++ b/packages/dotagents/src/subagents/format.ts @@ -1,5 +1,6 @@ import { stringify as tomlStringify } from "smol-toml"; import { isSerializedObject, type SerializedObject } from "@sentry/dotagents-lib"; +import { isString } from "../utils/type-guards.js"; export const DOTAGENTS_SUBAGENT_MARKER = "Generated by dotagents. Edit agents.toml instead."; const DOTAGENTS_SUBAGENT_MARKER_RE = escapeRegExp(DOTAGENTS_SUBAGENT_MARKER); @@ -70,10 +71,10 @@ function escapeRegExp(value: string): string { return value.replaceAll(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -function appendYamlField( +function appendYamlField( lines: string[], key: string, - value: unknown, + value: Value, indent = 0, ): void { if (value === undefined) {return;} @@ -92,7 +93,7 @@ function appendYamlField( return; } - if (typeof value === "string" && value.includes("\n")) { + if (isString(value) && value.includes("\n")) { const chomp = value.endsWith("\n") ? "|+" : "|-"; const body = value; lines.push(`${prefix}${toYamlKey(key)}: ${chomp}`); diff --git a/packages/dotagents/src/subagents/identity.ts b/packages/dotagents/src/subagents/identity.ts index 2abef320..b41eaa6f 100644 --- a/packages/dotagents/src/subagents/identity.ts +++ b/packages/dotagents/src/subagents/identity.ts @@ -5,13 +5,14 @@ import { type SerializedObject, } from "@sentry/dotagents-lib"; import type { SubagentConfigSpec, SubagentIdentityStrategy } from "./types.js"; +import { isNonEmptyString } from "../utils/type-guards.js"; export function subagentIdentityFromMarkdownMeta( strategy: SubagentIdentityStrategy, fileName: string | undefined, meta: SerializedObject, ): string | null { - const declaredName = typeof meta["name"] === "string" && meta["name"] ? meta["name"] : null; + const declaredName = isNonEmptyString(meta["name"]) ? meta["name"] : null; switch (strategy) { case "frontmatter-name": @@ -68,7 +69,7 @@ export function readSubagentFileIdentity( export function subagentIdentityFromTomlContent(content: string): string | null { try { const parsed = parseTOML(content); - if (typeof parsed["name"] === "string" && parsed["name"]) { + if (isNonEmptyString(parsed["name"])) { return parsed["name"]; } } catch { diff --git a/packages/dotagents/src/subagents/store.ts b/packages/dotagents/src/subagents/store.ts index 47daf044..9677a1ef 100644 --- a/packages/dotagents/src/subagents/store.ts +++ b/packages/dotagents/src/subagents/store.ts @@ -27,6 +27,7 @@ import type { SubagentDeclaration, SubagentIdentityStrategy, } from "./types.js"; +import { hasErrorCode, isNonEmptyString, isString } from "../utils/type-guards.js"; const DOTAGENTS_NATIVE_FIELD = "dotagents_native"; const NATIVE_SUBAGENT_TARGETS = ["claude", "cursor", "codex", "opencode"] satisfies NativeSubagentTarget[]; @@ -237,13 +238,18 @@ export async function pruneInstalledSubagents( } export function lockEntryForSubagent(resolved: ResolvedSubagent): LockedSubagent { - return { + if (resolved.type === "local") {return { source: resolved.source };} + if (!resolved.resolvedUrl || !resolved.resolvedPath || !resolved.commit) { + throw new Error(`Incomplete git resolution for subagent "${resolved.subagent.name}".`); + } + const entry: LockedSubagent = { source: resolved.source, - ...(resolved.resolvedUrl ? { resolved_url: resolved.resolvedUrl } : {}), - ...(resolved.resolvedPath ? { resolved_path: resolved.resolvedPath } : {}), - ...(resolved.resolvedRef ? { resolved_ref: resolved.resolvedRef } : {}), - ...(resolved.commit ? { resolved_commit: resolved.commit } : {}), + resolved_url: resolved.resolvedUrl, + resolved_path: resolved.resolvedPath, + resolved_commit: resolved.commit, }; + if (resolved.resolvedRef) {entry.resolved_ref = resolved.resolvedRef;} + return entry; } async function loadSubagentFromSource( @@ -309,11 +315,11 @@ async function discoverSubagent( assertSubagentNameMatches(subagent.name, config.name, relPath); } if (subagent.name !== config.name) {continue;} - const match = { + const match: DiscoveredSubagent = { path: relPath, subagent, - ...(scanDir.nativeTarget ? { nativeTarget: scanDir.nativeTarget } : {}), }; + if (scanDir.nativeTarget) {match.nativeTarget = scanDir.nativeTarget;} if (nameFromFile === config.name) { fileNameMatches.push(match); } else { @@ -358,7 +364,7 @@ async function loadSubagentFile( `Invalid subagent name "${name}" in ${filePath}; expected lowercase letters, numbers, and hyphens`, ); } - if (typeof meta["description"] !== "string" || !meta["description"]) { + if (!isNonEmptyString(meta["description"])) { throw new Error(`Missing 'description' in subagent frontmatter: ${filePath}`); } if (!body) { @@ -370,12 +376,13 @@ async function loadSubagentFile( native[opts.nativeTarget] = raw; } - return { + const subagent: SubagentDeclaration = { name, description: meta["description"], instructions: body, - ...(Object.keys(native).length > 0 ? { native } : {}), }; + if (Object.keys(native).length > 0) {subagent.native = native;} + return subagent; } async function loadCodexSubagentFile( @@ -401,7 +408,7 @@ async function loadCodexSubagentFile( } const name = parsed["name"]; - if (typeof name !== "string" || !name) { + if (!isNonEmptyString(name)) { throw new Error(`Missing 'name' in Codex subagent TOML: ${filePath}`); } @@ -415,12 +422,12 @@ async function loadCodexSubagentFile( } const description = parsed["description"]; - if (typeof description !== "string" || !description) { + if (!isNonEmptyString(description)) { throw new Error(`Missing 'description' in Codex subagent TOML: ${filePath}`); } const instructions = parsed["developer_instructions"]; - if (typeof instructions !== "string" || !instructions.trim()) { + if (!isNonEmptyString(instructions) || !instructions.trim()) { throw new Error(`Missing 'developer_instructions' in Codex subagent TOML: ${filePath}`); } @@ -450,17 +457,15 @@ function mergeDiscoveredSubagents( for (const match of matches) { for (const [target, content] of Object.entries(match.subagent.native ?? {})) { - const nativeTarget = target as NativeSubagentTarget; - native[nativeTarget] ??= content; + if (isNativeSubagentTarget(target)) {native[target] ??= content;} } } + const subagent: SubagentDeclaration = { ...base.subagent }; + if (Object.keys(native).length > 0) {subagent.native = native;} return { path: base.path, - subagent: { - ...base.subagent, - ...(Object.keys(native).length > 0 ? { native } : {}), - }, + subagent, }; } @@ -540,12 +545,13 @@ function assertSingleDiscoveryMatch( } function serializeInstalledSubagent(subagent: SubagentDeclaration): string { + const fields: SerializedObject = { + name: subagent.name, + description: subagent.description, + }; + if (subagent.native) {fields[DOTAGENTS_NATIVE_FIELD] = subagent.native;} return serializeMarkdownSubagent( - { - name: subagent.name, - description: subagent.description, - ...(subagent.native ? { [DOTAGENTS_NATIVE_FIELD]: subagent.native } : {}), - }, + fields, subagent.instructions, ); } @@ -620,15 +626,19 @@ function readNativeOverlays(meta: SerializedObject): NativeSubagentContent { const native: NativeSubagentContent = {}; for (const target of NATIVE_SUBAGENT_TARGETS) { const config = raw[target]; - if (typeof config === "string") { + if (isString(config)) { native[target] = config; - } else if (isSerializedObject(config) && typeof config["content"] === "string") { + } else if (isSerializedObject(config) && isString(config["content"])) { native[target] = config["content"]; } } return native; } -function isNotFoundError(err: unknown): boolean { - return err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT"; +function isNotFoundError(err: ErrorValue): boolean { + return hasErrorCode(err, "ENOENT"); +} + +function isNativeSubagentTarget(target: string): target is NativeSubagentTarget { + return target === "claude" || target === "cursor" || target === "codex" || target === "opencode"; } diff --git a/packages/dotagents/src/subagents/writer.test.ts b/packages/dotagents/src/subagents/writer.test.ts index 4f1270a4..7d58d842 100644 --- a/packages/dotagents/src/subagents/writer.test.ts +++ b/packages/dotagents/src/subagents/writer.test.ts @@ -13,7 +13,6 @@ import { } from "./writer.js"; import { DOTAGENTS_SUBAGENT_MARKER } from "./format.js"; import type { SubagentDeclaration } from "./types.js"; -import type { SerializedObject } from "@sentry/dotagents-lib"; const SUBAGENT: SubagentDeclaration = { name: "code-reviewer", @@ -27,6 +26,10 @@ const OTHER_SUBAGENT: SubagentDeclaration = { instructions: "Add regression coverage for the current change.", }; +function managedSubagent(name: string): string { + return `---\n# ${DOTAGENTS_SUBAGENT_MARKER}\nname: "${name}"\n---\n`; +} + describe("writeSubagentConfigs", () => { let dir: string; @@ -66,7 +69,7 @@ describe("writeSubagentConfigs", () => { const raw = await readFile(join(dir, ".codex", "agents", "code-reviewer.toml"), "utf-8"); expect(raw).toContain(DOTAGENTS_SUBAGENT_MARKER); - const content = parseTOML(raw) as SerializedObject; + const content = parseTOML(raw); expect(content["name"]).toBe("code-reviewer"); expect(content["description"]).toBe("Review code for correctness and missing tests."); expect(content["developer_instructions"]).toBe("Review the current diff and return findings."); @@ -93,7 +96,7 @@ describe("writeSubagentConfigs", () => { ); const codexRaw = await readFile(join(dir, ".codex", "agents", "code-reviewer.toml"), "utf-8"); - const codex = parseTOML(codexRaw) as SerializedObject; + const codex = parseTOML(codexRaw); expect(codex["developer_instructions"]).toBe("Native Codex instructions."); expect(codex["sandbox_mode"]).toBe("read-only"); expect(codexRaw).toContain("# upstream comment"); @@ -612,10 +615,8 @@ describe("reconcileSubagentConfigs", () => { await mkdir(targetDir, { recursive: true }); const declaredPath = join(targetDir, "failed-reviewer.md"); const stalePath = join(targetDir, "old-reviewer.md"); - const managed = (name: string) => - `---\n# ${DOTAGENTS_SUBAGENT_MARKER}\nname: "${name}"\n---\n`; - await writeFile(declaredPath, managed("failed-reviewer"), "utf-8"); - await writeFile(stalePath, managed("old-reviewer"), "utf-8"); + await writeFile(declaredPath, managedSubagent("failed-reviewer"), "utf-8"); + await writeFile(stalePath, managedSubagent("old-reviewer"), "utf-8"); const result = await reconcileSubagentConfigs( ["claude"], diff --git a/packages/dotagents/src/targets/definitions/helpers.ts b/packages/dotagents/src/targets/definitions/helpers.ts index eb8ba793..d94c1100 100644 --- a/packages/dotagents/src/targets/definitions/helpers.ts +++ b/packages/dotagents/src/targets/definitions/helpers.ts @@ -4,6 +4,11 @@ import type { SerializedObject, SerializedValue } from "@sentry/dotagents-lib"; type CommandHook = { type: "command"; command: string }; type ClaudeHookEntry = { matcher?: string; hooks: CommandHook[] }; +export interface CodexHeaders { + httpHeaders?: Record; + envHttpHeaders?: Record; +} + /** Build an agent-specific environment map from declared variable names. */ export function envRecord( env: string[] | undefined, @@ -11,7 +16,7 @@ export function envRecord( values?: Record, ): Record | undefined { if ((!env || env.length === 0) && !values) {return undefined;} - const rec: Record = { ...values }; + const rec = { ...values }; for (const key of env ?? []) {rec[key] = template(key);} return rec; } @@ -47,7 +52,7 @@ export function interpolateHeaders( */ export function extractCodexHeaders( headers: Record | undefined, -): { httpHeaders?: Record; envHttpHeaders?: Record } { +): CodexHeaders { if (!headers) {return {};} let httpHeaders: Record | undefined; let envHttpHeaders: Record | undefined; diff --git a/packages/dotagents/src/targets/hook-writer.test.ts b/packages/dotagents/src/targets/hook-writer.test.ts index f98a681f..7bd2c6a9 100644 --- a/packages/dotagents/src/targets/hook-writer.test.ts +++ b/packages/dotagents/src/targets/hook-writer.test.ts @@ -12,6 +12,7 @@ import { } from "./hook-writer.js"; import type { HookDeclaration } from "./types.js"; import type { HookConfig } from "../config/schema.js"; +import { isSerializedObject } from "@sentry/dotagents-lib"; const HOOKS: HookDeclaration[] = [ { event: "PreToolUse", matcher: "Bash", command: ".agents/hooks/block-rm.sh" }, @@ -90,8 +91,12 @@ describe("writeHookConfigs", () => { await writeHookConfigs(["cursor"], HOOKS, projectHookResolver(dir)); const content = JSON.parse(await readFile(join(dir, ".cursor", "hooks.json"), "utf-8")); + if (!isSerializedObject(content) || !isSerializedObject(content["hooks"])) { + throw new Error("expected serialized cursor hooks"); + } // Cursor hooks should not contain matcher - for (const entries of Object.values(content.hooks) as unknown[][]) { + for (const entries of Object.values(content["hooks"])) { + if (!Array.isArray(entries)) {throw new Error("expected cursor hook entries");} for (const entry of entries) { expect(entry).not.toHaveProperty("matcher"); } diff --git a/packages/dotagents/src/targets/mcp-writer.test.ts b/packages/dotagents/src/targets/mcp-writer.test.ts index b92c902f..ec0ca74e 100644 --- a/packages/dotagents/src/targets/mcp-writer.test.ts +++ b/packages/dotagents/src/targets/mcp-writer.test.ts @@ -13,7 +13,25 @@ import { writeMcpConfigs, } from "./mcp-writer.js"; import type { McpDeclaration } from "./types.js"; -import type { SerializedObject } from "@sentry/dotagents-lib"; +import { isSerializedObject, type SerializedObject } from "@sentry/dotagents-lib"; + +function parseJsoncObject(content: string): SerializedObject { + const parsed = parseJSONC(content); + if (!isSerializedObject(parsed)) {throw new Error("expected a serialized JSONC object");} + return parsed; +} + +function parseTomlObject(content: string): SerializedObject { + const parsed = parseTOML(content); + if (!isSerializedObject(parsed)) {throw new Error("expected a serialized TOML object");} + return parsed; +} + +function childObject(document: SerializedObject, key: string): SerializedObject { + const value = document[key]; + if (!isSerializedObject(value)) {throw new Error(`expected object field ${key}`);} + return value; +} const STDIO_SERVER: McpDeclaration = { name: "github", @@ -156,7 +174,7 @@ describe("writeMcpConfigs", () => { }); let raw = await readFile(filePath, "utf-8"); expect(raw).toContain("// Keep this setting and server"); - expect((parseJSONC(raw) as { mcp: SerializedObject }).mcp["plugin.qa.remote"]).toEqual({ + expect(childObject(parseJsoncObject(raw), "mcp")["plugin.qa.remote"]).toEqual({ type: "remote", url: "https://example.com/mcp", }); @@ -173,11 +191,11 @@ describe("writeMcpConfigs", () => { mode: "apply", }); raw = await readFile(filePath, "utf-8"); - const parsed = parseJSONC(raw) as { theme: string; mcp: SerializedObject }; + const parsed = parseJsoncObject(raw); expect(raw).toContain("// Keep this setting and server"); - expect(parsed.theme).toBe("dark"); - expect(parsed.mcp["manual"]).toEqual({ type: "local", command: ["manual"] }); - expect(parsed.mcp["plugin.qa.remote"]).toBeUndefined(); + expect(parsed["theme"]).toBe("dark"); + expect(childObject(parsed, "mcp")["manual"]).toEqual({ type: "local", command: ["manual"] }); + expect(childObject(parsed, "mcp")["plugin.qa.remote"]).toBeUndefined(); expect(existsSync(statePath)).toBe(false); }); @@ -269,8 +287,8 @@ describe("writeMcpConfigs", () => { // Codex const raw = await readFile(join(dir, ".codex", "config.toml"), "utf-8"); - const codex = parseTOML(raw) as Record>; - expect(codex["mcp_servers"]!["remote"]).toEqual({ + const codex = parseTomlObject(raw); + expect(childObject(codex, "mcp_servers")["remote"]).toEqual({ url: "https://mcp.example.com/mcp", http_headers: { Authorization: "Bearer tok" }, }); @@ -331,7 +349,7 @@ describe("writeMcpConfigs", () => { const nested = await readFile(nestedPath, "utf-8"); expect(nested).toContain("// Keep this comment"); expect( - (parseJSONC(nested) as Record)["mcp"]!["github"], + childObject(parseJsoncObject(nested), "mcp")["github"], ).toBeDefined(); expect(JSON.parse(await readFile(legacyPath, "utf-8"))).toEqual({ mcp: { legacy: { command: ["legacy"] } }, @@ -363,9 +381,9 @@ describe("writeMcpConfigs", () => { expect(raw).toContain("// Project theme must remain documented"); expect(raw).toContain("// User-owned server"); expect(raw).toMatch(/"github": \{[\s\S]*?\n },\n },\n}\n$/); - const content = parseJSONC(raw) as Record; - expect(content["mcp"]!["manual"]).toEqual({ type: "local", command: ["manual"] }); - expect(content["mcp"]!["github"]).toEqual({ + const content = parseJsoncObject(raw); + expect(childObject(content, "mcp")["manual"]).toEqual({ type: "local", command: ["manual"] }); + expect(childObject(content, "mcp")["github"]).toEqual({ type: "local", command: ["npx", "-y", "@mcp/server-github"], environment: { GITHUB_TOKEN: "${GITHUB_TOKEN}" }, @@ -464,10 +482,10 @@ describe("writeMcpConfigs", () => { headers: { "X-Api-Key": "${API_KEY}", Authorization: "Bearer ${TOKEN}" }, }); - const codex = parseTOML( + const codex = parseTomlObject( await readFile(join(dir, ".codex", "config.toml"), "utf-8"), - ) as Record>; - expect(codex["mcp_servers"]!["authed-api"]).toEqual({ + ); + expect(childObject(codex, "mcp_servers")["authed-api"]).toEqual({ url: "https://${API_HOST}/mcp", http_headers: { "X-Api-Key": "${API_KEY}", @@ -480,8 +498,8 @@ describe("writeMcpConfigs", () => { await writeMcpConfigs(["codex"], [HTTP_SERVER_WITH_ENV_REFS], projectMcpResolver(dir)); const raw = await readFile(join(dir, ".codex", "config.toml"), "utf-8"); - const content = parseTOML(raw) as Record>; - expect(content["mcp_servers"]!["authed-api"]).toEqual({ + const content = parseTomlObject(raw); + expect(childObject(content, "mcp_servers")["authed-api"]).toEqual({ url: "https://${API_HOST}/mcp", // Pure ref: X-Api-Key = "${API_KEY}" → env_http_headers.API_KEY = "X-Api-Key" env_http_headers: { API_KEY: "X-Api-Key" }, diff --git a/packages/dotagents/src/targets/mcp-writer.ts b/packages/dotagents/src/targets/mcp-writer.ts index 15899cf7..ace47f67 100644 --- a/packages/dotagents/src/targets/mcp-writer.ts +++ b/packages/dotagents/src/targets/mcp-writer.ts @@ -18,6 +18,7 @@ import type { } from "./types.js"; import type { McpConfig } from "../config/schema.js"; import { isSerializedObject, type SerializedObject } from "@sentry/dotagents-lib"; +import { hasErrorCode, isObject, isString } from "../utils/type-guards.js"; export interface McpResolvedTarget { filePath: string; @@ -353,7 +354,7 @@ function readServerRoot( ): SerializedObject { const root = document[rootKey]; if (root === undefined) {return {};} - if (root === null || typeof root !== "object" || Array.isArray(root) || root instanceof Date) { + if (!isObject(root) || Array.isArray(root) || root instanceof Date) { throw new TypeError(`MCP config root must contain an object: ${filePath}`); } return root; @@ -374,7 +375,7 @@ async function readExisting( ): Promise { const raw = await readFile(filePath, "utf-8"); const jsoncErrors: JsoncParseError[] = []; - const parsed: unknown = spec.format === "toml" + const parsed = spec.format === "toml" ? parseTOML(raw) : spec.format === "jsonc" ? parseJsonc(raw, jsoncErrors, { allowTrailingComma: true }) @@ -394,8 +395,8 @@ async function readExisting( return parsed; } -function isTomlObject(value: unknown): value is SerializedObject { - return value !== null && typeof value === "object" && !Array.isArray(value) && !(value instanceof Date); +function isTomlObject(value: Value): value is Value & SerializedObject { + return isObject(value) && !Array.isArray(value) && !(value instanceof Date); } async function writeReconciledDocument( @@ -454,15 +455,22 @@ async function readManagedMcpState( ): Promise<{ state?: ManagedMcpState; issue?: string }> { if (!existsSync(statePath)) {return {};} try { - const value: unknown = JSON.parse(await readFile(statePath, "utf-8")); - if (!isSerializedObject(value) || value["version"] !== 1 || !Array.isArray(value["servers"]) || - !value["servers"].every((name) => typeof name === "string")) { + const value = JSON.parse(await readFile(statePath, "utf-8")); + if (!isSerializedObject(value) || value["version"] !== 1) { + return { issue: `Invalid managed MCP state: ${statePath}` }; + } + const servers = value["servers"]; + if (!Array.isArray(servers)) { + return { issue: `Invalid managed MCP state: ${statePath}` }; + } + const validatedServers = servers.filter(isString); + if (validatedServers.length !== servers.length) { return { issue: `Invalid managed MCP state: ${statePath}` }; } return { state: { version: 1, - servers: [...new Set(value["servers"] as string[])].toSorted(), + servers: [...new Set(validatedServers)].toSorted(), }, }; } catch { @@ -499,6 +507,6 @@ async function writeFileIfChanged(filePath: string, content: string): Promise(err: ErrorValue): boolean { + return hasErrorCode(err, "ENOENT"); } diff --git a/packages/dotagents/src/utils/type-guards.ts b/packages/dotagents/src/utils/type-guards.ts new file mode 100644 index 00000000..b32aec21 --- /dev/null +++ b/packages/dotagents/src/utils/type-guards.ts @@ -0,0 +1,27 @@ +export interface ErrorWithCode extends Error { + code: string | number; +} + +export function isString(value: Value): value is Value & string { + return typeof value === "string"; +} + +export function isNonEmptyString(value: Value): value is Value & string { + return typeof value === "string" && value.length > 0; +} + +export function isNumber(value: Value): value is Value & number { + return typeof value === "number"; +} + +export function isObject(value: Value): value is Value & object { + return typeof value === "object" && value !== null; +} + +export function isStringArray(value: Value): value is Value & string[] { + return Array.isArray(value) && value.every(isString); +} + +export function hasErrorCode(value: Value, code: string): value is Value & ErrorWithCode { + return value instanceof Error && "code" in value && value.code === code; +} diff --git a/packages/dotagents/vitest.config.ts b/packages/dotagents/vitest.config.ts index 00cd312e..a84deb20 100644 --- a/packages/dotagents/vitest.config.ts +++ b/packages/dotagents/vitest.config.ts @@ -2,7 +2,7 @@ import { defineConfig } from "vitest/config"; import { fileURLToPath } from "node:url"; import { dirname, resolve } from "node:path"; -const __dirname = dirname(fileURLToPath(import.meta.url)); +const moduleDir = dirname(fileURLToPath(import.meta.url)); export default defineConfig({ test: { @@ -12,7 +12,7 @@ export default defineConfig({ }, resolve: { alias: { - "@sentry/dotagents-lib": resolve(__dirname, "../dotagents-lib/src/index.ts"), + "@sentry/dotagents-lib": resolve(moduleDir, "../dotagents-lib/src/index.ts"), }, }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5c7a16f0..c298482f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15,6 +15,9 @@ importers: '@ast-grep/cli': specifier: ^0.44.1 version: 0.44.1 + '@oxlint/plugins': + specifier: 1.79.0 + version: 1.79.0 '@types/node': specifier: ^25.2.1 version: 25.2.1 @@ -22,8 +25,8 @@ importers: specifier: ^16.2.7 version: 16.2.7 oxlint: - specifier: ^1.43.0 - version: 1.43.0 + specifier: 1.79.0 + version: 1.79.0 simple-git-hooks: specifier: ^2.13.1 version: 2.13.1 @@ -283,50 +286,132 @@ packages: '@jridgewell/sourcemap-codec@1.5.5': resolution: {integrity: sha512-cYQ9310grqxueWbl+WuIUIaiUaDcj7WOq5fVhEljNVgRfOUhY9fy2zTvfoqWsnebh8Sl70VScFbICvJnLKB0Og==} - '@oxlint/darwin-arm64@1.43.0': - resolution: {integrity: sha512-C/GhObv/pQZg34NOzB6Mk8x0wc9AKj8fXzJF8ZRKTsBPyHusC6AZ6bba0QG0TUufw1KWuD0j++oebQfWeiFXNw==} + '@oxlint/binding-android-arm-eabi@1.79.0': + resolution: {integrity: sha512-TebFaaMklO/RXzTv7PucaCq9l3X6D1gA+C8H6K4njtjFOV+zWE9MKLpulcJZN9bzytbUbQIY0mZuz12nQ5Kv4Q==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [arm] + os: [android] + + '@oxlint/binding-android-arm64@1.79.0': + resolution: {integrity: sha512-KqqnOtAVgNsPPF0YSodkFZA1O80jcKoCZCTu3bgsszxA+MrMP9TLzfXitKjEj1FmrPprKDMdRDMmY3weESO9sg==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [arm64] + os: [android] + + '@oxlint/binding-darwin-arm64@1.79.0': + resolution: {integrity: sha512-BVC2nsMzqQzRDPc5RhixkZ+m1p7iH4bxRRvqkbwDXX0PlQKm1BPy8J8cRjnAFafOq2QzI+BfO3vE8w2GZ3CBag==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [darwin] - '@oxlint/darwin-x64@1.43.0': - resolution: {integrity: sha512-4NjfUtEEH8ewRQ2KlZGmm6DyrvypMdHwBnQT92vD0dLScNOQzr0V9O8Ua4IWXdeCNl/XMVhAV3h4/3YEYern5A==} + '@oxlint/binding-darwin-x64@1.79.0': + resolution: {integrity: sha512-p6Lm+snmhGuLKL1+CpCV8L6ijkE/qJzK2H2jG9+eKJT0n31RbY4FLsdhexekgP3bLpw4Kgde+9DZuDZQ4yIInA==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [darwin] - '@oxlint/linux-arm64-gnu@1.43.0': - resolution: {integrity: sha512-75tf1HvwdZ3ebk83yMbSB+moAEWK98mYqpXiaFAi6Zshie7r+Cx5PLXZFUEqkscenoZ+fcNXakHxfn94V6nf1g==} + '@oxlint/binding-freebsd-x64@1.79.0': + resolution: {integrity: sha512-qDMm0dXZnoHyRqSL4N4xUq82T4sqK5cbKSjvd/dF/YbMUXc2R1wEPf+vmA5S0qUmi0nwXfNbjXBtZaIqzQLIMg==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [x64] + os: [freebsd] + + '@oxlint/binding-linux-arm-gnueabihf@1.79.0': + resolution: {integrity: sha512-2od7s0nuKPzqyUZAWk9KkCyGg7eI9dwFPZg+20lB15fKFkVZ0c9ZFxqPfiBAyDTlTkh9stPI0t+JlPCqMbItVA==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [arm] + os: [linux] + + '@oxlint/binding-linux-arm-musleabihf@1.79.0': + resolution: {integrity: sha512-ZOQUjkzDnvlhSE3+tWC3YXx94MMl+sYMlwH+u1+YGApGHOJP/YAc8ZBRFOXZ6eOBmxtXAWuS/fBcdZr8qqNO1A==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [arm] + os: [linux] + + '@oxlint/binding-linux-arm64-gnu@1.79.0': + resolution: {integrity: sha512-lu158FR4nGqGeRS3BQvtG85wRgU/Fy4MD5Cxp1hzJXizGiLo6u2742wJSCDKh8cFcZntvX7fcxlq4mMmfryH1g==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] libc: [glibc] - '@oxlint/linux-arm64-musl@1.43.0': - resolution: {integrity: sha512-BHV4fb36T2p/7bpA9fiJ5ayt7oJbiYX10nklW5arYp4l9/9yG/FQC5J4G1evzbJ/YbipF9UH0vYBAm5xbqGrvw==} + '@oxlint/binding-linux-arm64-musl@1.79.0': + resolution: {integrity: sha512-mbpKQeE2aflTjddaHK7MP8KP/OFbUM++lt5M635ENM8IyIdK0jm2t9pb+2v9mVVIvhF6TqA4l7F79Pll1mi+uw==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] libc: [musl] - '@oxlint/linux-x64-gnu@1.43.0': - resolution: {integrity: sha512-1l3nvnzWWse1YHibzZ4HQXdF/ibfbKZhp9IguElni3bBqEyPEyurzZ0ikWynDxKGXqZa+UNXTFuU1NRVX1RJ3g==} + '@oxlint/binding-linux-ppc64-gnu@1.79.0': + resolution: {integrity: sha512-WpGNua7gaxaHnpSDeog2ji8IDHn/QLPl9LPzwkR/FvVv58vT5BcXjRXnU+wbu3N75cpeha8CdC7ho/U2OIsB4g==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [ppc64] + os: [linux] + libc: [glibc] + + '@oxlint/binding-linux-riscv64-gnu@1.79.0': + resolution: {integrity: sha512-tK1E93A5LVzISg4ngpKJnfTs7EqtIUceGI7MQ4GyDjJiLi8wPCkEyKlj2xkyKWZ1yzkDJyLHTBJ5/iFWRdnJvg==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [riscv64] + os: [linux] + libc: [glibc] + + '@oxlint/binding-linux-riscv64-musl@1.79.0': + resolution: {integrity: sha512-qhQvUIrngXivA2A9pQ+xPCychztn/5qUv7yS3gDwXv3w7Rag+eTeeXWmRyx+t7XsW5x6LuY/8AsTq36UgFIblg==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [riscv64] + os: [linux] + libc: [musl] + + '@oxlint/binding-linux-s390x-gnu@1.79.0': + resolution: {integrity: sha512-sv6AaVgU/eE6u+6WFiQVDcPPwTxP6IJMSB9k701W2r/r6Tx465e8vPvVyRxquNH4Vy6KwRNu90mVbxXJN8+5gg==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [s390x] + os: [linux] + libc: [glibc] + + '@oxlint/binding-linux-x64-gnu@1.79.0': + resolution: {integrity: sha512-iFZL02deziHslb3jEX9KdqlAkYoo4fGyotchKDzdfK1f5mxlIBeiQeHhvK3iFpuEJSB4ma/qeFn9oxPiwnhUPQ==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] libc: [glibc] - '@oxlint/linux-x64-musl@1.43.0': - resolution: {integrity: sha512-+jNYgLGRFTJxJuaSOZJBwlYo5M0TWRw0+3y5MHOL4ArrIdHyCthg6r4RbVWrsR1qUfUE1VSSHQ2bfbC99RXqMg==} + '@oxlint/binding-linux-x64-musl@1.79.0': + resolution: {integrity: sha512-3DtZR2raqObnh7wXZoFYFd0Fw7skBvcb3f7A+/lkEiDuh8hrE6vv9b/62Qxao1a9/OeHLw/FcXlXzgsW9wTRFg==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] libc: [musl] - '@oxlint/win32-arm64@1.43.0': - resolution: {integrity: sha512-dvs1C/HCjCyGTURMagiHprsOvVTT3omDiSzi5Qw0D4QFJ1pEaNlfBhVnOUYgUfS6O7Mcmj4+G+sidRsQcWQ/kA==} + '@oxlint/binding-openharmony-arm64@1.79.0': + resolution: {integrity: sha512-Oatt4GuA1WJkqzk2ozx4HrWROOi7opV3AKDw/U8qDIqeTqzsjn5K2x3REJMNjU3/KU/Bkq96Zi3CknaiDTaC/Q==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [arm64] + os: [openharmony] + + '@oxlint/binding-win32-arm64-msvc@1.79.0': + resolution: {integrity: sha512-NAgZr9Qp8nIA9rpo0JEvwiabTF/2UVqBNnupBG9X4kxXcQoScJUTi+qHhvabb9s/thgj5wQ4XcIaJvb+ZMgoKw==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [win32] - '@oxlint/win32-x64@1.43.0': - resolution: {integrity: sha512-bSuItSU8mTSDsvmmLTepTdCL2FkJI6dwt9tot/k0EmiYF+ArRzmsl4lXVLssJNRV5lJEc5IViyTrh7oiwrjUqA==} + '@oxlint/binding-win32-ia32-msvc@1.79.0': + resolution: {integrity: sha512-+KyXjIvcpaXmWW/j9NNY5yWjrIVxaX18VyIheQy3jwc2GSYgpCr7MGI/HxIGQ/shAL5IWEKbhsqoMpAO5Stiog==} + engines: {node: ^20.19.0 || >=22.12.0} + cpu: [ia32] + os: [win32] + + '@oxlint/binding-win32-x64-msvc@1.79.0': + resolution: {integrity: sha512-mEelcCMMBS57sIXh2veGMNy+pQwuGtcMxHxGIZWQ5Ba9pJ5jCCUFOZB9E2JhBaxGsURe+WGe0zJp4RVre52gpQ==} + engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [win32] + '@oxlint/plugins@1.79.0': + resolution: {integrity: sha512-S0uyoxakDINJ4DPgqxGlEEvrdSMeQb7Z2lKVjxoY2gwsbZbfg2Xr8Klfeo5ZeraHmmdBCELFUHkSe6KEmBpMvg==} + engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} + '@rollup/rollup-android-arm-eabi@4.62.2': resolution: {integrity: sha512-6o7ZLZK+BeenkZCFNDXqpbjw9bD6nuWonvS/lwQJp7NoVVxm6p3qE7qQ5jGuBjiFsgvqjD8mZAU5oWxTmbOeOg==} cpu: [arm] @@ -735,15 +820,18 @@ packages: resolution: {integrity: sha512-VXJjc87FScF88uafS3JllDgvAm+c/Slfz06lorj2uAY34rlUu0Nt+v8wreiImcrgAjjIHp1rXpTDlLOGw29WwQ==} engines: {node: '>=18'} - oxlint@1.43.0: - resolution: {integrity: sha512-xiqTCsKZch+R61DPCjyqUVP2MhkQlRRYxLRBeBDi+dtQJ90MOgdcjIktvDCgXz0bgtx94EQzHEndsizZjMX2OA==} + oxlint@1.79.0: + resolution: {integrity: sha512-hVJ9hq9m2unPS+Of4eJJgCPdIeCC+3DHEUX3tkmrPJr3OK2hz7PhXwgC+ZP71ZcYu8cCDEtQrqLxWNvxBppBVg==} engines: {node: ^20.19.0 || >=22.12.0} hasBin: true peerDependencies: - oxlint-tsgolint: '>=0.11.2' + oxlint-tsgolint: '>=7.0.2001' + vite-plus: '*' peerDependenciesMeta: oxlint-tsgolint: optional: true + vite-plus: + optional: true pathe@2.0.3: resolution: {integrity: sha512-WUjGcAqP1gQacoQe+OBJsFA7Ld4DyXuUIjZ5cc75cLHvJ7dtNsTugphxIADwspS+AraAUePCKrSVtPLFj/F88w==} @@ -1087,30 +1175,65 @@ snapshots: '@jridgewell/sourcemap-codec@1.5.5': {} - '@oxlint/darwin-arm64@1.43.0': + '@oxlint/binding-android-arm-eabi@1.79.0': + optional: true + + '@oxlint/binding-android-arm64@1.79.0': + optional: true + + '@oxlint/binding-darwin-arm64@1.79.0': + optional: true + + '@oxlint/binding-darwin-x64@1.79.0': + optional: true + + '@oxlint/binding-freebsd-x64@1.79.0': + optional: true + + '@oxlint/binding-linux-arm-gnueabihf@1.79.0': + optional: true + + '@oxlint/binding-linux-arm-musleabihf@1.79.0': + optional: true + + '@oxlint/binding-linux-arm64-gnu@1.79.0': + optional: true + + '@oxlint/binding-linux-arm64-musl@1.79.0': + optional: true + + '@oxlint/binding-linux-ppc64-gnu@1.79.0': optional: true - '@oxlint/darwin-x64@1.43.0': + '@oxlint/binding-linux-riscv64-gnu@1.79.0': optional: true - '@oxlint/linux-arm64-gnu@1.43.0': + '@oxlint/binding-linux-riscv64-musl@1.79.0': optional: true - '@oxlint/linux-arm64-musl@1.43.0': + '@oxlint/binding-linux-s390x-gnu@1.79.0': optional: true - '@oxlint/linux-x64-gnu@1.43.0': + '@oxlint/binding-linux-x64-gnu@1.79.0': optional: true - '@oxlint/linux-x64-musl@1.43.0': + '@oxlint/binding-linux-x64-musl@1.79.0': optional: true - '@oxlint/win32-arm64@1.43.0': + '@oxlint/binding-openharmony-arm64@1.79.0': optional: true - '@oxlint/win32-x64@1.43.0': + '@oxlint/binding-win32-arm64-msvc@1.79.0': optional: true + '@oxlint/binding-win32-ia32-msvc@1.79.0': + optional: true + + '@oxlint/binding-win32-x64-msvc@1.79.0': + optional: true + + '@oxlint/plugins@1.79.0': {} + '@rollup/rollup-android-arm-eabi@4.62.2': optional: true @@ -1443,16 +1566,27 @@ snapshots: dependencies: mimic-function: 5.0.1 - oxlint@1.43.0: + oxlint@1.79.0: optionalDependencies: - '@oxlint/darwin-arm64': 1.43.0 - '@oxlint/darwin-x64': 1.43.0 - '@oxlint/linux-arm64-gnu': 1.43.0 - '@oxlint/linux-arm64-musl': 1.43.0 - '@oxlint/linux-x64-gnu': 1.43.0 - '@oxlint/linux-x64-musl': 1.43.0 - '@oxlint/win32-arm64': 1.43.0 - '@oxlint/win32-x64': 1.43.0 + '@oxlint/binding-android-arm-eabi': 1.79.0 + '@oxlint/binding-android-arm64': 1.79.0 + '@oxlint/binding-darwin-arm64': 1.79.0 + '@oxlint/binding-darwin-x64': 1.79.0 + '@oxlint/binding-freebsd-x64': 1.79.0 + '@oxlint/binding-linux-arm-gnueabihf': 1.79.0 + '@oxlint/binding-linux-arm-musleabihf': 1.79.0 + '@oxlint/binding-linux-arm64-gnu': 1.79.0 + '@oxlint/binding-linux-arm64-musl': 1.79.0 + '@oxlint/binding-linux-ppc64-gnu': 1.79.0 + '@oxlint/binding-linux-riscv64-gnu': 1.79.0 + '@oxlint/binding-linux-riscv64-musl': 1.79.0 + '@oxlint/binding-linux-s390x-gnu': 1.79.0 + '@oxlint/binding-linux-x64-gnu': 1.79.0 + '@oxlint/binding-linux-x64-musl': 1.79.0 + '@oxlint/binding-openharmony-arm64': 1.79.0 + '@oxlint/binding-win32-arm64-msvc': 1.79.0 + '@oxlint/binding-win32-ia32-msvc': 1.79.0 + '@oxlint/binding-win32-x64-msvc': 1.79.0 pathe@2.0.3: {} diff --git a/scripts/bump-version.mjs b/scripts/bump-version.mjs index 2687c372..c908887d 100755 --- a/scripts/bump-version.mjs +++ b/scripts/bump-version.mjs @@ -10,8 +10,8 @@ import { readFileSync, writeFileSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; -const __dirname = dirname(fileURLToPath(import.meta.url)); -const repoRoot = join(__dirname, ".."); +const moduleDir = dirname(fileURLToPath(import.meta.url)); +const repoRoot = join(moduleDir, ".."); // craft passes [OLD_VERSION, NEW_VERSION]; manual invocation passes just NEW_VERSION. const args = process.argv.slice(2); diff --git a/skills/dotagents-qa/scripts/qa-example.mjs b/skills/dotagents-qa/scripts/qa-example.mjs index b8339f8f..d25e4d26 100644 --- a/skills/dotagents-qa/scripts/qa-example.mjs +++ b/skills/dotagents-qa/scripts/qa-example.mjs @@ -18,8 +18,8 @@ import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; -const __dirname = dirname(fileURLToPath(import.meta.url)); -const repoRoot = resolve(__dirname, "../../.."); +const moduleDir = dirname(fileURLToPath(import.meta.url)); +const repoRoot = resolve(moduleDir, "../../.."); const cliPath = join(repoRoot, "packages", "dotagents", "dist", "cli", "index.js"); const exampleRoot = join(repoRoot, "examples", "full"); const sentinel = "DOTAGENTS_SUBAGENT_RUNTIME_PROOF_9b8e6f2c"; @@ -438,10 +438,10 @@ function proveCodexRuntime() { }, ); } catch (err) { - if (err && typeof err === "object" && "stdout" in err && typeof err.stdout === "string") { + if (isObjectValue(err) && "stdout" in err && isStringValue(err.stdout)) { writeFileSync(outputPath, err.stdout); } - if (err && typeof err === "object" && "stderr" in err && typeof err.stderr === "string") { + if (isObjectValue(err) && "stderr" in err && isStringValue(err.stderr)) { writeFileSync(stderrPath, err.stderr); } throw err; @@ -551,13 +551,6 @@ function assertFile(relativePath) { } } -function assertFileDoesNotExist(relativePath) { - const path = join(projectDir, relativePath); - if (existsSync(path)) { - throw new Error(`expected file not to exist: ${relativePath}`); - } -} - function assertSymlink(relativePath) { const path = join(projectDir, relativePath); if (!existsSync(path) || !lstatSync(path).isSymbolicLink()) { @@ -621,7 +614,7 @@ function assertCodexRuntimeEvents(output) { if (!line.trim()) {continue;} const event = JSON.parse(line); const states = event.item?.agents_states; - if (!states || typeof states !== "object") {continue;} + if (!isObjectValue(states)) {continue;} for (const state of Object.values(states)) { if (state?.message?.includes(sentinel)) { return; @@ -631,3 +624,11 @@ function assertCodexRuntimeEvents(output) { throw new Error("Codex runtime JSONL should include a waited child-agent response with the sentinel"); } + +function isObjectValue(value) { + return value !== null && Object(value) === value; +} + +function isStringValue(value) { + return Object.prototype.toString.call(value) === "[object String]"; +} diff --git a/tools/oxlint/anti-slop/LICENSE b/tools/oxlint/anti-slop/LICENSE new file mode 100644 index 00000000..69239ead --- /dev/null +++ b/tools/oxlint/anti-slop/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Dillon Mulroy + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/tools/oxlint/anti-slop/UPSTREAM.md b/tools/oxlint/anti-slop/UPSTREAM.md new file mode 100644 index 00000000..0d088b18 --- /dev/null +++ b/tools/oxlint/anti-slop/UPSTREAM.md @@ -0,0 +1,7 @@ +# anti-slop provenance + +Vendored from [`dmmulroy/anti-slop`](https://github.com/dmmulroy/anti-slop) +at commit `6d538555cb151d4121ed51a27db81890eacf8ae9`. + +This directory contains the runtime plugin sources from upstream's `src/` +directory. See `LICENSE` for the upstream MIT license. diff --git a/tools/oxlint/anti-slop/effect/index.ts b/tools/oxlint/anti-slop/effect/index.ts new file mode 100644 index 00000000..37247862 --- /dev/null +++ b/tools/oxlint/anti-slop/effect/index.ts @@ -0,0 +1,13 @@ +import { eslintCompatPlugin } from "@oxlint/plugins"; + +import { noServiceConstructorImportsRule } from "./rules/no-service-constructor-imports.ts"; + +/** Opt-in Oxlint rules for Effect service and Layer architecture. */ +const antiSlopEffectPlugin = eslintCompatPlugin({ + meta: { name: "anti-slop-effect" }, + rules: { + "no-service-constructor-imports": noServiceConstructorImportsRule, + }, +}); + +export default antiSlopEffectPlugin; diff --git a/tools/oxlint/anti-slop/effect/rules/no-service-constructor-imports.ts b/tools/oxlint/anti-slop/effect/rules/no-service-constructor-imports.ts new file mode 100644 index 00000000..55cefb7e --- /dev/null +++ b/tools/oxlint/anti-slop/effect/rules/no-service-constructor-imports.ts @@ -0,0 +1,52 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +const SERVICE_CONSTRUCTOR_NAME = /^make[A-Z]/u; +const TEST_FILE = /\.(?:test|spec)\.[cm]?[jt]sx?$/u; + +function isProjectLocalImport(source: string): boolean { + return source.startsWith("./") || source.startsWith("../"); +} + +function getImportedName(specifier: ESTree.ImportSpecifier): string { + if (specifier.imported.type === "Identifier") return specifier.imported.name; + return specifier.imported.value; +} + +/** Keep dependency-bearing Effect service constructors local to their owning capability modules. */ +export const noServiceConstructorImportsRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow project-local make imports outside test and spec files.", + }, + messages: { + serviceConstructorImport: + 'Do not import Effect service constructor "{{name}}" into runtime code. Import the owning Layer, yield the contextual service, and allow its requirements to propagate to the composition root.', + }, + }, + create(context) { + const isTestFile = TEST_FILE.test(context.filename.replaceAll("\\", "/")); + + return { + ImportDeclaration(node) { + if (isTestFile || !isProjectLocalImport(node.source.value)) return; + + for (const specifier of node.specifiers) { + if (specifier.type !== "ImportSpecifier") continue; + + const importedName = getImportedName(specifier); + if (!SERVICE_CONSTRUCTOR_NAME.test(importedName)) continue; + + context.report({ + node: specifier, + messageId: "serviceConstructorImport", + data: { name: importedName }, + }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/index.ts b/tools/oxlint/anti-slop/index.ts new file mode 100644 index 00000000..2b4ae222 --- /dev/null +++ b/tools/oxlint/anti-slop/index.ts @@ -0,0 +1,41 @@ +import { eslintCompatPlugin } from "@oxlint/plugins"; + +import { noChainedTypeAssertionsRule } from "./rules/no-chained-type-assertions.ts"; +import { noConditionalEmptyObjectSpreadRule } from "./rules/no-conditional-empty-object-spread.ts"; +import { noKnownValueWideningRule } from "./rules/no-known-value-widening.ts"; +import { noModuleMockingRule } from "./rules/no-module-mocking.ts"; +import { noObjectParametersRule } from "./rules/no-object-parameters.ts"; +import { noReflectApplyRule } from "./rules/no-reflect-apply.ts"; +import { noReflectGetRule } from "./rules/no-reflect-get.ts"; +import { noRuntimeTypeofRule } from "./rules/no-runtime-typeof.ts"; +import { noForbiddenTermInSymbolNamesRule } from "./rules/no-shape-in-symbol-names.ts"; +import { noUnknownParametersRule } from "./rules/no-unknown-parameters.ts"; +import { noUnknownReturnsRule } from "./rules/no-unknown-returns.ts"; +import { noUnknownTypeAliasesRule } from "./rules/no-unknown-type-aliases.ts"; +import { noUnsafeDictionaryTypeRule } from "./rules/no-unsafe-dictionary-type.ts"; +import { noWidenThenAssertRule } from "./rules/no-widen-then-assert.ts"; +import { requireSafetyCommentForTypeAssertionRule } from "./rules/require-safety-comment-for-type-assertion.ts"; + +/** Generic Oxlint rules that reject low-evidence and low-signal implementation patterns. */ +const antiSlopPlugin = eslintCompatPlugin({ + meta: { name: "anti-slop" }, + rules: { + "no-chained-type-assertions": noChainedTypeAssertionsRule, + "no-conditional-empty-object-spread": noConditionalEmptyObjectSpreadRule, + "no-known-value-widening": noKnownValueWideningRule, + "no-module-mocking": noModuleMockingRule, + "no-object-parameters": noObjectParametersRule, + "no-reflect-apply": noReflectApplyRule, + "no-reflect-get": noReflectGetRule, + "no-runtime-typeof": noRuntimeTypeofRule, + "no-unsafe-dictionary-type": noUnsafeDictionaryTypeRule, + "no-shape-in-symbol-names": noForbiddenTermInSymbolNamesRule, + "no-unknown-parameters": noUnknownParametersRule, + "no-unknown-returns": noUnknownReturnsRule, + "no-unknown-type-aliases": noUnknownTypeAliasesRule, + "no-widen-then-assert": noWidenThenAssertRule, + "require-safety-comment-for-type-assertion": requireSafetyCommentForTypeAssertionRule, + }, +}); + +export default antiSlopPlugin; diff --git a/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts b/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts new file mode 100644 index 00000000..0d118527 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts @@ -0,0 +1,77 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +type TypeAssertionExpression = ESTree.TSAsExpression | ESTree.TSTypeAssertion; + +function isTypeAssertionExpression(node: ESTree.Node): node is TypeAssertionExpression { + return node.type === "TSAsExpression" || node.type === "TSTypeAssertion"; +} + +function unwrapParenthesizedExpression(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while (current.type === "ParenthesizedExpression") { + current = current.expression; + } + return current; +} + +function isConstAssertion(node: TypeAssertionExpression): boolean { + const { typeAnnotation } = node; + return ( + typeAnnotation.type === "TSTypeReference" && + typeAnnotation.typeName.type === "Identifier" && + typeAnnotation.typeName.name === "const" + ); +} + +function isOutermostAssertionInChain(node: TypeAssertionExpression): boolean { + let current: ESTree.Expression = node; + let parent = node.parent; + + while (parent.type === "ParenthesizedExpression" && parent.expression === current) { + current = parent; + parent = parent.parent; + } + + return !isTypeAssertionExpression(parent) || parent.expression !== current; +} + +function isForbiddenAssertionChain(node: TypeAssertionExpression): boolean { + let assertionCount = 0; + let hasNonConstAssertion = false; + let current: ESTree.Expression = node; + + while (isTypeAssertionExpression(current)) { + assertionCount += 1; + hasNonConstAssertion ||= !isConstAssertion(current); + current = unwrapParenthesizedExpression(current.expression); + } + + return assertionCount > 1 && hasNonConstAssertion; +} + +/** Disallow nested TypeScript type assertions, while permitting chains made only of const assertions. */ +export const noChainedTypeAssertionsRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow chained TypeScript as and angle-bracket assertions, including parenthesized chains.", + }, + messages: { + chained: + "This assertion chain discards type evidence. Keep the original precise type, or parse untrusted input at its boundary before narrowing it.", + }, + }, + createOnce(context) { + const checkTypeAssertion = (node: TypeAssertionExpression) => { + if (!isOutermostAssertionInChain(node) || !isForbiddenAssertionChain(node)) return; + context.report({ node, messageId: "chained" }); + }; + + return { + TSAsExpression: checkTypeAssertion, + TSTypeAssertion: checkTypeAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts b/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts new file mode 100644 index 00000000..ae7248d3 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts @@ -0,0 +1,49 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +function unwrapParentheses(node: ESTree.Expression): ESTree.Expression { + let current = node; + while (current.type === "ParenthesizedExpression") { + current = current.expression; + } + return current; +} + +function isEmptyObjectExpression(node: ESTree.Expression): boolean { + return node.type === "ObjectExpression" && node.properties.length === 0; +} + +function isConditionalEmptyObjectSpread(node: ESTree.Expression): boolean { + const conditional = unwrapParentheses(node); + return ( + conditional.type === "ConditionalExpression" && + (isEmptyObjectExpression(conditional.consequent) || + isEmptyObjectExpression(conditional.alternate)) + ); +} + +/** Ban conditional empty-object spreads without changing their omission semantics. */ +export const noConditionalEmptyObjectSpreadRule = defineRule({ + meta: { + type: "suggestion", + docs: { + description: + "Disallow object spreads that conditionally spread an empty object to omit fields.", + }, + messages: { + avoid: + "This conditional spread hides property omission behind an empty object. Build the object in separate statements and add the property only when present.", + }, + }, + createOnce(context) { + return { + SpreadElement(node) { + if (node.parent.type !== "ObjectExpression") return; + + if (isConditionalEmptyObjectSpread(node.argument)) { + context.report({ node, messageId: "avoid" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-known-value-widening.ts b/tools/oxlint/anti-slop/rules/no-known-value-widening.ts new file mode 100644 index 00000000..2a6806c6 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-known-value-widening.ts @@ -0,0 +1,247 @@ +import { defineRule } from "@oxlint/plugins"; + +import { + classifyWideningTarget, + createTypeEnvironment, + isKnownEvidenceExpression, + type TypeEnvironment, + type WideningTarget, +} from "../shared/dictionary-types.ts"; + +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +type FunctionExpression = ESTree.ArrowFunctionExpression | ESTree.Function; + +function unwrapExpression(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSSatisfiesExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" + ) { + current = current.expression; + } + return current; +} + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function variableDeclarator(variable: Variable): ESTree.VariableDeclarator | null { + if (variable.defs.length !== 1) return null; + const [definition] = variable.defs; + return definition?.type === "Variable" && definition.node.type === "VariableDeclarator" + ? definition.node + : null; +} + +function isStableConstVariable(variable: Variable, declarator: ESTree.VariableDeclarator): boolean { + return ( + declarator.parent.type === "VariableDeclaration" && + declarator.parent.kind === "const" && + variable.references.every((reference) => reference.init || !reference.isWrite()) + ); +} + +function hasKnownEvidence( + sourceCode: SourceCode, + expression: ESTree.Expression, + visitedVariables = new Set(), +): boolean { + if (isKnownEvidenceExpression(expression)) return true; + const unwrapped = unwrapExpression(expression); + if (unwrapped.type !== "Identifier") return false; + const variable = resolveVariable(sourceCode, unwrapped); + if (variable === null || visitedVariables.has(variable)) return false; + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.init === null || + !isStableConstVariable(variable, declarator) + ) { + return false; + } + visitedVariables.add(variable); + return hasKnownEvidence(sourceCode, declarator.init, visitedVariables); +} + +function annotationTarget( + annotation: ESTree.TSTypeAnnotation | null | undefined, + environment: TypeEnvironment, +): WideningTarget | null { + return annotation === null || annotation === undefined + ? null + : classifyWideningTarget(annotation.typeAnnotation, environment); +} + +function enclosingFunction(node: ESTree.Node): FunctionExpression | null { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if ( + current.type === "ArrowFunctionExpression" || + current.type === "FunctionDeclaration" || + current.type === "FunctionExpression" + ) { + return current; + } + current = current.parent; + } + return null; +} + +function sourceKeyName(sourceCode: SourceCode, key: ESTree.PropertyKey): string { + if (key.type === "Identifier" || key.type === "PrivateIdentifier") return key.name; + if (key.type === "Literal") return String(key.value); + return sourceCode.getText(key); +} + +function functionName(sourceCode: SourceCode, owner: FunctionExpression | null): string { + if (owner === null) return "anonymous function"; + if (owner.id !== null) return owner.id.name; + const parent = owner.parent; + if (parent.type === "VariableDeclarator" && parent.id.type === "Identifier") + return parent.id.name; + if (parent.type === "MethodDefinition") return sourceKeyName(sourceCode, parent.key); + return "anonymous function"; +} + +function isEmptyObjectExpression(expression: ESTree.Expression): boolean { + const unwrapped = unwrapExpression(expression); + return unwrapped.type === "ObjectExpression" && unwrapped.properties.length === 0; +} + +function isDictionaryAccumulatorTarget(destination: WideningTarget): boolean { + return destination.kind === "open dictionary" || destination.kind === "generic container"; +} + +function hasParentAssertion(node: ESTree.Node): boolean { + return node.parent?.type === "TSAsExpression" || node.parent?.type === "TSTypeAssertion"; +} + +/** Detect sound syntactic cases where a known value is explicitly widened and loses evidence. */ +export const noKnownValueWideningRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow syntactically established values from flowing into explicitly broad or anonymous target types that discard useful evidence.", + }, + messages: { + widening: + "The explicit {{target}} type on {{subject}} discards known type evidence. Keep inference, validate with `satisfies`, or use a named owner contract.", + }, + }, + createOnce(context) { + let environment: TypeEnvironment | null = null; + + const reportFlow = ( + expression: ESTree.Expression, + destination: WideningTarget | null, + subject: string, + ) => { + if (destination === null) return; + if ( + isDictionaryAccumulatorTarget(destination) && + isEmptyObjectExpression(expression) + ) { + return; + } + if (!hasKnownEvidence(context.sourceCode, expression)) return; + context.report({ + node: expression, + messageId: "widening", + data: { subject, target: destination.kind }, + }); + }; + + const targetFromAnnotation = (annotation: ESTree.TSTypeAnnotation | null | undefined) => + environment === null ? null : annotationTarget(annotation, environment); + + return { + Program(node) { + environment = createTypeEnvironment(node); + }, + VariableDeclarator(node) { + if (node.init === null || node.id.type !== "Identifier") return; + reportFlow( + node.init, + targetFromAnnotation(node.id.typeAnnotation), + `binding \`${node.id.name}\``, + ); + }, + PropertyDefinition(node) { + if (node.value === null) return; + reportFlow( + node.value, + targetFromAnnotation(node.typeAnnotation), + `property \`${sourceKeyName(context.sourceCode, node.key)}\``, + ); + }, + AccessorProperty(node) { + if (node.value === null) return; + reportFlow( + node.value, + targetFromAnnotation(node.typeAnnotation), + `property \`${sourceKeyName(context.sourceCode, node.key)}\``, + ); + }, + AssignmentExpression(node) { + if (node.operator !== "=" || node.left.type !== "Identifier") return; + const variable = resolveVariable(context.sourceCode, node.left); + if (variable === null) return; + const declarator = variableDeclarator(variable); + if (declarator === null || declarator.id.type !== "Identifier") return; + reportFlow( + node.right, + targetFromAnnotation(declarator.id.typeAnnotation), + `binding \`${declarator.id.name}\``, + ); + }, + ReturnStatement(node) { + if (node.argument === null) return; + const owner = enclosingFunction(node); + reportFlow( + node.argument, + targetFromAnnotation(owner?.returnType), + `return value of \`${functionName(context.sourceCode, owner)}\``, + ); + }, + ArrowFunctionExpression(node) { + if (node.body.type === "BlockStatement") return; + reportFlow( + node.body, + targetFromAnnotation(node.returnType), + `return value of \`${functionName(context.sourceCode, node)}\``, + ); + }, + TSAsExpression(node) { + if (environment === null || hasParentAssertion(node)) return; + reportFlow( + node.expression, + classifyWideningTarget(node.typeAnnotation, environment), + "assertion", + ); + }, + TSTypeAssertion(node) { + if (environment === null || hasParentAssertion(node)) return; + reportFlow( + node.expression, + classifyWideningTarget(node.typeAnnotation, environment), + "assertion", + ); + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-module-mocking.ts b/tools/oxlint/anti-slop/rules/no-module-mocking.ts new file mode 100644 index 00000000..d6fb5b45 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-module-mocking.ts @@ -0,0 +1,91 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +const moduleMockMethods = new Set(["doMock", "mock", "unstable_mockModule"]); + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function importedName(node: ESTree.Node): string | null { + if (node.type !== "ImportSpecifier") return null; + return node.imported.type === "Identifier" ? node.imported.name : node.imported.value; +} + +function isTestFrameworkObject( + sourceCode: SourceCode, + expression: ESTree.Expression, +): expression is ESTree.IdentifierReference { + if (expression.type !== "Identifier") return false; + if ( + (expression.name === "vi" || expression.name === "jest") && + sourceCode.isGlobalReference(expression) + ) { + return true; + } + + const variable = resolveVariable(sourceCode, expression); + if (variable === null || variable.defs.length === 0) { + return expression.name === "vi" || expression.name === "jest"; + } + return variable.defs.some((definition) => { + if (definition.type !== "ImportBinding" || definition.parent?.type !== "ImportDeclaration") { + return false; + } + const source = definition.parent.source.value; + const name = importedName(definition.node); + return (source === "vitest" && name === "vi") || (source === "@jest/globals" && name === "jest"); + }); +} + +function moduleMockCall(sourceCode: SourceCode, callee: ESTree.Expression): boolean { + if (!("property" in callee) || !("object" in callee) || !("computed" in callee)) return false; + if (!isTestFrameworkObject(sourceCode, callee.object)) return false; + const property = callee.property; + const method = callee.computed + ? property.type === "Literal" && + (property.value === "doMock" || + property.value === "mock" || + property.value === "unstable_mockModule") + ? property.value + : null + : property.type === "Identifier" + ? property.name + : null; + return method !== null && moduleMockMethods.has(method); +} + +/** Ban test framework module mocking in favor of real dependency seams. */ +export const noModuleMockingRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Vitest and Jest module mocking; tests must replace dependencies through real interfaces.", + }, + messages: { + moduleMock: + "Replace module mocking with dependency injection through a real interface, service layer, or faithful test implementation.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (moduleMockCall(context.sourceCode, node.callee)) { + context.report({ node, messageId: "moduleMock" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-object-parameters.ts b/tools/oxlint/anti-slop/rules/no-object-parameters.ts new file mode 100644 index 00000000..29b990f3 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-object-parameters.ts @@ -0,0 +1,126 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, SourceCode } from "@oxlint/plugins"; + +import { lexicalTypeParameterNames } from "../shared/lexical-type-parameters.ts"; + +type Parameter = ESTree.ParamPattern; +type ParameterOwner = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function parameterAnnotation(parameter: Parameter): ESTree.TSTypeAnnotation | null | undefined { + if (parameter.type === "TSParameterProperty") { + return parameterAnnotation(parameter.parameter); + } + if (parameter.type === "RestElement") { + return parameter.typeAnnotation ?? parameterAnnotation(parameter.argument); + } + if (parameter.type === "AssignmentPattern") { + return parameter.typeAnnotation ?? parameter.left.typeAnnotation; + } + return parameter.typeAnnotation; +} + +function parameterName(parameter: Parameter, sourceCode: SourceCode): string { + return parameter.type === "Identifier" + ? parameter.name + : sourceCode.getText(parameter).replace(/\s*:\s*object\s*$/u, ""); +} + +/** Ban the broad object type on function inputs, including local aliases to object. */ +export const noObjectParametersRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow object function parameters; inputs must use an owner-provided type and be parsed at their boundary.", + }, + messages: { + objectParameter: + "Parameter `{{parameter}}` uses the broad `object` type. Accept a named owner type; parse external input at its boundary before calling this function.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToObject = ( + type: ESTree.TSType, + shadowedAliases: ReadonlySet, + visited = new Set(), + ): boolean => { + if (type.type === "TSObjectKeyword") return true; + if (type.type === "TSParenthesizedType") + return resolvesToObject(type.typeAnnotation, shadowedAliases, visited); + if (type.type === "TSUnionType") { + return type.types.some((member) => + resolvesToObject(member, shadowedAliases, visited), + ); + } + if ( + type.type !== "TSTypeReference" || + type.typeName.type !== "Identifier" || + (type.typeArguments !== null && + type.typeArguments !== undefined && + type.typeArguments.params.length > 0) || + visited.has(type.typeName.name) || + shadowedAliases.has(type.typeName.name) + ) { + return false; + } + const alias = aliases.get(type.typeName.name); + if (alias === undefined) return false; + const nextVisited = new Set(visited); + nextVisited.add(type.typeName.name); + return resolvesToObject(alias, shadowedAliases, nextVisited); + }; + + const checkParameters = (node: ParameterOwner) => { + const shadowedAliases = lexicalTypeParameterNames( + node, + context.sourceCode.visitorKeys, + ); + for (const parameter of node.params) { + const annotation = parameterAnnotation(parameter); + if (annotation === null || annotation === undefined) continue; + if (!resolvesToObject(annotation.typeAnnotation, shadowedAliases)) continue; + context.report({ + node: annotation.typeAnnotation, + messageId: "objectParameter", + data: { parameter: parameterName(parameter, context.sourceCode) }, + }); + } + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if ( + declaration?.type === "TSTypeAliasDeclaration" && + (declaration.typeParameters === null || declaration.typeParameters === undefined) + ) { + aliases.set(declaration.id.name, declaration.typeAnnotation); + } + } + }, + ArrowFunctionExpression: checkParameters, + FunctionDeclaration: checkParameters, + FunctionExpression: checkParameters, + TSCallSignatureDeclaration: checkParameters, + TSConstructSignatureDeclaration: checkParameters, + TSConstructorType: checkParameters, + TSDeclareFunction: checkParameters, + TSEmptyBodyFunctionExpression: checkParameters, + TSFunctionType: checkParameters, + TSMethodSignature: checkParameters, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-reflect-apply.ts b/tools/oxlint/anti-slop/rules/no-reflect-apply.ts new file mode 100644 index 00000000..2cc30451 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-reflect-apply.ts @@ -0,0 +1,28 @@ +import { defineRule } from "@oxlint/plugins"; + +import { isGlobalReflectMethodCall } from "../shared/reflect-method.ts"; + +/** Ban Reflect.apply, which bypasses ordinary typed function calls. */ +export const noReflectApplyRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Reflect.apply; call typed functions directly or model dynamic dispatch behind an interface.", + }, + messages: { + reflectApply: + "Replace `Reflect.apply` with a typed function call. Model dynamic dispatch behind a named interface.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (isGlobalReflectMethodCall(context.sourceCode, node.callee, "apply")) { + context.report({ node, messageId: "reflectApply" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-reflect-get.ts b/tools/oxlint/anti-slop/rules/no-reflect-get.ts new file mode 100644 index 00000000..cf630ecc --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-reflect-get.ts @@ -0,0 +1,28 @@ +import { defineRule } from "@oxlint/plugins"; + +import { isGlobalReflectMethodCall } from "../shared/reflect-method.ts"; + +/** Ban Reflect.get, which bypasses ordinary property access and useful type evidence. */ +export const noReflectGetRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Reflect.get; use typed property access or parse dynamic input into a domain type.", + }, + messages: { + reflectGet: + "Replace `Reflect.get` with typed property access. Parse dynamic input into a named domain type before reading it.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (isGlobalReflectMethodCall(context.sourceCode, node.callee, "get")) { + context.report({ node, messageId: "reflectGet" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts b/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts new file mode 100644 index 00000000..6a25c247 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts @@ -0,0 +1,67 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +type RuntimeFunction = ESTree.ArrowFunctionExpression | ESTree.Function; + +function isRuntimeFunction(node: ESTree.Node): node is RuntimeFunction { + return ( + node.type === "ArrowFunctionExpression" || + node.type === "FunctionDeclaration" || + node.type === "FunctionExpression" + ); +} + +function isInsideTypeGuard(node: ESTree.Node): boolean { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (isRuntimeFunction(current)) { + return current.returnType?.typeAnnotation.type === "TSTypePredicate"; + } + current = current.parent; + } + return false; +} + +/** Disallow runtime typeof checks that narrow unparsed values instead of decoding them. */ +export const noRuntimeTypeofRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow runtime typeof checks; external values must be decoded into meaningful types at their I/O boundary.", + }, + messages: { + runtimeTypeof: + "A `typeof` check narrows a representation without establishing its contract. Parse input at its I/O boundary, then branch on the domain value.", + }, + schema: [ + { + type: "object", + properties: { + allowInTypeGuards: { type: "boolean" }, + }, + additionalProperties: false, + }, + ], + defaultOptions: [{ allowInTypeGuards: false }], + }, + createOnce(context) { + return { + UnaryExpression(node) { + const option = context.options?.[0]; + const allowInTypeGuards = + typeof option === "object" && + option !== null && + !Array.isArray(option) && + option.allowInTypeGuards === true; + if ( + node.operator === "typeof" && + (!allowInTypeGuards || !isInsideTypeGuard(node)) + ) { + context.report({ node, messageId: "runtimeTypeof" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts b/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts new file mode 100644 index 00000000..afc00dd4 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts @@ -0,0 +1,39 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +const FORBIDDEN_SYMBOL_NAME = "shape"; + +function containsForbiddenSymbolName(name: string): boolean { + return name.toLowerCase().includes(FORBIDDEN_SYMBOL_NAME); +} + +/** Ban the case-insensitive substring "shape" in every JavaScript and TypeScript symbol name. */ +export const noForbiddenTermInSymbolNamesRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + 'Disallow the case-insensitive substring "shape" in JavaScript, TypeScript, private, and JSX symbol names.', + }, + messages: { + forbiddenSymbolName: + 'Rename symbol "{{name}}" for its domain role; "shape" describes structure rather than ownership.', + }, + }, + createOnce(context) { + const reportForbiddenSymbolName = (node: ESTree.Node & { name: string }) => { + if (!containsForbiddenSymbolName(node.name)) return; + context.report({ + node, + messageId: "forbiddenSymbolName", + data: { name: node.name }, + }); + }; + + return { + Identifier: reportForbiddenSymbolName, + PrivateIdentifier: reportForbiddenSymbolName, + JSXIdentifier: reportForbiddenSymbolName, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts b/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts new file mode 100644 index 00000000..cdc6c235 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts @@ -0,0 +1,83 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +type Parameter = ESTree.ParamPattern; +type ParameterOwner = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function parameterAnnotation(parameter: Parameter): ESTree.TSTypeAnnotation | null | undefined { + if (parameter.type === "TSParameterProperty") { + return parameterAnnotation(parameter.parameter); + } + if (parameter.type === "RestElement") { + return parameter.typeAnnotation ?? parameterAnnotation(parameter.argument); + } + if (parameter.type === "AssignmentPattern") { + return parameter.typeAnnotation ?? parameter.left.typeAnnotation; + } + return parameter.typeAnnotation; +} + +function parameterName(parameter: Parameter, sourceText: string): string { + if (parameter.type === "TSParameterProperty") { + return parameterName(parameter.parameter, sourceText); + } + if (parameter.type === "AssignmentPattern") { + return parameterName(parameter.left, sourceText); + } + if (parameter.type === "RestElement") { + return parameterName(parameter.argument, sourceText); + } + return parameter.type === "Identifier" + ? parameter.name + : sourceText.replace(/\s*:\s*unknown\s*$/u, ""); +} + +/** Disallow unknown inputs except explicitly named error-cause enrichment. */ +export const noUnknownParametersRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow explicitly unknown function parameters except `cause`; decode unknown input at its I/O boundary instead.", + }, + messages: { + unknownParameter: + "Parameter `{{parameter}}` leaves input unparsed. Accept a named domain type; run the expected schema or parser at the I/O boundary before calling this function.", + }, + }, + createOnce(context) { + const checkParameters = (node: ParameterOwner) => { + for (const parameter of node.params) { + const annotation = parameterAnnotation(parameter); + if (annotation?.typeAnnotation.type !== "TSUnknownKeyword") continue; + const name = parameterName(parameter, context.sourceCode.getText(parameter)); + if (name === "cause") continue; + context.report({ + node: annotation.typeAnnotation, + messageId: "unknownParameter", + data: { parameter: name }, + }); + } + }; + + return { + ArrowFunctionExpression: checkParameters, + FunctionDeclaration: checkParameters, + FunctionExpression: checkParameters, + TSCallSignatureDeclaration: checkParameters, + TSConstructSignatureDeclaration: checkParameters, + TSConstructorType: checkParameters, + TSDeclareFunction: checkParameters, + TSEmptyBodyFunctionExpression: checkParameters, + TSFunctionType: checkParameters, + TSMethodSignature: checkParameters, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-returns.ts b/tools/oxlint/anti-slop/rules/no-unknown-returns.ts new file mode 100644 index 00000000..4b16d6ef --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-returns.ts @@ -0,0 +1,115 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +import { lexicalTypeParameterNames } from "../shared/lexical-type-parameters.ts"; + +type FunctionWithReturnType = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function referencedAliasName(type: ESTree.TSType): string | null { + if (type.type === "TSParenthesizedType") return referencedAliasName(type.typeAnnotation); + if (type.type !== "TSTypeReference" || type.typeName.type !== "Identifier") return null; + return type.typeArguments === null || + type.typeArguments === undefined || + type.typeArguments.params.length === 0 + ? type.typeName.name + : null; +} + +/** Ban function contracts that return unknown instead of a parsed domain type. */ +export const noUnknownReturnsRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow functions whose explicit return contract is unknown or Promise.", + }, + messages: { + unknownReturn: + "This function exposes `unknown` to its caller. Parse the value at its boundary and return a named domain type.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToUnknown = ( + type: ESTree.TSType, + shadowedAliases: ReadonlySet, + visited = new Set(), + ): boolean => { + if (type.type === "TSUnknownKeyword") return true; + if (type.type === "TSParenthesizedType") { + return resolvesToUnknown(type.typeAnnotation, shadowedAliases, visited); + } + if (type.type === "TSUnionType") { + return type.types.some((member) => + resolvesToUnknown(member, shadowedAliases, visited), + ); + } + if ( + type.type === "TSTypeReference" && + type.typeName.type === "Identifier" && + (type.typeName.name === "Promise" || type.typeName.name === "PromiseLike") + ) { + const value = type.typeArguments?.params[0]; + return value !== undefined && resolvesToUnknown(value, shadowedAliases, visited); + } + const name = referencedAliasName(type); + if (name === null || visited.has(name) || shadowedAliases.has(name)) return false; + const alias = aliases.get(name); + if ( + alias === undefined || + (alias.typeParameters !== null && alias.typeParameters !== undefined) + ) { + return false; + } + const nextVisited = new Set(visited); + nextVisited.add(name); + return resolvesToUnknown(alias.typeAnnotation, shadowedAliases, nextVisited); + }; + + const checkReturnType = (node: FunctionWithReturnType) => { + const annotation = node.returnType; + if (annotation === null || annotation === undefined) return; + if ( + !resolvesToUnknown( + annotation.typeAnnotation, + lexicalTypeParameterNames(node, context.sourceCode.visitorKeys), + ) + ) { + return; + } + context.report({ node: annotation.typeAnnotation, messageId: "unknownReturn" }); + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if (declaration?.type === "TSTypeAliasDeclaration") { + aliases.set(declaration.id.name, declaration); + } + } + }, + ArrowFunctionExpression: checkReturnType, + FunctionDeclaration: checkReturnType, + FunctionExpression: checkReturnType, + TSCallSignatureDeclaration: checkReturnType, + TSConstructSignatureDeclaration: checkReturnType, + TSConstructorType: checkReturnType, + TSDeclareFunction: checkReturnType, + TSEmptyBodyFunctionExpression: checkReturnType, + TSFunctionType: checkReturnType, + TSMethodSignature: checkReturnType, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts b/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts new file mode 100644 index 00000000..3e328fdf --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts @@ -0,0 +1,70 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +function referencedAliasName(type: ESTree.TSType): string | null { + if (type.type === "TSParenthesizedType") return referencedAliasName(type.typeAnnotation); + if (type.type !== "TSTypeReference" || type.typeName.type !== "Identifier") return null; + return type.typeArguments === null || + type.typeArguments === undefined || + type.typeArguments.params.length === 0 + ? type.typeName.name + : null; +} + +/** Ban named aliases that merely conceal TypeScript's unknown top type. */ +export const noUnknownTypeAliasesRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow type aliases whose resolved type is unknown; unknown must remain visible at an allowed boundary.", + }, + messages: { + unknownAlias: + "Type alias `{{alias}}` hides `unknown`. Keep `unknown` explicit at the parsing boundary or on an allowed `cause` field; otherwise use the parsed owner type.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToUnknown = (type: ESTree.TSType, visited = new Set()): boolean => { + if (type.type === "TSUnknownKeyword") return true; + if (type.type === "TSParenthesizedType") + return resolvesToUnknown(type.typeAnnotation, visited); + const name = referencedAliasName(type); + if (name === null || visited.has(name)) return false; + const alias = aliases.get(name); + if ( + alias === undefined || + (alias.typeParameters !== null && alias.typeParameters !== undefined) + ) { + return false; + } + const nextVisited = new Set(visited); + nextVisited.add(name); + return resolvesToUnknown(alias.typeAnnotation, nextVisited); + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if (declaration?.type === "TSTypeAliasDeclaration") { + aliases.set(declaration.id.name, declaration); + } + } + for (const alias of aliases.values()) { + if (!resolvesToUnknown(alias.typeAnnotation, new Set([alias.id.name]))) continue; + context.report({ + node: alias.id, + messageId: "unknownAlias", + data: { alias: alias.id.name }, + }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts b/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts new file mode 100644 index 00000000..8c45eed2 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts @@ -0,0 +1,134 @@ +import { defineRule } from "@oxlint/plugins"; + +import { + classifyUnsafeDictionary, + classifyUnsafeDictionaryValue, + createTypeEnvironment, + type TypeEnvironment, +} from "../shared/dictionary-types.ts"; + +import type { ESTree } from "@oxlint/plugins"; + +const typeNodeKinds: ReadonlySet = new Set([ + "JSDocNonNullableType", + "JSDocNullableType", + "JSDocUnknownType", + "TSAnyKeyword", + "TSArrayType", + "TSBigIntKeyword", + "TSBooleanKeyword", + "TSConditionalType", + "TSConstructorType", + "TSFunctionType", + "TSImportType", + "TSIndexedAccessType", + "TSInferType", + "TSIntersectionType", + "TSIntrinsicKeyword", + "TSLiteralType", + "TSMappedType", + "TSNamedTupleMember", + "TSNeverKeyword", + "TSNullKeyword", + "TSNumberKeyword", + "TSObjectKeyword", + "TSParenthesizedType", + "TSStringKeyword", + "TSSymbolKeyword", + "TSTemplateLiteralType", + "TSThisType", + "TSTupleType", + "TSTypeLiteral", + "TSTypeOperator", + "TSTypePredicate", + "TSTypeQuery", + "TSTypeReference", + "TSUndefinedKeyword", + "TSUnionType", + "TSUnknownKeyword", + "TSVoidKeyword", +]); + +function isTypeNode(node: ESTree.Node): node is ESTree.TSType { + return typeNodeKinds.has(node.type); +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isInsideTypeAliasDeclaration(node: ESTree.Node): boolean { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (current.type === "TSTypeAliasDeclaration") return true; + current = current.parent; + } + return false; +} + +function isPlainAliasConsumerUse(node: ESTree.TSType, environment: TypeEnvironment): boolean { + if (node.type !== "TSTypeReference" || node.typeArguments?.params.length) return false; + const name = typeReferenceName(node); + return name !== null && environment.aliases.has(name) && !isInsideTypeAliasDeclaration(node); +} + +function shouldReportType(node: ESTree.TSType, environment: TypeEnvironment): boolean { + if (isPlainAliasConsumerUse(node, environment)) return false; + if (classifyUnsafeDictionary(node, environment) === null) return false; + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (isTypeNode(current) && classifyUnsafeDictionary(current, environment) !== null) + return false; + current = current.parent; + } + return true; +} + +/** Disallow object-dictionary contracts whose direct value type is an unsafe escape hatch. */ +export const noUnsafeDictionaryTypeRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow object-dictionary contracts whose direct value type is unknown, any, object, {}, or a union/alias containing one of those escape hatches.", + }, + messages: { + unsafeDictionary: + "This dictionary's {{value}} value type gives callers no concrete value contract. Use an owner/schema-derived value type; parse external payloads before insertion.", + }, + }, + createOnce(context) { + let environment: TypeEnvironment | null = null; + const report = (node: ESTree.Node, value: string) => { + context.report({ node, messageId: "unsafeDictionary", data: { value } }); + }; + const reportIfUnsafe = (node: ESTree.TSType) => { + if (environment === null || !shouldReportType(node, environment)) return; + const unsafe = classifyUnsafeDictionary(node, environment); + if (unsafe === null) return; + report(node, unsafe.unsafeValue); + }; + + return { + Program(node) { + environment = createTypeEnvironment(node); + }, + TSTypeReference: reportIfUnsafe, + TSTypeLiteral: reportIfUnsafe, + TSMappedType: reportIfUnsafe, + TSIndexSignature(node) { + if ( + environment === null || + node.typeAnnotation === null || + node.parent.type === "TSTypeLiteral" + ) + return; + const unsafe = classifyUnsafeDictionaryValue( + node.typeAnnotation.typeAnnotation, + environment, + ); + if (unsafe !== null) report(node, unsafe.unsafeValue); + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts b/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts new file mode 100644 index 00000000..c5e07f7f --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts @@ -0,0 +1,366 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree, Variable } from "@oxlint/plugins"; + +type BroadTypeKind = "top" | "object" | "record"; + +type KnownValueEvidence = { + readonly type: ESTree.TSType | null; +}; + +const functionBoundaryTypes = new Set([ + "ArrowFunctionExpression", + "FunctionDeclaration", + "FunctionExpression", + "TSDeclareFunction", + "TSEmptyBodyFunctionExpression", +]); + +function unwrapExpressionParentheses(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while (current.type === "ParenthesizedExpression") current = current.expression; + return current; +} + +function unwrapTypeParentheses(type: ESTree.TSType): ESTree.TSType { + let current = type; + while (current.type === "TSParenthesizedType") current = current.typeAnnotation; + return current; +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isUnknownOrAnyType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + return unwrapped.type === "TSUnknownKeyword" || unwrapped.type === "TSAnyKeyword"; +} + +function isBroadRecordKeyType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + if ( + unwrapped.type === "TSStringKeyword" || + unwrapped.type === "TSNumberKeyword" || + unwrapped.type === "TSSymbolKeyword" + ) { + return true; + } + if (unwrapped.type === "TSUnionType") return unwrapped.types.every(isBroadRecordKeyType); + return unwrapped.type === "TSTypeReference" && typeReferenceName(unwrapped) === "PropertyKey"; +} + +function isBroadRecordType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + + if (unwrapped.type === "TSTypeReference") { + if (typeReferenceName(unwrapped) === "Readonly") { + const [inner] = unwrapped.typeArguments?.params ?? []; + return inner !== undefined && isBroadRecordType(inner); + } + + if (typeReferenceName(unwrapped) !== "Record") return false; + const parameters = unwrapped.typeArguments?.params ?? []; + return ( + parameters.length === 2 && + parameters[0] !== undefined && + parameters[1] !== undefined && + isBroadRecordKeyType(parameters[0]) && + isUnknownOrAnyType(parameters[1]) + ); + } + + if (unwrapped.type !== "TSTypeLiteral" || unwrapped.members.length !== 1) return false; + const [member] = unwrapped.members; + const [parameter] = member?.type === "TSIndexSignature" ? member.parameters : []; + return ( + member?.type === "TSIndexSignature" && + member.parameters.length === 1 && + parameter !== undefined && + isBroadRecordKeyType(parameter.typeAnnotation.typeAnnotation) && + isUnknownOrAnyType(member.typeAnnotation.typeAnnotation) + ); +} + +function broadTypeKind(type: ESTree.TSType): BroadTypeKind | null { + const unwrapped = unwrapTypeParentheses(type); + if (unwrapped.type === "TSUnknownKeyword" || unwrapped.type === "TSAnyKeyword") return "top"; + if (unwrapped.type === "TSObjectKeyword") return "object"; + return isBroadRecordType(unwrapped) ? "record" : null; +} + +function assertedExpression( + node: ESTree.TSAsExpression | ESTree.TSTypeAssertion, +): ESTree.Expression { + return unwrapExpressionParentheses(node.expression); +} + +function assertionFromExpression( + expression: ESTree.Expression, +): ESTree.TSAsExpression | ESTree.TSTypeAssertion | null { + const unwrapped = unwrapExpressionParentheses(expression); + return unwrapped.type === "TSAsExpression" || unwrapped.type === "TSTypeAssertion" + ? unwrapped + : null; +} + +function normalizedTypeText(sourceText: string, type: ESTree.TSType): string { + return sourceText.slice(type.start, type.end).replaceAll(/\s+/gu, ""); +} + +function typesHaveSameSyntax( + sourceText: string, + left: ESTree.TSType | null, + right: ESTree.TSType, +): boolean { + return ( + left !== null && + normalizedTypeText(sourceText, unwrapTypeParentheses(left)) === + normalizedTypeText(sourceText, unwrapTypeParentheses(right)) + ); +} + +function isDefinitelyObjectType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + switch (unwrapped.type) { + case "TSArrayType": + case "TSConstructorType": + case "TSFunctionType": + case "TSMappedType": + case "TSObjectKeyword": + case "TSTupleType": + return true; + case "TSTypeLiteral": + return unwrapped.members.length > 0; + case "TSIntersectionType": + return unwrapped.types.every(isDefinitelyObjectType); + case "TSTypeOperator": + return unwrapped.operator === "readonly" && isDefinitelyObjectType(unwrapped.typeAnnotation); + default: + return false; + } +} + +function isDefinitelyNarrowerRecordType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type !== "TSIndexSignature"); + } + + if (unwrapped.type !== "TSTypeReference") return false; + if (typeReferenceName(unwrapped) === "Readonly") { + const [inner] = unwrapped.typeArguments?.params ?? []; + return inner !== undefined && isDefinitelyNarrowerRecordType(inner); + } + if (typeReferenceName(unwrapped) !== "Record") return false; + + const parameters = unwrapped.typeArguments?.params ?? []; + return ( + parameters.length === 2 && parameters[1] !== undefined && !isUnknownOrAnyType(parameters[1]) + ); +} + +function functionBoundary(node: ESTree.Node): ESTree.Node | null { + let current = node.parent; + while (current !== null && current.type !== "Program") { + if (functionBoundaryTypes.has(current.type)) return current; + current = current.parent; + } + return null; +} + +function resolvedVariableForIdentifier( + scopes: readonly { + readonly references: readonly { + readonly identifier: ESTree.Node; + readonly resolved: Variable | null; + }[]; + }[], + identifier: ESTree.IdentifierReference, +): Variable | null { + for (const scope of scopes) { + const reference = scope.references.find( + (candidate) => + candidate.identifier.start === identifier.start && + candidate.identifier.end === identifier.end, + ); + if (reference !== undefined) return reference.resolved; + } + return null; +} + +function variableDeclarator(variable: Variable): ESTree.VariableDeclarator | null { + for (const definition of variable.defs) { + if (definition.type === "Variable" && definition.node.type === "VariableDeclarator") { + return definition.node; + } + } + return null; +} + +function knownValueEvidence( + expression: ESTree.Expression, + scopes: Parameters[0], + boundary: ESTree.Node | null, + visitedVariables: ReadonlySet, +): KnownValueEvidence | null { + const unwrapped = unwrapExpressionParentheses(expression); + + if (unwrapped.type === "TSAsExpression" || unwrapped.type === "TSTypeAssertion") { + if (broadTypeKind(unwrapped.typeAnnotation) !== null) return null; + return { type: unwrapped.typeAnnotation }; + } + + if (unwrapped.type === "Literal" || unwrapped.type === "TemplateLiteral") { + return { type: null }; + } + + if ( + unwrapped.type === "ArrayExpression" || + unwrapped.type === "ArrowFunctionExpression" || + unwrapped.type === "ClassExpression" || + unwrapped.type === "FunctionExpression" || + unwrapped.type === "NewExpression" || + unwrapped.type === "ObjectExpression" + ) { + return { type: null }; + } + + if (unwrapped.type !== "Identifier") return null; + const variable = resolvedVariableForIdentifier(scopes, unwrapped); + if (variable === null || visitedVariables.has(variable)) return null; + + const annotatedIdentifier = variable.identifiers.find( + (identifier) => identifier.typeAnnotation !== null && identifier.typeAnnotation !== undefined, + ); + const annotation = annotatedIdentifier?.typeAnnotation?.typeAnnotation; + if (annotation !== undefined && annotatedIdentifier !== undefined) { + if (functionBoundary(annotatedIdentifier) !== boundary || broadTypeKind(annotation) !== null) { + return null; + } + return { type: annotation }; + } + + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.parent.type !== "VariableDeclaration" || + declarator.parent.kind !== "const" || + declarator.init === null || + variable.references.some((reference) => reference.isWrite() && !reference.init) || + functionBoundary(declarator) !== boundary + ) { + return null; + } + + return knownValueEvidence( + declarator.init, + scopes, + boundary, + new Set([...visitedVariables, variable]), + ); +} + +function widenedBinding( + variable: Variable, + scopes: Parameters[0], +): { + readonly broadKind: BroadTypeKind; + readonly evidence: KnownValueEvidence; + readonly declaredAt: number; + readonly boundary: ESTree.Node | null; +} | null { + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.parent.type !== "VariableDeclaration" || + declarator.parent.kind !== "const" || + declarator.id.type !== "Identifier" || + declarator.init === null || + variable.references.some((reference) => reference.isWrite() && !reference.init) + ) { + return null; + } + + const boundary = functionBoundary(declarator); + const declaredType = declarator.id.typeAnnotation?.typeAnnotation; + const initializerAssertion = assertionFromExpression(declarator.init); + const initializerBroadKind = + initializerAssertion === null ? null : broadTypeKind(initializerAssertion.typeAnnotation); + const declaredBroadKind = declaredType === undefined ? null : broadTypeKind(declaredType); + const broadKind = declaredBroadKind ?? initializerBroadKind; + if (broadKind === null) return null; + + const originalExpression = + initializerAssertion !== null && initializerBroadKind !== null + ? assertedExpression(initializerAssertion) + : declarator.init; + const evidence = knownValueEvidence(originalExpression, scopes, boundary, new Set([variable])); + return evidence === null ? null : { broadKind, evidence, declaredAt: declarator.end, boundary }; +} + +function assertionIsNarrower( + sourceText: string, + broadKind: BroadTypeKind, + evidence: KnownValueEvidence, + assertedType: ESTree.TSType, +): boolean { + if (broadTypeKind(assertedType) !== null) return false; + if (broadKind === "top") return true; + if (typesHaveSameSyntax(sourceText, evidence.type, assertedType)) return true; + if (broadKind === "object") return isDefinitelyObjectType(assertedType); + return isDefinitelyNarrowerRecordType(assertedType); +} + +/** Detect immutable local bindings that erase a known type and are later asserted back to a narrower type. */ +export const noWidenThenAssertRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow local const flows that explicitly widen a known value before asserting the widened binding to a narrower type.", + }, + messages: { + widenThenAssert: + 'Binding "{{name}}" discards type evidence and later recreates it with an assertion. Keep the precise type from initialization through use; parse boundary input once.', + }, + }, + createOnce(context) { + let scopes: Parameters[0] = []; + + const checkAssertion = (node: ESTree.TSAsExpression | ESTree.TSTypeAssertion) => { + const expression = assertedExpression(node); + if (expression.type !== "Identifier") return; + + const variable = resolvedVariableForIdentifier(scopes, expression); + if (variable === null) return; + const widened = widenedBinding(variable, scopes); + if ( + widened === null || + node.start <= widened.declaredAt || + functionBoundary(node) !== widened.boundary || + !assertionIsNarrower( + context.sourceCode.text, + widened.broadKind, + widened.evidence, + node.typeAnnotation, + ) + ) { + return; + } + + context.report({ + node, + messageId: "widenThenAssert", + data: { name: expression.name }, + }); + }; + + return { + Program() { + scopes = context.sourceCode.scopeManager.scopes; + }, + TSAsExpression: checkAssertion, + TSTypeAssertion: checkAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts b/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts new file mode 100644 index 00000000..f1a2ffcf --- /dev/null +++ b/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts @@ -0,0 +1,62 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, SourceCode } from "@oxlint/plugins"; + +type TypeAssertion = ESTree.TSAsExpression | ESTree.TSTypeAssertion; + +const commentOwnerKinds = new Set([ + "ExpressionStatement", + "PropertyDefinition", + "ReturnStatement", + "ThrowStatement", + "VariableDeclaration", +]); + +function isConstAssertion(node: TypeAssertion): boolean { + return ( + node.typeAnnotation.type === "TSTypeReference" && + node.typeAnnotation.typeName.type === "Identifier" && + node.typeAnnotation.typeName.name === "const" + ); +} + +function hasSafetyComment(sourceCode: SourceCode, node: TypeAssertion): boolean { + let current: ESTree.Node = node; + while (true) { + if ( + sourceCode + .getCommentsBefore(current) + .some((comment) => comment.end <= node.start && /\bSAFETY\s*:/u.test(comment.value)) + ) { + return true; + } + if (commentOwnerKinds.has(current.type) || current.parent.type === "Program") return false; + current = current.parent; + } +} + +/** Require every non-const type assertion to state the invariant TypeScript cannot express. */ +export const requireSafetyCommentForTypeAssertionRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Require a nearby SAFETY comment for every TypeScript type assertion except const assertions.", + }, + messages: { + missingSafetyComment: + "This type assertion has no `SAFETY:` justification. State the checked invariant immediately before the assertion or its containing statement.", + }, + }, + createOnce(context) { + const checkAssertion = (node: TypeAssertion) => { + if (isConstAssertion(node) || hasSafetyComment(context.sourceCode, node)) return; + context.report({ node, messageId: "missingSafetyComment" }); + }; + + return { + TSAsExpression: checkAssertion, + TSTypeAssertion: checkAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/shared/dictionary-types.ts b/tools/oxlint/anti-slop/shared/dictionary-types.ts new file mode 100644 index 00000000..86517004 --- /dev/null +++ b/tools/oxlint/anti-slop/shared/dictionary-types.ts @@ -0,0 +1,502 @@ +import type { ESTree } from "@oxlint/plugins"; + +const BUILT_INS = new Set([ + "Record", + "Readonly", + "Partial", + "Required", + "Pick", + "Omit", + "PropertyKey", + "NonNullable", +]); +const TRANSPARENT_WRAPPERS = new Set(["Readonly", "Partial", "Required", "NonNullable"]); + +type TypeAliasEnvironment = ReadonlyMap; + +type ResolvedType = { + readonly type: ESTree.TSType; + readonly substitutions: TypeAliasEnvironment; +}; + +export type UnsafeDictionary = { + readonly kind: "unsafe-dictionary"; + readonly unsafeValue: "any" | "empty-object" | "object" | "union" | "unknown"; +}; + +export type WideningTargetKind = + | "anonymous object" + | "generic container" + | "object" + | "open dictionary" + | "unknown"; + +export type WideningTarget = { + readonly kind: WideningTargetKind; +}; + +export type TypeEnvironment = { + readonly aliases: ReadonlyMap; + readonly interfaces: ReadonlyMap; + readonly shadowedBuiltIns: ReadonlySet; +}; + +function declaredStatement(statement: ESTree.Statement): ESTree.Node | null { + return statement.type === "ExportNamedDeclaration" || + statement.type === "ExportDefaultDeclaration" + ? (statement.declaration ?? null) + : statement; +} + +export function createTypeEnvironment(program: ESTree.Program): TypeEnvironment { + const aliases = new Map(); + const interfaces = new Map(); + const shadowedBuiltIns = new Set(); + + for (const statement of program.body) { + const declaration = declaredStatement(statement); + if (declaration?.type === "ImportDeclaration") { + for (const specifier of declaration.specifiers) { + if (BUILT_INS.has(specifier.local.name)) shadowedBuiltIns.add(specifier.local.name); + } + continue; + } + + if (declaration?.type === "TSTypeAliasDeclaration") { + const existing = aliases.get(declaration.id.name); + if (existing === undefined) aliases.set(declaration.id.name, declaration); + else shadowedBuiltIns.add(declaration.id.name); + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if (declaration?.type === "TSInterfaceDeclaration") { + const declarations = interfaces.get(declaration.id.name) ?? []; + declarations.push(declaration); + interfaces.set(declaration.id.name, declarations); + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if (declaration?.type === "TSEnumDeclaration") { + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if ( + (declaration?.type === "ClassDeclaration" || + declaration?.type === "FunctionDeclaration") && + declaration.id !== null + ) { + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + } + } + + return { aliases, interfaces, shadowedBuiltIns }; +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isBuiltIn(name: string, environment: TypeEnvironment): boolean { + return BUILT_INS.has(name) && !environment.shadowedBuiltIns.has(name); +} + +function isUnappliedReferenceTo(type: ESTree.TSType, name: string): boolean { + const unwrapped = unwrapTransparentType(type); + return ( + unwrapped.type === "TSTypeReference" && + typeReferenceName(unwrapped) === name && + (unwrapped.typeArguments === null || + unwrapped.typeArguments === undefined || + unwrapped.typeArguments.params.length === 0) + ); +} + +function unwrapTransparentType(type: ESTree.TSType): ESTree.TSType { + let current = type; + while ( + current.type === "TSParenthesizedType" || + (current.type === "TSTypeOperator" && current.operator === "readonly") + ) { + current = current.typeAnnotation; + } + return current; +} + +function isNeverType(type: ESTree.TSType): boolean { + return unwrapTransparentType(type).type === "TSNeverKeyword"; +} + +function isEffectivelyEmptyMember(member: ESTree.TSSignature): boolean { + return ( + member.type === "TSPropertySignature" && + member.optional === true && + member.typeAnnotation !== null && + member.typeAnnotation !== undefined && + isNeverType(member.typeAnnotation.typeAnnotation) + ); +} + +function isEffectivelyEmptyTypeLiteral(type: ESTree.TSTypeLiteral): boolean { + return type.members.length === 0 || type.members.every(isEffectivelyEmptyMember); +} + +function isEffectivelyEmptyInterface( + declarations: readonly ESTree.TSInterfaceDeclaration[], +): boolean { + if (declarations.length !== 1) return false; + const [type] = declarations; + return ( + type !== undefined && + type.extends.length === 0 && + (type.body.body.length === 0 || type.body.body.every(isEffectivelyEmptyMember)) + ); +} + +function resolvedSubstitutionArgument( + type: ESTree.TSType, + base: TypeAliasEnvironment, + resolving: ReadonlySet = new Set(), +): ESTree.TSType { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type !== "TSTypeReference") return type; + const name = typeReferenceName(unwrapped); + if (name === null || resolving.has(name)) return type; + const substitution = base.get(name); + if (substitution === undefined) return type; + const nextResolving = new Set(resolving); + nextResolving.add(name); + return resolvedSubstitutionArgument(substitution, base, nextResolving); +} + +function aliasSubstitution( + alias: ESTree.TSTypeAliasDeclaration, + type: ESTree.TSTypeReference, + base: TypeAliasEnvironment, +): TypeAliasEnvironment | null { + const parameters = alias.typeParameters?.params ?? []; + const arguments_ = type.typeArguments?.params ?? []; + const next = new Map(base); + for (const [index, parameter] of parameters.entries()) { + const argument = arguments_[index] ?? parameter.default; + if (argument === null || argument === undefined) return null; + next.set(parameter.name.name, resolvedSubstitutionArgument(argument, next)); + } + return next; +} + +function unsafeDirectValue( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): UnsafeDictionary["unsafeValue"] | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return "unknown"; + if (unwrapped.type === "TSAnyKeyword") return "any"; + if (unwrapped.type === "TSObjectKeyword") return "object"; + if (unwrapped.type === "TSTypeLiteral" && isEffectivelyEmptyTypeLiteral(unwrapped)) + return "empty-object"; + if (unwrapped.type === "TSUnionType") { + return unwrapped.types.some( + (member) => unsafeDirectValue(member, environment, substitutions, resolvingAliases) !== null, + ) + ? "union" + : null; + } + if (unwrapped.type === "TSIntersectionType") { + const unsafeMembers = unwrapped.types.map((member) => + unsafeDirectValue(member, environment, substitutions, resolvingAliases), + ); + if (unsafeMembers.includes("any")) return "any"; + return unsafeMembers.length > 0 && unsafeMembers.every((member) => member !== null) + ? unsafeMembers[0] + : null; + } + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? null + : unsafeDirectValue(wrapped, environment, substitutions, resolvingAliases); + } + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? null + : unsafeDirectValue(substitution, environment, substitutions, resolvingAliases); + } + const interfaceDeclarations = environment.interfaces.get(name); + if (interfaceDeclarations !== undefined) { + return isEffectivelyEmptyInterface(interfaceDeclarations) ? "empty-object" : null; + } + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return null; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return null; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return unsafeDirectValue(alias.typeAnnotation, environment, nextSubstitutions, nextResolving); +} + +function dictionaryValueTypes( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): readonly ResolvedType[] { + const unwrapped = unwrapTransparentType(type); + + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.flatMap((member): readonly ResolvedType[] => + member.type === "TSIndexSignature" && member.typeAnnotation !== null + ? [{ type: member.typeAnnotation.typeAnnotation, substitutions }] + : [], + ); + } + + if (unwrapped.type === "TSMappedType") { + return unwrapped.typeAnnotation === null + ? [] + : [{ type: unwrapped.typeAnnotation, substitutions }]; + } + + if (unwrapped.type !== "TSTypeReference") return []; + const name = typeReferenceName(unwrapped); + if (name === null) return []; + + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? [] + : dictionaryValueTypes(substitution, environment, substitutions, resolvingAliases); + } + + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? [] + : dictionaryValueTypes(wrapped, environment, substitutions, resolvingAliases); + } + + if (name === "Record" && isBuiltIn(name, environment)) { + const value = unwrapped.typeArguments?.params[1] ?? null; + return value === null ? [] : [{ type: value, substitutions }]; + } + + if ((name === "Pick" || name === "Omit") && isBuiltIn(name, environment)) { + const source = unwrapped.typeArguments?.params[0]; + return source === undefined + ? [] + : dictionaryValueTypes(source, environment, substitutions, resolvingAliases); + } + + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return []; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return []; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return dictionaryValueTypes(alias.typeAnnotation, environment, nextSubstitutions, nextResolving); +} + +export function classifyUnsafeDictionaryValue( + valueType: ESTree.TSType, + environment: TypeEnvironment, +): UnsafeDictionary | null { + const unsafeValue = unsafeDirectValue(valueType, environment, new Map(), new Set()); + return unsafeValue === null ? null : { kind: "unsafe-dictionary", unsafeValue }; +} + +export function classifyUnsafeDictionary( + type: ESTree.TSType, + environment: TypeEnvironment, +): UnsafeDictionary | null { + for (const valueType of dictionaryValueTypes(type, environment, new Map(), new Set())) { + const unsafeValue = unsafeDirectValue( + valueType.type, + environment, + valueType.substitutions, + new Set(), + ); + if (unsafeValue !== null) return { kind: "unsafe-dictionary", unsafeValue }; + } + return null; +} + +function resolvesToDictionary( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): boolean { + return dictionaryValueTypes(type, environment, substitutions, resolvingAliases).length > 0; +} + +export function classifyWideningTarget( + type: ESTree.TSType, + environment: TypeEnvironment, +): WideningTarget | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return { kind: "unknown" }; + if (unwrapped.type === "TSObjectKeyword") return { kind: "object" }; + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type === "TSIndexSignature") + ? { kind: "open dictionary" } + : unwrapped.members.length > 0 + ? { kind: "anonymous object" } + : null; + } + if (unwrapped.type === "TSMappedType") return { kind: "open dictionary" }; + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined ? null : classifyWideningTarget(wrapped, environment); + } + if (name === "Record" && isBuiltIn(name, environment)) return { kind: "open dictionary" }; + const alias = environment.aliases.get(name); + if (alias === undefined) return null; + if ((alias.typeParameters?.params.length ?? 0) > 0) { + const substitutions = aliasSubstitution(alias, unwrapped, new Map()); + return substitutions !== null && + resolvesToDictionary(alias.typeAnnotation, environment, substitutions, new Set([name])) + ? { kind: "generic container" } + : null; + } + const substitutions = aliasSubstitution(alias, unwrapped, new Map()); + if (substitutions === null) return null; + const resolved = classifyAliasBroadTarget( + alias.typeAnnotation, + environment, + substitutions, + new Set([name]), + ); + return resolved; +} + +function isBroadMappedKey( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, +): boolean { + const unwrapped = unwrapTransparentType(type); + if ( + unwrapped.type === "TSStringKeyword" || + unwrapped.type === "TSNumberKeyword" || + unwrapped.type === "TSSymbolKeyword" + ) { + return true; + } + if (unwrapped.type === "TSUnionType") { + return unwrapped.types.every((member) => + isBroadMappedKey(member, environment, substitutions), + ); + } + if (unwrapped.type !== "TSTypeReference") return false; + const name = typeReferenceName(unwrapped); + if (name === null) return false; + const substitution = substitutions.get(name); + if (substitution !== undefined && !isUnappliedReferenceTo(substitution, name)) { + return isBroadMappedKey(substitution, environment, substitutions); + } + return name === "PropertyKey" && isBuiltIn(name, environment); +} + +function classifyAliasBroadTarget( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): WideningTarget | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return { kind: "unknown" }; + if (unwrapped.type === "TSObjectKeyword") return { kind: "object" }; + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type === "TSIndexSignature") + ? { kind: "open dictionary" } + : null; + } + if (unwrapped.type === "TSMappedType") { + return isBroadMappedKey(unwrapped.constraint, environment, substitutions) + ? { kind: "open dictionary" } + : null; + } + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? null + : classifyAliasBroadTarget( + substitution, + environment, + substitutions, + resolvingAliases, + ); + } + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? null + : classifyAliasBroadTarget(wrapped, environment, substitutions, resolvingAliases); + } + if (name === "Record" && isBuiltIn(name, environment)) { + return { kind: "open dictionary" }; + } + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return null; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return null; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return classifyAliasBroadTarget( + alias.typeAnnotation, + environment, + nextSubstitutions, + nextResolving, + ); +} + +export function isPopulatedObjectExpression(expression: ESTree.Expression): boolean { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" + ) { + current = current.expression; + } + return current.type === "ObjectExpression" && current.properties.length > 0; +} + +export function isKnownEvidenceExpression(expression: ESTree.Expression): boolean { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" || + current.type === "TSSatisfiesExpression" + ) { + current = current.expression; + } + if (current.type === "ObjectExpression") return true; + return ( + current.type === "ArrayExpression" || + current.type === "ArrowFunctionExpression" || + current.type === "ClassExpression" || + current.type === "FunctionExpression" || + current.type === "NewExpression" || + current.type === "Literal" || + current.type === "TemplateLiteral" || + current.type === "UnaryExpression" + ); +} diff --git a/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts b/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts new file mode 100644 index 00000000..7cdb18c9 --- /dev/null +++ b/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts @@ -0,0 +1,61 @@ +import type { ESTree } from "@oxlint/plugins"; + +type VisitorKeys = Readonly>; + +function isNode(value: unknown): value is ESTree.Node { + return ( + typeof value === "object" && + value !== null && + "type" in value && + typeof value.type === "string" + ); +} + +function collectInferTypeParameterNames( + node: ESTree.Node, + visitorKeys: VisitorKeys, + names: Set, +): void { + if (node.type === "TSInferType") names.add(node.typeParameter.name.name); + const record = node as unknown as Readonly>; + for (const key of visitorKeys[node.type] ?? []) { + const value = record[key]; + if (isNode(value)) { + collectInferTypeParameterNames(value, visitorKeys, names); + continue; + } + if (!Array.isArray(value)) continue; + for (const child of value) { + if (isNode(child)) collectInferTypeParameterNames(child, visitorKeys, names); + } + } +} + +/** Collect type binders that are in scope at a node and can shadow module aliases. */ +export function lexicalTypeParameterNames( + node: ESTree.Node, + visitorKeys: VisitorKeys, +): ReadonlySet { + const names = new Set(); + let descendant: ESTree.Node = node; + let current: ESTree.Node | null = node; + while (current !== null && current.type !== "Program") { + if ("typeParameters" in current) { + for (const parameter of current.typeParameters?.params ?? []) { + names.add(parameter.name.name); + } + } + if ( + current.type === "TSMappedType" && + (descendant === current.nameType || descendant === current.typeAnnotation) + ) { + names.add(current.key.name); + } + if (current.type === "TSConditionalType" && descendant === current.trueType) { + collectInferTypeParameterNames(current.extendsType, visitorKeys, names); + } + descendant = current; + current = current.parent; + } + return names; +} diff --git a/tools/oxlint/anti-slop/shared/reflect-method.ts b/tools/oxlint/anti-slop/shared/reflect-method.ts new file mode 100644 index 00000000..39bc218c --- /dev/null +++ b/tools/oxlint/anti-slop/shared/reflect-method.ts @@ -0,0 +1,35 @@ +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function isGlobalReflect(sourceCode: SourceCode, expression: ESTree.Expression): boolean { + if (expression.type !== "Identifier" || expression.name !== "Reflect") return false; + if (sourceCode.isGlobalReference(expression)) return true; + const variable = resolveVariable(sourceCode, expression); + return variable === null || variable.defs.length === 0; +} + +/** Reports whether a call target names one method on the global Reflect object. */ +export function isGlobalReflectMethodCall( + sourceCode: SourceCode, + callee: ESTree.Expression, + methodName: string, +): boolean { + if (!("property" in callee) || !("object" in callee) || !("computed" in callee)) return false; + if (!isGlobalReflect(sourceCode, callee.object)) return false; + const property = callee.property; + return callee.computed + ? property.type === "Literal" && property.value === methodName + : property.type === "Identifier" && property.name === methodName; +}