Skip to content

test(lib): add comprehensive unit tests for resolve-subagent-path (#328) #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AVPthegreat wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test(lib): add comprehensive unit tests for resolve-subagent-path (#328) #628

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 35 additions & 1 deletion __tests__/lib/resolve-subagent-path.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,25 @@
import { mkdtemp, mkdir, rm, writeFile } from "fs/promises";
import { tmpdir } from "os";
import { dirname, join, relative } from "path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { resolveSubagentPath } from "@/lib/resolve-subagent-path";

let mockAccessError: NodeJS.ErrnoException | null = null;

vi.mock("fs/promises", async () => {
const actual = await vi.importActual<typeof import("fs/promises")>("fs/promises");
return {
...actual,
access: vi.fn(async (path, mode) => {
if (mockAccessError) {
throw mockAccessError;
}
return actual.access(path, mode);
}),
};
});

describe("resolveSubagentPath", () => {
let fixtureRoot: string;
let projectsPath: string;
Expand All @@ -15,12 +30,14 @@ describe("resolveSubagentPath", () => {
const agentId = "abc123";

beforeEach(async () => {
mockAccessError = null;
fixtureRoot = await mkdtemp(join(tmpdir(), "failproofai-subagents-"));
projectsPath = join(fixtureRoot, "projects");
await mkdir(join(projectsPath, projectName, sessionId, "subagents"), { recursive: true });
});

afterEach(async () => {
mockAccessError = null;
Comment on lines +33 to +40

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file and related symbols =="
git ls-files | rg '(^|/)resolve-subagent-path\.(ts|js)$|resolve-subagent-path.test.ts' || true

echo
echo "== excerpt test file around mentioned lines =="
if [ -f "__tests__/lib/resolve-subagent-path.test.ts" ]; then
  wc -l "__tests__/lib/resolve-subagent-path.test.ts"
  sed -n '1,150p' "__tests__/lib/resolve-subagent-path.test.ts" | nl -ba -v1
fi

echo
echo "== find source file(s) =="
fd -a 'resolve-subagent-path' . | sed 's#^\./##'

Repository: FailproofAI/failproofai

Length of output: 409

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== test file =="
cat -n "__tests__/lib/resolve-subagent-path.test.ts"

echo
echo "== source file =="
cat -n "lib/resolve-subagent-path.ts"

Repository: FailproofAI/failproofai

Length of output: 6744

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== package vitest availability =="
if [ -f package.json ]; then
  cat package.json | sed -n '1,160p'
fi

echo
echo "== deterministic call-history behavior for the mocked fs/promises access =="
node - <<'JS'
const calls = [];
const original = async (path) => {
  if (path.endsWith("candidate2.jsonl")) throw Object.assign(new Error("EACCES"), { code: "EACCES" });
};
async function resolveSubagentPath() {
  const candidatePaths = ["candidate1.jsonl", "candidate2.jsonl", "candidate3.jsonl"];
  for (const candidatePath of candidatePaths) {
    try {
      await original(candidatePath);
    } catch (e) {
      if ((e).code === "ENOENT") continue;
      break;
    }
  }
  return null;
}

(async () => {
  const result = await resolveSubagentPath();
  console.log(JSON.stringify({ result, totalCalls: calls.length }));
})();
JS

echo
echo "== test assertions for access.calls/callCount =="
rg -n "access\\.calls|\.calls\\.length|callCount|mocked\(" "__tests__" "lib" || true

Repository: FailproofAI/failproofai

Length of output: 48531

Assert that unexpected errors stop candidate search.

The EACCES expectation only checks result === null; it would still pass if the loop checked the remaining candidates. Clear the mock call history in beforeEach, then assert that access was called exactly once with the project-level candidate in the error case.

Also applies to: 103-110

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/lib/resolve-subagent-path.test.ts` around lines 33 - 40, Update the
test setup in beforeEach to clear the access mock’s call history, then
strengthen the EACCES test assertions to verify access is called exactly once
with the project-level candidate path, ensuring candidate search stops on
unexpected errors. Apply the same assertion update to the related test around
the second referenced range.

await rm(fixtureRoot, { recursive: true, force: true });
});

Expand Down Expand Up @@ -74,4 +91,21 @@ describe("resolveSubagentPath", () => {

await expect(resolveSubagentPath(projectsPath, projectName, sessionId, traversalAgentId)).resolves.toBeNull();
});

it("prevents path traversal when projectName attempts to escape projectsPath", async () => {
const traversalProject = "../../outside";
const escapedCandidate = join(projectsPath, traversalProject, `agent-${agentId}.jsonl`);
await touch(escapedCandidate);

await expect(resolveSubagentPath(projectsPath, traversalProject, sessionId, agentId)).resolves.toBeNull();
});
Comment on lines +95 to +101

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Keep traversal fixtures inside the per-test temporary directory.

../../outside escapes above fixtureRoot, so touch() creates an artifact that the test cleanup will not remove. Use ../outside (or another path beneath fixtureRoot) while remaining outside projectsPath.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/lib/resolve-subagent-path.test.ts` around lines 95 - 101, Update
the traversalProject fixture in the resolveSubagentPath traversal test to use
../outside or another path that remains beneath the per-test fixtureRoot while
still escaping projectsPath. Keep the escaped-candidate setup and expected null
result unchanged.


it("stops searching and returns null when an unexpected error occurs (e.g., EACCES)", async () => {
const err = new Error("Permission denied") as NodeJS.ErrnoException;
err.code = "EACCES";
mockAccessError = err;

const result = await resolveSubagentPath(projectsPath, projectName, sessionId, agentId);
expect(result).toBeNull();
});
});