Fix deferred minors from #55–#57; add small-team setup guide

CHANGELOG.md

@@ -4,6 +4,21 @@ User-visible changes are recorded here. Alpha releases are for evaluation; stabl
 
 ## Unreleased
 
+- Add a [team setup guide](docs/team-setup.md) for a small team on GitHub and
+  Google Workspace. The starter workflows pass `PERMESH_GOOGLE_CLIENT_SECRET` and
+  `PERMESH_GOOGLE_REFRESH_TOKEN`, so Google can run in refresh-token mode in CI.
+- Fix: someone with `status: left` is expected to hold nothing in `profile check`,
+  even when a directory group still assigns them profiles.
+- `access review` refuses existing plan files before discovery, escapes Markdown
+  in `--detail` summaries, and names the folder and the CI remedy when a reviewed
+  `index.json` has an old format.
+- The starter verify workflow accepts only a finished post-merge plan run from
+  this repository's default branch (also one that exited 4 or 6), and exits like the CLI for the worst plan
+  (errors, then 3, 4, 6).
+- `permesh mcp`: a failure to reach the client exits 5 instead of blaming the
+  protocol version; symlinks that leave the workspace read as "File not found", so
+  they reveal nothing about their target; empty `profiles` or `remove` lists get a
+  specific message.
 - Declare the formats added since the first stability declaration stable: the
   `check` and `profiles` configuration, people and profile files, policy
   `version: 2`, the check report, profile results, access plan files and the

crates/permesh-cli/src/access/review.rs

@@ -11,7 +11,7 @@ use crate::{
     artifact::{self, Artifact},
     error::AppError,
     offboard,
-    output::safe,
+    output::{markdown, safe},
 };
 use permesh_config::profiles::ProfileSet;
 use permesh_core as core;
@@ -31,11 +31,12 @@ pub(crate) const INDEX_VERSION: u32 = 1;
 pub(crate) fn index_value(rows: &[&IndexRow]) -> Value {
     json!({"access_review_index_version": INDEX_VERSION, "plans": rows})
 }
