Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .ade/ade.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ processes:
- id: dbun9idy
name: dogfood code review
command:
- /Users/admin/Projects/ADE/scripts/dogfood.sh
- scripts/dogfood.sh
- code-review
cwd: /Users/admin/Projects/ADE
cwd: .
gracefulShutdownMs: 7000
stackButtons: []
testSuites: []
Expand Down
58 changes: 57 additions & 1 deletion .claude/commands/finalize.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The only outputs are the Phase 4 summary and any error messages for genuinely fa

```
Phase 1: Analyze code changes and batch simplification work (lead)
Phase 2: Parallel execution (simplify + docs) (agents)
Phase 2: Parallel execution (simplify + docs + mobile parity)(agents)
Phase 3: CI sync + local verification (lead)
Phase 4: Summary (lead)
```
Expand Down Expand Up @@ -185,6 +185,56 @@ This validator only covers the Mintlify site. For internal docs, self-check:
Report what docs were updated and what was changed.
```

### Mobile parity agent

Spawn a general-purpose agent with this prompt:

```
You are the mobile parity reviewer for the ADE project.

Analyze all work on the current branch vs main, including changes that are
already under review and any simplifications made during `/finalize`. Determine
whether the iOS companion app under `apps/ios/` needs matching updates.

Step 1: Get branch context
git diff main --name-only
git diff main --stat | tail -30
git log main..HEAD --oneline

