diff --git a/package.json b/package.json index 34e2f1b..8343ac0 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "cli": "tsx src/bin/index.ts", "cli:index": "tsx src/bin/index.ts index", "cli:search": "tsx src/bin/index.ts search", - "test:integration": "tsx test/augment-provider.ts && tsx test/cli-agent.ts", + "test:integration": "tsx test/augment-provider.ts && tsx test/cli-agent.ts && tsx test/resolved-ref.ts", "format": "biome format --write .", "lint": "biome check .", "lint:fix": "biome check --write ." diff --git a/src/clients/multi-index-runner.test.ts b/src/clients/multi-index-runner.test.ts new file mode 100644 index 0000000..8bea9a0 --- /dev/null +++ b/src/clients/multi-index-runner.test.ts @@ -0,0 +1,115 @@ +/** + * Tests for createSourceFromState + * + * These tests verify that createSourceFromState correctly uses resolvedRef + * from state metadata when creating source instances. + * + * We mock GitHub and Website sources to capture what config gets passed + * to the constructors, without needing API credentials. + * + * Since all VCS sources (GitHub, GitLab, BitBucket) use the same getRef() logic, + * we only test GitHub as the representative case. + */ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { IndexStateSearchOnly, SourceMetadata } from "../core/types.js"; + +// Mock only the sources we actually test +vi.mock("../sources/github.js", () => ({ + GitHubSource: vi.fn().mockImplementation((config) => ({ + type: "github" as const, + config, + })), +})); + +vi.mock("../sources/website.js", () => ({ + WebsiteSource: vi.fn().mockImplementation((config) => ({ + type: "website" as const, + config, + })), +})); + +// Import the function under test and mocked sources +import { createSourceFromState } from "./multi-index-runner.js"; +import { GitHubSource } from "../sources/github.js"; +import { WebsiteSource } from "../sources/website.js"; + +// Create mock state with specific source metadata +const createMockState = (source: SourceMetadata): IndexStateSearchOnly => ({ + version: 1, + contextState: { + version: 1, + } as any, + source, +}); + +describe("createSourceFromState", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + // All VCS sources (GitHub, GitLab, BitBucket) use the same getRef() logic: + // resolvedRef ?? config.ref + // We test this once with GitHub as the representative case. + + it("uses resolvedRef when present", async () => { + const state = createMockState({ + type: "github", + config: { owner: "test-owner", repo: "test-repo", ref: "main" }, + resolvedRef: "abc123sha", + syncedAt: new Date().toISOString(), + }); + + await createSourceFromState(state); + + expect(GitHubSource).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "abc123sha", + }); + }); + + it("falls back to config.ref when resolvedRef is missing", async () => { + const state = createMockState({ + type: "github", + config: { owner: "test-owner", repo: "test-repo", ref: "main" }, + // No resolvedRef + syncedAt: new Date().toISOString(), + }); + + await createSourceFromState(state); + + expect(GitHubSource).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + ref: "main", + }); + }); + + it("website source works without resolvedRef", async () => { + const state = createMockState({ + type: "website", + config: { url: "https://example.com", maxDepth: 2 }, + syncedAt: new Date().toISOString(), + }); + + await createSourceFromState(state); + + expect(WebsiteSource).toHaveBeenCalledWith({ + url: "https://example.com", + maxDepth: 2, + }); + }); + + it("throws error for unknown source type", async () => { + const state = createMockState({ + type: "unknown" as any, + config: {} as any, + syncedAt: new Date().toISOString(), + }); + + await expect(createSourceFromState(state)).rejects.toThrow( + "Unknown source type: unknown" + ); + }); +}); diff --git a/src/clients/multi-index-runner.ts b/src/clients/multi-index-runner.ts index f15027f..8ea531c 100644 --- a/src/clients/multi-index-runner.ts +++ b/src/clients/multi-index-runner.ts @@ -38,18 +38,38 @@ export interface MultiIndexRunnerConfig { searchOnly?: boolean; } -/** Create a Source from index state metadata */ -async function createSourceFromState(state: IndexStateSearchOnly): Promise { +/** + * Create a Source from index state metadata. + * + * For VCS sources (GitHub, GitLab, BitBucket), uses `resolvedRef` (the indexed commit SHA) + * if available, falling back to `config.ref` (branch name) if not. + * + * **Why resolvedRef matters:** + * - `resolvedRef` is the exact commit SHA that was indexed for search + * - Using it ensures `listFiles` and `readFile` return content from the same commit + * that was indexed, so file operations match search results + * - If we used `config.ref` (branch name), the branch might have moved since indexing, + * causing file operations to return different content than what search indexed + * + * @internal Exported for testing + */ +export async function createSourceFromState(state: IndexStateSearchOnly): Promise { const meta = state.source; + + // For VCS sources, use resolvedRef (indexed commit SHA) if available. + // This ensures file operations (listFiles, readFile) return content from + // the same commit that was indexed, so results match search. + // Falls back to config.ref for backwards compatibility with older indexes. + if (meta.type === "github") { const { GitHubSource } = await import("../sources/github.js"); - return new GitHubSource(meta.config); + return new GitHubSource({ ...meta.config, ref: meta.resolvedRef ?? meta.config.ref }); } else if (meta.type === "gitlab") { const { GitLabSource } = await import("../sources/gitlab.js"); - return new GitLabSource(meta.config); + return new GitLabSource({ ...meta.config, ref: meta.resolvedRef ?? meta.config.ref }); } else if (meta.type === "bitbucket") { const { BitBucketSource } = await import("../sources/bitbucket.js"); - return new BitBucketSource(meta.config); + return new BitBucketSource({ ...meta.config, ref: meta.resolvedRef ?? meta.config.ref }); } else if (meta.type === "website") { const { WebsiteSource } = await import("../sources/website.js"); return new WebsiteSource(meta.config); diff --git a/test/README.md b/test/README.md index a62db8a..2e8cc45 100644 --- a/test/README.md +++ b/test/README.md @@ -36,6 +36,7 @@ This directory contains integration tests that require real credentials and make |------|-------------| | `augment-provider.ts` | Tests the Augment provider SDK integration (credentials, model, API calls, tool calling) | | `cli-agent.ts` | Tests the built `ctxc` CLI binary end-to-end (indexes augmentcode/auggie, then runs agent) | +| `resolved-ref.ts` | Tests that GitHubSource operations use the exact commit SHA provided as ref (honors resolvedRef) | ## Note diff --git a/test/cli-agent.ts b/test/cli-agent.ts index 9e31095..b42b96f 100644 --- a/test/cli-agent.ts +++ b/test/cli-agent.ts @@ -14,6 +14,7 @@ import { spawn } from "child_process"; import { mkdtemp, rm } from "fs/promises"; +import { resolve } from "path"; import { tmpdir } from "os"; import { join } from "path"; @@ -26,12 +27,15 @@ interface TestResult { let testIndexPath: string | null = null; +// Path to the local CLI build +const CLI_PATH = resolve(import.meta.dirname, "../dist/bin/index.js"); + async function runCLI( args: string[], timeoutMs = 60000 ): Promise<{ stdout: string; stderr: string; exitCode: number }> { return new Promise((resolve) => { - const proc = spawn("npx", ["ctxc", ...args], { + const proc = spawn(process.execPath, [CLI_PATH, ...args], { cwd: process.cwd(), env: process.env, timeout: timeoutMs, diff --git a/test/resolved-ref.ts b/test/resolved-ref.ts new file mode 100644 index 0000000..d835c6a --- /dev/null +++ b/test/resolved-ref.ts @@ -0,0 +1,241 @@ +/** + * Integration tests for resolvedRef handling in GitHubSource. + * + * These tests verify that when a specific commit SHA is provided as ref, + * the listFiles and readFile operations use that exact commit, not the + * latest commit on a branch. + * + * This validates the fix in createSourceFromState() that ensures file + * operations use resolvedRef (the indexed commit SHA) instead of config.ref + * (the branch name). + * + * Prerequisites: + * - GITHUB_TOKEN environment variable set + * + * Usage: + * npx tsx test/resolved-ref.test.ts + */ + +import { GitHubSource } from "../dist/sources/github.js"; + +// Skip if no GitHub token +const GITHUB_TOKEN = process.env.GITHUB_TOKEN; +if (!GITHUB_TOKEN) { + console.log("โญ๏ธ Skipping: GITHUB_TOKEN not set"); + process.exit(0); +} + +// Known commits from octocat/Hello-World repository +// These are stable and won't change +const OLDEST_COMMIT = "553c2077f0edc3d5dc5d17262f6aa498e69d6f8e"; +const NEWEST_COMMIT = "7fd1a60b01f91b314f59955a4e4d4e80d8edf11d"; + +// Expected content at specific commits (base64 decoded): +// OLDEST_COMMIT README: "Hello World!" (no trailing newline) +// NEWEST_COMMIT README: "Hello World!\n" (with trailing newline) +const OLDEST_COMMIT_README_CONTENT = "Hello World!"; +const NEWEST_COMMIT_README_CONTENT = "Hello World!\n"; + +interface TestResult { + name: string; + passed: boolean; + message?: string; +} + +const results: TestResult[] = []; + +function test(name: string, passed: boolean, message?: string) { + results.push({ name, passed, message }); + console.log(`${passed ? "โœ“" : "โœ—"} ${name}`); + if (message && !passed) { + console.log(` ${message}`); + } +} + +async function testListFilesHonorsRef() { + console.log("\n๐Ÿ“ Testing listFiles honors ref...\n"); + + // Test 1: listFiles with oldest commit should work + try { + const source = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: OLDEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const files = await source.listFiles(); + const hasReadme = files.some((f) => f.path === "README"); + + test( + "listFiles with commit SHA returns files", + files.length > 0 && hasReadme, + `Expected README in files, got: ${JSON.stringify(files.map((f) => f.path))}` + ); + } catch (error) { + test("listFiles with commit SHA returns files", false, String(error)); + } + + // Test 2: listFiles with newest commit should also work + try { + const source = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: NEWEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const files = await source.listFiles(); + const hasReadme = files.some((f) => f.path === "README"); + + test( + "listFiles with different commit SHA also works", + files.length > 0 && hasReadme, + `Expected README in files, got: ${JSON.stringify(files.map((f) => f.path))}` + ); + } catch (error) { + test("listFiles with different commit SHA also works", false, String(error)); + } +} + +async function testReadFileHonorsRef() { + console.log("\n๐Ÿ“„ Testing readFile honors ref...\n"); + + // Test 1: readFile at oldest commit returns content from that commit + try { + const source = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: OLDEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const content = await source.readFile("README"); + const matches = content === OLDEST_COMMIT_README_CONTENT; + + test( + "readFile at oldest commit returns correct content", + matches, + `Expected "${OLDEST_COMMIT_README_CONTENT}", got "${content}"` + ); + } catch (error) { + test("readFile at oldest commit returns correct content", false, String(error)); + } + + // Test 2: readFile at newest commit returns different content + try { + const source = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: NEWEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const content = await source.readFile("README"); + const matches = content === NEWEST_COMMIT_README_CONTENT; + + test( + "readFile at newest commit returns correct content", + matches, + `Expected "${NEWEST_COMMIT_README_CONTENT}", got "${content}"` + ); + } catch (error) { + test("readFile at newest commit returns correct content", false, String(error)); + } + + // Test 3: Two sources with different refs return different content + try { + const oldSource = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: OLDEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const newSource = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: NEWEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const oldContent = await oldSource.readFile("README"); + const newContent = await newSource.readFile("README"); + + const contentsDiffer = oldContent !== newContent; + + test( + "Different commit SHAs return different file content", + contentsDiffer, + `Expected different content, both returned: "${oldContent}"` + ); + } catch (error) { + test("Different commit SHAs return different file content", false, String(error)); + } +} + +async function testMetadataResolvedRef() { + console.log("\n๐Ÿ“‹ Testing metadata resolvedRef...\n"); + + // Test: getMetadata returns the exact commit SHA as resolvedRef + try { + const source = new GitHubSource({ + owner: "octocat", + repo: "Hello-World", + ref: OLDEST_COMMIT, + token: GITHUB_TOKEN, + }); + + const metadata = await source.getMetadata(); + + test( + "getMetadata returns correct resolvedRef", + metadata.type === "github" && metadata.resolvedRef === OLDEST_COMMIT, + `Expected resolvedRef=${OLDEST_COMMIT}, got ${metadata.type === "github" ? metadata.resolvedRef : "non-github type"}` + ); + + // Also verify config.ref is preserved separately + test( + "getMetadata preserves config.ref", + metadata.type === "github" && metadata.config.ref === OLDEST_COMMIT, + `Expected config.ref=${OLDEST_COMMIT}, got ${metadata.type === "github" ? metadata.config.ref : "non-github type"}` + ); + } catch (error) { + test("getMetadata returns correct resolvedRef", false, String(error)); + } +} + +async function main() { + console.log("๐Ÿงช Resolved Ref Integration Tests\n"); + console.log("=".repeat(50)); + console.log("\nThese tests verify that GitHubSource operations use the"); + console.log("exact commit SHA provided as ref, ensuring file operations"); + console.log("return content from the indexed commit, not the latest.\n"); + + await testListFilesHonorsRef(); + await testReadFileHonorsRef(); + await testMetadataResolvedRef(); + + // Summary + console.log("\n" + "=".repeat(50)); + const passed = results.filter((r) => r.passed).length; + const total = results.length; + + if (passed === total) { + console.log(`\nโœ… All ${total} tests passed!`); + process.exit(0); + } else { + console.log(`\nโŒ ${passed}/${total} tests passed`); + const failed = results.filter((r) => !r.passed); + console.log("\nFailed tests:"); + for (const f of failed) { + console.log(` - ${f.name}: ${f.message}`); + } + process.exit(1); + } +} + +main().catch((error) => { + console.error("Test runner error:", error); + process.exit(1); +});