Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions crates/ask/src/commands/ensure_checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,22 @@ pub fn ensure_checkout(
.and_then(|r| r.store_path.clone())
.unwrap_or_else(|| checkout_dir.clone());

// Keep `ref` consistent with the actual checkout dir (its basename), so the
// `ask src --json` contract never reports a ref that disagrees with the path.
// Keep `ref` consistent with the actual checkout dir. Prefer the
// fetcher's `meta.ref` (the real winning ref) over the checkout's
// basename — slash-containing tags like `@tanstack/react-query@5.101.2`
// are encoded (`/` → `__`) in the directory name, so the basename is
// only a fallback for fetchers that do not report `meta.ref`.
let actual_ref = if resolved_checkout_dir == checkout_dir {
reference
} else {
resolved_checkout_dir
.file_name()
.map(|n| n.to_string_lossy().into_owned())
fetch_result
.as_ref()
.and_then(|r| r.meta.ref_.clone())
.or_else(|| {
resolved_checkout_dir
.file_name()
.map(|n| n.to_string_lossy().into_owned())
})
.unwrap_or(reference)
};

Expand Down
65 changes: 56 additions & 9 deletions crates/ask/src/resolvers/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,22 @@ pub fn resolve(
})?;

// Monorepo packages (repository.directory present) use changesets-style
// `<pkgName>@<version>` / `@v<version>` tags; scoped names use the unscoped
// part (`@vercel/ai` → `ai`).
// `<pkgName>@<version>` / `@v<version>` tags. Scoped monorepos
// (`@tanstack/*`, `@trpc/*`, ...) tag with the FULL scoped name
// (`@tanstack/react-query@5.101.2`) — try that first, then the unscoped
// part (`react-query@5.101.2`) used by repos like `vercel/ai`.
let mut fallback_refs = Vec::new();
if repo_field.and_then(RepoField::directory).is_some() {
let unscoped = if let Some(rest) = name.strip_prefix('@') {
rest.split_once('/').map(|(_, n)| n).unwrap_or(rest)
if let Some(rest) = name.strip_prefix('@') {
fallback_refs.push(format!("{name}@{resolved_version}"));
fallback_refs.push(format!("{name}@v{resolved_version}"));
let unscoped = rest.split_once('/').map(|(_, n)| n).unwrap_or(rest);
fallback_refs.push(format!("{unscoped}@{resolved_version}"));
fallback_refs.push(format!("{unscoped}@v{resolved_version}"));
} else {
name
};
fallback_refs.push(format!("{unscoped}@{resolved_version}"));
fallback_refs.push(format!("{unscoped}@v{resolved_version}"));
fallback_refs.push(format!("{name}@{resolved_version}"));
fallback_refs.push(format!("{name}@v{resolved_version}"));
}
}
fallback_refs.push(resolved_version.clone());

Expand Down Expand Up @@ -205,7 +210,49 @@ mod tests {
let c = MockClient::new().with("https://registry.npmjs.org/@vercel/ai", 200, meta);
let r = resolve(&c, "@vercel/ai", "latest").unwrap();
assert_eq!(r.repo, "vercel/ai");
assert_eq!(r.fallback_refs, vec!["ai@5.0.0", "ai@v5.0.0", "5.0.0"]);
assert_eq!(
r.fallback_refs,
vec![
"@vercel/ai@5.0.0",
"@vercel/ai@v5.0.0",
"ai@5.0.0",
"ai@v5.0.0",
"5.0.0"
]
);
}

#[test]
fn monorepo_unscoped_package_has_no_scoped_fallbacks() {
let meta = r#"{
"dist-tags": { "latest": "6.0.158" },
"versions": { "6.0.158": {} },
"repository": { "url": "https://github.com/vercel/ai.git", "directory": "packages/ai" }
}"#;
let c = MockClient::new().with("https://registry.npmjs.org/ai", 200, meta);
let r = resolve(&c, "ai", "latest").unwrap();
assert_eq!(
r.fallback_refs,
vec!["ai@6.0.158", "ai@v6.0.158", "6.0.158"]
);
}

