diff --git a/package-lock.json b/package-lock.json index 43e3ba1a..299ce873 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,10 @@ "resolved": "packages/argent-cli", "link": true }, + "node_modules/@argent/configuration-core": { + "resolved": "packages/configuration-core", + "link": true + }, "node_modules/@argent/installer": { "resolved": "packages/argent-installer", "link": true @@ -3697,7 +3701,9 @@ "name": "@argent/cli", "version": "0.9.0", "dependencies": { - "@argent/tools-client": "file:../argent-tools-client" + "@argent/configuration-core": "file:../configuration-core", + "@argent/tools-client": "file:../argent-tools-client", + "picocolors": "^1.1.1" }, "devDependencies": { "@types/node": "^25.9.0", @@ -3791,6 +3797,7 @@ "name": "@argent/mcp", "version": "0.9.0", "dependencies": { + "@argent/configuration-core": "file:../configuration-core", "@argent/tools-client": "file:../argent-tools-client", "@modelcontextprotocol/sdk": "^1.29.0" }, @@ -3961,6 +3968,46 @@ "dev": true, "license": "MIT" }, + "packages/configuration-core": { + "name": "@argent/configuration-core", + "version": "0.9.0", + "devDependencies": { + "@types/node": "^25.9.0", + "typescript": "^6.0.3", + "vitest": "^4.1.6" + } + }, + "packages/configuration-core/node_modules/@types/node": { + "version": "25.9.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-25.9.1.tgz", + "integrity": "sha512-xfrlY7UD5rMJk3ZVJP8BNzS28J36YJg+xp+LPXV1TdWxr8uMH5A860QNxYDGQe/ylDSgjxE52Q9VnO7p75tJxg==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": ">=7.24.0 <7.24.7" + } + }, + "packages/configuration-core/node_modules/typescript": { + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-6.0.3.tgz", + "integrity": "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + }, + "packages/configuration-core/node_modules/undici-types": { + "version": "7.24.6", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.24.6.tgz", + "integrity": "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg==", + "dev": true, + "license": "MIT" + }, "packages/native-devtools-android": { "name": "@argent/native-devtools-android", "version": "0.9.0", @@ -4044,6 +4091,7 @@ "name": "@argent/tool-server", "version": "0.9.0", "dependencies": { + "@argent/configuration-core": "file:../configuration-core", "@argent/native-devtools-android": "file:../native-devtools-android", "@argent/native-devtools-ios": "file:../native-devtools-ios", "@argent/registry": "file:../registry", diff --git a/packages/argent-cli/package.json b/packages/argent-cli/package.json index d5cbacb7..9a0a8fed 100644 --- a/packages/argent-cli/package.json +++ b/packages/argent-cli/package.json @@ -12,7 +12,9 @@ "typecheck:tests": "tsc --noEmit -p tsconfig.test.json" }, "dependencies": { - "@argent/tools-client": "file:../argent-tools-client" + "@argent/configuration-core": "file:../configuration-core", + "@argent/tools-client": "file:../argent-tools-client", + "picocolors": "^1.1.1" }, "devDependencies": { "@types/node": "^25.9.0", diff --git a/packages/argent-cli/src/flags.ts b/packages/argent-cli/src/flags.ts index ffbe86b3..68f9bf78 100644 --- a/packages/argent-cli/src/flags.ts +++ b/packages/argent-cli/src/flags.ts @@ -1,192 +1,39 @@ -// Feature-flag storage + CLI for argent. +// Feature-flag CLI for argent: the `enable` / `disable` / `flags` commands. // -// Flags are simple boolean toggles stored as JSON in: -// ~/.argent/flags.json (global, default scope) -// /.argent/flags.json (project scope) +// This module is the command layer only — argv parsing and console output. The +// registry, JSON storage, and `isFlagEnabled` live in `@argent/configuration-core` +// (the pure source of truth); the primitives are imported below. // -// `enable` writes `true`, `disable` removes the entry at the chosen scope. -// `isFlagEnabled` walks project → global, so an entry at the project scope -// shadows the same key at the global scope. To opt a single project out of -// a globally-enabled flag, hand-edit `/.argent/flags.json` to -// `{"flags":{"name":false}}` — there is no CLI for an explicit override. -// -// FLAG_REGISTRY below is the single source of truth for which flags exist: -// `enable` only accepts a name listed there, and `argent flags` documents -// every entry. `disable` stays lenient (so a flag removed from the registry -// can still be cleared from storage), and `isFlagEnabled` only reads storage — -// it never consults the registry, keeping runtime callers decoupled from CLI -// validation. -// -// Deprecating a flag is safe: only the *write* path (`enable`) consults the -// registry. Every read path (readFlags / isFlagEnabled / `argent flags`) loads -// whatever booleans are stored regardless of the registry, so removing an entry -// from FLAG_REGISTRY never errors on a flags.json that still contains it. -// `argent flags` lists such leftovers under an "unrecognized" section so they -// can be cleaned up. - -import * as fs from "node:fs"; -import * as path from "node:path"; -import { homedir } from "node:os"; - -export type FlagScope = "global" | "project"; - -interface FlagsFile { - flags?: Record; -} - -// A recognized feature flag. `name` is what users pass to enable/disable and -// what `isFlagEnabled` reads; `description` is shown by `argent flags`. -export interface FlagDefinition { - readonly name: string; - readonly description: string; -} - -// The flags argent recognizes. Adding one entry here is the only change needed -// to make `argent enable ` accept it and `argent flags` document it. -// An empty registry means no flags are available yet. -export const FLAG_REGISTRY: readonly FlagDefinition[] = [ - // { - // name: "example-flag", - // description: "One-line summary shown by `argent flags`.", - // }, -]; - -// Look up a flag's definition — exported for consumers that want the -// description alongside isFlagEnabled(). Defaults to the built-in registry. -export function getFlagDefinition( - name: string, - registry: readonly FlagDefinition[] = FLAG_REGISTRY -): FlagDefinition | undefined { - return registry.find((def) => def.name === name); -} - -// Markers used to find the project root for --scope project. Trimmed to the -// minimum needed: an existing `.argent` (so subsequent runs in subdirs find -// the dir created by the first run), a git repo, or an npm package. -const PROJECT_MARKERS = [".argent", ".git", "package.json"]; - -export interface FlagsPathOptions { - cwd?: string; - homeDir?: string; -} - -export function resolveProjectRoot(startDir: string): string { - const initial = path.resolve(startDir); - let current = initial; - while (true) { - for (const marker of PROJECT_MARKERS) { - if (fs.existsSync(path.join(current, marker))) return current; - } - const parent = path.dirname(current); - if (parent === current) return initial; - current = parent; - } -} - -export function getFlagsPath(scope: FlagScope, options: FlagsPathOptions = {}): string { - const home = options.homeDir ?? homedir(); - if (scope === "global") { - return path.join(home, ".argent", "flags.json"); - } - const cwd = options.cwd ?? process.cwd(); - return path.join(resolveProjectRoot(cwd), ".argent", "flags.json"); -} - -function readFlagsFile(filePath: string): Record { - let raw: string; - try { - raw = fs.readFileSync(filePath, "utf8"); - } catch { - return {}; - } - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch { - return {}; - } - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; - const flags = (parsed as FlagsFile).flags; - if (!flags || typeof flags !== "object" || Array.isArray(flags)) return {}; - const out: Record = {}; - for (const [k, v] of Object.entries(flags)) { - if (typeof v === "boolean") out[k] = v; - } - return out; -} - -function writeFlagsFile(filePath: string, flags: Record): void { - // No flags left ⇒ remove the file (and the .argent dir if it becomes empty) - // so disable-after-enable round trips leave a clean tree. Sibling files - // (tool-server.json, tool-server.log, etc.) keep the dir alive when present. - if (Object.keys(flags).length === 0) { - if (fs.existsSync(filePath)) fs.rmSync(filePath, { force: true }); - const parent = path.dirname(filePath); - try { - if (fs.existsSync(parent) && fs.readdirSync(parent).length === 0) { - fs.rmdirSync(parent); - } - } catch { - // non-fatal - } - return; - } - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - // Atomic write via tmp+rename so a reader never observes a torn/partial JSON - // payload. (Concurrent read-modify-write is still last-writer-wins, but two - // argent CLI invocations racing on the same flag file are not expected.) - const tmp = `${filePath}.${process.pid}.tmp`; - fs.writeFileSync(tmp, JSON.stringify({ flags } satisfies FlagsFile, null, 2) + "\n"); - fs.renameSync(tmp, filePath); -} - -export function readFlags( - scope: FlagScope, - options: FlagsPathOptions = {} -): Record { - return readFlagsFile(getFlagsPath(scope, options)); +// `enable` writes `true` for a registry-listed flag; `disable` removes the entry +// at the chosen scope. `enable` is the only path that consults the registry — +// `disable` stays lenient so a flag removed from the registry can still be +// cleared from storage, and `argent flags` lists such leftovers under an +// "unrecognized" section so they can be cleaned up. + +import pc from "picocolors"; +import { + FLAG_REGISTRY, + getFlagDefinition, + getFlagsPath, + readFlags, + setFlag, + unsetFlag, + type FlagScope, + type FlagDefinition, +} from "@argent/configuration-core"; + +// Green for enabled, red for disabled. The label is padded first, then wrapped, +// so column alignment is computed from the plain text and never thrown off by +// ANSI escapes. We only colorize for an interactive TTY: picocolors' own +// auto-detection also turns colors on when the CI env var is set (e.g. GitHub +// Actions), which would leak escapes into captured/piped output, so we gate on +// isTTY ourselves to keep piped/redirected/CI output plain. +function colorState(enabled: boolean): string { + const label = (enabled ? "enabled" : "disabled").padEnd(8); + if (!process.stdout.isTTY) return label; + return enabled ? pc.green(label) : pc.red(label); } -export function setFlag( - name: string, - value: boolean, - scope: FlagScope, - options: FlagsPathOptions = {} -): void { - const filePath = getFlagsPath(scope, options); - const current = readFlagsFile(filePath); - current[name] = value; - writeFlagsFile(filePath, current); -} - -// Removes the entry from the given scope so the next layer (or the default) -// takes effect. Returns true when an entry was removed. -export function unsetFlag(name: string, scope: FlagScope, options: FlagsPathOptions = {}): boolean { - const filePath = getFlagsPath(scope, options); - const current = readFlagsFile(filePath); - // hasOwn, not `in`: flag names like "toString"/"constructor" are valid per - // FLAG_NAME_RE but live on Object.prototype, so `in` would report them as - // present (and delete a no-op) even when they were never stored. - if (!Object.hasOwn(current, name)) return false; - delete current[name]; - writeFlagsFile(filePath, current); - return true; -} - -// Effective value: project overrides global. Returns false when the flag is -// not set in either scope — flags are opt-in. -export function isFlagEnabled(name: string, options: FlagsPathOptions = {}): boolean { - // hasOwn, not `in`: otherwise prototype keys ("toString", "constructor", …) - // resolve to a truthy Object.prototype member for a flag that was never set. - const projectFlags = readFlags("project", options); - if (Object.hasOwn(projectFlags, name)) return projectFlags[name]!; - const globalFlags = readFlags("global", options); - if (Object.hasOwn(globalFlags, name)) return globalFlags[name]!; - return false; -} - -// ── CLI handlers ────────────────────────────────────────────────────────────── - // Flag names: start with a letter, then letters/digits/dot/underscore/dash. // Keeps file contents predictable and avoids shell-quoting surprises. const FLAG_NAME_RE = /^[a-zA-Z][a-zA-Z0-9._-]*$/; @@ -251,7 +98,19 @@ function parseScope(raw: string): FlagScope { throw new Error(`--scope must be "project" or "global", got "${raw}"`); } -function printToggleHelp(command: "enable" | "disable"): void { +// Renders the registry as an indented "Available flags:" block for --help +// output: one line per flag, `name description`, so users can see +// what they can toggle without running `argent flags` first. +function formatAvailableFlags(registry: readonly FlagDefinition[]): string { + if (registry.length === 0) { + return "Available flags:\n (none defined)"; + } + const maxName = registry.reduce((m, def) => Math.max(m, def.name.length), 0); + const lines = registry.map((def) => ` ${def.name.padEnd(maxName)} ${def.description}`); + return ["Available flags:", ...lines].join("\n"); +} + +function printToggleHelp(command: "enable" | "disable", registry: readonly FlagDefinition[]): void { const summary = command === "enable" ? "Enable a predefined feature flag (see `argent flags`) at the chosen scope." @@ -261,6 +120,8 @@ function printToggleHelp(command: "enable" | "disable"): void { ${summary} +${formatAvailableFlags(registry)} + Storage: ~/.argent/flags.json (global, default) /.argent/flags.json (project, with --scope project) @@ -277,7 +138,7 @@ function runToggle( registry: readonly FlagDefinition[] ): void { if (argv.includes("--help") || argv.includes("-h")) { - printToggleHelp(command); + printToggleHelp(command, registry); return; } @@ -334,6 +195,8 @@ export function flags(argv: string[], registry: readonly FlagDefinition[] = FLAG List the available feature flags and their current state. Flags are predefined; project-scoped values override global ones. +${formatAvailableFlags(registry)} + Options: --json Print machine-readable JSON `); @@ -396,9 +259,8 @@ Options: console.log("Feature flags (project overrides global):"); const maxName = registryView.reduce((m, f) => Math.max(m, f.name.length), 0); for (const f of registryView) { - const stateLabel = f.enabled ? "enabled" : "disabled"; const scopeLabel = f.scope ? ` (${f.scope})` : ""; - console.log(` ${f.name.padEnd(maxName, " ")} ${stateLabel.padEnd(8)}${scopeLabel}`); + console.log(` ${f.name.padEnd(maxName, " ")} ${colorState(f.enabled)}${scopeLabel}`); console.log(` ${" ".repeat(maxName)} ${f.description}`); } } @@ -407,8 +269,7 @@ Options: console.log("\nStored but no longer recognized (safe to `argent disable`):"); const maxName = unrecognized.reduce((m, f) => Math.max(m, f.name.length), 0); for (const f of unrecognized) { - const stateLabel = f.enabled ? "enabled" : "disabled"; - console.log(` ${f.name.padEnd(maxName, " ")} ${stateLabel.padEnd(8)} (${f.scope})`); + console.log(` ${f.name.padEnd(maxName, " ")} ${colorState(f.enabled)} (${f.scope})`); } } diff --git a/packages/argent-cli/src/index.ts b/packages/argent-cli/src/index.ts index 8e4f9f50..a208254f 100644 --- a/packages/argent-cli/src/index.ts +++ b/packages/argent-cli/src/index.ts @@ -1,13 +1,14 @@ export { run, type RunCommandOptions } from "./run.js"; export { tools, type ToolsCommandOptions } from "./tools.js"; export { server } from "./server.js"; +export { enable, disable, flags } from "./flags.js"; +// Backward-compat re-export: the flag primitives now live in +// @argent/configuration-core, but @argent/cli's public surface keeps exposing +// them so existing importers (and the publish bundle) are unaffected. export { - enable, - disable, - flags, isFlagEnabled, getFlagDefinition, FLAG_REGISTRY, type FlagScope, type FlagDefinition, -} from "./flags.js"; +} from "@argent/configuration-core"; diff --git a/packages/argent-cli/test/flags.test.ts b/packages/argent-cli/test/flags.test.ts index d7141d62..64ebaff0 100644 --- a/packages/argent-cli/test/flags.test.ts +++ b/packages/argent-cli/test/flags.test.ts @@ -2,20 +2,14 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import * as fs from "node:fs"; import * as path from "node:path"; import * as os from "node:os"; +import { disable, enable, flags as flagsCmd } from "../src/flags.js"; import { - disable, - enable, - flags as flagsCmd, - FLAG_REGISTRY, - getFlagDefinition, getFlagsPath, isFlagEnabled, readFlags, - resolveProjectRoot, setFlag, - unsetFlag, type FlagDefinition, -} from "../src/flags.js"; +} from "@argent/configuration-core"; // Hermetic registry for the CLI tests so they never depend on which flags ship // in the production FLAG_REGISTRY. enable()/flags() take an injectable registry @@ -39,10 +33,6 @@ let originalHome: string | undefined; let originalUserProfile: string | undefined; let originalCwd: string; -function readJsonFile(filePath: string): unknown { - return JSON.parse(fs.readFileSync(filePath, "utf8")); -} - beforeEach(() => { // realpath unwraps macOS's /var → /private/var tmpdir symlink so the path // we hand back from getFlagsPath matches what process.cwd() reports after @@ -71,216 +61,6 @@ afterEach(() => { fs.rmSync(tmpProject, { recursive: true, force: true }); }); -describe("getFlagsPath", () => { - it("defaults global path under ~/.argent/flags.json", () => { - expect(getFlagsPath("global")).toBe(path.join(tmpHome, ".argent", "flags.json")); - }); - - it("project path lives at /.argent/flags.json", () => { - expect(getFlagsPath("project")).toBe(path.join(tmpProject, ".argent", "flags.json")); - }); - - it("respects explicit cwd / homeDir overrides", () => { - const altHome = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-alt-home-")) - ); - const altProj = fs.realpathSync( - fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-alt-proj-")) - ); - fs.writeFileSync(path.join(altProj, "package.json"), "{}"); - try { - expect(getFlagsPath("global", { homeDir: altHome })).toBe( - path.join(altHome, ".argent", "flags.json") - ); - expect(getFlagsPath("project", { cwd: altProj })).toBe( - path.join(altProj, ".argent", "flags.json") - ); - } finally { - fs.rmSync(altHome, { recursive: true, force: true }); - fs.rmSync(altProj, { recursive: true, force: true }); - } - }); -}); - -describe("resolveProjectRoot", () => { - it("walks up to the nearest marker", () => { - const nested = path.join(tmpProject, "a", "b", "c"); - fs.mkdirSync(nested, { recursive: true }); - expect(resolveProjectRoot(nested)).toBe(tmpProject); - }); - - it("returns startDir when no marker exists in ancestry", () => { - // A bare tmpdir guaranteed to have no project markers between it and / - const bare = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-noroot-"))); - try { - expect(resolveProjectRoot(bare)).toBe(bare); - } finally { - fs.rmSync(bare, { recursive: true, force: true }); - } - }); - - it("treats existing .argent as a marker", () => { - fs.rmSync(path.join(tmpProject, "package.json")); - const argentDir = path.join(tmpProject, ".argent"); - fs.mkdirSync(argentDir); - const nested = path.join(tmpProject, "sub"); - fs.mkdirSync(nested); - expect(resolveProjectRoot(nested)).toBe(tmpProject); - }); - - it("treats existing .git as a marker", () => { - fs.rmSync(path.join(tmpProject, "package.json")); - fs.mkdirSync(path.join(tmpProject, ".git")); - const nested = path.join(tmpProject, "sub"); - fs.mkdirSync(nested); - expect(resolveProjectRoot(nested)).toBe(tmpProject); - }); -}); - -describe("setFlag / unsetFlag / readFlags", () => { - it("writes the flag to disk and reads it back", () => { - setFlag("alpha", true, "global"); - expect(readFlags("global")).toEqual({ alpha: true }); - - const file = readJsonFile(getFlagsPath("global")); - expect(file).toEqual({ flags: { alpha: true } }); - }); - - it("preserves other flags when setting one", () => { - setFlag("alpha", true, "global"); - setFlag("beta", false, "global"); - expect(readFlags("global")).toEqual({ alpha: true, beta: false }); - }); - - it("overwrites a flag with a new value", () => { - setFlag("alpha", true, "global"); - setFlag("alpha", false, "global"); - expect(readFlags("global")).toEqual({ alpha: false }); - }); - - it("unsetFlag removes the entry and reports whether it existed", () => { - setFlag("alpha", true, "global"); - setFlag("beta", true, "global"); - expect(unsetFlag("alpha", "global")).toBe(true); - expect(readFlags("global")).toEqual({ beta: true }); - expect(unsetFlag("missing", "global")).toBe(false); - }); - - it("removes the file (and empty .argent dir) when the last flag is unset", () => { - setFlag("alpha", true, "global"); - unsetFlag("alpha", "global"); - expect(fs.existsSync(getFlagsPath("global"))).toBe(false); - expect(fs.existsSync(path.join(tmpHome, ".argent"))).toBe(false); - }); - - it("keeps the .argent dir when sibling state lives next to flags.json", () => { - setFlag("alpha", true, "global"); - const sibling = path.join(tmpHome, ".argent", "tool-server.json"); - fs.writeFileSync(sibling, "{}"); - unsetFlag("alpha", "global"); - expect(fs.existsSync(getFlagsPath("global"))).toBe(false); - expect(fs.existsSync(sibling)).toBe(true); - expect(fs.existsSync(path.join(tmpHome, ".argent"))).toBe(true); - }); - - it("project + global live in separate files", () => { - setFlag("alpha", true, "global"); - setFlag("alpha", false, "project"); - expect(readFlags("global")).toEqual({ alpha: true }); - expect(readFlags("project")).toEqual({ alpha: false }); - }); - - it("recovers from malformed JSON by treating storage as empty", () => { - const file = getFlagsPath("global"); - fs.mkdirSync(path.dirname(file), { recursive: true }); - fs.writeFileSync(file, "not json"); - expect(readFlags("global")).toEqual({}); - setFlag("alpha", true, "global"); - expect(readFlags("global")).toEqual({ alpha: true }); - }); - - it("ignores non-boolean values in the stored flags object", () => { - const file = getFlagsPath("global"); - fs.mkdirSync(path.dirname(file), { recursive: true }); - fs.writeFileSync( - file, - JSON.stringify({ flags: { real: true, bogus: "yes", numeric: 1, nested: {} } }) - ); - expect(readFlags("global")).toEqual({ real: true }); - }); - - it("treats missing .argent dir as empty without throwing", () => { - expect(readFlags("global")).toEqual({}); - expect(readFlags("project")).toEqual({}); - }); - - it("writes are atomic — no .tmp leftover in the .argent dir", () => { - setFlag("alpha", true, "global"); - setFlag("beta", true, "global"); - const dir = path.join(tmpHome, ".argent"); - const leftovers = fs.readdirSync(dir).filter((f) => f.includes(".tmp")); - expect(leftovers).toEqual([]); - }); -}); - -describe("isFlagEnabled", () => { - it("returns false for an unknown flag", () => { - expect(isFlagEnabled("unknown")).toBe(false); - }); - - it("project value overrides global value (explicit false masks global true)", () => { - setFlag("alpha", true, "global"); - setFlag("alpha", false, "project"); - expect(isFlagEnabled("alpha")).toBe(false); - - setFlag("beta", false, "global"); - setFlag("beta", true, "project"); - expect(isFlagEnabled("beta")).toBe(true); - }); - - it("falls through to global when project does not set it", () => { - setFlag("alpha", true, "global"); - expect(isFlagEnabled("alpha")).toBe(true); - }); - - it("falls through to project-only when global is unset", () => { - setFlag("alpha", true, "project"); - expect(isFlagEnabled("alpha")).toBe(true); - }); -}); - -describe("prototype-named flags (Object.prototype keys)", () => { - // FLAG_NAME_RE allows names like "toString"/"constructor"/"valueOf", which - // also exist on Object.prototype. A naive `name in obj` check would treat - // these as set (returning a truthy prototype member) even when storage is - // empty — these guard that hasOwn semantics are used throughout. - const protoNames = ["toString", "constructor", "valueOf", "hasOwnProperty"]; - - for (const name of protoNames) { - it(`isFlagEnabled("${name}") is false (and a real boolean) when unset`, () => { - const result = isFlagEnabled(name); - expect(result).toBe(false); - expect(typeof result).toBe("boolean"); - }); - - it(`unsetFlag("${name}") on storage without it returns false and is a no-op`, () => { - setFlag("real", true, "global"); - expect(unsetFlag(name, "global")).toBe(false); - expect(readFlags("global")).toEqual({ real: true }); - }); - - it(`enable/disable round trip works for a flag literally named "${name}"`, () => { - setFlag(name, true, "global"); - expect(readFlags("global")).toEqual({ [name]: true }); - expect(isFlagEnabled(name)).toBe(true); - expect(unsetFlag(name, "global")).toBe(true); - expect(isFlagEnabled(name)).toBe(false); - }); - } -}); - -// ── CLI ────────────────────────────────────────────────────────────────────── - interface CapturedConsole { stdout: string; stderr: string; @@ -410,6 +190,20 @@ describe("enable / disable CLI", () => { expect(readFlags("global")).toEqual({}); }); + it("--help lists every available flag with its description", () => { + const out = captureConsole(() => enable(["--help"], TEST_REGISTRY)); + expect(out.stdout).toContain("Available flags:"); + expect(out.stdout).toContain("my-feature-flag"); + expect(out.stdout).toContain("Primary test flag."); + expect(out.stdout).toContain("proj-flag"); + expect(out.stdout).toContain("Project-scoped test flag."); + // -h is an alias for --help and shows the same listing. + const short = captureConsole(() => disable(["-h"], TEST_REGISTRY)); + expect(short.stdout).toContain("Available flags:"); + expect(short.stdout).toContain("Listing flag A."); + expect(readFlags("global")).toEqual({}); + }); + it("enable → disable round trip leaves a clean tree (no stub entry)", () => { captureConsole(() => enable(["x"], TEST_REGISTRY)); captureConsole(() => disable(["x"])); @@ -425,9 +219,10 @@ describe("enable / disable CLI", () => { expect(readFlags("global")).toEqual({}); }); - it("rejects every enable when the (default) registry is empty", () => { - // No registry argument → production FLAG_REGISTRY, which ships empty. - const out = expectExit(2, () => enable(["anything"])); + it("rejects enabling the legacy auto-shutdown flag name (never wired)", () => { + // ARGENT_AUTO_SHUTDOWN stayed an env var (renamed), so `auto-shutdown` was + // never added to the registry — `argent enable auto-shutdown` must fail. + const out = expectExit(2, () => enable(["auto-shutdown"])); expect(out.stderr).toContain("Unknown feature flag"); expect(readFlags("global")).toEqual({}); }); @@ -442,14 +237,6 @@ describe("enable / disable CLI", () => { }); describe("flags (list) CLI", () => { - it("prints a friendly message when the registry is empty", () => { - // Default (production) registry ships empty. - const out = captureConsole(() => flagsCmd([])); - expect(out.stdout).toContain("No feature flags are defined."); - expect(out.stdout).toContain(getFlagsPath("global")); - expect(out.stdout).toContain(getFlagsPath("project")); - }); - it("lists every registry flag with its description and effective scope", () => { setFlag("a", true, "global"); setFlag("b", true, "global"); @@ -500,22 +287,13 @@ describe("flags (list) CLI", () => { const out = captureConsole(() => flagsCmd(["--help"])); expect(out.stdout).toContain("Usage: argent flags"); }); -}); - -describe("FLAG_REGISTRY / getFlagDefinition", () => { - it("getFlagDefinition returns the entry or undefined", () => { - expect(getFlagDefinition("my-feature-flag", TEST_REGISTRY)?.description).toBe( - "Primary test flag." - ); - expect(getFlagDefinition("nope", TEST_REGISTRY)).toBeUndefined(); - }); - it("every shipped registry entry has a non-empty name and description", () => { - // Guards against a half-filled entry being added to the production registry. - for (const def of FLAG_REGISTRY) { - expect(def.name).toMatch(/^[a-zA-Z][a-zA-Z0-9._-]*$/); - expect(def.description.trim().length).toBeGreaterThan(0); - } + it("--help lists every available flag with its description", () => { + const out = captureConsole(() => flagsCmd(["--help"], TEST_REGISTRY)); + expect(out.stdout).toContain("Available flags:"); + expect(out.stdout).toContain("my-feature-flag"); + expect(out.stdout).toContain("Primary test flag."); + expect(out.stdout).toContain("Listing flag B."); }); }); @@ -523,15 +301,6 @@ describe("deprecating a flag (removed from FLAG_REGISTRY)", () => { // A flag that was enabled before being deprecated still lives in flags.json // but is absent from the registry. Loading/reading it must never throw. - it("reading a deprecated stored flag never throws and returns its value", () => { - setFlag("deprecated-flag", true, "global"); - setFlag("deprecated-false", false, "project"); - // Neither name is in any registry — these are pure storage reads. - expect(readFlags("global")).toEqual({ "deprecated-flag": true }); - expect(isFlagEnabled("deprecated-flag")).toBe(true); - expect(isFlagEnabled("deprecated-false")).toBe(false); - }); - it("`argent flags` lists a deprecated stored flag under 'unrecognized' (no throw)", () => { setFlag("deprecated-flag", true, "global"); let out!: CapturedConsole; diff --git a/packages/argent-mcp/package.json b/packages/argent-mcp/package.json index eeca89af..0907a42d 100644 --- a/packages/argent-mcp/package.json +++ b/packages/argent-mcp/package.json @@ -12,6 +12,7 @@ "typecheck:tests": "tsc --noEmit -p tsconfig.test.json" }, "dependencies": { + "@argent/configuration-core": "file:../configuration-core", "@argent/tools-client": "file:../argent-tools-client", "@modelcontextprotocol/sdk": "^1.29.0" }, diff --git a/packages/argent-mcp/src/auto-screenshot.ts b/packages/argent-mcp/src/auto-screenshot.ts index b6e4d377..542785ea 100644 --- a/packages/argent-mcp/src/auto-screenshot.ts +++ b/packages/argent-mcp/src/auto-screenshot.ts @@ -6,6 +6,8 @@ * All tunables live in this module so they can be tested in isolation. */ +import { isFlagEnabled, type FlagsPathOptions } from "@argent/configuration-core"; + export const AUTO_SCREENSHOT_TOOLS = new Set([ "gesture-tap", "gesture-swipe", @@ -44,9 +46,11 @@ export const AUTO_SCREENSHOT_DELAY_MS_BY_TOOL: Record = { const DEFAULT_DELAY_MS = 1400; -export function autoScreenshotEnabled(): boolean { - const v = process.env.ARGENT_AUTO_SCREENSHOT; - return v === undefined || v === "" || v === "1" || v.toLowerCase() === "true"; +// Auto-screenshot is on by default; the opt-out is the off-by-default +// `disable-auto-screenshot` flag. `options` mirrors isFlagEnabled so tests can +// point storage at a temp dir. +export function autoScreenshotEnabled(options?: FlagsPathOptions): boolean { + return !isFlagEnabled("disable-auto-screenshot", options); } export function getUdidFromArgs(args: unknown): string | undefined { diff --git a/packages/argent-mcp/src/mcp-server.ts b/packages/argent-mcp/src/mcp-server.ts index 06f2c2e4..158bcaf4 100644 --- a/packages/argent-mcp/src/mcp-server.ts +++ b/packages/argent-mcp/src/mcp-server.ts @@ -76,6 +76,10 @@ export interface StartMcpServerOptions { } export async function startMcpServer(options: StartMcpServerOptions): Promise { + // isFlagEnabled hits disk, so resolve it once at startup rather than on every + // tool call. A flag change therefore needs an MCP restart to take effect. + const autoScreenshotOn = autoScreenshotEnabled(); + let TOOLS_URL: string; if (process.env.ARGENT_TOOLS_URL) { TOOLS_URL = process.env.ARGENT_TOOLS_URL; @@ -219,7 +223,7 @@ export async function startMcpServer(options: StartMcpServerOptions): Promise 0) await new Promise((r) => setTimeout(r, delayMs)); @@ -291,7 +295,7 @@ export async function startMcpServer(options: StartMcpServerOptions): Promise { fetch(`${TOOLS_URL}/shutdown`, { method: "POST" }).catch(() => {}); setTimeout(() => process.exit(0), 5_000); diff --git a/packages/argent-mcp/test/auto-screenshot.test.ts b/packages/argent-mcp/test/auto-screenshot.test.ts index c1df365c..bfbb9011 100644 --- a/packages/argent-mcp/test/auto-screenshot.test.ts +++ b/packages/argent-mcp/test/auto-screenshot.test.ts @@ -1,4 +1,8 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as os from "node:os"; +import { setFlag } from "@argent/configuration-core"; import { AUTO_SCREENSHOT_TOOLS, AUTO_SCREENSHOT_DELAY_MS_BY_TOOL, @@ -91,44 +95,43 @@ describe("shouldAutoScreenshot", () => { }); // --------------------------------------------------------------------------- -// autoScreenshotEnabled +// autoScreenshotEnabled — driven by the off-by-default `disable-auto-screenshot` +// flag (auto-screenshot is on unless the flag is set). // --------------------------------------------------------------------------- describe("autoScreenshotEnabled", () => { - const original = process.env.ARGENT_AUTO_SCREENSHOT; + let tmpHome: string; + let tmpProject: string; - afterEach(() => { - if (original === undefined) delete process.env.ARGENT_AUTO_SCREENSHOT; - else process.env.ARGENT_AUTO_SCREENSHOT = original; - }); - - it("returns true when env var is unset", () => { - delete process.env.ARGENT_AUTO_SCREENSHOT; - expect(autoScreenshotEnabled()).toBe(true); + beforeEach(() => { + tmpHome = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-screenshot-home-"))); + tmpProject = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-screenshot-proj-"))); + // Marker so resolveProjectRoot stops at tmpProject instead of walking up. + fs.writeFileSync(path.join(tmpProject, "package.json"), "{}"); }); - it("returns true when env var is empty string", () => { - process.env.ARGENT_AUTO_SCREENSHOT = ""; - expect(autoScreenshotEnabled()).toBe(true); + afterEach(() => { + fs.rmSync(tmpHome, { recursive: true, force: true }); + fs.rmSync(tmpProject, { recursive: true, force: true }); }); - it('returns true when env var is "1"', () => { - process.env.ARGENT_AUTO_SCREENSHOT = "1"; - expect(autoScreenshotEnabled()).toBe(true); + it("is on by default when the flag is unset", () => { + expect(autoScreenshotEnabled({ homeDir: tmpHome, cwd: tmpProject })).toBe(true); }); - it('returns true when env var is "true" (case-insensitive)', () => { - process.env.ARGENT_AUTO_SCREENSHOT = "True"; - expect(autoScreenshotEnabled()).toBe(true); + it("is off when the flag is enabled globally", () => { + setFlag("disable-auto-screenshot", true, "global", { homeDir: tmpHome }); + expect(autoScreenshotEnabled({ homeDir: tmpHome, cwd: tmpProject })).toBe(false); }); - it('returns false when env var is "0"', () => { - process.env.ARGENT_AUTO_SCREENSHOT = "0"; - expect(autoScreenshotEnabled()).toBe(false); + it("is off when the flag is enabled at project scope", () => { + setFlag("disable-auto-screenshot", true, "project", { cwd: tmpProject }); + expect(autoScreenshotEnabled({ homeDir: tmpHome, cwd: tmpProject })).toBe(false); }); - it('returns false when env var is "false"', () => { - process.env.ARGENT_AUTO_SCREENSHOT = "false"; - expect(autoScreenshotEnabled()).toBe(false); + it("project scope overrides a global disable (explicit false re-enables)", () => { + setFlag("disable-auto-screenshot", true, "global", { homeDir: tmpHome }); + setFlag("disable-auto-screenshot", false, "project", { cwd: tmpProject }); + expect(autoScreenshotEnabled({ homeDir: tmpHome, cwd: tmpProject })).toBe(true); }); }); diff --git a/packages/argent/scripts/bundle-tools.cjs b/packages/argent/scripts/bundle-tools.cjs index ee8aa232..16d1fa8e 100644 --- a/packages/argent/scripts/bundle-tools.cjs +++ b/packages/argent/scripts/bundle-tools.cjs @@ -19,6 +19,10 @@ const TOOLS_CLIENT_ENTRY = path.resolve( const INSTALLER_ENTRY = path.resolve(WORKSPACE_ROOT, "packages/argent-installer/src/index.ts"); const MCP_ENTRY = path.resolve(WORKSPACE_ROOT, "packages/argent-mcp/src/index.ts"); const CLI_ENTRY = path.resolve(WORKSPACE_ROOT, "packages/argent-cli/src/index.ts"); +const CONFIGURATION_ENTRY = path.resolve( + WORKSPACE_ROOT, + "packages/configuration-core/src/index.ts" +); const OUT_FILE = path.resolve(__dirname, "../dist/tool-server.cjs"); const INSTALLER_OUT_FILE = path.resolve(__dirname, "../dist/installer.mjs"); const MCP_OUT_FILE = path.resolve(__dirname, "../dist/mcp-server.mjs"); @@ -32,6 +36,7 @@ const ALIASES = { "@argent/installer": INSTALLER_ENTRY, "@argent/mcp": MCP_ENTRY, "@argent/cli": CLI_ENTRY, + "@argent/configuration-core": CONFIGURATION_ENTRY, }; // esbuild on platform:"node" defaults mainFields to ["main","module"], which diff --git a/packages/configuration-core/package.json b/packages/configuration-core/package.json new file mode 100644 index 00000000..95ebaaed --- /dev/null +++ b/packages/configuration-core/package.json @@ -0,0 +1,19 @@ +{ + "private": true, + "name": "@argent/configuration-core", + "version": "0.9.0", + "description": "Source of truth for argent feature flags: registry, JSON storage, and isFlagEnabled", + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "scripts": { + "build": "rm -rf dist tsconfig.tsbuildinfo && tsc", + "test": "vitest run", + "typecheck:tests": "tsc --noEmit -p tsconfig.test.json" + }, + "devDependencies": { + "@types/node": "^25.9.0", + "typescript": "^6.0.3", + "vitest": "^4.1.6" + } +} diff --git a/packages/configuration-core/src/flags.ts b/packages/configuration-core/src/flags.ts new file mode 100644 index 00000000..ff52ea4c --- /dev/null +++ b/packages/configuration-core/src/flags.ts @@ -0,0 +1,190 @@ +// Feature-flag source of truth for argent. +// +// Flags are simple boolean toggles stored as JSON in: +// ~/.argent/flags.json (global, default scope) +// /.argent/flags.json (project scope) +// +// This package is the pure library: the registry, JSON storage, and +// `isFlagEnabled`. It has no console I/O — the `enable`/`disable`/`flags` CLI +// commands live in `@argent/cli` and import the primitives below. +// +// `setFlag` writes a boolean, `unsetFlag` removes the entry at the chosen scope. +// `isFlagEnabled` walks project → global, so an entry at the project scope +// shadows the same key at the global scope. To opt a single project out of +// a globally-enabled flag, hand-edit `/.argent/flags.json` to +// `{"flags":{"name":false}}` — there is no CLI for an explicit override. +// +// FLAG_REGISTRY below is the single source of truth for which flags exist: +// `argent enable` only accepts a name listed there, and `argent flags` +// documents every entry. `isFlagEnabled` only reads storage — it never +// consults the registry, keeping runtime callers decoupled from CLI validation. +// +// Deprecating a flag is safe: only the *write* path (`argent enable`) consults +// the registry. Every read path (readFlags / isFlagEnabled / `argent flags`) +// loads whatever booleans are stored regardless of the registry, so removing an +// entry from FLAG_REGISTRY never errors on a flags.json that still contains it. + +import * as fs from "node:fs"; +import * as path from "node:path"; +import { homedir } from "node:os"; + +export type FlagScope = "global" | "project"; + +interface FlagsFile { + flags?: Record; +} + +// A recognized feature flag. `name` is what users pass to enable/disable and +// what `isFlagEnabled` reads; `description` is shown by `argent flags`. +export interface FlagDefinition { + readonly name: string; + readonly description: string; +} + +// The flags argent recognizes. Adding one entry here is the only change needed +// to make `argent enable ` accept it and `argent flags` document it. +export const FLAG_REGISTRY: readonly FlagDefinition[] = [ + { + name: "disable-auto-screenshot", + description: "Disable the automatic screenshot captured after interaction tools.", + }, + { + name: "disable-update-notification", + description: + 'Suppress the agent-facing "update available" notification note. The `argent update` command and `update-argent` tool still work.', + }, +]; + +// Look up a flag's definition — exported for consumers that want the +// description alongside isFlagEnabled(). Defaults to the built-in registry. +export function getFlagDefinition( + name: string, + registry: readonly FlagDefinition[] = FLAG_REGISTRY +): FlagDefinition | undefined { + return registry.find((def) => def.name === name); +} + +// Markers used to find the project root for --scope project. Trimmed to the +// minimum needed: an existing `.argent` (so subsequent runs in subdirs find +// the dir created by the first run), a git repo, or an npm package. +const PROJECT_MARKERS = [".argent", ".git", "package.json"]; + +export interface FlagsPathOptions { + cwd?: string; + homeDir?: string; +} + +export function resolveProjectRoot(startDir: string): string { + const initial = path.resolve(startDir); + let current = initial; + while (true) { + for (const marker of PROJECT_MARKERS) { + if (fs.existsSync(path.join(current, marker))) return current; + } + const parent = path.dirname(current); + if (parent === current) return initial; + current = parent; + } +} + +export function getFlagsPath(scope: FlagScope, options: FlagsPathOptions = {}): string { + const home = options.homeDir ?? homedir(); + if (scope === "global") { + return path.join(home, ".argent", "flags.json"); + } + const cwd = options.cwd ?? process.cwd(); + return path.join(resolveProjectRoot(cwd), ".argent", "flags.json"); +} + +function readFlagsFile(filePath: string): Record { + let raw: string; + try { + raw = fs.readFileSync(filePath, "utf8"); + } catch { + return {}; + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return {}; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + const flags = (parsed as FlagsFile).flags; + if (!flags || typeof flags !== "object" || Array.isArray(flags)) return {}; + const out: Record = {}; + for (const [k, v] of Object.entries(flags)) { + if (typeof v === "boolean") out[k] = v; + } + return out; +} + +function writeFlagsFile(filePath: string, flags: Record): void { + // No flags left ⇒ remove the file (and the .argent dir if it becomes empty) + // so disable-after-enable round trips leave a clean tree. Sibling files + // (tool-server.json, tool-server.log, etc.) keep the dir alive when present. + if (Object.keys(flags).length === 0) { + if (fs.existsSync(filePath)) fs.rmSync(filePath, { force: true }); + const parent = path.dirname(filePath); + try { + if (fs.existsSync(parent) && fs.readdirSync(parent).length === 0) { + fs.rmdirSync(parent); + } + } catch { + // non-fatal + } + return; + } + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + // Atomic write via tmp+rename so a reader never observes a torn/partial JSON + // payload. (Concurrent read-modify-write is still last-writer-wins, but two + // argent CLI invocations racing on the same flag file are not expected.) + const tmp = `${filePath}.${process.pid}.tmp`; + fs.writeFileSync(tmp, JSON.stringify({ flags } satisfies FlagsFile, null, 2) + "\n"); + fs.renameSync(tmp, filePath); +} + +export function readFlags( + scope: FlagScope, + options: FlagsPathOptions = {} +): Record { + return readFlagsFile(getFlagsPath(scope, options)); +} + +export function setFlag( + name: string, + value: boolean, + scope: FlagScope, + options: FlagsPathOptions = {} +): void { + const filePath = getFlagsPath(scope, options); + const current = readFlagsFile(filePath); + current[name] = value; + writeFlagsFile(filePath, current); +} + +// Removes the entry from the given scope so the next layer (or the default) +// takes effect. Returns true when an entry was removed. +export function unsetFlag(name: string, scope: FlagScope, options: FlagsPathOptions = {}): boolean { + const filePath = getFlagsPath(scope, options); + const current = readFlagsFile(filePath); + // hasOwn, not `in`: flag names like "toString"/"constructor" are valid + // identifiers but live on Object.prototype, so `in` would report them as + // present (and delete a no-op) even when they were never stored. + if (!Object.hasOwn(current, name)) return false; + delete current[name]; + writeFlagsFile(filePath, current); + return true; +} + +// Effective value: project overrides global. Returns false when the flag is +// not set in either scope — flags are opt-in. +export function isFlagEnabled(name: string, options: FlagsPathOptions = {}): boolean { + // hasOwn, not `in`: otherwise prototype keys ("toString", "constructor", …) + // resolve to a truthy Object.prototype member for a flag that was never set. + const projectFlags = readFlags("project", options); + if (Object.hasOwn(projectFlags, name)) return projectFlags[name]!; + const globalFlags = readFlags("global", options); + if (Object.hasOwn(globalFlags, name)) return globalFlags[name]!; + return false; +} diff --git a/packages/configuration-core/src/index.ts b/packages/configuration-core/src/index.ts new file mode 100644 index 00000000..6d6ca77c --- /dev/null +++ b/packages/configuration-core/src/index.ts @@ -0,0 +1,13 @@ +export { + FLAG_REGISTRY, + getFlagDefinition, + resolveProjectRoot, + getFlagsPath, + readFlags, + setFlag, + unsetFlag, + isFlagEnabled, + type FlagScope, + type FlagDefinition, + type FlagsPathOptions, +} from "./flags.js"; diff --git a/packages/configuration-core/tests/flags.test.ts b/packages/configuration-core/tests/flags.test.ts new file mode 100644 index 00000000..4d7e46c4 --- /dev/null +++ b/packages/configuration-core/tests/flags.test.ts @@ -0,0 +1,300 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as os from "node:os"; +import { + FLAG_REGISTRY, + getFlagDefinition, + getFlagsPath, + isFlagEnabled, + readFlags, + resolveProjectRoot, + setFlag, + unsetFlag, + type FlagDefinition, +} from "../src/flags.js"; + +// Hermetic registry for getFlagDefinition's injectable-registry path so the +// test never depends on which flags ship in the production FLAG_REGISTRY. +const TEST_REGISTRY: readonly FlagDefinition[] = [ + { name: "my-feature-flag", description: "Primary test flag." }, +]; + +// All tests redirect global+project storage into tmp dirs by mutating +// process.env.HOME (consumed by os.homedir()) and process.cwd(). + +let tmpHome: string; +let tmpProject: string; +let originalHome: string | undefined; +let originalUserProfile: string | undefined; +let originalCwd: string; + +function readJsonFile(filePath: string): unknown { + return JSON.parse(fs.readFileSync(filePath, "utf8")); +} + +beforeEach(() => { + // realpath unwraps macOS's /var → /private/var tmpdir symlink so the path + // we hand back from getFlagsPath matches what process.cwd() reports after + // chdir(). + tmpHome = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-home-"))); + tmpProject = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-proj-"))); + // Drop a marker so resolveProjectRoot stops here instead of walking up to + // the actual user's repo and writing into it. + fs.writeFileSync(path.join(tmpProject, "package.json"), "{}"); + + originalHome = process.env.HOME; + originalUserProfile = process.env.USERPROFILE; + originalCwd = process.cwd(); + process.env.HOME = tmpHome; + process.env.USERPROFILE = tmpHome; + process.chdir(tmpProject); +}); + +afterEach(() => { + process.chdir(originalCwd); + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + fs.rmSync(tmpHome, { recursive: true, force: true }); + fs.rmSync(tmpProject, { recursive: true, force: true }); +}); + +describe("getFlagsPath", () => { + it("defaults global path under ~/.argent/flags.json", () => { + expect(getFlagsPath("global")).toBe(path.join(tmpHome, ".argent", "flags.json")); + }); + + it("project path lives at /.argent/flags.json", () => { + expect(getFlagsPath("project")).toBe(path.join(tmpProject, ".argent", "flags.json")); + }); + + it("respects explicit cwd / homeDir overrides", () => { + const altHome = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-alt-home-")) + ); + const altProj = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-alt-proj-")) + ); + fs.writeFileSync(path.join(altProj, "package.json"), "{}"); + try { + expect(getFlagsPath("global", { homeDir: altHome })).toBe( + path.join(altHome, ".argent", "flags.json") + ); + expect(getFlagsPath("project", { cwd: altProj })).toBe( + path.join(altProj, ".argent", "flags.json") + ); + } finally { + fs.rmSync(altHome, { recursive: true, force: true }); + fs.rmSync(altProj, { recursive: true, force: true }); + } + }); +}); + +describe("resolveProjectRoot", () => { + it("walks up to the nearest marker", () => { + const nested = path.join(tmpProject, "a", "b", "c"); + fs.mkdirSync(nested, { recursive: true }); + expect(resolveProjectRoot(nested)).toBe(tmpProject); + }); + + it("returns startDir when no marker exists in ancestry", () => { + // A bare tmpdir guaranteed to have no project markers between it and / + const bare = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "argent-flags-noroot-"))); + try { + expect(resolveProjectRoot(bare)).toBe(bare); + } finally { + fs.rmSync(bare, { recursive: true, force: true }); + } + }); + + it("treats existing .argent as a marker", () => { + fs.rmSync(path.join(tmpProject, "package.json")); + const argentDir = path.join(tmpProject, ".argent"); + fs.mkdirSync(argentDir); + const nested = path.join(tmpProject, "sub"); + fs.mkdirSync(nested); + expect(resolveProjectRoot(nested)).toBe(tmpProject); + }); + + it("treats existing .git as a marker", () => { + fs.rmSync(path.join(tmpProject, "package.json")); + fs.mkdirSync(path.join(tmpProject, ".git")); + const nested = path.join(tmpProject, "sub"); + fs.mkdirSync(nested); + expect(resolveProjectRoot(nested)).toBe(tmpProject); + }); +}); + +describe("setFlag / unsetFlag / readFlags", () => { + it("writes the flag to disk and reads it back", () => { + setFlag("alpha", true, "global"); + expect(readFlags("global")).toEqual({ alpha: true }); + + const file = readJsonFile(getFlagsPath("global")); + expect(file).toEqual({ flags: { alpha: true } }); + }); + + it("preserves other flags when setting one", () => { + setFlag("alpha", true, "global"); + setFlag("beta", false, "global"); + expect(readFlags("global")).toEqual({ alpha: true, beta: false }); + }); + + it("overwrites a flag with a new value", () => { + setFlag("alpha", true, "global"); + setFlag("alpha", false, "global"); + expect(readFlags("global")).toEqual({ alpha: false }); + }); + + it("unsetFlag removes the entry and reports whether it existed", () => { + setFlag("alpha", true, "global"); + setFlag("beta", true, "global"); + expect(unsetFlag("alpha", "global")).toBe(true); + expect(readFlags("global")).toEqual({ beta: true }); + expect(unsetFlag("missing", "global")).toBe(false); + }); + + it("removes the file (and empty .argent dir) when the last flag is unset", () => { + setFlag("alpha", true, "global"); + unsetFlag("alpha", "global"); + expect(fs.existsSync(getFlagsPath("global"))).toBe(false); + expect(fs.existsSync(path.join(tmpHome, ".argent"))).toBe(false); + }); + + it("keeps the .argent dir when sibling state lives next to flags.json", () => { + setFlag("alpha", true, "global"); + const sibling = path.join(tmpHome, ".argent", "tool-server.json"); + fs.writeFileSync(sibling, "{}"); + unsetFlag("alpha", "global"); + expect(fs.existsSync(getFlagsPath("global"))).toBe(false); + expect(fs.existsSync(sibling)).toBe(true); + expect(fs.existsSync(path.join(tmpHome, ".argent"))).toBe(true); + }); + + it("project + global live in separate files", () => { + setFlag("alpha", true, "global"); + setFlag("alpha", false, "project"); + expect(readFlags("global")).toEqual({ alpha: true }); + expect(readFlags("project")).toEqual({ alpha: false }); + }); + + it("recovers from malformed JSON by treating storage as empty", () => { + const file = getFlagsPath("global"); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, "not json"); + expect(readFlags("global")).toEqual({}); + setFlag("alpha", true, "global"); + expect(readFlags("global")).toEqual({ alpha: true }); + }); + + it("ignores non-boolean values in the stored flags object", () => { + const file = getFlagsPath("global"); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + JSON.stringify({ flags: { real: true, bogus: "yes", numeric: 1, nested: {} } }) + ); + expect(readFlags("global")).toEqual({ real: true }); + }); + + it("treats missing .argent dir as empty without throwing", () => { + expect(readFlags("global")).toEqual({}); + expect(readFlags("project")).toEqual({}); + }); + + it("writes are atomic — no .tmp leftover in the .argent dir", () => { + setFlag("alpha", true, "global"); + setFlag("beta", true, "global"); + const dir = path.join(tmpHome, ".argent"); + const leftovers = fs.readdirSync(dir).filter((f) => f.includes(".tmp")); + expect(leftovers).toEqual([]); + }); +}); + +describe("isFlagEnabled", () => { + it("returns false for an unknown flag", () => { + expect(isFlagEnabled("unknown")).toBe(false); + }); + + it("project value overrides global value (explicit false masks global true)", () => { + setFlag("alpha", true, "global"); + setFlag("alpha", false, "project"); + expect(isFlagEnabled("alpha")).toBe(false); + + setFlag("beta", false, "global"); + setFlag("beta", true, "project"); + expect(isFlagEnabled("beta")).toBe(true); + }); + + it("falls through to global when project does not set it", () => { + setFlag("alpha", true, "global"); + expect(isFlagEnabled("alpha")).toBe(true); + }); + + it("falls through to project-only when global is unset", () => { + setFlag("alpha", true, "project"); + expect(isFlagEnabled("alpha")).toBe(true); + }); + + it("respects explicit cwd / homeDir options instead of process state", () => { + setFlag("alpha", true, "global", { homeDir: tmpHome }); + expect(isFlagEnabled("alpha", { homeDir: tmpHome, cwd: tmpProject })).toBe(true); + }); +}); + +describe("prototype-named flags (Object.prototype keys)", () => { + // Names like "toString"/"constructor"/"valueOf" also exist on + // Object.prototype. A naive `name in obj` check would treat these as set + // (returning a truthy prototype member) even when storage is empty — these + // guard that hasOwn semantics are used throughout. + const protoNames = ["toString", "constructor", "valueOf", "hasOwnProperty"]; + + for (const name of protoNames) { + it(`isFlagEnabled("${name}") is false (and a real boolean) when unset`, () => { + const result = isFlagEnabled(name); + expect(result).toBe(false); + expect(typeof result).toBe("boolean"); + }); + + it(`unsetFlag("${name}") on storage without it returns false and is a no-op`, () => { + setFlag("real", true, "global"); + expect(unsetFlag(name, "global")).toBe(false); + expect(readFlags("global")).toEqual({ real: true }); + }); + + it(`set/unset round trip works for a flag literally named "${name}"`, () => { + setFlag(name, true, "global"); + expect(readFlags("global")).toEqual({ [name]: true }); + expect(isFlagEnabled(name)).toBe(true); + expect(unsetFlag(name, "global")).toBe(true); + expect(isFlagEnabled(name)).toBe(false); + }); + } +}); + +describe("FLAG_REGISTRY / getFlagDefinition", () => { + it("getFlagDefinition returns the entry or undefined", () => { + expect(getFlagDefinition("my-feature-flag", TEST_REGISTRY)?.description).toBe( + "Primary test flag." + ); + expect(getFlagDefinition("nope", TEST_REGISTRY)).toBeUndefined(); + }); + + it("getFlagDefinition defaults to the shipped registry", () => { + expect(getFlagDefinition("disable-auto-screenshot")?.description).toMatch(/auto/i); + }); + + it("ships the disable-update-notification flag with a description", () => { + expect(getFlagDefinition("disable-update-notification")?.description).toMatch(/update/i); + }); + + it("every shipped registry entry has a non-empty name and description", () => { + // Guards against a half-filled entry being added to the production registry. + for (const def of FLAG_REGISTRY) { + expect(def.name).toMatch(/^[a-zA-Z][a-zA-Z0-9._-]*$/); + expect(def.description.trim().length).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/configuration-core/tsconfig.json b/packages/configuration-core/tsconfig.json new file mode 100644 index 00000000..4a7d44e1 --- /dev/null +++ b/packages/configuration-core/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "outDir": "./dist", + "rootDir": "./src", + "types": ["node"], + "strict": true, + "composite": true, + "declaration": true, + "sourceMap": true + }, + "include": ["src/**/*"] +} diff --git a/packages/configuration-core/tsconfig.test.json b/packages/configuration-core/tsconfig.test.json new file mode 100644 index 00000000..9f6d71f0 --- /dev/null +++ b/packages/configuration-core/tsconfig.test.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "composite": false, + "noEmit": true, + "rootDir": ".", + "declaration": false, + "declarationMap": false + }, + "include": ["src/**/*", "tests/**/*", "vitest.config.ts"] +} diff --git a/packages/configuration-core/vitest.config.ts b/packages/configuration-core/vitest.config.ts new file mode 100644 index 00000000..6161f133 --- /dev/null +++ b/packages/configuration-core/vitest.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + include: ["tests/**/*.test.ts"], + globals: true, + }, +}); diff --git a/packages/tool-server/package.json b/packages/tool-server/package.json index 1f61c8bb..b1b20267 100644 --- a/packages/tool-server/package.json +++ b/packages/tool-server/package.json @@ -13,6 +13,7 @@ "typecheck:tests": "tsc --noEmit -p tsconfig.test.json" }, "dependencies": { + "@argent/configuration-core": "file:../configuration-core", "@argent/native-devtools-ios": "file:../native-devtools-ios", "@argent/native-devtools-android": "file:../native-devtools-android", "@argent/registry": "file:../registry", diff --git a/packages/tool-server/src/http.ts b/packages/tool-server/src/http.ts index 6619a27b..d99e10fe 100644 --- a/packages/tool-server/src/http.ts +++ b/packages/tool-server/src/http.ts @@ -5,6 +5,7 @@ import { createIdleTimer } from "./utils/idle-timer"; import { DependencyMissingError, ensureDeps } from "./utils/check-deps"; import { formatErrorForAgent } from "./utils/format-error"; import { getUpdateState, isUpdateNoteSuppressed, suppressUpdateNote } from "./utils/update-checker"; +import { updateNotificationDisabled } from "./utils/update-reminder"; import { buildUpdateNote } from "./update-utils"; import { createPreviewRouter } from "./preview"; import { @@ -188,7 +189,11 @@ export function createHttpApp(registry: Registry, options?: HttpAppOptions): Htt // Gate on `updateInstallable` (not `updateAvailable`) and advertise the // version the resolver would install — both account for the release-age policy. const { updateInstallable, currentVersion, installableVersion } = getUpdateState(); - const shouldNotify = updateInstallable && !isUpdateNoteSuppressed(); + // Order matters: `updateInstallable` (cheap in-memory bool) stays first so + // the flags file is read only when an update is actually installable — the + // common no-update path does zero extra fs work. + const shouldNotify = + updateInstallable && !updateNotificationDisabled() && !isUpdateNoteSuppressed(); if (shouldNotify) { // Best-effort: a persistence failure here must not fail the user's tool call. // Worst case: the note appears again on the next request. diff --git a/packages/tool-server/src/utils/update-reminder.ts b/packages/tool-server/src/utils/update-reminder.ts new file mode 100644 index 00000000..51ff868e --- /dev/null +++ b/packages/tool-server/src/utils/update-reminder.ts @@ -0,0 +1,8 @@ +import { isFlagEnabled, type FlagsPathOptions } from "@argent/configuration-core"; + +// Permanent opt-out for the agent-facing "update available" note. Gates ONLY the +// note — `argent update` and the `update-argent` tool are unaffected, and the +// temporary dismiss-update / auto-suppress mechanism is orthogonal and stays. +export function updateNotificationDisabled(options?: FlagsPathOptions): boolean { + return isFlagEnabled("disable-update-notification", options); +} diff --git a/packages/tool-server/test/http-update-note.test.ts b/packages/tool-server/test/http-update-note.test.ts index 1af80509..a492e695 100644 --- a/packages/tool-server/test/http-update-note.test.ts +++ b/packages/tool-server/test/http-update-note.test.ts @@ -5,6 +5,7 @@ import type { UpdateState } from "../src/utils/update-checker"; import type { Registry } from "@argent/registry"; let suppressed = false; +let updateNotificationOff = false; /** Build a full UpdateState from partial overrides so tests stay terse. */ function mkState(overrides: Partial = {}): UpdateState { @@ -27,6 +28,11 @@ vi.mock("../src/utils/update-checker", () => ({ suppressUpdateNote: vi.fn(), })); +// Mock the permanent opt-out flag so tests need not touch the real flags file. +vi.mock("../src/utils/update-reminder", () => ({ + updateNotificationDisabled: vi.fn(() => updateNotificationOff), +})); + import { getUpdateState, suppressUpdateNote } from "../src/utils/update-checker"; const mockGetUpdateState = vi.mocked(getUpdateState); @@ -57,6 +63,7 @@ describe("HTTP update note injection", () => { beforeEach(async () => { suppressed = false; + updateNotificationOff = false; mockGetUpdateState.mockReturnValue(mkState()); mockSuppressUpdateNote.mockClear(); }); @@ -314,4 +321,43 @@ describe("HTTP update note injection", () => { expect(mockSuppressUpdateNote).not.toHaveBeenCalled(); }); + + // ── Permanent opt-out flag (disable-update-notification) ────────── + + it("does NOT include a note when the disable-update-notification flag is on", async () => { + updateNotificationOff = true; + mockGetUpdateState.mockReturnValue( + mkState({ + updateAvailable: true, + updateInstallable: true, + latestVersion: "1.2.3", + installableVersion: "1.2.3", + }) + ); + + handle = createHttpApp(stubRegistry()); + + const res = await request(handle.app).post("/tools/test-tool").send({}).expect(200); + + expect(res.body).toHaveProperty("data"); + expect(res.body).not.toHaveProperty("note"); + }); + + it("does NOT call suppressUpdateNote when the flag is on (must not burn the temporary window)", async () => { + updateNotificationOff = true; + mockGetUpdateState.mockReturnValue( + mkState({ + updateAvailable: true, + updateInstallable: true, + latestVersion: "1.2.3", + installableVersion: "1.2.3", + }) + ); + + handle = createHttpApp(stubRegistry()); + + await request(handle.app).post("/tools/test-tool").send({}).expect(200); + + expect(mockSuppressUpdateNote).not.toHaveBeenCalled(); + }); }); diff --git a/packages/tool-server/tsconfig.json b/packages/tool-server/tsconfig.json index 3b8ac147..c19a5893 100644 --- a/packages/tool-server/tsconfig.json +++ b/packages/tool-server/tsconfig.json @@ -7,6 +7,7 @@ "rootDir": "./src" }, "references": [ + { "path": "../configuration-core" }, { "path": "../registry" }, { "path": "../update-core" }, { "path": "../native-devtools-ios" }, diff --git a/tsconfig.json b/tsconfig.json index 76ac2f8b..1f8d6bec 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,6 +7,7 @@ { "path": "packages/native-devtools-android" }, { "path": "packages/tool-server" }, { "path": "packages/argent-tools-client" }, + { "path": "packages/configuration-core" }, { "path": "packages/argent-installer" }, { "path": "packages/argent-mcp" }, { "path": "packages/argent-cli" },