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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **`fallow coverage upload-source-maps` now uploads each map's repo-relative path, so the source-evidence viewer can resolve monorepo sub-package source.** A bundled map under a sub-package (e.g. `dashboard/dist/assets/X.js.map`) lists its sources relative to the map file (`../../src/components/X.tsx`); the cloud previously had only the basename and collapsed that to `src/components/X.tsx`, which never matched the package-prefixed runtime path `dashboard/src/components/X.tsx`, so the viewer reported "source not in maps" even though the file was in an uploaded map. The CLI now sends the map's path relative to the repo root alongside the existing `fileName`, letting the cloud resolve each source against the map's directory and recover `dashboard/src/components/X.tsx`. The field is omitted when a map is not under the repo root (an absolute `--dir` outside it), in which case the cloud falls back to its previous behavior. Run `upload-source-maps` from the repo root so the prefix is correct. No change for single-package projects. (Closes [#260](https://github.com/fallow-rs/fallow-cloud/issues/260).)

- **`fallow flags` now surfaces the configuration surface when it finds nothing.** The empty-result line (`No feature flags detected`) is no longer byte-identical whether a project truly has no flags or just uses an SDK fallow does not recognize. On full defaults, the human output now lists the built-in env-var prefixes and SDK providers it scanned for, then points at `flags.sdkPatterns`, `flags.configObjectHeuristics`, and the configuration docs, so you can tell a true negative from a missing detector and add your own SDK (PostHog, in-house, anything not listed). Projects that already configured custom `flags.*` patterns get a single terse line acknowledging their config instead of the discovery block. The enumerated detectors are derived from fallow's built-in tables, so the hint stays in sync as defaults grow. JSON, SARIF, compact, markdown, and CodeClimate output are unchanged, and `--quiet` suppresses the hint. (Closes [#562](https://github.com/fallow-rs/fallow/issues/562).)
- **`fallow impact` reports what fallow has done for you, opt-in and local-only.** A new `fallow impact` command shows how many issues fallow is currently surfacing, the trend since the previous recorded run, and how many commits its pre-commit gate blocked then cleared. Enable it with `fallow impact enable` (with `disable` and `status` siblings); once enabled, each `fallow audit` run appends a small record to a single rolling `.fallow/impact.json` (gitignored, never uploaded). The generated `fallow init --hooks` pre-commit hook now tags gate runs so a blocked-then-fixed commit is recorded as contained. Writes are best-effort and never change a command's exit code or output. Human, `--format json`, and `--format markdown` output are available, and the JSON shape ships in the published output schema. `fallow impact` now also credits per-finding **resolutions**: when a finding you previously saw goes away because you fixed the code, it counts as resolved, and when it goes away because you added a `fallow-ignore`, it is reported separately as intentionally managed and never counted as a win. It distinguishes the two by capturing which suppressions are present each run, and it ignores findings that merely moved rather than being removed (within a file, or relocated to another file, including across separate commits). Resolution attribution covers dead code, complexity, and duplication, accrues from your local runs (it is a local-developer signal, not a CI metric, since it lives in `.fallow/impact.json`), and adds `resolved_total`, `suppressed_total`, and a recent-resolutions list to all three output formats and the published schema.
- **The Fallow Impact value report is now available over MCP.** A new read-only `impact` MCP tool wraps `fallow impact --format json`, so AI agents can read the local report (current surfacing, trend since the last recorded run, pre-commit gate containment, and, on impact v1.5+, resolved/suppressed attribution) the same way they call `check_health` or `audit`. It runs no analysis, so it takes only a `root`; the mutating `enable`/`disable` lifecycle is intentionally not exposed, and on a never-enabled project it returns a populated `{"enabled": false, ...}` report (never `{}`) so an agent can tell "not set up" from "set up, no history yet" and recommend `fallow impact enable` rather than toggling it. Because impact is a local-developer signal, the tool surfaces an empty report in ephemeral CI runners and should not be used as a CI metric.
Expand Down
100 changes: 94 additions & 6 deletions crates/cli/src/coverage/upload_source_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ fn run_inner(args: &UploadSourceMapsArgs, root: &Path) -> Result<(), UploadSourc
let exclude = compile_glob_set(&args.exclude, "--exclude")?;
let repo = resolve_repo_name(args.repo.as_deref(), root)?;
let git_sha = resolve_git_sha(args.git_sha.as_deref(), root)?;
let maps = collect_source_maps(&build_dir, &include, &exclude, args.strip_path)?;
let maps = collect_source_maps(root, &build_dir, &include, &exclude, args.strip_path)?;