#[test]
fn scoped_monorepo_tag_candidate_matches_tanstack_convention() {
// pleaseai/ask#121: the real tag on TanStack/query is
// `@tanstack/react-query@5.101.2` (full scoped name, contains `/`).
let meta = r#"{
"dist-tags": { "latest": "5.101.2" },
"versions": { "5.101.2": {} },
"repository": { "url": "https://github.com/TanStack/query.git", "directory": "packages/react-query" }
}"#;
let c = MockClient::new().with(
"https://registry.npmjs.org/@tanstack/react-query",
200,
meta,
);
let r = resolve(&c, "@tanstack/react-query", "latest").unwrap();
assert_eq!(r.fallback_refs[0], "@tanstack/react-query@5.101.2");
}

#[test]
Expand Down
52 changes: 51 additions & 1 deletion crates/ask/src/sources/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,12 @@ pub fn fetch(
store_path: Some(store_dir),
store_subpath: opts.docs_path.clone(),
from_store_cache: true,
// Report the WINNING candidate, not the requested ref — the
// store dir is keyed by the candidate, and slash-containing
// tags are encoded in the path, so callers cannot recover
// the real ref from the directory basename.
meta: FetchMeta {
ref_: Some(reference.clone()),
ref_: Some(candidate),
..Default::default()
},
});
Expand Down Expand Up @@ -752,6 +756,52 @@ mod tests {
);
}

#[test]
fn scoped_monorepo_tag_clones_into_encoded_store_dir() {
// pleaseai/ask#121: tags like `@tanstack/react-query@5.101.2` contain
// `/` — the clone must succeed and land under the encoded dir name,
// while meta.ref reports the REAL tag.
if !has_git() {
return;
}
let tmp = tempfile::tempdir().unwrap();
let remote = create_local_remote(tmp.path());
let scoped_tag = "@tanstack/react-query@5.101.2";
git(&["tag", scoped_tag, "v1.0.0"], &tmp.path().join("work"));
// Re-push the new tag into the bare remote.
git(
&["push", remote.to_str().unwrap(), scoped_tag],
&tmp.path().join("work"),
);
let ask_home = tmp.path().join("ask-home");
let opts = GithubOptions {
name: "react-query".into(),
version: "5.101.2".into(),
repo: "TanStack/query".into(),
tag: Some("v5.101.2".into()),
fallback_refs: vec![scoped_tag.to_string()],
docs_path: Some("docs".into()),
remote_url: Some(remote.to_string_lossy().into_owned()),
..Default::default()
};
let result = fetch(&no_client(), &opts, &ask_home).unwrap();

let store_path = result.store_path.clone().unwrap();
assert_eq!(
store_path.file_name().unwrap().to_str().unwrap(),
"@tanstack__react-query@5.101.2-8cd04c22"
);
assert_eq!(result.meta.ref_.as_deref(), Some(scoped_tag));
assert!(store_path.join("docs/guide.md").exists());

// Second fetch: store-cache hit on the same encoded path, still
// reporting the real tag.
let cached = fetch(&no_client(), &opts, &ask_home).unwrap();
assert!(cached.from_store_cache);
assert_eq!(cached.store_path.as_deref(), Some(store_path.as_path()));
assert_eq!(cached.meta.ref_.as_deref(), Some(scoped_tag));
}

