Coding standards, file-size limit, safe path output, stricter acceptance scanner

AGENTS.md

@@ -1,5 +1,9 @@
 This project uses Tink to manage Agent Skills under `.agents/skills/`.
 
+## Coding standards
+
+- Read [`CODING_STANDARDS.md`](CODING_STANDARDS.md) before writing or reviewing code. It links area checklists, such as filesystem mutations.
+
 ## Testing rules
 
 - Never write unit tests after you write code.

CODING_STANDARDS.md

@@ -0,0 +1,33 @@
+# Coding standards
+
+Read during review. A reviewer applies every rule to the diff and reports each violation with file and line. Mechanical rules live in CI (`cargo fmt`, `clippy -D warnings`, tests), not here.
+
+Disclosed standards, read when the diff touches the named area:
+
+- Anything that creates, replaces, or removes files (skills, skillsets, receipts, manifests, library, binary): [`docs/standards/filesystem-mutations.md`](docs/standards/filesystem-mutations.md)
+
+## Structure
+
+**One owner.** Each act and fact has one owning module. Count the edit sites for a plausible change; more than one is a finding. Entry points that install from the library share one placement function. The library write functions (`deposit_at`, `deposit_create_only_at`, `deposit_refresh_at`, `sync_from_installed_at`) move toward one write policy; a new promote path does not add another.
+
+**Typed origin.** A closed enum (such as `AddOrigin { Source, Library }`) replaces mutually exclusive booleans, and every `match` over it is exhaustive. Reports and exit messages read the enum.
+
+**Whole rename.** A rename lands in one change across module names, APIs, user strings, and docs. Two words for one thing block approval (the retired `stash` beside `library`); keep the on-disk layout stable and rename the code.
+
+**Distinct outcomes.** Each observable result has its own variant. A repaired install, an unchanged no-op, and a created install print three different reports and carry three different values.
+
+**Shared predicate.** One function decides "hidden entry", "receipt-backed", and "canonical name". Every scan of the same tree calls it, so `skill check`, `skill list`, and `load_project_skills` agree.
+
+**Scoped gate.** A drift or currentness check runs in the commands that name it (`check`, `lock`). Placing it in the shared loader blocks unrelated commands.
+
+**Round trip.** Every file tink writes is read back by tink. `lock` output passes `verify` and `sync`; an empty project emits `skills = []`; a pinned source restores the locked bytes, not the current binary's embedded copy.
+
+**Idempotent publish.** CI release steps rerun cleanly: a completed older tag succeeds before any monotonic-version guard fires, and digest comparison uses artifacts that are stable across runs.
+
+**Honest copy.** README, SKILL.md, ACCEPTANCE.md, and help text state what the code does. When a contract changes, search for each restatement (flag names, the manage-tink workflow, acceptance rows) and fix them together. An acceptance row stays `partial` until its sensor compares content before and after, not mere existence.
+
+**Earned layer.** Add a wrapper, trait, or module split for a second caller.
+
+## Judgement over the diff
+
+Prefer deleting a layer to rearranging it. The question for each finding: "what is the smallest change that makes this concept disappear?"

clippy.toml

@@ -0,0 +1,5 @@
+# Paths can come from untrusted repositories. `output::display_path` escapes control
+# characters so a filename cannot inject terminal escape sequences into output.
+disallowed-methods = [
+    { path = "std::path::Path::display", reason = "use output::display_path (escapes terminal control characters)" },
+]

docs/standards/filesystem-mutations.md

