-
Notifications
You must be signed in to change notification settings - Fork 0
Fix search/list/read operations to use indexed commit ref #13
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.
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
base: main
Are you sure you want to change the base?
Conversation
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.
|
augment review |
|
augment review |
🤖 Augment PR SummarySummary: 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:
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 👎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "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", |
There was a problem hiding this comment.
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.
🤖 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"); |
There was a problem hiding this comment.
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).
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Problem
When
SearchClientperforms file operations (listFiles,readFile), it creates a Source from stored index state. The Source was being created with the original branchref(e.g., "main"), but theresolvedRef(commit SHA that was actually indexed) was not passed to the source.This caused inconsistency:
listFilesandreadFilequeried 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()inmulti-index-runner.tsto useresolvedRef(when present) instead ofconfig.reffor VCS sources. This ensures thatlistFilesandreadFileoperate on the exact commit that was indexed.Changes
Core Fix
meta.resolvedRef ?? meta.config.reffor GitHub, GitLab, and BitBucket sourcesUnit Tests
resolvedRefis used when presentconfig.refwhenresolvedRefis missingIntegration Tests
listFileshonors the commit SHAreadFilereturns content from the specific commitresolvedRefTest Infrastructure Improvements
node dist/bin/index.js) instead ofnpx ctxcfor reliable testingdist/instead ofsrc/to test compiled outputTesting
npm run test:integration)Pull Request opened by Augment Code with guidance from the PR author