#[test]
fn second_fetch_hits_the_store_cache() {
if !has_git() {
Expand Down
73 changes: 71 additions & 2 deletions crates/ask/src/store/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,29 @@ pub fn npm_store_path(ask_home: &Path, pkg: &str, version: &str) -> anyhow::Resu
assert_contained(ask_home, &candidate)
}

/// Encode a git ref for use as a single store-path segment. Git refs may
/// legitimately contain `/` — scoped monorepo tags like
/// `@tanstack/react-query@5.101.2` and branch names like `release/v1.2.3` —
/// but the store layout maps each ref to exactly one directory name.
/// Separators become `__` (same convention as the skills spec-key encoding)
/// instead of being rejected; `..` and other traversal inputs are still
/// rejected by [`assert_safe_segment`] after encoding.
///
/// The `__` substitution alone is not injective (`release/v1.0.0` and a
/// literal `release__v1.0.0` tag would share a cache dir, silently shadowing
/// each other), so separator-containing refs also get a short hash of the
/// ORIGINAL ref appended. Refs without separators — the overwhelmingly
/// common case — map to themselves unchanged.
pub fn encode_ref_segment(reference: &str) -> String {
if !reference.contains(['/', '\\']) {
return reference.to_string();
}
let mut hasher = Sha256::new();
hasher.update(reference.as_bytes());
let hash = format!("{:x}", hasher.finalize());
format!("{}-{}", reference.replace(['/', '\\'], "__"), &hash[..8])
}

/// The PM-style nested layout for a github entry:
/// `<askHome>/github/<host>/<owner>/<repo>/<tag>/`.
pub fn github_store_path(
Expand All @@ -107,9 +130,16 @@ pub fn github_store_path(
assert_safe_segment("host", host)?;
assert_safe_segment("owner", owner)?;
assert_safe_segment("repo", repo)?;
assert_safe_segment("tag", tag)?;
// Refs may contain `/` (scoped monorepo tags, `release/*` branches) —
// encode to a single directory name, then run the traversal guard.
let encoded_tag = encode_ref_segment(tag);
assert_safe_segment("tag", &encoded_tag)?;
let github_root = ask_home.join("github");
let candidate = github_root.join(host).join(owner).join(repo).join(tag);
let candidate = github_root
.join(host)
.join(owner)
.join(repo)
.join(&encoded_tag);
assert_contained(&github_root, &candidate)
}

Expand Down Expand Up @@ -194,6 +224,45 @@ mod tests {
assert!(github_store_path(home, "github.com", "o", "repo", "").is_err());
}

#[test]
fn github_path_encodes_slash_containing_refs() {
// pleaseai/ask#121: scoped monorepo tags and `release/*` branches
// contain `/` — encoded to `__` + 8-char sha256 of the original ref.
let home = Path::new("/store");
assert_eq!(
github_store_path(
home,
"github.com",
"TanStack",
"query",
"@tanstack/react-query@5.101.2"
)
.unwrap(),
PathBuf::from(
"/store/github/github.com/TanStack/query/@tanstack__react-query@5.101.2-8cd04c22"
)
);
assert_eq!(
github_store_path(home, "github.com", "o", "repo", "release/v1.2.3").unwrap(),
PathBuf::from("/store/github/github.com/o/repo/release__v1.2.3-3101f387")
);
// Traversal hidden inside a slash-containing ref is still rejected.
assert!(github_store_path(home, "github.com", "o", "repo", "@scope/../../escape").is_err());
}

#[test]
fn encode_ref_segment_is_injective_for_underscore_twins() {
// A literal `release__v1.2.3` tag (no separator) maps to itself;
// `release/v1.2.3` carries a hash suffix — never the same dir.
assert_eq!(encode_ref_segment("release__v1.2.3"), "release__v1.2.3");
assert_ne!(
encode_ref_segment("release__v1.2.3"),
encode_ref_segment("release/v1.2.3")
);
// Plain tags are unchanged.
assert_eq!(encode_ref_segment("v1.0.0"), "v1.0.0");
}

#[test]
fn assert_contained_blocks_escape() {
let parent = Path::new("/store/github");
Expand Down
14 changes: 8 additions & 6 deletions packages/cli/src/commands/ensure-checkout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,14 @@ export async function ensureCheckout(
const resolvedCheckoutDir = fetchResult?.storePath ?? checkoutDir

// When a fallbackRef (or a `v`-rescued ref) wins, the store dir lives
// under the WINNING ref, not the requested one. The store layout ends
// in `.../<owner>/<repo>/<ref>/`, so the actual ref is the checkout's
// basename. Keep `ref` consistent with `checkoutDir` — otherwise the
// `ask src --json` contract would report a `ref` that disagrees with
// the path downstream tools actually index.
const actualRef = resolvedCheckoutDir === checkoutDir ? ref : path.basename(resolvedCheckoutDir)
// under the WINNING ref, not the requested one. Prefer the fetcher's
// `meta.ref` (the real winning ref) over the checkout's basename —
// slash-containing tags like `@tanstack/react-query@5.101.2` are
// encoded (`/` → `__`) in the directory name, so the basename is only
// a fallback for fetchers that do not report `meta.ref`.
const actualRef = resolvedCheckoutDir === checkoutDir
? ref
: fetchResult?.meta?.ref ?? path.basename(resolvedCheckoutDir)

// `GithubSource.fetch` can satisfy the request from its own store-hit
// path (a ref-candidate variant like `v<ref>` or `master`) with zero
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/resolvers/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,14 @@ export class NpmResolver implements EcosystemResolver {

// For monorepo packages (those with repository.directory), prepend pkg-name@version tags.
// Changesets convention uses `<pkgName>@<version>` and `<pkgName>@v<version>`.
// Scoped packages like `@vercel/ai` use only the unscoped part (e.g. `ai`).
// Scoped monorepos (`@tanstack/*`, `@trpc/*`, ...) tag with the FULL scoped
// name (`@tanstack/react-query@5.101.2`) — try that first, then the
// unscoped part (`react-query@5.101.2`) used by repos like `vercel/ai`.
const monorepoFallbacks: string[] = []
if (typeof repoField === 'object' && repoField?.directory) {
if (name.startsWith('@')) {
monorepoFallbacks.push(`${name}@${resolvedVersion}`, `${name}@v${resolvedVersion}`)
}
const unscopedName = name.startsWith('@') ? name.split('/')[1] : name
monorepoFallbacks.push(`${unscopedName}@${resolvedVersion}`, `${unscopedName}@v${resolvedVersion}`)
}
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/sources/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,11 @@ export class GithubSource implements DocSource {
storePath: storeDir,
storeSubpath: docsPath,
fromStoreCache: true,
meta: { ref },
// Report the WINNING candidate, not the requested ref — the
// store dir is keyed by the candidate, and slash-containing
// tags are encoded in the path, so callers cannot recover the
// real ref from the directory basename.
meta: { ref: candidate },
}
}
quarantineEntry(askHome, storeDir)
Expand Down
31 changes: 29 additions & 2 deletions packages/cli/src/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,30 @@ function assertSafeSegment(name: string, value: string): void {
}
}

const RE_PATH_SEPARATORS = /[/\\]/g

/**
* Encode a git ref for use as a single store-path segment. Git refs may
* legitimately contain `/` — scoped monorepo tags like
* `@tanstack/react-query@5.101.2` and branch names like `release/v1.2.3`
* — but the store layout maps each ref to exactly one directory name.
* Separators become `__` (same convention as the skills spec-key
* encoding) instead of being rejected; `..` and other traversal inputs
* are still rejected by `assertSafeSegment` after encoding.
*
* The `__` substitution alone is not injective (`release/v1.0.0` and a
* literal `release__v1.0.0` tag would share a cache dir, silently
* shadowing each other), so separator-containing refs also get a short
* hash of the ORIGINAL ref appended. Refs without separators — the
* overwhelmingly common case — map to themselves unchanged.
*/
export function encodeRefSegment(ref: string): string {
if (!ref.includes('/') && !ref.includes('\\'))
return ref
const hash = crypto.createHash('sha256').update(ref).digest('hex').slice(0, 8)
return `${ref.replace(RE_PATH_SEPARATORS, '__')}-${hash}`
}
Comment thread
amondnet marked this conversation as resolved.

export function githubStorePath(
askHome: string,
host: string,
Expand All @@ -105,9 +129,12 @@ export function githubStorePath(
assertSafeSegment('host', host)
assertSafeSegment('owner', owner)
assertSafeSegment('repo', repo)
assertSafeSegment('tag', tag)
// Refs may contain `/` (scoped monorepo tags, `release/*` branches) —
// encode to a single directory name, then run the traversal guard.
const encodedTag = encodeRefSegment(tag)
assertSafeSegment('tag', encodedTag)
const githubRoot = path.join(askHome, 'github')
const candidate = path.join(githubRoot, host, owner, repo, tag)
const candidate = path.join(githubRoot, host, owner, repo, encodedTag)
return assertContained(githubRoot, candidate)
}

Expand Down
Loading