@@ -0,0 +1,33 @@
+# Filesystem mutations
+
+Applies to every code path that writes under a project, `TINK_HOME`, the library, or the install location. Each rule below came from a reviewer finding on a real diff; apply all of them to the whole diff, then to the callers.
+
+## Writing
+
+**Stage, then rename.** Write the new tree or file beside the target and rename it over the directory entry. `fs::write` on an existing path follows hard links and truncates the shared inode; this includes `.tink-source.json` receipts.
+
+**Every ancestor.** Refuse symlinks at each component between the project root and the target (`.agents`, `.agents/skills`, each `sourceRoot` segment), not only at the endpoint. Canonicalize each member and require it to stay beneath the root.
+
+**Any entry shape.** A receipt path may be a file, a symlink, a hard link, or an empty directory. State which shapes are replaceable drift and which refuse, and test each.
+
+**Revalidate at the write.** Fingerprint the installed tree when planning; re-check it immediately before the first write of a batch. Long network work between plan and apply opens a window for local edits.
+
+**Bounded disk.** Hold one checkout at a time. Keep a staged copy of each candidate skill and release the clone after preflight.
+
+**Read-only means no writes.** `check`, `list`, and `verify` create no files, including temp directories. Compare against an in-memory value or a precomputed digest.
+
+**Recoverable.** A failure after the old tree is removed (`fs::remove_dir_all`) leaves the previous state restorable. Reuse `replace_verified` for refresh writes.
+
+## Reading remote and user input
+
+**Revisions are objects.** A tree URL revision can be a branch, a tag, or a 40-hex commit. Clone, fetch the revision (tags included for tag-only commits), then check it out. `git clone --branch <sha>` fails.
+
+**Decode URLs.** Percent-decode path segments before resolving a boundary (`skill%20collections`).
+
+**Root skill naming.** A skill at the repository root compares its canonical name to the repository basename or skips the comparison; the temp checkout directory name is an implementation detail.
+
+**Boundary skill wins.** When the selected boundary is itself a skill, report exactly that skill and zero skillsets; discovery stops at it.
+
+**Terminal-safe errors.** Paths in errors and output pass through `output::display_path`.
+
+**Advisory output.** Text written after a completed mutation (`Next:` lines) is best-effort; a closed stdout cannot change the exit status.

src/destroy.rs

@@ -30,7 +30,7 @@ pub fn destroy_project(project_root: &Path, yes: bool) -> Result<DestroyReport,
         if !agents.is_dir() {
             return Err(Error::msg(format!(
                 "Refusing to remove non-directory: {}",
-                agents.display()
+                crate::output::display_path(&agents)
             )));
         }
     }
@@ -39,7 +39,7 @@ pub fn destroy_project(project_root: &Path, yes: bool) -> Result<DestroyReport,
         if !skills.is_dir() {
             return Err(Error::msg(format!(
                 "Refusing to remove non-directory: {}",
-                skills.display()
+                crate::output::display_path(&skills)
             )));
         }
     }

src/doctor.rs

@@ -74,7 +74,7 @@ fn probe_home(home: Option<&Path>) -> ProbeRow {
         Ok(Some(root)) => ProbeRow {
             name: "home",
             outcome: ProbeOutcome::Pass,
-            detail: root.display().to_string(),
+            detail: crate::output::display_path(&root),
         },
         Ok(None) => ProbeRow {
             name: "home",

src/harvest.rs

@@ -256,7 +256,7 @@ pub(crate) fn harvest_at(home: Option<&Path>, cwd: &Path) -> Result<HarvestRepor
                 name: path
                     .file_name()
                     .map(|s| s.to_string_lossy().into_owned())
-                    .unwrap_or_else(|| path.display().to_string()),
+                    .unwrap_or_else(|| crate::output::display_path(&path)),
                 source: path,
                 action: HarvestAction::Skipped,
                 detail: Some("under library".into()),

src/home.rs

@@ -121,7 +121,7 @@ pub fn existing_inventory_root(root: Option<&Path>) -> Result<Option<PathBuf>, E
     if !root.is_dir() {
         return Err(Error::msg(format!(
             "Refusing to read non-directory inventory root: {}",
-            root.display()
+            output::display_path(&root)
         )));
     }
     validate_layout_marker(&root, &root.join(LAYOUT_FILENAME))?;
@@ -161,7 +161,7 @@ pub fn ensure_inventory_root(root: Option<&Path>) -> Result<(PathBuf, bool), Err
     if root.exists() && !root.is_dir() {
         return Err(Error::msg(format!(
             "Refusing to replace non-directory inventory root: {}",
-            root.display()
+            output::display_path(&root)
         )));
     }
     preflight_inventory_root(&root)?;
@@ -197,7 +197,7 @@ fn preflight_inventory_root(root: &Path) -> Result<(), Error> {
     }
     Err(Error::msg(format!(
         "Refusing to initialize non-empty directory as Tink home: {}",
-        root.display()
+        output::display_path(root)
     )))
 }
 
@@ -255,16 +255,20 @@ fn validate_layout_marker(root: &Path, layout: &Path) -> Result<(), Error> {
     if !layout.is_file() {
         return Err(Error::msg(format!(
             "Not a Tink home inventory: {}",
-            root.display()
+            output::display_path(root)
         )));
     }
     let raw = fs::read_to_string(layout).map_err(|e| map_io(layout, e))?;
-    let value: serde_json::Value = serde_json::from_str(&raw)
-        .map_err(|_| Error::msg(format!("Not a Tink home inventory: {}", root.display())))?;
+    let value: serde_json::Value = serde_json::from_str(&raw).map_err(|_| {
+        Error::msg(format!(
+            "Not a Tink home inventory: {}",
+            output::display_path(root)
+        ))
+    })?;
     if value.get("kind").and_then(serde_json::Value::as_str) != Some(LAYOUT_KIND) {
         return Err(Error::msg(format!(
             "Not a Tink home inventory: {}",
-            root.display()
+            output::display_path(root)
         )));
     }
     Ok(())

src/lib.rs

@@ -439,7 +439,7 @@ fn dispatch(cli: Cli, cwd: PathBuf) -> Result<(), Error> {
                     println!(
                         "{} {}",
                         style.success("Removed"),
-                        style.accent(path.display())
+                        style.accent(output::display_path(path))
                     );
                 }
             }
@@ -780,7 +780,7 @@ fn dispatch_skillset(cwd: &Path, command: SkillsetCommand) -> Result<(), Error>
             println!(
                 "{} {}",
                 style.success("Removed"),
-                style.accent(path.display())
+                style.accent(output::display_path(&path))
             );
             Ok(())
         }
