Add VCI snapshot download support #7
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdds a new Directory.fromVCI helper that builds a Directory from the published VCI snapshot JSON, and extends tests to cover VCI fetching behavior while refactoring shared directory test data into a reusable fixture-style helper. Sequence diagram for Directory.fromVCI VCI snapshot download flowsequenceDiagram
actor Caller
participant Directory
participant FetchAPI
participant VciSnapshotService
Caller->>Directory: fromVCI()
Directory->>FetchAPI: fetch(vci_snapshot_url)
FetchAPI->>VciSnapshotService: HTTP GET /vci_snapshot.json
VciSnapshotService-->>FetchAPI: HTTP response
alt response status ok
FetchAPI-->>Directory: JSON body
Directory->>Directory: fromJSON(vciDirectoryJson)
Directory-->>Caller: Directory instance
else response status not ok
FetchAPI-->>Directory: non ok response
Directory-->>Caller: Error
end
Class diagram for Directory with new fromVCI helperclassDiagram
class Directory {
- issuerInfo
+ getIssuerInfo()
+ static fromJSON(directoryJson)
+ static fromVCI() Promise_Directory
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In the tests that monkey-patch
globalThis.fetch, consider wrapping the override in atry/finally(or using anafterEachhook) to ensurefetchis always restored even if the test fails, rather than relying on manual reassignment at the end of each test. - You may want to make the VCI snapshot URL configurable (e.g., an optional parameter to
fromVCIor a constant exported from the module), which would improve flexibility for different environments and simplify testing without hardcoding the URL in the method.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the tests that monkey-patch `globalThis.fetch`, consider wrapping the override in a `try/finally` (or using an `afterEach` hook) to ensure `fetch` is always restored even if the test fails, rather than relying on manual reassignment at the end of each test.
- You may want to make the VCI snapshot URL configurable (e.g., an optional parameter to `fromVCI` or a constant exported from the module), which would improve flexibility for different environments and simplify testing without hardcoding the URL in the method.
## Individual Comments
### Comment 1
<location> `src/shc/directory.ts:44-45` </location>
<code_context>
+ * const directory = await Directory.fromVCI()
+ */
+ static async fromVCI(): Promise<Directory> {
+ const vciSnapshotResponse = await fetch(
+ 'https://raw.githubusercontent.com/the-commons-project/vci-directory/main/logs/vci_snapshot.json'
+ )
+ if (!vciSnapshotResponse.ok) {
</code_context>
<issue_to_address>
**suggestion:** The hard-coded VCI snapshot URL reduces flexibility and testability.
Inlining the snapshot URL makes it harder to adapt if the endpoint changes, to point to other environments (staging/mirror), or to override in tests. Please extract this URL into a constant, configuration value, or optional parameter so callers can override it without code changes.
Suggested implementation:
```typescript
/**
* This helper fetches a well-known VCI snapshot JSON file and delegates to
* `Directory.fromJSON` to produce a `Directory` instance. If the snapshot
* cannot be retrieved (non-2xx response) the function throws an Error.
*
* @param snapshotUrl Optional URL to the VCI snapshot JSON. Defaults to the
* public GitHub-hosted snapshot but can be overridden for alternative
* environments or tests.
* @returns A {@link Directory} populated from the VCI snapshot
* @throws Error when the VCI snapshot HTTP fetch returns a non-OK status
* @example
* const directory = await Directory.fromVCI()
* @example
* const directory = await Directory.fromVCI('https://staging.example.com/vci_snapshot.json')
*/
static async fromVCI(
snapshotUrl: string = 'https://raw.githubusercontent.com/the-commons-project/vci-directory/main/logs/vci_snapshot.json'
): Promise<Directory> {
const vciSnapshotResponse = await fetch(snapshotUrl)
```
```typescript
if (!vciSnapshotResponse.ok) {
throw new Error(
`Failed to fetch VCI Directory snapshot with status ${vciSnapshotResponse.status}`
)
}
const vciDirectoryJson = await vciSnapshotResponse.json()
return Directory.fromJSON(vciDirectoryJson)
}
```
If there are existing tests or call sites that rely on `Directory.fromVCI()` without parameters, they will continue to work unchanged because the new `snapshotUrl` parameter has a default value. You may optionally:
1. Add new tests that call `Directory.fromVCI(customUrl)` to verify behavior against alternate endpoints.
2. If your project prefers centralized configuration over default parameters, you could also introduce a shared `DEFAULT_VCI_SNAPSHOT_URL` constant and reuse it in this method and elsewhere.
</issue_to_address>
### Comment 2
<location> `test/shc/directory.test.ts:113-122` </location>
<code_context>
+ it('should create a directory from the VCI snapshot', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add assertions that `Directory.fromVCI` calls `fetch` with the expected snapshot URL
This test only checks the happy-path behavior with the mocked response body; it doesn’t verify that `Directory.fromVCI` calls `fetch` with the correct snapshot URL or that it’s called once. To make the test more robust against URL or call-pattern regressions, add expectations like:
```ts
expect(fetchMock).toHaveBeenCalledTimes(1)
expect(fetchMock).toHaveBeenCalledWith(
'https://raw.githubusercontent.com/the-commons-project/vci-directory/main/logs/vci_snapshot.json'
)
```
Suggested implementation:
```typescript
it('should create a directory from the VCI snapshot', async () => {
const originalFetch = globalThis.fetch
const fetchMock = vi.fn().mockImplementation((url: string) => {
if (
url ===
'https://raw.githubusercontent.com/the-commons-project/vci-directory/main/logs/vci_snapshot.json'
) {
return Promise.resolve({
ok: true,
json: async () => SAMPLE_DIRECTORY_JSON,
})
}
return Promise.resolve({ ok: false, status: 404 })
```
To fully implement your suggestion, you should also, within this same test:
1. Assign the mock to `globalThis.fetch` before invoking the code under test:
```ts
globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch
```
2. After the `await Directory.fromVCI()` (or equivalent call), add:
```ts
expect(fetchMock).toHaveBeenCalledTimes(1)
expect(fetchMock).toHaveBeenCalledWith(
'https://raw.githubusercontent.com/the-commons-project/vci-directory/main/logs/vci_snapshot.json'
)
```
3. Restore `globalThis.fetch` to `originalFetch` at the end of the test (if not already done) to avoid leaking the mock into other tests:
```ts
globalThis.fetch = originalFetch
```
These additions will ensure the test asserts both the exact URL used for the VCI snapshot and that `fetch` is called exactly once.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
fjsj
reviewed
Dec 2, 2025
fjsj
approved these changes
Dec 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add support for building a Directory instance from the hosted VCI snapshot and extend tests to cover snapshot fetching and error handling.
New Features:
Tests:
Closes #4