style: strip narration comments from bundle provenance code

Function docs that restated behavior already evident from names,
signatures, and code are removed; only comments carrying invariants
the code cannot express remain.
This commit is contained in:
2026-08-24 11:15:13 -06:00
parent 4987d850f9
commit 2d1bf372d8
2 changed files with 27 additions and 82 deletions
+10 -31
View File
@@ -173,8 +173,6 @@ impl BundleStore {
self.bundles.keys().map(String::as_str).collect() self.bundles.keys().map(String::as_str).collect()
} }
/// Look up the record installed from `url`, comparing canonical source URLs
/// so https/scp/`.git` spellings of the same remote all match.
pub(crate) fn find_by_source(&self, url: &str) -> Option<(&str, &BundleRecord)> { pub(crate) fn find_by_source(&self, url: &str) -> Option<(&str, &BundleRecord)> {
let canonical = canonical_source_url(url); let canonical = canonical_source_url(url);
self.bundles self.bundles
@@ -183,12 +181,9 @@ impl BundleStore {
.map(|(name, record)| (name.as_str(), record)) .map(|(name, record)| (name.as_str(), record))
} }
/// Decide the record key for an install from `url`, matching by canonical /// A URL already tracked under a different key migrates to the new key —
/// source URL first and name second. If the URL is already tracked under a /// the same URL never gets a second record. A name held by a
/// different key (manifest name added, renamed, or removed since install), /// different-source bundle is owner-qualified instead.
/// the existing record is migrated to the new key — the same URL never gets
/// a second record. A name held by a different-source bundle is
/// owner-qualified instead. Both cases print a notice.
pub(crate) fn resolve_bundle_name( pub(crate) fn resolve_bundle_name(
&mut self, &mut self,
url: &str, url: &str,
@@ -287,10 +282,6 @@ impl BundleStore {
Ok(resolved) Ok(resolved)
} }
/// Create or update the record's metadata and persist it. Repeated installs
/// of the same bundle (e.g. with different filters) merge into one record:
/// metadata is refreshed, files accumulate, and the original install time
/// is kept.
pub(crate) fn upsert_bundle(&mut self, name: &str, metadata: InstallMetadata) -> Result<()> { pub(crate) fn upsert_bundle(&mut self, name: &str, metadata: InstallMetadata) -> Result<()> {
match self.bundles.get_mut(name) { match self.bundles.get_mut(name) {
Some(record) => { Some(record) => {
@@ -322,7 +313,6 @@ impl BundleStore {
self.save() self.save()
} }
/// Stamp the record with the time of its most recent update from source.
pub(crate) fn mark_updated(&mut self, name: &str) -> Result<()> { pub(crate) fn mark_updated(&mut self, name: &str) -> Result<()> {
self.ensure_bundle_exists(name)?; self.ensure_bundle_exists(name)?;
let record = self let record = self
@@ -333,8 +323,6 @@ impl BundleStore {
self.save() self.save()
} }
/// Drop one path from a bundle's owned files and persist. Used when the
/// bundle no longer ships the file and it is gone (or deleted) locally.
pub(crate) fn remove_file_record(&mut self, bundle: &str, path: &str) -> Result<()> { pub(crate) fn remove_file_record(&mut self, bundle: &str, path: &str) -> Result<()> {
self.ensure_bundle_exists(bundle)?; self.ensure_bundle_exists(bundle)?;
let record = self let record = self
@@ -345,8 +333,6 @@ impl BundleStore {
self.save() self.save()
} }
/// Drop one mcp.json entry from a bundle's owned servers, matched by the
/// key it occupies in mcp.json, and persist.
pub(crate) fn remove_mcp_record(&mut self, bundle: &str, effective_key: &str) -> Result<()> { pub(crate) fn remove_mcp_record(&mut self, bundle: &str, effective_key: &str) -> Result<()> {
self.ensure_bundle_exists(bundle)?; self.ensure_bundle_exists(bundle)?;
let record = self let record = self
@@ -359,16 +345,14 @@ impl BundleStore {
self.save() self.save()
} }
/// Remove a bundle's record entirely and persist. Used once an uninstall
/// has released everything the record owned.
pub(crate) fn remove_bundle(&mut self, name: &str) -> Result<()> { pub(crate) fn remove_bundle(&mut self, name: &str) -> Result<()> {
self.ensure_bundle_exists(name)?; self.ensure_bundle_exists(name)?;
self.bundles.remove(name); self.bundles.remove(name);
self.save() self.save()
} }
/// Record one written file and persist immediately, so an install aborted /// Persists per call, so an install aborted partway through still has
/// partway through still has provenance for everything already on disk. /// provenance for everything already on disk.
/// A path owned by another bundle transfers to `bundle`. /// A path owned by another bundle transfers to `bundle`.
pub(crate) fn record_file(&mut self, bundle: &str, file: FileRecord) -> Result<()> { pub(crate) fn record_file(&mut self, bundle: &str, file: FileRecord) -> Result<()> {
self.ensure_bundle_exists(bundle)?; self.ensure_bundle_exists(bundle)?;
@@ -386,12 +370,9 @@ impl BundleStore {
self.save() self.save()
} }
/// Record the mcp.json entries an install wrote, in one persisted flush. /// An entry whose key any bundle already owns transfers to `bundle`, and
/// An entry whose key any bundle already owns — including `bundle` itself /// its `replaced` action upgrades to `transferred` — plain `replaced`
/// on an update — transfers to `bundle`: the old owner drops it, and a /// marks a pre-existing user entry that uninstall must never delete.
/// `replaced` action is upgraded to `transferred` (removable at uninstall
/// — plain `replaced` marks a pre-existing user entry that uninstall must
/// never delete).
pub(crate) fn record_mcp_servers( pub(crate) fn record_mcp_servers(
&mut self, &mut self,
bundle: &str, bundle: &str,
@@ -491,10 +472,8 @@ pub(crate) struct BundleListRow {
pub(crate) drift: DriftSummary, pub(crate) drift: DriftSummary,
} }
/// Build one listing row per installed bundle, hashing each owned file under /// An unreadable file counts as locally modified: it exists but its integrity
/// `config_dir` against its recorded checksum: a match is intact, a mismatch /// cannot be verified.
/// (or unreadable file) counts as locally modified, and an absent file is
/// missing. Read-only: the store is never mutated by listing.
pub(crate) fn bundle_list_rows(store: &BundleStore, config_dir: &Path) -> Vec<BundleListRow> { pub(crate) fn bundle_list_rows(store: &BundleStore, config_dir: &Path) -> Vec<BundleListRow> {
store store
.iter() .iter()
+17 -51
View File
@@ -120,9 +120,6 @@ fn classify_install_target(value: &str, installed_names: &[String]) -> InstallTa
InstallTarget::Unknown InstallTarget::Unknown
} }
/// A value is treated as a source when it is a URL, an scp-style
/// `[user@]host:path`, or an explicit local path. Bare names never are: they
/// must match an installed bundle.
fn looks_like_remote_source(value: &str) -> bool { fn looks_like_remote_source(value: &str) -> bool {
if value.contains("://") if value.contains("://")
|| value.starts_with("./") || value.starts_with("./")
@@ -140,9 +137,6 @@ fn looks_like_remote_source(value: &str) -> bool {
} }
} }
/// Unified entry point behind `--install` and the REPL's `.install <value>`:
/// asset categories are redirected to `--install-builtins`, installed bundle
/// names become updates, and anything shaped like a source is installed.
pub fn install_or_update(value: &str, filter: Option<InstallFilter>, force: bool) -> Result<()> { pub fn install_or_update(value: &str, filter: Option<InstallFilter>, force: bool) -> Result<()> {
let store = BundleStore::load()?; let store = BundleStore::load()?;
let installed: Vec<String> = store let installed: Vec<String> = store
@@ -204,11 +198,8 @@ pub fn install_or_update_from_repl_args(args: &str) -> Result<()> {
install_or_update(&value, filter, force) install_or_update(&value, filter, force)
} }
/// Update an installed bundle from its recorded source. `spec` is the bundle /// The whole remote is always processed — including categories a filtered
/// name, optionally suffixed with `#<ref>` to move a pinned ref. The whole /// install excluded — because filtered installs merge into a single record.
/// remote is always processed — including categories a filtered install
/// excluded — because filtered installs merge into a single record and an
/// update brings that record in line with everything the remote now ships.
pub fn update_bundle(spec: &str) -> Result<()> { pub fn update_bundle(spec: &str) -> Result<()> {
let (name, ref_override) = parse_url_with_ref(spec)?; let (name, ref_override) = parse_url_with_ref(spec)?;
@@ -279,11 +270,6 @@ pub fn update_bundle(spec: &str) -> Result<()> {
Ok(()) Ok(())
} }
/// A conflict on a file this bundle owns whose on-disk content still matches
/// the recorded hash is not a real conflict: the bundle wrote that content and
/// the user never touched it, so an update refreshes it without prompting.
/// Files the user modified — or that another bundle owns — keep the normal
/// conflict semantics.
fn reclassify_owned_unmodified( fn reclassify_owned_unmodified(
mut plan: InstallPlan, mut plan: InstallPlan,
store: &BundleStore, store: &BundleStore,
@@ -320,10 +306,8 @@ enum ObsoleteAction {
Delete, Delete,
} }
/// Reconcile owned files the remote no longer ships. A file already gone from /// Kept files stay in the record, so a later uninstall still offers to
/// disk just drops out of the record; a file still present is kept by default /// remove them.
/// (the user may rely on it) and only deleted on explicit confirmation. Kept
/// files stay in the record, so a later uninstall still offers to remove them.
fn handle_obsolete_files(store: &mut BundleStore, bundle: &str, plan: &InstallPlan) -> Result<()> { fn handle_obsolete_files(store: &mut BundleStore, bundle: &str, plan: &InstallPlan) -> Result<()> {
let planned: HashSet<String> = plan let planned: HashSet<String> = plan
.files .files
@@ -423,11 +407,7 @@ struct UninstallMcpSummary {
kept: Vec<String>, kept: Vec<String>,
} }
/// Uninstall a bundle: delete the files it owns, remove its mcp.json entries, /// Kept and failed items stay in the record, so a re-run offers them again.
/// and drop its store record once nothing is left. `spec` is the bundle name
/// or its source URL. Locally modified items are kept unless explicitly
/// confirmed at a prompt; kept and failed items stay in the record, so a
/// re-run offers them again.
pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> { pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> {
let mut store = BundleStore::load()?; let mut store = BundleStore::load()?;
let name = match store.get(spec) { let name = match store.get(spec) {
@@ -522,13 +502,8 @@ pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> {
Ok(()) Ok(())
} }
/// Process the files a bundle owns under `config_dir`. Intact files (content /// A recorded path that could escape `config_dir` (absolute, or containing
/// still matches the recorded hash) are deleted outright; missing files just /// anything but plain components) is never touched; its record survives.
/// drop out of the record; modified files are kept unless confirmed for
/// deletion. A failed deletion keeps the record so a re-run can retry, and
/// never aborts the remaining files. A recorded path that could escape
/// `config_dir` (absolute, or containing anything but plain components) is
/// never touched; its record survives.
fn uninstall_owned_files( fn uninstall_owned_files(
store: &mut BundleStore, store: &mut BundleStore,
bundle: &str, bundle: &str,
@@ -674,14 +649,9 @@ fn resolve_uninstall_action(
} }
} }
/// Remove a bundle's entries from mcp.json. An entry is removed outright only /// mcp.json is written before any ownership record is dropped: a failed write
/// when its current content still matches the recorded hash; a modified (or /// leaves every record intact for a retry, while a crash after it leaves
/// legacy, hash-less) entry is kept unless confirmed for deletion, and a /// stale records the absent-server branch cleans up on re-run.
/// pre-existing server the bundle replaced is never removed. Keys the bundle
/// does not own are never touched. mcp.json is written before any ownership
/// record is dropped, so a failed write leaves every record intact for a
/// retry, while a crash after it leaves stale records the absent-server
/// branch cleans up on re-run.
fn uninstall_mcp_entries( fn uninstall_mcp_entries(
store: &mut BundleStore, store: &mut BundleStore,
bundle: &str, bundle: &str,
@@ -982,8 +952,8 @@ pub(crate) struct BundleManifest {
pub(crate) homepage: Option<String>, pub(crate) homepage: Option<String>,
} }
/// Resolve the bundle's identity for this install and create or refresh its /// Returns the record key — possibly migrated or owner-qualified — that all
/// provenance record. Returns the record key subsequent recording uses. /// subsequent recording must use in place of the requested name.
fn register_bundle( fn register_bundle(
store: &mut BundleStore, store: &mut BundleStore,
url: &str, url: &str,
@@ -1505,12 +1475,9 @@ fn apply_plan(
Ok(report) Ok(report)
} }
/// Provenance is recorded per written file, not at the end of the loop: a /// Kept and identical files are never recorded — ownership means "this
/// conflict prompt can abort the install after earlier files already landed /// content exists because of this bundle" — so a file another bundle's
/// on disk, and those must not become untracked orphans. Files the user kept /// record already owns stays with that owner.
/// (or that were identical) are never recorded — ownership means "this
/// content exists because of this bundle" — so a kept file that another
/// bundle's record already owns stays with that owner.
fn record_written_file( fn record_written_file(
store: &mut BundleStore, store: &mut BundleStore,
bundle: &str, bundle: &str,
@@ -1533,9 +1500,8 @@ fn provenance_path(dst: &Path) -> String {
rel.to_string_lossy().replace('\\', "/") rel.to_string_lossy().replace('\\', "/")
} }
/// Record the mcp.json entries the merge actually wrote, in one flush right /// Entries the merge kept local are deliberately absent: an entry another
/// after the merged file itself. Entries the merge kept local are deliberately /// bundle already owns stays with that owner.
/// absent: an entry another bundle already owns stays with that owner.
fn record_mcp_merge(store: &mut BundleStore, bundle: &str, report: &McpMergeReport) -> Result<()> { fn record_mcp_merge(store: &mut BundleStore, bundle: &str, report: &McpMergeReport) -> Result<()> {
let mut entries: Vec<McpServerRecord> = Vec::new(); let mut entries: Vec<McpServerRecord> = Vec::new();
entries.extend(report.added.iter().map(|name| McpServerRecord { entries.extend(report.added.iter().map(|name| McpServerRecord {