@@ -1088,13 +1088,13 @@ fn dispatch_init(
         println!(
             "{} {}",
             style.success("Created"),
-            style.accent(report.skills_path.display())
+            style.accent(output::display_path(&report.skills_path))
         );
     } else {
         println!(
             "{} {}",
             style.success("Ready"),
-            style.accent(report.skills_path.display())
+            style.accent(output::display_path(&report.skills_path))
         );
     }
     if report.agents_written {
@@ -1110,13 +1110,13 @@ fn dispatch_init(
         println!(
             "{} {}",
             style.success("New home at"),
-            style.accent(report.inventory_home.display())
+            style.accent(output::display_path(&report.inventory_home))
         );
     } else {
         println!(
             "{} {}",
             style.muted("Home at"),
-            style.accent(report.inventory_home.display())
+            style.accent(output::display_path(&report.inventory_home))
         );
     }
     Ok(())
@@ -1257,7 +1257,7 @@ fn dispatch_skill_remove(cwd: &Path, name: &str) -> Result<(), Error> {
     println!(
         "{} {}",
         style.success("Removed"),
-        style.accent(report.removed.display())
+        style.accent(output::display_path(&report.removed))
     );
     Ok(())
 }
@@ -1273,7 +1273,7 @@ fn dispatch_skill_harvest(cwd: &Path) -> Result<(), Error> {
                     "{} {} {}",
                     out.success("Harvested"),
                     out.skill(&event.name),
-                    out.muted(event.source.display())
+                    out.muted(output::display_path(&event.source))
                 );
             }
             harvest::HarvestAction::Unchanged => {
@@ -1285,7 +1285,7 @@ fn dispatch_skill_harvest(cwd: &Path) -> Result<(), Error> {
                     "{} {} {} ({})",
                     err.warn("Skipped"),
                     err.skill(&event.name),
-                    err.muted(event.source.display()),
+                    err.muted(output::display_path(&event.source)),
                     detail
                 );
             }
@@ -1302,7 +1302,11 @@ fn dispatch_skill_harvest(cwd: &Path) -> Result<(), Error> {
         out.accent(report.skipped),
         out.muted("skipped")
     );
-    println!("{} {}", out.muted("Home"), out.accent(home.display()));
+    println!(
+        "{} {}",
+        out.muted("Home"),
+        out.accent(output::display_path(&home))
+    );
     Ok(())
 }
 
@@ -1318,7 +1322,7 @@ fn dispatch_skill_promote(cwd: &Path, name: &str, replace: bool) -> Result<(), E
         "{} {} {}\n{} {}\n{} {}",
         action,
         style.skill(name),
-        style.accent(outcome.destination.display()),
+        style.accent(output::display_path(&outcome.destination)),
         style.muted("Origin"),
         style.accent(format!("project skill {name}")),
         style.muted("Digest"),

src/library.rs

