feat(cli): add dynamic integration discovery#131
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesDynamic Integration Catalog & Search
Sequence DiagramsequenceDiagram
participant CLI as CLI User
participant Router as runIntegration
participant Loader as loadAvailableIntegrationCatalog
participant Cache as DiskCache
participant Fetcher as fetchIntegrationCatalogPath
participant HTTP as RemoteCatalogAPI
participant Filter as filterIntegrationCatalog
CLI->>Router: integration search <query>
Router->>Loader: loadAvailableIntegrationCatalog(refresh=false)
Loader->>Cache: check dynamic cache key
Cache-->>Loader: cache miss
Loader->>Fetcher: fetchIntegrationCatalogPath(dynamic=true)
Fetcher->>HTTP: GET /api/v1/integrations/catalog?dynamic=true
HTTP-->>Fetcher: 200 OK + catalog JSON
Fetcher->>Cache: write dynamic cache key
Fetcher-->>Loader: providers list
Loader-->>Filter: filterIntegrationCatalog(backend, query)
Filter-->>Router: matching entries
Router-->>CLI: display results (JSON or table)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/relayfile-cli/main.go (1)
837-895:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the fallback catalog without also failing the command.
This helper returns
fallbackIntegrationCatalog()and a non-nil error on transport/HTTP/JSON failures. Callers likerunIntegrationAvailableabort on that error, sointegration available/searchstill hard-fails instead of degrading to the built-in catalog. If fallback is intentional here, return a nil error when the fallback is usable, or split diagnostics from the returned error path.🤖 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 `@cmd/relayfile-cli/main.go` around lines 837 - 895, The helper fetchIntegrationCatalogPath currently returns fallbackIntegrationCatalog() together with non-nil errors (e.g., on url.Parse, http.NewRequestWithContext, http.Client.Do, io.ReadAll, json.Unmarshal, and the non-2xx apiError branch) which causes callers like runIntegrationAvailable to abort; change those paths to return the fallback catalog with a nil error when the fallback is usable instead of propagating the error (i.e., replace occurrences of "return fallbackIntegrationCatalog(), err" and the non-2xx "return fallbackIntegrationCatalog(), &apiError{...}" with "return fallbackIntegrationCatalog(), nil" so callers can degrade gracefully), while preserving normal error returns where no fallback exists; keep writeIntegrationCatalogCache and successful json.Unmarshal behavior unchanged.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@cmd/relayfile-cli/main.go`:
- Around line 1410-1415: The wrapper runIntegrationSearch assumes args[0] is the
query which breaks when callers pass flags before the positional (e.g.
--backend); update runIntegrationSearch to normalize/parse flags from args
before pulling the query and then reconstruct the arg list for
runIntegrationAvailable (i.e., parse flags like --backend and --json out of
args, identify the first non-flag token as QUERY, and call
runIntegrationAvailable with append([]string{"--search", QUERY},
remainingArgs...) so flags are preserved and the correct positional query is
used).
- Around line 397-398: Update the help/usage strings to advertise the --refresh
flag: add " [--refresh]" to the top-level usage line that currently shows
"relayfile integration available [--search QUERY] [--backend BACKEND] [--json]"
and also to the "relayfile integration search QUERY [--backend BACKEND]
[--json]" string so both "integration available" and "integration search"
mention --refresh; also apply the same change to the duplicate usage text around
the other occurrence referenced (lines showing the same two usage strings near
1412-1413) so the cache-bypass flag is discoverable everywhere.
---
Outside diff comments:
In `@cmd/relayfile-cli/main.go`:
- Around line 837-895: The helper fetchIntegrationCatalogPath currently returns
fallbackIntegrationCatalog() together with non-nil errors (e.g., on url.Parse,
http.NewRequestWithContext, http.Client.Do, io.ReadAll, json.Unmarshal, and the
non-2xx apiError branch) which causes callers like runIntegrationAvailable to
abort; change those paths to return the fallback catalog with a nil error when
the fallback is usable instead of propagating the error (i.e., replace
occurrences of "return fallbackIntegrationCatalog(), err" and the non-2xx
"return fallbackIntegrationCatalog(), &apiError{...}" with "return
fallbackIntegrationCatalog(), nil" so callers can degrade gracefully), while
preserving normal error returns where no fallback exists; keep
writeIntegrationCatalogCache and successful json.Unmarshal behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: efaea5a0-c96a-4093-9286-738c32af9581
📒 Files selected for processing (2)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.go
khaliqgant
force-pushed
the
codex/dynamic-integration-discovery
branch
from
dffd9c3 to
d371022
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| func loadAvailableIntegrationCatalog(cloudAPIURL string, refresh bool) ([]integrationCatalogEntry, error) { | ||
| cacheKey := dynamicIntegrationCatalogCacheKey(cloudAPIURL) | ||
| if !refresh { | ||
| if entry, ok := readIntegrationCatalogCache(); ok && cachedCatalogIsFresh(entry, cacheKey) { | ||
| return entry.Providers, nil | ||
| } | ||
| } | ||
| return fetchIntegrationCatalogPath(cloudAPIURL, "", true, cacheKey) | ||
| } |
There was a problem hiding this comment.
🟡 Dynamic and regular catalog caches share a single-entry file, mutually evicting each other
Both loadIntegrationCatalog and loadAvailableIntegrationCatalog read from and write to the same single-entry cache file (catalog-cache.json via readIntegrationCatalogCache/writeIntegrationCatalogCache). They use different APIURL values as cache keys (cloudAPIURL vs cloudAPIURL + "?dynamic=true"), so each write overwrites the other's cached entry. For example: a user runs relayfile integration available (stores dynamic entry) → then relayfile setup calls loadIntegrationCatalog which finds a key mismatch, refetches, and overwrites the dynamic cache. A subsequent integration available call will again miss the cache. The test TestIntegrationAvailableUsesDynamicCatalogCache only verifies two consecutive dynamic calls and never interleaves with loadIntegrationCatalog, so it doesn't catch this mutual eviction.
Prompt for agents
The functions loadIntegrationCatalog and loadAvailableIntegrationCatalog both use the same single-file cache (catalog-cache.json) via readIntegrationCatalogCache/writeIntegrationCatalogCache (cmd/relayfile-cli/main.go:774, 786). They store different APIURL values as discriminators but only one entry can exist in the file at a time, so writes from one function evict the other's cached data.
Possible fixes:
1. Use separate cache files (e.g. catalog-cache.json and catalog-dynamic-cache.json) so each catalog type has its own independent cache slot.
2. Change the cache format to store multiple entries keyed by APIURL, so both entries can coexist.
3. Accept the current behavior as a tradeoff if the alternating usage pattern is considered rare.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/relayfile-cli/main.go (1)
819-831: 💤 Low valueConsider separate cache storage for dynamic vs non-dynamic catalogs.
The dynamic catalog (
?dynamic=true) and the regular catalog share a single cache file but use different cache keys. Whenintegration availablewrites the dynamic catalog and thensetupruns (or vice versa), the cache key mismatch causes a fresh network fetch despite the data being recent.This isn't incorrect—both paths get valid data—but it means users who alternate between
setupandintegration available/searchwill experience unnecessary network requests.A straightforward fix would be separate cache files (e.g.,
catalog-cache.jsonandcatalog-cache-dynamic.json), or a multi-entry cache keyed by URL.🤖 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 `@cmd/relayfile-cli/main.go` around lines 819 - 831, The cache currently mixes dynamic and non-dynamic catalogs because loadAvailableIntegrationCatalog uses dynamicIntegrationCatalogCacheKey but readIntegrationCatalogCache stores/reads a single cache file; change the cache storage to be keyed by cacheKey (or use two files) so dynamic and regular catalogs are stored separately: update readIntegrationCatalogCache and its writer to accept a cacheKey (or to choose filename like catalog-cache-dynamic.json when dynamicIntegrationCatalogCacheKey(...) contains "?dynamic=true"), ensure cachedCatalogIsFresh still checks freshness per cacheKey, and keep fetchIntegrationCatalogPath calls passing the same cacheKey so reads/writes reference the same per-key cache entry.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@cmd/relayfile-cli/main.go`:
- Around line 819-831: The cache currently mixes dynamic and non-dynamic
catalogs because loadAvailableIntegrationCatalog uses
dynamicIntegrationCatalogCacheKey but readIntegrationCatalogCache stores/reads a
single cache file; change the cache storage to be keyed by cacheKey (or use two
files) so dynamic and regular catalogs are stored separately: update
readIntegrationCatalogCache and its writer to accept a cacheKey (or to choose
filename like catalog-cache-dynamic.json when
dynamicIntegrationCatalogCacheKey(...) contains "?dynamic=true"), ensure
cachedCatalogIsFresh still checks freshness per cacheKey, and keep
fetchIntegrationCatalogPath calls passing the same cacheKey so reads/writes
reference the same per-key cache entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 622efe87-edcf-414a-8fdf-7e425c2feec5
📒 Files selected for processing (2)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/relayfile-cli/main_test.go
khaliqgant
force-pushed
the
codex/dynamic-integration-discovery
branch
2 times, most recently
from
6af8eb7 to
1b3ed4c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@cmd/relayfile-cli/main.go`:
- Around line 823-830: loadAvailableIntegrationCatalog currently computes
cacheKey via dynamicIntegrationCatalogCacheKey but still reads/writes a single
on-disk file (catalog-cache.json) so dynamic and static catalogs overwrite each
other; change the persistence to be keyed by cacheKey (either by using one cache
file per cacheKey or by storing a map of cacheKey->entry) and update
readIntegrationCatalogCache and the writer used by fetchIntegrationCatalogPath
to accept/return the cacheKey-scoped entry; ensure cachedCatalogIsFresh
continues to receive the same cacheKey when validating freshness so reads and
writes are isolated per cacheKey.
- Around line 1456-1460: The backend filter is removing all fallback providers
because fallback entries lack backend/backends metadata; after calling
loadAvailableIntegrationCatalog (which may return fallbackIntegrationCatalog
entries), check the returned entries for any non-empty backend or backends
fields and only call filterIntegrationCatalog when at least one entry contains
backend metadata; otherwise skip the backend filtering so fallback entries are
preserved. Locate the entries variable returned by
loadAvailableIntegrationCatalog and modify the logic before calling
filterIntegrationCatalog to perform this metadata presence check and
conditionally apply filterIntegrationCatalog.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cece65f3-a517-4692-b163-4bf67aa6142e
📒 Files selected for processing (2)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.go
khaliqgant
force-pushed
the
codex/dynamic-integration-discovery
branch
from
1b3ed4c to
868088e
Compare
1 participant
Summary
relayfile integration availableandrelayfile integration search?dynamic=true, cache it for one hour, and support--refresh--searchand--backendso users can discover Nango providers and Composio toolkitsVerification
go test ./cmd/relayfile-cliNote:
package-lock.jsonwas already dirty in this worktree and is not part of this PR.