Step 2: Identify cross-platform changes
- Shared contracts: apps/desktop/src/shared/**, preload IPC types, sync payloads,
PR mobile snapshots, chat/session models, lane summaries, config schemas.
- Desktop behavior with a mobile surface: PR workflows, lanes, Work chat,
files, sync/multi-device, settings exposed on iOS, model/session controls.
- Renderer-only desktop preferences are only mobile-applicable when the iOS app
has the same user-facing concept and a native implementation path.

Step 3: Inspect iOS equivalents
- Search `apps/ios/ADE` and `apps/ios/ADETests` for the affected model, view,
service, or workflow names.
- If the branch adds or changes a host/mobile contract, update Swift Codable
models and iOS tests as needed.
- If the branch changes user-facing behavior that iOS already exposes, update
the SwiftUI view using native iOS controls and existing ADE design patterns.
- If the change is not applicable to iOS, explain why in the report.

Step 4: Apply required iOS updates
- Keep edits scoped to `apps/ios/` unless a shared contract fix is required.
- Prefer existing SwiftUI patterns and native controls.
- Preserve Dynamic Type, VoiceOver labels, and 44x44 tap targets.
- Add or update targeted tests in `apps/ios/ADETests` for contract changes.

Step 5: Validate what you touched
- At minimum: `xcrun swiftc -parse <changed swift files>` when a full Xcode
build/test run is unavailable.
- Prefer an iOS build/test when the local simulator/runtime environment supports it.

Report:
- iOS files changed, or "No iOS changes required"
- Why each desktop/shared change was applicable or not applicable to mobile
- Validation run and any environment limitations
```

Wait for all agents to complete.

---
Expand Down Expand Up @@ -326,6 +376,11 @@ If Phase 3e fails only inside files the simplifier touched, revert the simplifie
- Docs checked but unchanged: [list]
- Doc validation: PASS

### Mobile Parity:
- iOS changes: [list or "none required"]
- Applicability notes: [brief list]
- Validation: PASS / blocked with reason

### CI Verification:
- Lock files in sync: PASS
- Typecheck (desktop): PASS
Expand All @@ -347,6 +402,7 @@ If Phase 3e fails only inside files the simplifier touched, revert the simplifie
Before marking complete:
- [ ] Code simplification completed on all batches
- [ ] Documentation updated for all affected areas
- [ ] Mobile parity reviewed; applicable iOS updates made and validated
- [ ] CI workflow sync verified (no orphaned test files)
- [ ] Lock files in sync (no dirty lock files after install)
- [ ] Typecheck passed (desktop + mcp-server + web)
Expand Down
72 changes: 62 additions & 10 deletions apps/desktop/src/main/services/ai/tools/grepSearch.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { execFile } from "node:child_process";
import { afterEach, describe, expect, it, vi } from "vitest";
import { createGrepSearchTool } from "./grepSearch";
import {
__testResetRipgrepExecFile,
__testSetRipgrepExecFile,
createGrepSearchTool,
} from "./grepSearch";

const tmpDirs: string[] = [];
function makeTmpDir(prefix: string): string {
Expand All @@ -18,6 +23,7 @@ function writeFixtureFile(root: string, relativePath: string, content: string):
}

afterEach(() => {
__testResetRipgrepExecFile();
vi.restoreAllMocks();
for (const dir of tmpDirs) {
try {
Expand All @@ -29,22 +35,19 @@ afterEach(() => {
tmpDirs.length = 0;
});

// Helper to force JS fallback by making execFile reject for rg
// Force JS fallback by making ripgrep's exec path reject (matches real "rg missing" behavior).
function forceJsFallback(): void {
const cp = require("node:child_process");
const originalExecFile = cp.execFile;
vi.spyOn(cp, "execFile").mockImplementation(
(cmd: unknown, ...rest: unknown[]) => {
__testSetRipgrepExecFile(
((cmd: unknown, ...rest: unknown[]) => {
if (cmd === "rg") {
// Make the promisified version reject
const cb = rest[rest.length - 1];
if (typeof cb === "function") {
process.nextTick(() => cb(new Error("rg not available")));
process.nextTick(() => (cb as (err: Error) => void)(new Error("rg not available")));
return;
}
}
return originalExecFile(cmd, ...rest);
},
return (execFile as (typeof import("node:child_process"))["execFile"])(cmd as never, ...rest as never[]);
}) as typeof execFile,
);
}

Expand Down Expand Up @@ -199,6 +202,19 @@ describe("createGrepSearchTool", () => {
expect(result.matches[0].displayPath).toBe("src/app.ts");
});

it("repo-wide JS fallback skips root .ade but still searches .github", async () => {
const cwd = makeTmpDir("grep-hidden-root-");
writeFixtureFile(cwd, ".ade/secrets.txt", "SECRET_MARKER");
writeFixtureFile(cwd, ".github/workflows/ci.yml", "SECRET_MARKER");
writeFixtureFile(cwd, "src/app.ts", "SECRET_MARKER");
forceJsFallback();

const tool = createGrepSearchTool(cwd);
const result = await tool.execute({ pattern: "SECRET_MARKER", context: 0 });
const paths = result.matches.map((m) => m.displayPath).sort();
expect(paths).toEqual([".github/workflows/ci.yml", "src/app.ts"]);
});

it("handles brace expansion in file glob: *.{ts,tsx}", async () => {
const cwd = makeTmpDir("grep-brace-");
writeFixtureFile(cwd, "app.ts", "const val = 1;");
Expand Down Expand Up @@ -254,6 +270,42 @@ describe("createGrepSearchTool", () => {
expect(result.error).toBeDefined();
expect(result.matchCount).toBe(0);
});

it("surfaces a descriptive 'Invalid regex pattern' error for malformed patterns (JS fallback)", async () => {
const cwd = makeTmpDir("grep-bad-regex-");
writeFixtureFile(cwd, "code.ts", "const x = 1;");
forceJsFallback();

const tool = createGrepSearchTool(cwd);
// Unmatched `[` — a SyntaxError from `new RegExp`.
const result = await tool.execute({ pattern: "[", context: 0 });

expect(result.matchCount).toBe(0);
expect(result.error).toBeDefined();
expect(result.error).toContain("Invalid regex pattern");
});
});

// --------------------------------------------------------------------------
// Glob edge cases
// --------------------------------------------------------------------------

describe("glob handling", () => {
it("matches bare filenames under a **/*.ts glob (JS fallback)", async () => {
// Globs are applied to `entry.name` in the fallback, so `**/*.ts`
// must collapse to `*.ts` to match bare `foo.ts` — aligning with ripgrep.
const cwd = makeTmpDir("grep-starstar-");
writeFixtureFile(cwd, "foo.ts", "const marker = 1;");
writeFixtureFile(cwd, "src/bar.ts", "const marker = 2;");
writeFixtureFile(cwd, "readme.md", "marker");
forceJsFallback();

const tool = createGrepSearchTool(cwd);
const result = await tool.execute({ pattern: "marker", glob: "**/*.ts", context: 0 });

const paths = result.matches.map((m) => m.displayPath).sort();
expect(paths).toEqual(["foo.ts", "src/bar.ts"]);
});
});