@@ -75,7 +75,7 @@ fn promotion_source(project_root: &Path, name: &str) -> Result<Skill, Error> {
     if path.join(".git").exists() || path.join(".git").is_symlink() {
         return Err(Error::msg(format!(
             "Refusing repository metadata in skill: {}",
-            path.display()
+            crate::output::display_path(&path)
         )));
     }
     skills::validate_skill_tree(&path)?;
@@ -110,7 +110,7 @@ fn promote_at(
         if !target.is_dir() {
             return Err(Error::msg(format!(
                 "Refusing to replace non-directory library skill: {}",
-                target.display()
+                crate::output::display_path(&target)
             )));
         }
         crate::skillsets::ensure_standalone_source(&target, name)?;
@@ -227,7 +227,7 @@ fn iter_library_skills(library: &Path) -> Result<Vec<Skill>, Error> {
     if !library.is_dir() {
         return Err(Error::msg(format!(
             "Refusing to read non-directory library: {}",
-            library.display()
+            crate::output::display_path(library)
         )));
     }
 
@@ -407,7 +407,7 @@ fn load_library_skill(library: &Path, name: &str) -> Result<Skill, Error> {
     if path.is_symlink() {
         return Err(Error::msg(format!(
             "Refusing to follow symlink: {}",
-            path.display()
+            crate::output::display_path(&path)
         )));
     }
     if !path.is_dir() {
@@ -441,7 +441,7 @@ pub(crate) fn load_existing_at(home: Option<&Path>, name: &str) -> Result<Skill,
     if !library.is_dir() {
         return Err(Error::msg(format!(
             "Refusing to read non-directory library: {}",
-            library.display()
+            crate::output::display_path(&library)
         )));
     }
     load_library_skill(&library, name)

src/manifest.rs

@@ -127,7 +127,10 @@ fn read_toml<T: for<'de> Deserialize<'de>>(
     refuse_symlink(&root.join(DIRECTORY))?;
     refuse_symlink(file)?;
     if !file.is_file() {
-        return Err(Error::msg(format!("Missing {label}: {}", file.display())));
+        return Err(Error::msg(format!(
+            "Missing {label}: {}",
+            output::display_path(file)
+        )));
     }
     let text = fs::read_to_string(file).map_err(|e| map_io(file, e))?;
     toml::from_str(&text).map_err(|e| Error::msg(format!("Invalid {label}: {e}")))
@@ -490,7 +493,7 @@ fn write_atomic(root: &Path, manifest: &str, lock: &str) -> Result<(), Error> {
     if !directory.is_dir() {
         return Err(Error::msg(format!(
             "Refusing non-directory manifest root: {}",
-            directory.display()
+            output::display_path(&directory)
         )));
     }
     let manifest_path = directory.join(MANIFEST_FILE);

src/mount.rs

@@ -247,7 +247,7 @@ fn link_into_active(project_root: &Path, skill_name: &str, src: &Path) -> Result
     if target.is_dir() && !target.is_symlink() {
         return Err(Error::msg(format!(
             "Refusing to overwrite non-symlink directory: {}",
-            target.display()
+            crate::output::display_path(&target)
         )));
     }
 
@@ -415,7 +415,7 @@ pub fn unmount_skill(project_root: &Path, skill_name: &str) -> Result<UnmountOut
     if target.is_dir() && !target.is_symlink() {
         return Err(Error::msg(format!(
             "Refusing to remove non-symlink directory: {}",
-            target.display()
+            crate::output::display_path(&target)
         )));
     }
 

src/paths.rs

@@ -263,7 +263,7 @@ mod tests {
                 .and_then(|name| name.to_str())
                 .is_some_and(|name| name.starts_with(".tink-orphan-target-")),
             "unexpected orphan name: {}",
-            recovery.display()
+            output::display_path(&recovery)
         );
         assert_eq!(
             fs::read(recovery.join("payload.txt")).unwrap(),

src/remove.rs

@@ -37,7 +37,7 @@ pub fn remove_skill(project_root: &Path, name: &str) -> Result<RemoveReport, Err
     if !target.is_dir() {
         return Err(Error::msg(format!(
             "Refusing to remove non-directory: {}",
-            target.display()
+            crate::output::display_path(&target)
         )));
     }
     if crate::skillsets::has_receipt_entry(&target) {

src/skills.rs

@@ -1136,10 +1136,10 @@ mod tests {
                 .and_then(|name| name.to_str())
                 .is_some_and(|name| name.starts_with(".tink-orphan-target-")),
             "unexpected orphan name: {}",
-            orphan.display()
+            output::display_path(orphan)
         );
         assert!(
-            error.to_string().contains(&orphan.display().to_string()),
+            error.to_string().contains(&output::display_path(orphan)),
             "{error}"
         );
         assert_eq!(fs::read(orphan.join("original")).unwrap(), b"preserve me");