if maps.is_empty() {
return Err(UploadSourceMapsError::Validation(format!(
Expand Down Expand Up @@ -276,22 +276,33 @@ struct SourceMapCandidate {
path: PathBuf,
rel_path: PathBuf,
file_name: String,
/// The map's path relative to the REPO ROOT (e.g.
/// `dashboard/dist/assets/app.js.map`), posix-separated. `None` when the map
/// is not under the repo root (an absolute `--dir` outside it) or the path
/// fails the same safety checks as `file_name`. The cloud resolves each
/// `sources[]` entry against this path's directory so a monorepo
/// sub-package map's `../../src/x` resolves to `dashboard/src/x` instead of
/// collapsing to `src/x` and losing the package prefix (issue #260). It is
/// distinct from `file_name`, which keys the upload's storage + identity.
map_path: Option<String>,
bytes: u64,
}

fn collect_source_maps(
repo_root: &Path,
dir: &Path,
include: &GlobSet,
exclude: &GlobSet,
strip_path: bool,
) -> Result<Vec<SourceMapCandidate>, UploadSourceMapsError> {
let mut maps = Vec::new();
collect_source_maps_inner(dir, dir, include, exclude, strip_path, &mut maps)?;
collect_source_maps_inner(repo_root, dir, dir, include, exclude, strip_path, &mut maps)?;
maps.sort_by(|a, b| a.rel_path.cmp(&b.rel_path));
Ok(maps)
}

fn collect_source_maps_inner(
repo_root: &Path,
root: &Path,
dir: &Path,
include: &GlobSet,
Expand All @@ -315,24 +326,36 @@ fn collect_source_maps_inner(
continue;
}
if file_type.is_dir() {
collect_source_maps_inner(root, &path, include, exclude, strip_path, maps)?;
collect_source_maps_inner(repo_root, root, &path, include, exclude, strip_path, maps)?;
continue;
}
if !include.is_match(&rel_path) || !path.is_file() {
continue;
}
let bytes = entry.metadata().map_or(0, |metadata| metadata.len());
let file_name = map_file_name(&rel_path, strip_path)?;
let map_path = repo_relative_map_path(repo_root, &path);
maps.push(SourceMapCandidate {
path,
rel_path,
file_name,
map_path,
bytes,
});
}
Ok(())
}

/// The map file's path relative to `repo_root`, posix-separated, or `None` when
/// the map is not under the repo root or the result is not a safe relative path
/// (issue #260). Only `Some` paths are sent as `mapPath`; the cloud falls back
/// to root-anchored normalization when absent, so dropping it is always safe.
fn repo_relative_map_path(repo_root: &Path, path: &Path) -> Option<String> {
let rel = path.strip_prefix(repo_root).ok()?;
let value = to_posix_string(rel);
validate_file_name(&value).ok().map(|()| value)
}

fn map_file_name(rel_path: &Path, strip_path: bool) -> Result<String, UploadSourceMapsError> {
let value = if strip_path {
rel_path
Expand Down Expand Up @@ -584,6 +607,11 @@ struct SourceMapRequest<'a> {
git_sha: &'a str,
#[serde(rename = "fileName")]
file_name: &'a str,
/// The map's repo-relative path, omitted when not resolvable (#260). The
/// cloud resolves `sources[]` against its directory for monorepo
/// sub-packages; an older cloud ignores the field.
#[serde(rename = "mapPath", skip_serializing_if = "Option::is_none")]
map_path: Option<&'a str>,
#[serde(rename = "sourceMap")]
source_map: &'a serde_json::Value,
}
Expand Down Expand Up @@ -620,6 +648,7 @@ fn send_source_map(
let payload = SourceMapRequest {
git_sha,
file_name: &map.candidate.file_name,
map_path: map.candidate.map_path.as_deref(),
source_map: &map.source_map,
};
let mut response = agent
Expand Down Expand Up @@ -806,11 +835,13 @@ fn print_dry_run(
display_endpoint_url(endpoint_override, repo)
);
for map in maps.iter().take(20) {
let map_path = map.map_path.as_deref().unwrap_or("-");
println!(
" - {} ({}) -> fileName={}",
" - {} ({}) -> fileName={} mapPath={}",
map.rel_path.display(),
format_bytes(map.bytes),
map.file_name
map.file_name,
map_path
);
}
if maps.len() > 20 {
Expand Down Expand Up @@ -907,12 +938,68 @@ mod tests {

let include = compile_glob_set(&["**/*.map".to_owned()], "--include").unwrap();
let exclude = compile_glob_set(&["**/node_modules/**".to_owned()], "--exclude").unwrap();
let maps = collect_source_maps(dir.path(), &include, &exclude, false).unwrap();
let maps = collect_source_maps(dir.path(), dir.path(), &include, &exclude, false).unwrap();

let file_names: Vec<&str> = maps.iter().map(|map| map.file_name.as_str()).collect();
assert_eq!(file_names, vec!["assets/app.js.map", "root.js.map"]);
}

#[test]
fn map_path_is_repo_root_relative_when_build_dir_is_a_subdirectory() {
// CI runs `upload-source-maps --dir dashboard/dist` from the repo root,
// so the repo root is the parent of `dashboard/dist`. mapPath must carry
// the full `dashboard/dist/...` prefix even though fileName is stripped.
let repo_root = tempdir().expect("tempdir");
let build_dir = repo_root.path().join("dashboard/dist");
std::fs::create_dir_all(build_dir.join("assets")).expect("assets dir");
std::fs::write(build_dir.join("assets/app-a1b2.js.map"), "{}").expect("map");

let include = compile_glob_set(&["**/*.map".to_owned()], "--include").unwrap();
let exclude = compile_glob_set(&["**/node_modules/**".to_owned()], "--exclude").unwrap();
let maps =
collect_source_maps(repo_root.path(), &build_dir, &include, &exclude, true).unwrap();

assert_eq!(maps.len(), 1);
// fileName is the stripped basename (storage identity), unchanged.
assert_eq!(maps[0].file_name, "app-a1b2.js.map");
// mapPath carries the repo-relative path so the cloud can resolve the
// map's `sources[]` against `dashboard/dist/assets`.
assert_eq!(
maps[0].map_path.as_deref(),
Some("dashboard/dist/assets/app-a1b2.js.map")
);
}

#[test]
fn repo_relative_map_path_is_none_for_a_map_outside_the_repo_root() {
let repo_root = tempdir().expect("repo root");
let elsewhere = tempdir().expect("elsewhere");
let outside = elsewhere.path().join("app.js.map");
assert_eq!(repo_relative_map_path(repo_root.path(), &outside), None);
}

#[test]
fn request_serializes_map_path_and_omits_it_when_absent() {
let source_map = serde_json::json!({ "version": 3, "sources": [], "mappings": "" });
let with_path = SourceMapRequest {
git_sha: "abcdef1",
file_name: "app.js.map",
map_path: Some("dashboard/dist/assets/app.js.map"),
source_map: &source_map,
};
let json = serde_json::to_string(&with_path).unwrap();
assert!(json.contains(r#""mapPath":"dashboard/dist/assets/app.js.map""#));

let without_path = SourceMapRequest {
git_sha: "abcdef1",
file_name: "app.js.map",
map_path: None,
source_map: &source_map,
};
let json = serde_json::to_string(&without_path).unwrap();
assert!(!json.contains("mapPath"));
}

#[test]
fn endpoint_url_encodes_repo_as_one_segment() {
assert_eq!(
Expand Down Expand Up @@ -947,6 +1034,7 @@ mod tests {
path: PathBuf::from("dist/app.js.map"),
rel_path: PathBuf::from("dist/app.js.map"),
file_name: "dist/app.js.map".to_owned(),
map_path: Some("dist/app.js.map".to_owned()),
bytes: 10,
};
let outcomes = [MapOutcome::failed(
Expand Down
6 changes: 6 additions & 0 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,12 @@ enum CoverageCli {
/// the selected repo + git SHA. The production beacon reports bundled
/// paths; the cloud resolver uses these maps to remap runtime coverage back
/// to original source files.
///
/// Each upload also carries the map's path relative to the repo root, so the
/// source-evidence viewer can resolve a monorepo sub-package map's relative
/// `sources[]` (e.g. `../../src/X`) to the package-prefixed source path
/// (e.g. `dashboard/src/X`). Run from the repo root so this prefix is
/// correct.
UploadSourceMaps {
/// Directory to scan recursively for source maps.
#[arg(long, value_name = "PATH", default_value = "dist")]
Expand Down
Loading