-fn index_rows(index: Value) -> Result<Vec<IndexRow>, AppError> {
+fn index_rows(dir: &Path, index: Value) -> Result<Vec<IndexRow>, AppError> {
     let unsupported = || {
-        AppError::input(
-            "Unsupported index.json format; regenerate the reviewed plans with this Permesh version",
-        )
+        AppError::input(format!(
+            "Unsupported index.json format in {}; regenerate the reviewed plans with this Permesh version (in the starter CI, re-run the pull request's Permesh plan job)",
+            safe(&dir.display().to_string())
+        ))
     };
     let Value::Object(mut index) = index else {
         return Err(unsupported());
@@ -209,7 +210,7 @@ fn valid_file(name: &str) -> bool {
 /// The reviewed plans by (identity, kind), checked against their index.
 pub(crate) fn load_reviewed(dir: &Path) -> Result<Reviewed, AppError> {
     let mismatch = || AppError::input("Reviewed plans do not match their index.json");
-    let rows = index_rows(artifact::load_document(&dir.join(INDEX))?)?;
+    let rows = index_rows(dir, artifact::load_document(&dir.join(INDEX))?)?;
     let mut reviewed = BTreeMap::new();
     for row in rows {
         if !valid_file(&row.file) {
@@ -325,22 +326,32 @@ pub(crate) fn summary(
             let kind = serde_json::to_value(row.kind).unwrap_or_default();
             text.push_str(&format!(
                 "\n## {} {}\n\n",
-                safe(kind.as_str().unwrap_or_default()),
-                safe(&row.identity)
+                markdown(kind.as_str().unwrap_or_default()),
+                markdown(&row.identity)
             ));
             for step in &plan.steps {
                 let operation = serde_json::to_value(&step.operation).unwrap_or_default();
-                text.push_str(&format!("- {}\n", words(&operation)));
+                text.push_str(&format!("- {}\n", markdown(&words(&operation))));
             }
         }
     }
     text
 }
 
-/// Fail before capture when the fixed output files already exist.
+/// Fail before capture when output files already exist, plans included.
 pub(crate) fn check_out(out: &Path, overwrite: bool) -> Result<(), AppError> {
     for name in [INDEX, SUMMARY] {
         artifact::check_destination(&out.join(name), overwrite)?;
     }
+    let Ok(entries) = std::fs::read_dir(out) else {
+        return Ok(());
+    };
+    for entry in entries.flatten() {
+        let name = entry.file_name();
+        let name = name.to_string_lossy();
+        if valid_file(&name.replace(".execution.json", ".json")) {
+            artifact::check_destination(&entry.path(), overwrite)?;
+        }
+    }
     Ok(())
 }

crates/permesh-cli/src/mcp/mod.rs

@@ -39,6 +39,13 @@ pub(crate) async fn serve(cli: &Cli, blocking: &BlockingPool) -> Result<(), AppE
     let running = match session {
         Ok(running) => running,
         Err(ServerInitializeError::ConnectionClosed(_)) => return Ok(()),
+        Err(ServerInitializeError::Cancelled) => return Err(AppError::new(130, "Cancelled")),
+        Err(ServerInitializeError::TransportError { .. }) => {
+            return Err(AppError::new(
+                5,
+                "The MCP session could not start: Permesh could not reach the assistant over stdio.",
+            ));
+        }
         Err(_) => {
             return Err(AppError::input(
                 "The MCP session could not start. permesh mcp supports MCP protocol 2026-07-28 only; update the assistant or its MCP client.",

crates/permesh-cli/src/mcp/paths.rs

@@ -57,15 +57,16 @@ fn canonical_root(root: &Path) -> Result<PathBuf, AppError> {
     std::fs::canonicalize(root).map_err(|_| AppError::input(MISSING))
 }
 
-/// An input file inside the root; symlinks are followed and must stay inside.
+/// An input file inside the root; symlinks are followed and must stay inside. A link
+/// that leaves the root reads as missing, so it never tells whether its target exists.
 pub(crate) fn existing(root: &Path, relative: &str) -> Result<PathBuf, AppError> {
     let path = lexical(relative)?;
     let root = canonical_root(root)?;
     let resolved = std::fs::canonicalize(root.join(path)).map_err(|_| AppError::input(MISSING))?;
     if resolved.starts_with(&root) {
         Ok(resolved)
     } else {
-        Err(AppError::input(OUTSIDE))
+        Err(AppError::input(MISSING))
     }
 }
 
@@ -87,7 +88,7 @@ pub(crate) fn new_file(root: &Path, relative: &str) -> Result<PathBuf, AppError>
     if parent.starts_with(&root) {
         Ok(parent.join(name))
     } else {
-        Err(AppError::input(OUTSIDE))
+        Err(AppError::input(MISSING))
     }
 }
 
@@ -140,14 +141,18 @@ mod tests {
         let dir = tempfile::tempdir().unwrap();
         std::os::unix::fs::symlink(outside.path().join("file"), dir.path().join("link")).unwrap();
         std::os::unix::fs::symlink(outside.path(), dir.path().join("out")).unwrap();
-        assert_eq!(
-            message(existing(dir.path(), "link")),
-            "Path is outside the workspace"
-        );
-        assert_eq!(
-            message(new_file(dir.path(), "out/new.json")),
-            "Path is outside the workspace"
-        );
+        std::os::unix::fs::symlink(outside.path().join("none"), dir.path().join("gone")).unwrap();
+        std::os::unix::fs::symlink(outside.path().join("none"), dir.path().join("nodir")).unwrap();
+        // Same answer whether the target outside exists, so links reveal nothing about it.
+        for (link, gone) in [("link", "gone"), ("out/new.json", "nodir/new.json")] {
+            let (link, gone) = if link == "link" {
+                (existing(dir.path(), link), existing(dir.path(), gone))
+            } else {
+                (new_file(dir.path(), link), new_file(dir.path(), gone))
+            };
+            assert_eq!(message(link), "File not found in the workspace");
+            assert_eq!(message(gone), "File not found in the workspace");
+        }
     }
 
     #[test]

crates/permesh-cli/src/mcp/server.rs

@@ -15,7 +15,7 @@ use rmcp::{
 };
 use std::{borrow::Cow, path::PathBuf, sync::Arc};
 
-pub(crate) const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Every tool result has `complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access.";
+pub(crate) const INSTRUCTIONS: &str = "Permesh answers who has access to what, read-only. Successful tool results have `report.complete`: when it is false, some providers could not be fully read and missing access is not proof of absence, so say you cannot tell. Plan tools write a new plan file for a person to review and carry out; Permesh never changes access.";
 const VERSIONS: &[ProtocolVersion] = &[ProtocolVersion::V_2026_07_28];
 
 pub(crate) struct Server {

crates/permesh-cli/src/mcp/tools.rs

@@ -135,6 +135,20 @@ fn plan_properties(mut properties: Value) -> Value {
 fn names(description: &str) -> Value {
     json!({"type": "array", "items": {"type": "string"}, "description": description})
 }
+/// A required profile list, which the plan command needs at least one name in.
+fn required_names(description: &str) -> Value {
+    let mut value = names(description);
+    value["minItems"] = json!(1);
+    value
+}
+fn at_least_one(field: &str, values: &[String]) -> Result<(), AppError> {
+    if values.is_empty() {
+        return Err(AppError::input(format!(
+            "Name at least one profile in {field}"
+        )));
+    }
+    Ok(())
+}
 const SNAPSHOT: &str =
     "Saved snapshot to read offline instead of fresh discovery; a path relative to the workspace.";
 
@@ -194,7 +208,7 @@ pub(crate) fn definitions() -> Vec<Definition> {
             "plan_join",
             "Prepare a reviewable plan file of the access a person needs for added profiles, including accounts still to create. Permesh never applies it.",
             object(
-                plan_properties(json!({"profiles": names("Profiles to add.")})),
+                plan_properties(json!({"profiles": required_names("Profiles to add.")})),
                 &["person", "profiles", "output"],
             ),
         ),
@@ -203,7 +217,7 @@ pub(crate) fn definitions() -> Vec<Definition> {
             "Prepare a reviewable plan file for a profile change: removals only for access no remaining profile justifies, and shared access listed as kept. Permesh never applies it.",
             object(
                 plan_properties(
-                    json!({"remove": names("Profiles to take away."), "add": names("Profiles to add.")}),
+                    json!({"remove": required_names("Profiles to take away."), "add": names("Profiles to add.")}),
                 ),
                 &["person", "remove", "output"],
             ),
@@ -298,12 +312,14 @@ impl Tool {
                 argv.extend(file("policy", &args.policy)?);
             }
             Tool::PlanJoin(args) => {
+                at_least_one("profiles", &args.profiles)?;
                 argv.extend(["access".into(), "join".into()]);
                 argv.extend(args.profiles.iter().map(|p| option("profile", p)));
                 argv.extend(common(&args.output, &args.snapshot, args.expires_in_hours)?);
                 argv.extend(["--".into(), (&args.person).into()]);
             }
             Tool::PlanMove(args) => {
+                at_least_one("remove", &args.remove)?;
                 argv.extend(["access".into(), "move".into()]);
                 argv.extend(args.remove.iter().map(|p| option("remove", p)));
                 argv.extend(args.add.iter().map(|p| option("add", p)));
@@ -456,4 +472,37 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn empty_profile_lists_name_the_argument() {
+        let dir = tempfile::tempdir().unwrap();
+        let workspace = dir.path().join("permesh.yaml");
+        let join = Tool::PlanJoin(JoinArgs {
+            person: "a@example.com".into(),
+            profiles: vec![],
+            output: "new.json".into(),
+            snapshot: None,
+            expires_in_hours: None,
+        });
+        let error = join.argv(&workspace).unwrap_err();
+        assert_eq!(error.message, "Name at least one profile in profiles");
+        let movement = Tool::PlanMove(MoveArgs {
+            person: "a@example.com".into(),
+            remove: vec![],
+            add: vec!["b".into()],
+            output: "new.json".into(),
+            snapshot: None,
+            expires_in_hours: None,
+        });
+        let error = movement.argv(&workspace).unwrap_err();
+        assert_eq!(error.message, "Name at least one profile in remove");
+        let listed = definitions();
+        let schema = |name: &str, field: &str| {
+            listed.iter().find(|t| t.name == name).unwrap().input_schema["properties"][field]
+                ["minItems"]
+                .clone()
+        };
+        assert_eq!(schema("plan_join", "profiles"), json!(1));
+        assert_eq!(schema("plan_move", "remove"), json!(1));
+    }
 }

crates/permesh-cli/src/output.rs

@@ -14,6 +14,19 @@ pub(crate) fn safe(value: &str) -> String {
         })
         .collect()
 }
+/// `safe`, then backslash-escape Markdown syntax so provider names render as text.
+pub(crate) fn markdown(value: &str) -> String {
+    safe(value)
+        .chars()
+        .flat_map(|c| {
+            if "\\`*_{}[]()<>#!|~&".contains(c) {
+                vec!['\\', c]
+            } else {
+                vec![c]
+            }
+        })
+        .collect()
+}
 pub(crate) fn field<'a>(value: &'a serde_json::Value, name: &str) -> &'a str {
     value[name].as_str().unwrap_or("")
 }

crates/permesh-cli/src/profile_command.rs

@@ -92,6 +92,7 @@ pub(crate) fn input(set: &ProfileSet, artifact: &Artifact) -> core::ProfileInput
                 profiles: d.profiles.clone(),
             })
             .collect(),
