Skip to content

Conversation

@richhankins
Copy link
Contributor

@richhankins richhankins commented Jan 30, 2026

Problem

When SearchClient performs file operations (listFiles, readFile), it creates a Source from stored index state. The Source was being created with the original branch ref (e.g., "main"), but the resolvedRef (commit SHA that was actually indexed) was not passed to the source.

This caused inconsistency:

  • Search results came from the indexed version (correct)
  • But listFiles and readFile queried the current latest commit on the branch (wrong!)

This means search might find code that doesn't exist in the version being read, or vice versa.

Solution

Modified createSourceFromState() in multi-index-runner.ts to use resolvedRef (when present) instead of config.ref for VCS sources. This ensures that listFiles and readFile operate on the exact commit that was indexed.

// For VCS sources, use resolvedRef (indexed commit SHA) if available.
// This ensures file operations return content from the same commit that was indexed.
return new GitHubSource({ ...meta.config, ref: meta.resolvedRef ?? meta.config.ref });

Changes

Core Fix

  • src/clients/multi-index-runner.ts - Use meta.resolvedRef ?? meta.config.ref for GitHub, GitLab, and BitBucket sources

Unit Tests

  • src/clients/multi-index-runner.test.ts - 4 tests verifying:
    • resolvedRef is used when present
    • Falls back to config.ref when resolvedRef is missing
    • Website sources work correctly (no ref concept)
    • Unknown source type throws error

Integration Tests

  • test/resolved-ref.ts - 7 tests verifying actual GitHub API behavior:
    • listFiles honors the commit SHA
    • readFile returns content from the specific commit
    • Different commits return different content
    • Metadata correctly reports resolvedRef

Test Infrastructure Improvements

  • test/cli-agent.ts - Use local build directly (node dist/bin/index.js) instead of npx ctxc for reliable testing
  • test/resolved-ref.ts - Import from dist/ instead of src/ to test compiled output

Testing

  • ✅ All 167 unit tests pass
  • ✅ All integration tests pass locally (npm run test:integration)
  • ✅ CI passing

Pull Request opened by Augment Code with guidance from the PR author

Updated the createSourceFromState function to override the ref in the
config with meta.resolvedRef when available for GitHub, GitLab, and
BitBucket sources.

This ensures that listFiles and readFile operations use the exact
commit SHA that was indexed, rather than the branch name that may
have moved since indexing.

Agent-Id: agent-ac8ae6ab-1e3e-43f0-99fb-bdad5a5fb83d
Add tests to verify that createSourceFromState correctly uses
resolvedRef from state metadata when creating source instances:

- Test that GitHub source receives resolvedRef as ref when present
- Test that GitLab source receives resolvedRef as ref when present
- Test that BitBucket source receives resolvedRef as ref when present
- Test that original config.ref is used when resolvedRef is missing
- Test that website sources work correctly without resolvedRef

Uses vi.mock to mock the source constructors and verify they receive
the correct ref parameter.

Agent-Id: agent-cb10d5cd-ffb8-488e-81b3-914f1aeda05a
The tests require the Augment SDK which needs API credentials.
Following the pattern used in other test files (mcp-server.test.ts,
search-client.test.ts), skip the tests when the SDK fails to load
or when API credentials are not available.

Agent-Id: agent-83532e9b-da56-46ec-a480-58d2ec43c946
- Export createSourceFromState for testing
- Rewrite tests to call the function directly instead of going through MultiIndexRunner
- Tests now run as unit tests without requiring API credentials
- Added test for unknown source type error handling
Agent-Id: agent-2c5b2cfc-9cfd-4602-83db-8b2c64a1f8b7
…ests

All VCS sources (GitHub, GitLab, BitBucket) use identical getRef() logic,
so testing each provider separately was redundant. Reduced from 7 tests
to 4 tests while maintaining the same effective coverage:

- Uses resolvedRef when present (tests shared VCS logic)
- Falls back to config.ref when resolvedRef missing (tests shared VCS logic)
- Website source works without resolvedRef (different code path)
- Throws error for unknown source type (error handling)
Created test/resolved-ref.test.ts that verifies GitHubSource operations
(listFiles, readFile) use the exact commit SHA provided as ref, ensuring
file operations return content from the indexed commit.

Test cases:
- listFiles works with commit SHA refs
- readFile returns content from specific commit SHA
- Different commit SHAs return different file content
- getMetadata returns correct resolvedRef

Uses octocat/Hello-World repository with known commits that have
slightly different README content to prove ref handling works correctly.

Agent-Id: agent-d2d9f141-b65f-43fe-82b7-9bc2962f9f5e
- Rename test/resolved-ref.test.ts to test/resolved-ref.ts
- Add to test:integration script in package.json
- Update test/README.md
Instead of a separate getRef helper that couldn't be properly typed
(SourceMetadata is a discriminated union), inline the logic directly
where TypeScript can leverage the narrowed type from the if-checks.
Instead of using 'npx ctxc' which may pick up a globally installed
version, run the local build directly with 'node dist/bin/index.js'.

This makes the test more reliable and reproducible across different
environments.
Import from dist/ instead of src/ to test the actual compiled code
that gets shipped, consistent with how cli-agent.ts tests the CLI.
@richhankins
Copy link
Contributor Author

augment review

@richhankins richhankins requested a review from igor0 January 30, 2026 21:57
@richhankins
Copy link
Contributor Author

augment review

@augment-app-staging
Copy link

🤖 Augment PR Summary

Summary: This PR fixes a mismatch where search results came from an indexed commit, but follow-up file operations read/listed from the moving branch head.

Changes:

  • Updated and exported createSourceFromState() to prefer resolvedRef (indexed commit SHA) over config.ref for GitHub/GitLab/Bitbucket sources.
  • Added unit tests covering resolved-ref preference, fallback behavior, website source creation, and unknown source types.
  • Added an integration test that asserts GitHubSource listFiles/readFile honor an explicit commit SHA.
  • Adjusted the CLI integration test to run the locally built CLI entrypoint from dist/bin/index.js.
  • Extended the integration test runner script and test README to include the new resolved-ref checks.
Technical Notes: Using resolvedRef ensures file reads/lists and search operate against the same exact repository snapshot, even if the branch advances after indexing.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augment-app-staging augment-app-staging bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

"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",

Choose a reason for hiding this comment

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

test:integration now runs scripts that execute/import from dist/, but the script itself doesn’t ensure a build has happened first (fresh checkouts will fail with missing dist). Consider making the integration script self-contained by ensuring npm run build is run as a pre-step.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

let testIndexPath: string | null = null;

// Path to the local CLI build
const CLI_PATH = resolve(import.meta.dirname, "../dist/bin/index.js");

Choose a reason for hiding this comment

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

import.meta.dirname isn’t available in all Node/runner versions, and if it’s undefined this test will crash before spawning the CLI. Consider deriving the directory from import.meta.url (or otherwise ensuring the runtime guarantees import.meta.dirname).

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants