feat(cli): port recent opensrc features (fetch command, lockfile parsers, GITHUB_TOKEN auth)#117
Merged
Conversation
Pulls in 9 upstream commits (2026-04-14 .. 2026-06-23): - opensrc fetch subcommand + shared core::fetcher module (#53, #56) - Bitbucket Cloud support via BITBUCKET_TOKEN + auth docs (#52) - Lockfile parsers rewritten with transitive pnpm resolution (#51) - Structured error handling via thiserror across the CLI (#56) - Authenticated clone host validation fix (#66) - Trusted npm publishing, v0.7.2/v0.7.3 releases, docs favicon
…olution Port of vercel-labs/opensrc#51. The previous regex-based parsers misidentified versions in common real-world cases: yarn multi-specifier headers only matched the first specifier, pnpm peer-dep suffixes like 18.2.0(react@17.0.0) leaked false matches for the inner package, and pnpm monorepos returned whichever version the regex engine hit first. - pnpm: indent-aware stack parser covering v5 through v9 (importers, devDependencies, optionalDependencies, nested peer-dep suffixes) with BFS transitive resolution over the snapshots/packages dep graph - yarn: block-based parser handling classic v1 and Berry v2+ in one path - package.json fallback now skips protocol versions (workspace:*, link:, file:, git+..., npm: aliases) via isRegistryVersion - fixtures (yarn v1, yarn Berry, pnpm v9 monorepo) and 58 unit tests ported from upstream
Port of opensrc fetch (vercel-labs/opensrc#53/#56). ask fetch <spec...> resolves each spec through the shared ensureCheckout helper without printing paths — the prefetch counterpart to ask src / ask docs. - supports multiple specs, -q/--quiet, per-spec ✓ fetched / already cached lines and a summary; per-spec failures print to stderr and the command exits non-zero at the end - EnsureCheckoutResult gains a fromCache flag so callers can tell a store hit from a fresh fetch - warms the cache that ask add's offline-first docs-path prompt reads, removing the need to abuse ask docs for prefetching
…lidation Port of vercel-labs/opensrc#52 + #66 (scoped to GitHub — ASK's store layout is github.com-only for MVP, so the GitLab/Bitbucket token paths do not apply). - authenticatedCloneUrl parses the clone URL and injects x-access-token:<GITHUB_TOKEN> ONLY on an exact https://github.com host match; prefix-confusable hosts (github.com.evil.com, evilgithub.com, subdomains), non-https schemes, and ssh remotes pass through untouched (#66 host-confusion fix) - auth is applied at the exec boundary (clone, ls-remote); error messages keep the token-free URL and redactToken scrubs git/ execFileSync output so the secret never reaches logs or thrown errors - tar.gz fallback switches to the api.github.com tarball endpoint when a token is set: undici drops the Authorization header on the cross-origin redirect to codeload, but GitHub embeds a temporary token in the redirect Location, so private downloads still work
Machines with global tag.gpgsign=true failed createLocalRemote setup — git tag <name> becomes a signed annotated tag that requires a message. Pass -c tag.gpgsign=false / -c commit.gpgsign=false so the fixtures are hermetic regardless of the developer's global git config.
- README / SKILL.md: add ask fetch to the command tables, note the cache-warming use case and private-repo token support - CLAUDE.md: update the top-level command list, point cache-warming guidance at ask fetch, and add gotchas for the format-aware lockfile parsers and the exact-host token injection rule
dosubot
Bot
added
size:XL
📝 WalkthroughWalkthroughAdds a new Changesask fetch command and cache-hit reporting
Lockfile parser rewrite
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as fetchCmd
participant RunFetch as runFetch
participant EnsureCheckout as ensureCheckout
participant Github as GithubSource
CLI->>RunFetch: specs, quiet
loop each spec
RunFetch->>EnsureCheckout: ensureCheckout(spec)
alt cache miss
EnsureCheckout->>Github: fetch(spec)
Github-->>EnsureCheckout: fromStoreCache flag
end
EnsureCheckout-->>RunFetch: result.fromCache
RunFetch->>RunFetch: increment fetched/cached, log unless quiet
end
RunFetch-->>CLI: log summary, exit(1) if any spec failed
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 6 critical 2 high |
🟢 Metrics 246 complexity · 1 duplication
Metric Results Complexity 246 Duplication 1
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Deploying ask with
|
| Latest commit: |
14e5584
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f3b3a228.ask-6im.pages.dev |
| Branch Preview URL: | https://amondnet-opensrc.ask-6im.pages.dev |
Deploying ask-registry with
|
| Latest commit: |
14e5584
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://17141118.ask-registry.pages.dev |
| Branch Preview URL: | https://amondnet-opensrc.ask-registry.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 20 files
Architecture diagram
sequenceDiagram
participant User as CLI User
participant FetchCmd as fetchCmd (citty)
participant RunFetch as runFetch()
participant EnsureCheckout as ensureCheckout()
participant GithubSrc as GithubSource
participant Git as git process
participant Env as Environment
participant Lockfiles as npmEcosystemReader
participant PnpmParser as parsePnpmLock
participant YarnParser as parseYarnLock
participant Helpers as parse-helpers
Note over User,Git: Flow 1: ask fetch command
User->>FetchCmd: ask fetch -q github:owner/repo@v1.0.0 react
FetchCmd->>RunFetch: runFetch({ specs, projectDir, quiet: true })
loop for each spec
RunFetch->>EnsureCheckout: ensureCheckout({ spec, projectDir })
alt Spec not in ~/.ask store
EnsureCheckout->>GithubSrc: cloneAtTag(remoteUrl, candidate)
alt GITHUB_TOKEN set
GithubSrc->>GithubSrc: authenticatedCloneUrl(remoteUrl, token)
Note over GithubSrc: Parses URL, checks hostname === 'github.com'<br/>and protocol === 'https:'<br/>Rejects github.com.evil.com, evilgithub.com, subdomains
GithubSrc->>Git: execFileSync('git clone', authenticatedUrl)
Git-->>GithubSrc: stdout/stderr
GithubSrc->>GithubSrc: redactToken(output) → log/error safe
else no token
GithubSrc->>Git: execFileSync('git clone', remoteUrl)
end
Git-->>GithubSrc: result
GithubSrc->>EnsureCheckout: resolvedCheckoutDir
EnsureCheckout-->>RunFetch: { fromCache: false, ... }
else Cache hit (store exists)
EnsureCheckout-->>RunFetch: { fromCache: true, ... }
end
RunFetch->>RunFetch: Format display line
alt quiet mode
Note over RunFetch: No log lines (errors still to stderr)
else verbose
RunFetch->>RunFetch: log "✓ fetched" or "already cached"
end
end
RunFetch->>RunFetch: Print summary (X fetched, Y already cached)
alt any spec failed
RunFetch->>RunFetch: error to stderr per failure
RunFetch->>RunFetch: exit(1)
end
Note over User,PnpmParser: Flow 2: Lockfile parsing (npm entries)
User->>Lockfiles: ask install (npm spec)
Lockfiles->>Lockfiles: Try bun.lock → package-lock.json
Lockfiles->>PnpmParser: read pnpm-lock.yaml content
PnpmParser->>Helpers: isRegistryVersion, stripPeerSuffix, splitPkgSpec
alt Direct match in importers or top-level deps
PnpmParser-->>Lockfiles: version (e.g., "18.2.0")
else No direct match -> transitive BFS
PnpmParser->>PnpmParser: build dep graph from snapshots/packages
PnpmParser->>PnpmParser: BFS from root->roots to find reachable version
alt Reachable version exists
PnpmParser-->>Lockfiles: version
else Unreachable -> fallback to packages key
PnpmParser->>Helpers: isRegistryVersion check
PnpmParser-->>Lockfiles: version (or null)
end
end
alt Protocol version (workspace:*, link:, file:)
Helpers-->>PnpmParser: isRegistryVersion false -> skip
end
Lockfiles->>YarnParser: read yarn.lock content
YarnParser->>Helpers: cleanValue, stripPeerSuffix, etc.
YarnParser->>YarnParser: Split into blocks, parse header for specifier matching pkg
alt Block mentions pkg
YarnParser->>Helpers: isRegistryVersion check on version line
alt Valid registry version
YarnParser-->>Lockfiles: version
else Workspace sentinel or protocol -> skip, check next block
end
end
Lockfiles-->>User: exact version for install
Note over User,Env: Flow 3: Authenticated clone with host validation
User->>Env: export GITHUB_TOKEN=ghp_...
User->>Git: ask src github:private/repo
Git->>Git: probeRemoteTag(remoteUrl) -> authenticatedCloneUrl
alt exact https://github.com match
Git->>GithubSrc: authenticated URL used for ls-remote
GithubSrc-->>Env: token injected, host validated (no prefix confusion)
else any other host (e.g., gist.github.com, github.com.evil.com)
Git->>GithubSrc: original URL passed through unchanged
end
Git->>Git: shallowCloneRef -> authenticatedCloneUrl
Note over Git: token redacted from error messages by redactToken()
Git-->>GithubSrc: commit SHA, checkout dir
GithubSrc-->>User: path to cached source
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryThis PR ports three features from
Confidence Score: 5/5Safe to merge — all three feature areas are correctly implemented, all edge cases are covered by tests, and the security-sensitive auth path is well-hardened. The host-validation logic in No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant fetchCmd
participant runFetch
participant ensureCheckout
participant GithubSource
participant Store
User->>fetchCmd: ask fetch spec1 spec2
fetchCmd->>runFetch: "specs=[spec1,spec2], projectDir"
loop for each spec
runFetch->>ensureCheckout: "{spec, projectDir}"
ensureCheckout->>Store: fs.existsSync(checkoutDir)?
alt cache hit
Store-->>ensureCheckout: dir exists
ensureCheckout-->>runFetch: "{fromCache: true, ...}"
runFetch->>User: ✓ spec already cached
else cache miss
Store-->>ensureCheckout: miss
ensureCheckout->>GithubSource: fetch(opts)
alt git available
GithubSource->>GithubSource: authenticatedCloneUrl(remoteUrl, GITHUB_TOKEN)
GithubSource->>Store: shallowClone → cpDirAtomic → stampEntry
GithubSource-->>ensureCheckout: "{storePath, fromStoreCache?}"
else fallback
GithubSource->>GithubSource: build archiveUrl (api.github.com if token)
GithubSource->>GithubSource: "fetch(archiveUrl, {Authorization: Bearer TOKEN})"
GithubSource->>Store: tar xz → cpDirAtomic → stampEntry
GithubSource-->>ensureCheckout: "{storePath}"
end
ensureCheckout-->>runFetch: "{fromCache: fromStoreCache??false, ...}"
runFetch->>User: "✓ Fetched spec@ref"
end
end
runFetch->>User: summary (N fetched, M already cached)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant fetchCmd
participant runFetch
participant ensureCheckout
participant GithubSource
participant Store
User->>fetchCmd: ask fetch spec1 spec2
fetchCmd->>runFetch: "specs=[spec1,spec2], projectDir"
loop for each spec
runFetch->>ensureCheckout: "{spec, projectDir}"
ensureCheckout->>Store: fs.existsSync(checkoutDir)?
alt cache hit
Store-->>ensureCheckout: dir exists
ensureCheckout-->>runFetch: "{fromCache: true, ...}"
runFetch->>User: ✓ spec already cached
else cache miss
Store-->>ensureCheckout: miss
ensureCheckout->>GithubSource: fetch(opts)
alt git available
GithubSource->>GithubSource: authenticatedCloneUrl(remoteUrl, GITHUB_TOKEN)
GithubSource->>Store: shallowClone → cpDirAtomic → stampEntry
GithubSource-->>ensureCheckout: "{storePath, fromStoreCache?}"
else fallback
GithubSource->>GithubSource: build archiveUrl (api.github.com if token)
GithubSource->>GithubSource: "fetch(archiveUrl, {Authorization: Bearer TOKEN})"
GithubSource->>Store: tar xz → cpDirAtomic → stampEntry
GithubSource-->>ensureCheckout: "{storePath}"
end
ensureCheckout-->>runFetch: "{fromCache: fromStoreCache??false, ...}"
runFetch->>User: "✓ Fetched spec@ref"
end
end
runFetch->>User: summary (N fetched, M already cached)
Reviews (5): Last reviewed commit: "fix: restore null-safe access to the inj..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- pnpm parser: support v5 /name/version keys in the packages:/snapshots: pass with a last-slash split (scoped names intact, _peerhash stripped) and canonical name@version node keys so v5 fallback and transitive resolution connect (cubic review) - ensure-checkout: propagate GithubSource's store-hit via a new FetchResult.fromStoreCache flag so fromCache is not misreported as a fresh fetch when the source found a ref-candidate cache entry with zero network I/O (cubic review)
Contributor
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: Ports major features like format-aware lockfile parsers (pnpm/yarn) and a new fetch command, plus GitHub token authentication. These changes touch core logic and auth paths, carrying high risk and warrant human review.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/cli/src/commands/fetch.ts (1)
29-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting per-spec handling/summary logic.
SonarCloud flags this function's cognitive complexity (25 vs. the 15 allowed threshold). Splitting the per-spec try/catch body and the summary-string composition into small helpers (e.g.
processSpec,buildSummary) would make each piece independently testable and bring complexity down without behavior changes.🤖 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 `@packages/cli/src/commands/fetch.ts` around lines 29 - 74, The runFetch function is too complex because it mixes per-spec checkout handling, error capture, and summary composition in one loop. Refactor the logic in runFetch by extracting the try/catch spec-processing block into a helper such as processSpec, and move the fetched/cached summary string building into a helper such as buildSummary; keep behavior unchanged while using the existing ensureCheckout, log, error, and exit dependencies.Source: Linters/SAST tools
CLAUDE.md (1)
95-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix markdownlint MD038 (space inside code span).
markdownlint flags a code span with extraneous internal spacing around line 96.
🤖 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 `@CLAUDE.md` around lines 95 - 96, The markdownlint MD038 issue comes from an inline code span in the CLAUDE.md guidance text having extra spaces inside the backticks. Update the affected sentence in that release/publish guidance so the inline code token is written without leading or trailing spaces, keeping the wording unchanged otherwise.Source: Linters/SAST tools
packages/cli/src/sources/github.ts (2)
462-477: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the nested ternary for readability.
isTagCandidate ? 'tags' : 'heads'nested inside thetoken ? ... : ...ternary is harder to scan.♻️ Suggested extraction
- const token = githubToken() - const archiveUrl = token - ? `https://api.github.com/repos/${repo}/tarball/${candidate}` - : `https://github.com/${repo}/archive/refs/${isTagCandidate ? 'tags' : 'heads'}/${candidate}.tar.gz` + const token = githubToken() + const refKind = isTagCandidate ? 'tags' : 'heads' + const archiveUrl = token + ? `https://api.github.com/repos/${repo}/tarball/${candidate}` + : `https://github.com/${repo}/archive/refs/${refKind}/${candidate}.tar.gz`🤖 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 `@packages/cli/src/sources/github.ts` around lines 462 - 477, The archive URL construction in github source is harder to read because the isTagCandidate branch is nested inside the token ternary. Refactor the archiveUrl assignment in github.ts by extracting the path segment choice into a separate named value or small helper around the fetch/archiveUrl logic, so the token-based URL selection and the tags/heads selection are not nested together.Source: Linters/SAST tools
85-93: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
redactTokenrelies on literal string matching; percent-encoded credentials would slip through.
parsed.password = tokeninauthenticatedCloneUrlgets percent-encoded byURL.toString()for any userinfo-unsafe characters. If a token ever contains such characters, the string embedded in a clone URL (and thus inerr.message) would differ from the rawtoken, andmsg.split(token).join('***')would fail to redact it. Current GitHub token formats (ghp_,github_pat_, fine-grained PATs) are alphanumeric/underscore only, so this is low risk today.🛡️ Defensive option
function redactToken(msg: string, token: string | undefined = githubToken()): string { - return token ? msg.split(token).join('***') : msg + if (!token) + return msg + const encoded = encodeURIComponent(token) + return msg.split(token).join('***').split(encoded).join('***') }🤖 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 `@packages/cli/src/sources/github.ts` around lines 85 - 93, The redaction in redactionToken only matches the raw token string, so it can miss percent-encoded credentials that appear in authenticatedCloneUrl output or err.message. Update redactionToken to also handle the encoded form of githubToken() by comparing against the URL-encoded token before replacing, and keep the existing callers in github.ts using the same helper so all logged or thrown messages are scrubbed consistently.packages/cli/src/lockfiles/yarn.ts (1)
29-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReduce cognitive complexity of
parseYarnLock.SonarCloud reports cognitive complexity at 44 vs. an allowed 15 and surfaces it as a failing check. The block-splitting/header-matching/version-extraction logic is correct (verified against v1, Berry, multi-specifier,
__metadata-skip, and CRLF test cases), but combining block splitting, header parsing, and version-line scanning in one function makes it hard to follow.Consider extracting
splitIntoBlocks(text),parseBlockHeader(block)(returns header/body), andmatchesPackage(headerBody, pkg)as separate helpers to bring this under the complexity threshold.🤖 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 `@packages/cli/src/lockfiles/yarn.ts` around lines 29 - 106, Refactor parseYarnLock to reduce its cognitive complexity while preserving the current behavior for v1, Berry, multi-specifier headers, __metadata skipping, and CRLF handling. Extract the block-building logic into splitIntoBlocks(text), move header/body detection into parseBlockHeader(block), and isolate package matching into matchesPackage(headerBody, pkg) so parseYarnLock only orchestrates these helpers and scans for the version value. Keep the version extraction and registry-version filtering behavior unchanged.Source: Linters/SAST tools
packages/cli/src/lockfiles/pnpm.ts (1)
153-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-null assertions flagged as forbidden by static analysis.
stack.at(-1)!(lines 154, 162) andqueue.shift()!(line 274) are functionally safe — the comment at lines 151-152 correctly explains the root frame never pops andqueue.length > 0is checked beforeshift()— but the lint rule explicitly flags these as "Forbidden non-null assertion," which suggests the project's ESLint config disallows!. This could fail CI lint checks.Consider a small typed helper to centralize the assertion with an explicit runtime check instead of
!:♻️ Suggested helper
+function peek<T>(items: readonly T[]): T { + const last = items[items.length - 1] + if (last === undefined) + throw new Error('unreachable: stack/queue must not be empty here') + return last +}Then replace
stack.at(-1)!withpeek(stack)andqueue.shift()!with a similar pattern (or keepshift()and usepeek+spliceif a non-mutating check is preferred).Also applies to: 274-274
🤖 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 `@packages/cli/src/lockfiles/pnpm.ts` around lines 153 - 162, The lockfile parser uses forbidden non-null assertions in the pnpm helper, which will trip ESLint/CI. In the relevant parsing logic in the pnpm lockfile code, replace `stack.at(-1)!` and `queue.shift()!` with a small typed helper that performs an explicit runtime check before returning the value. Use the helper at the `stack` access points and in the `queue` consumption path so the behavior stays the same without any `!` assertions.Source: Linters/SAST tools
🤖 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 `@packages/cli/src/lockfiles/pnpm.ts`:
- Around line 106-259: `parsePnpmLock` is failing the cognitive complexity gate
because its entire line-by-line state machine is implemented in one large
switch. Refactor the parsing flow in `parsePnpmLock` by extracting the
`root`/`importers`/`importer` handling, the `depGroup`/`depBlock` handling, and
the `packages`/`snapshots`/`pkgEntry`/`pkgDeps` handling into small named
helpers that operate on the shared `stack` and `graph`. Keep the return logic
and behavior unchanged, but move each case branch’s parsing rules into helpers
so the main function becomes a thin dispatcher.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 95-96: The markdownlint MD038 issue comes from an inline code span
in the CLAUDE.md guidance text having extra spaces inside the backticks. Update
the affected sentence in that release/publish guidance so the inline code token
is written without leading or trailing spaces, keeping the wording unchanged
otherwise.
In `@packages/cli/src/commands/fetch.ts`:
- Around line 29-74: The runFetch function is too complex because it mixes
per-spec checkout handling, error capture, and summary composition in one loop.
Refactor the logic in runFetch by extracting the try/catch spec-processing block
into a helper such as processSpec, and move the fetched/cached summary string
building into a helper such as buildSummary; keep behavior unchanged while using
the existing ensureCheckout, log, error, and exit dependencies.
In `@packages/cli/src/lockfiles/pnpm.ts`:
- Around line 153-162: The lockfile parser uses forbidden non-null assertions in
the pnpm helper, which will trip ESLint/CI. In the relevant parsing logic in the
pnpm lockfile code, replace `stack.at(-1)!` and `queue.shift()!` with a small
typed helper that performs an explicit runtime check before returning the value.
Use the helper at the `stack` access points and in the `queue` consumption path
so the behavior stays the same without any `!` assertions.
In `@packages/cli/src/lockfiles/yarn.ts`:
- Around line 29-106: Refactor parseYarnLock to reduce its cognitive complexity
while preserving the current behavior for v1, Berry, multi-specifier headers,
__metadata skipping, and CRLF handling. Extract the block-building logic into
splitIntoBlocks(text), move header/body detection into parseBlockHeader(block),
and isolate package matching into matchesPackage(headerBody, pkg) so
parseYarnLock only orchestrates these helpers and scans for the version value.
Keep the version extraction and registry-version filtering behavior unchanged.
In `@packages/cli/src/sources/github.ts`:
- Around line 462-477: The archive URL construction in github source is harder
to read because the isTagCandidate branch is nested inside the token ternary.
Refactor the archiveUrl assignment in github.ts by extracting the path segment
choice into a separate named value or small helper around the fetch/archiveUrl
logic, so the token-based URL selection and the tags/heads selection are not
nested together.
- Around line 85-93: The redaction in redactionToken only matches the raw token
string, so it can miss percent-encoded credentials that appear in
authenticatedCloneUrl output or err.message. Update redactionToken to also
handle the encoded form of githubToken() by comparing against the URL-encoded
token before replacing, and keep the existing callers in github.ts using the
same helper so all logged or thrown messages are scrubbed consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a307ae7-539e-4c60-8d42-3f4e1091b2e1
⛔ Files ignored due to path filters (2)
packages/cli/test/lockfiles/fixtures/yarn-berry.lockis excluded by!**/*.lockpackages/cli/test/lockfiles/fixtures/yarn-v1.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
CLAUDE.mdREADME.mdpackages/cli/src/commands/ensure-checkout.tspackages/cli/src/commands/fetch.tspackages/cli/src/index.tspackages/cli/src/lockfiles/package-json.tspackages/cli/src/lockfiles/parse-helpers.tspackages/cli/src/lockfiles/pnpm.tspackages/cli/src/lockfiles/yarn.tspackages/cli/src/sources/github.tspackages/cli/src/sources/index.tspackages/cli/test/commands/ensure-checkout.test.tspackages/cli/test/commands/fetch.test.tspackages/cli/test/lockfiles/fixtures/pnpm-v9-workspace.yamlpackages/cli/test/lockfiles/parsers.test.tspackages/cli/test/sources/github-auth.test.tspackages/cli/test/sources/github-monorepo.test.tspackages/cli/test/sources/github.test.tsskills/ask/SKILL.mdvendor/opensrc
Addresses the SonarCloud cognitive-complexity finding (85 > 15) raised via CodeRabbit review. The line loop now delegates to one small handler per frame kind (root/importers/importer/depGroup/depBlock/packages/ pkgEntry/pkgDeps) over a shared ParseState, with the root scope represented by an empty stack instead of a sentinel frame. Also clears the Codacy warnings on the same function: no while(true), no non-null assertions (currentFrame returns Frame | undefined; resolveTransitive uses index-pointer BFS instead of queue.shift()!). Behavior unchanged — covered by the 84 lockfile parser tests.
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Ports upstream opensrc features: rewritten pnpm/yarn lockfile parsers with transitive resolution, new ask fetch command, and GITHUB_TOKEN auth with host validation. All tests pass.
Re-trigger cubic
fetcher.fetch() returns a non-nullable FetchResult, so the ?. on fromStoreCache access was dead — flagged by Codacy.
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Ports new fetch command, format-aware lockfile parsers, and GITHUB_TOKEN auth—these changes affect core dependency resolution and security, requiring human review.
Re-trigger cubic
bec57a0 dropped the optional chain because FetchResult was declared non-nullable, but the ensureCheckout test seam legitimately simulates a successful fetch by materializing the checkout dir without returning a FetchResult — cache-sharing.test.ts crashed in CI. Widen the seam type to Promise<FetchResult | undefined> so the type states the real contract and the ?. is justified rather than dead.
|
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Core logic changes to lockfile parsers and auth handling affect critical dependency resolution and security; higher risk than low-impact changes.
Re-trigger cubic
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Ports the features introduced upstream in vercel-labs/opensrc between our previous submodule pin (
a3b9d21, 2026-04-10) and v0.7.3 (f96078a) into the ASK CLI, and bumps thevendor/opensrcsubmodule to that tag.1. Format-aware pnpm/yarn lockfile parsers (port of vercel-labs/opensrc#51)
Replaces the regex-based parsers in
packages/cli/src/lockfiles/, which misidentified versions in common real-world cases: pnpm peer-dep suffixes like18.2.0(react@17.0.0)leaked false matches for the inner package, yarn multi-specifier headers only matched the first specifier, and pnpm monorepos returned whichever version the regex hit first.snapshots:/packages:dependency graph — unreachable versions lose to root-reachable onesworkspace:*,link:,file:,git+…,npm:aliases) viaisRegistryVersion2.
ask fetchcommand (port of vercel-labs/opensrc#53/#56)New top-level command that warms the checkout cache via the shared
ensureCheckouthelper without printing paths — the prefetch counterpart toask src/ask docs.-q/--quiet, per-spec✓ fetched/already cachedlines + summaryEnsureCheckoutResultgains afromCacheflagask docs <spec>just to warm the cache beforeask add's offline-first docs-path prompt3. GITHUB_TOKEN authenticated fetch with strict host validation (port of vercel-labs/opensrc#52 + #66)
authenticatedCloneUrlparses the clone URL and injectsx-access-token:<GITHUB_TOKEN>only on an exacthttps://github.comhost match — prefix-confusable hosts (github.com.evil.com,evilgithub.com, subdomains), non-https schemes, and ssh remotes pass through untouched (Resolve pre-existing tsc errors blocking bun run build in packages/cli #66 host-confusion fix)redactTokenscrubs git/execFileSync output so the secret never reaches logs or thrown errors (hardening beyond upstream)api.github.comtarball endpoint when a token is set: undici drops theAuthorizationheader on the cross-origin redirect to codeload, but GitHub embeds a temporary token in the redirectLocation, so private downloads still workIntentionally not ported
exitcallbacksAlso included
test(cli): git test fixtures now pass-c tag.gpgsign=false/-c commit.gpgsign=falseso they are hermetic on machines with global gpg signing enabled (21 pre-existing local failures)docs: README,skills/ask/SKILL.md, and CLAUDE.md updated for the new command and token supportchore:vendor/opensrcsubmodule bumped to v0.7.3Test plan
bun run --cwd packages/cli test— 621 pass / 0 failbun run --cwd packages/cli lint/build— cleanask fetch github:sindresorhus/is-plain-obj@v4.1.0(fresh fetch → cache hit →-q),ask fetch <bogus>exits 1 with stderr errorauthenticatedCloneUrlunit tests cover exact-host match, host-prefix/suffix confusion, subdomains, non-https schemes, ssh remotesSummary by cubic
Ports recent
opensrcfeatures into the ASK CLI: addsask fetch, rewritespnpm/yarnlockfile parsers (incl.pnpmv5 keys), enables private GitHub fetch viaGITHUB_TOKEN, improves cache‑hit reporting, and bumpsvendor/opensrcto v0.7.3.New Features
ask fetch <spec...>warms the source cache; supports multiple specs,-q/--quiet, per‑spec errors, non‑zero exit on any failure; reports “fetched” vs “already cached.”pnpmv5–v9 with transitive resolution (v5/name/version[_peerhash]keys canonicalized), andyarnv1 + Berry;package.jsonfallback skips protocol versions viaisRegistryVersion.GITHUB_TOKENonly for exacthttps://github.comremotes; tokens redacted; API tarballs used when a token is set. Cache hits are reported accurately viaEnsureCheckoutResult.fromCacheandFetchResult.fromStoreCache.Bug Fixes
ensure-checkout: null‑safe access to injected fetcher results (fetcher.fetch(): Promise<FetchResult | undefined>), keeping cache‑hit reporting correct when tests/materialization bypass aFetchResult.Written for commit 14e5584. Summary will update on new commits.
Summary by CodeRabbit
New Features
ask fetchto warm the cache for specs without printing paths.GITHUB_TOKENis set.Bug Fixes
Documentation
askworkflow and cache behavior.