+        left: set.left.clone(),
         observable: artifact
             .providers
             .iter()
@@ -116,13 +117,7 @@ pub(crate) fn review(
     now: OffsetDateTime,
 ) -> Result<ProfileReview, AppError> {
     let (snapshots, aliases) = crate::resource_command::domain(artifact);
-    // Someone who left is expected to hold nothing: remaining access is unexplained,
-    // and their former profiles are not missing.
-    let mut input = input.clone();
-    for identity in &set.left {
-        input.people.insert(identity.clone(), Default::default());
-    }
-    let comparison = core::compare_profiles(&snapshots, &aliases, &input)?;
+    let comparison = core::compare_profiles(&snapshots, &aliases, input)?;
     let (mut missing, mut unexplained, mut cannot_tell, mut excepted, mut covered) =
         (0, 0, 0, 0, 0);
     let mut identities = Vec::new();

crates/permesh-cli/tests/access.rs

@@ -802,6 +802,61 @@ fn review_plans_changed_people_only() {
     assert!(summary.contains("bob@example.com"), "{summary}");
 }
 
+#[test]
+fn detail_summary_escapes_markdown() {
+    let identity = "<b>x</b>_[a](https://e.example)@example.com";
+    let head = demo(&[
+        ("profiles/backend.yaml", BACKEND),
+        (
+            "people/x.yaml",
+            &format!("identity: '{identity}'\nprofiles: [backend]\n"),
+        ),
+    ]);
+    let base = base_checkout(head.path(), &[("profiles/backend.yaml", BACKEND)]);
+    let (code, result) = review(head.path(), base.path(), "detailed", &["--detail"]);
+    assert_eq!(code, Some(0), "{result}");
+    let summary = fs::read_to_string(head.path().join("detailed/summary.md")).unwrap();
+    // Names render as text: no HTML, no link.
+    assert!(
+        summary.contains(r"\<b\>x\</b\>\_\[a\]\(https://e.example\)@example.com"),
+        "{summary}"
+    );
+    assert!(
+        !summary.contains("<b>") && !summary.contains("]("),
+        "{summary}"
+    );
+}
+
+#[test]
+fn existing_plan_file_is_refused_before_capture() {
+    let head = demo(&[
+        ("profiles/backend.yaml", BACKEND),
+        ("people/bob.yaml", &person("bob@example.com", "backend")),
+    ]);
+    let base = base_checkout(head.path(), &[("profiles/backend.yaml", BACKEND)]);
+    fs::create_dir_all(head.path().join("plans")).unwrap();
+    fs::write(head.path().join("plans/000.json"), "{}").unwrap();
+    // The missing snapshot would fail the capture; the output check must come first.
+    let (code, result) = review(
+        head.path(),
+        base.path(),
+        "plans",
+        &["--snapshot", "missing.json"],
+    );
+    assert_eq!(code, Some(2), "{result}");
+    assert!(
+        result["error"]["message"]
+            .as_str()
+            .unwrap()
+            .starts_with("Output exists"),
+        "{result}"
+    );
+    assert_eq!(
+        fs::read_to_string(head.path().join("plans/000.json")).unwrap(),
+        "{}"
+    );
+}
+
 #[test]
 fn deleted_people_file_is_a_move_and_left_is_a_leave() {
     let head = demo(&[
@@ -1069,7 +1124,7 @@ fn bare_array_index_is_refused() {
     assert_eq!(code, Some(2), "{result}");
     assert_eq!(
         result["error"]["message"],
-        "Unsupported index.json format; regenerate the reviewed plans with this Permesh version"
+        "Unsupported index.json format in r1; regenerate the reviewed plans with this Permesh version (in the starter CI, re-run the pull request's Permesh plan job)"
     );
     assert!(!head.path().join("r2/000.execution.json").exists());
 }