// --------------------------------------------------------------------------
Expand Down
86 changes: 77 additions & 9 deletions apps/desktop/src/main/services/ai/tools/grepSearch.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,35 @@
import { executableTool as tool } from "./executableTool";
import { z } from "zod";
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import { execFile, type ExecFileOptionsWithStringEncoding } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import { getErrorMessage, resolvePathWithinRoot } from "../../shared/utils";

const execFileAsync = promisify(execFile);
/** Swappable for Vitest — defaults to Node's `execFile`. */
let execFileForRipgrep: typeof execFile = execFile;

/** @internal Used by grepSearch.test.ts to force the JS fallback path. */
export function __testSetRipgrepExecFile(fn: typeof execFile): void {
execFileForRipgrep = fn;
}

/** @internal */
export function __testResetRipgrepExecFile(): void {
execFileForRipgrep = execFile;
}

function execFileAsync(
file: string,
args: readonly string[] | null | undefined,
options: ExecFileOptionsWithStringEncoding,
): Promise<{ stdout: string; stderr: string }> {
return new Promise((resolve, reject) => {
execFileForRipgrep(file, args, options, (error, stdout, stderr) => {
if (error) reject(error);
else resolve({ stdout, stderr });
});
});
}

type GrepMatch = {
path: string;
Expand Down Expand Up @@ -77,10 +100,13 @@ export function createGrepSearchTool(cwd: string) {
const matches = jsFallbackGrep(root, pattern, target, fileGlob);
return { matches, matchCount: matches.length, root: target };
} catch (err) {
const message = getErrorMessage(err);
return {
matches: [],
matchCount: 0,
error: `Search failed: ${getErrorMessage(err)}`,
error: message.startsWith("Invalid regex pattern")
? message
: `Search failed: ${message}`,
};
}
},
Expand Down Expand Up @@ -126,9 +152,17 @@ function jsFallbackGrep(
target: string,
fileGlob: string | undefined
): GrepMatch[] {
const regex = new RegExp(pattern);
let regex: RegExp;
try {
regex = new RegExp(pattern);
} catch (error) {
// Surface a user-facing message distinct from generic "Search failed".
// Ripgrep itself returns a descriptive error for malformed patterns; match that ergonomic on the fallback path.
throw new Error(`Invalid regex pattern: ${getErrorMessage(error)}`);
}
const results: GrepMatch[] = [];
const files = collectFiles(target, fileGlob);
const searchWholeRepo = path.resolve(target) === path.resolve(root);
const files = collectFiles(target, fileGlob, searchWholeRepo);

for (const filePath of files) {
if (results.length >= 500) break;
Expand All @@ -151,16 +185,38 @@ function jsFallbackGrep(

const SKIP_DIRS = new Set(["node_modules", ".git", "dist", "build", ".next", "coverage"]);

/** Hidden first-segment dirs under the repo root we still want repo-wide search to enter. */
const ALLOW_HIDDEN_ROOT_DIRS = new Set([".github"]);

function shouldSkipHiddenDirUnderRepoRoot(
rootReal: string,
parentAbs: string,
dirName: string,
searchWholeRepo: boolean,
): boolean {
if (!searchWholeRepo) return false;
if (!dirName.startsWith(".") || dirName === "." || dirName === "..") return false;
if (ALLOW_HIDDEN_ROOT_DIRS.has(dirName)) return false;
const childAbs = path.resolve(path.join(parentAbs, dirName));
const rel = path.relative(rootReal, childAbs);
if (!rel || rel.startsWith("..") || path.isAbsolute(rel)) return false;
const first = rel.split(path.sep)[0] ?? "";
// Only skip direct children of the repo root (e.g. `.ade`, `.env`) — not `src/.cache`.
return first === dirName;
}

function collectFiles(
dir: string,
fileGlob: string | undefined,
searchWholeRepo: boolean,
maxFiles = 5000
): string[] {
const stat = fs.statSync(dir);
if (stat.isFile()) return [dir];

const files: string[] = [];
const globRegex = fileGlob ? globToRegex(fileGlob) : null;
const rootReal = fs.realpathSync(dir);

function walk(current: string): void {
if (files.length >= maxFiles) return;
Expand All @@ -173,9 +229,14 @@ function collectFiles(
for (const entry of entries) {
if (files.length >= maxFiles) return;
if (entry.isDirectory()) {
if (!SKIP_DIRS.has(entry.name) && !entry.name.startsWith(".")) {
walk(path.join(current, entry.name));
if (SKIP_DIRS.has(entry.name)) continue;
const next = path.join(current, entry.name);
if (
shouldSkipHiddenDirUnderRepoRoot(rootReal, current, entry.name, searchWholeRepo)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

This change only skips dot-directories when searchWholeRepo is true, so targeted directory searches no longer preserve the old hidden-dir guard. In the new code:

// apps/desktop/src/main/services/ai/tools/grepSearch.ts
if (entry.isDirectory()) {
  if (SKIP_DIRS.has(entry.name)) continue;
  const next = path.join(current, entry.name);
  if (shouldSkipHiddenDirUnderRepoRoot(rootReal, current, entry.name, searchWholeRepo)) {
    continue;
  }
  walk(next);
}

tool.execute({ path: "src", ... }) will now recurse into src/.cache, src/.storybook, etc., whereas the previous implementation skipped all entry.name.startsWith(".") directories and the PR note says targeted subdirectory searches are unchanged. That broadens the agent-visible search surface beyond the intended .github exception. Restore the old hidden-directory exclusion for non-root searches, and only special-case direct children of the repo root when searchWholeRepo is true.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

The new walker removed the old entry.name.startsWith(".") guard and now only skips hidden directories when they are direct children of the repo root in a repo-wide search:

// apps/desktop/src/main/services/ai/tools/grepSearch.ts
if (entry.isDirectory()) {
  if (SKIP_DIRS.has(entry.name)) continue;
  const next = path.join(current, entry.name);
  if (shouldSkipHiddenDirUnderRepoRoot(rootReal, current, entry.name, searchWholeRepo)) {

That changes fallback behavior from ripgrep’s default filtering (rg skips hidden files/directories unless explicitly opted in): a targeted search like path: "src" will now recurse into src/.cache, and a repo-wide search will also descend into nested hidden directories such as src/.storybook. Since this code path is used exactly when rg is unavailable, those environments now get noisier, inconsistent results that the new tests do not cover. Restore the general hidden-directory skip in collectFiles and add the .github exception only for direct children of the repo root (while still allowing an explicitly targeted hidden root to be searched).

) {
continue;
}
walk(next);
} else if (entry.isFile()) {
const fullPath = path.join(current, entry.name);
if (!globRegex || globRegex.test(entry.name)) {
Expand All @@ -190,9 +251,16 @@ function collectFiles(
}

function globToRegex(glob: string): RegExp {
// Globs are matched against bare filenames (`entry.name`) in the fallback,
// so strip directory components before applying glob rules. Collapse `**`
// first so `**/*.ts` → `*/*.ts` → `*.ts`; if any `/` remains, keep only the
// last segment (the filename pattern) so `src/*.ts` still matches `foo.ts`.
let pattern = glob.replace(/\*\*/g, "*");
const lastSlash = pattern.lastIndexOf("/");
if (lastSlash !== -1) pattern = pattern.slice(lastSlash + 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

globToRegex now drops every path segment before compiling the fallback regex, so when rg is unavailable a request like src/*.ts or services/**/*.ts no longer restricts results to that subtree — it matches any *.ts / index.ts anywhere under the scan root. That changes the meaning of the glob argument relative to the primary ripgrep path and can return unrelated files to agents in fallback-only environments. Preserve and match against a relative file path instead of slicing to the last segment; only special-case the **/ collapse if needed.

// apps/desktop/src/main/services/ai/tools/grepSearch.ts
let pattern = glob.replace(/\*\*/g, "*");
const lastSlash = pattern.lastIndexOf("/");
if (lastSlash !== -1) pattern = pattern.slice(lastSlash + 1);

// Escape special regex chars except * and ? first, BEFORE brace expansion.
// This avoids escaping the parens/pipe that brace expansion introduces.
let pattern = glob.replace(/[.+^$[\]\\]/g, "\\$&");
pattern = pattern.replace(/[.+^$[\]\\]/g, "\\$&");
// Replace glob wildcards
pattern = pattern.replace(/\*/g, ".*");
pattern = pattern.replace(/\?/g, ".");
Expand Down
Loading
Loading