Merge pull request #997 from finitecomputer/claude/refresh-the-moved-ref

refresh reads again only the ref its writer moved

Author: futurepaul

cell/src/fragment.rs

@@ -1088,7 +1088,11 @@ impl FragmentCell {
             }
             (Method::Delete, ["api", "secrets", key]) => self.delete_secret(&caller, key),
             (Method::Get, ["api", "storage-token"]) => self.storage_token(&caller).await,
-            (Method::Post, ["api", "refresh"]) => self.refresh(&caller).await,
+            (Method::Post, ["api", "refresh"]) => {
+                // the ref its writer moved, or both (fragment_proto::Refresh)
+                let refs = fragment_proto::Refresh::refs_of(&req.bytes().await?).map_err(CellError::invalid)?;
+                self.refresh(&caller, &refs).await
+            }
             (Method::Get, ["api", "files"]) => self.files(&caller).await,
             (Method::Post, ["api", "files"]) => {
                 let body = body_json(&mut req).await?;

cell/src/plane.rs

@@ -43,7 +43,7 @@ pub const QUIET_PASS_MS: i64 = 24 * 3600 * 1000;
 /// fails to follow: a token lives 15 minutes, so this is generous for
 /// whoever pushes with it, and outlasts a code.storage outage of hours.
 const MAY_LAG_MS: i64 = 24 * 3600 * 1000;
-pub const REFS: [&str; 2] = ["main", "live"];
+pub const REFS: [&str; 2] = fragment_proto::PINNED_REFS;
 
 pub struct PinMove {
     pub changed: bool,
@@ -791,9 +791,11 @@ impl FragmentCell {
         self.schedule().await
     }
 
-    pub(crate) async fn refresh(&self, caller: &Caller) -> CellResult<Response> {
+    /// `POST /api/refresh`: `refs` read again (the one its writer moved,
+    /// or both), their moves taken before it answers.
+    pub(crate) async fn refresh(&self, caller: &Caller, refs: &[&str]) -> CellResult<Response> {
         self.require(caller, false, Role::Editor)?;
-        let moved = self.interpret(&REFS).await?;
+        let moved = self.interpret(refs).await?;
         let refs: serde_json::Map<String, Value> = moved
             .into_iter()
             .map(|(which, m)| {

cli/src/main.rs

@@ -1890,7 +1890,7 @@ fn run(cli: Cli) -> Result<()> {
             let author = Author::writer(&writer_id(&c));
             let new_tip = storage.restore_live(&target, &live_tip, &format!("rollback {name} to {target}"), &author)
                 .map_err(cs_anyhow)?;
-            sync::refresh_pins(&c, &name);
+            sync::refresh_pins(&c, &name, LIVE);
             json_exit(j, &json!({ "rolledBackTo": target, "liveTip": new_tip }));
             println!("rolled back to {}: live is now {}", &target[..8.min(target.len())], &new_tip[..8.min(new_tip.len())]);
         }

cli/src/session/mod.rs

@@ -486,7 +486,7 @@ fn write(c: &Client, name: &str, spool: &Spool) -> Result<Written> {
     let indexes = Indexes { c, name, storage: &storage, author: &author };
     let (entries, indexed) = indexes.write(head, merge.entries, merge.indexes, &written)?;
     if total > 0 || indexed {
-        crate::sync::refresh_pins(c, name);
+        crate::sync::refresh_pins(c, name, crate::codestorage::MAIN);
     }
     Ok(Written { added, updated, unchanged, entries })
 }

cli/src/sync.rs

@@ -535,7 +535,7 @@ pub fn sync_once(client: &Client, name: &str, dir: &Path, opts: &SyncOptions) ->
     let storage = CodeStorage::connect(client, name)?;
     let report = pass_over(client, &storage, name, dir, opts, read_local(dir, name)?)?;
     if report.landed {
-        refresh_pins(client, name);
+        refresh_pins(client, name, MAIN);
     }
     Ok(report)
 }
@@ -743,12 +743,14 @@ pub fn pass_over(client: &Client, storage: &CodeStorage, name: &str, dir: &Path,
     Ok(report)
 }
 
-/// Tells the cell its fragment's git moved. Our commits and ref moves are
+/// Tells the cell its fragment's `moved` ref moved. Our commits and ref moves are
 /// EXTERNAL pushes from the cell's perspective: without this nudge its
 /// pins wait out the 5-minute poll backstop before app reads and serving
 /// see them. Best-effort: the poll covers a missed nudge.
-pub fn refresh_pins(client: &Client, name: &str) {
-    if let Err(e) = client.post_json(&format!("/api/f/{name}/refresh"), &serde_json::json!({})) {
+pub fn refresh_pins(client: &Client, name: &str, moved: &str) {
+    // the cell reads again only the ref we moved (a cell before that reads both)
+    let asked = fragment_proto::Refresh { moved: Some(moved.to_string()) };
+    if let Err(e) = client.post_json(&format!("/api/f/{name}/refresh"), &asked) {
         eprintln!("warning: cell pin refresh failed ({e:#}); the poll backstop will catch up");
     }
 }

cli/src/watch.rs

@@ -121,7 +121,7 @@ impl<'a> Watcher<'a> {
         let passed = self.storage.get(client).map_err(SyncError::from).and_then(|storage| sync::pass_over(client, storage, name, dir, opts, local));
         let report = self.refused_on_auth(passed)?;
         if report.landed {
-            sync::refresh_pins(client, name);
+            sync::refresh_pins(client, name, MAIN);
         }
         self.head = report.head.clone();
         Ok(Served::Passed(Box::new(report)))

crates/e2e/src/lanes/plane.rs

@@ -76,6 +76,16 @@ fn files_lane(s: &mut Suite, api: &Api) -> Result<()> {
     let r = api.signed(&owner, "POST", &format!("/api/f/{name}/refresh"), Some(&json!({})))?;
     s.ok("refresh moves main at once", r.status == 200 && r.body["refs"]["main"]["moved"] == true && r.body["refs"]["live"]["absent"] == true, &r);
     s.ok("refresh made the file visible", read(api, &owner, &name, "quiet.md").as_deref() == Some("unannounced"), "");
+    // a writer names the ref it moved, and only that one is read again
+    s.fake.external_commit(&repo, "main", &[("named.md", Some(b"main alone"))], "unannounced");
+    let r = api.signed(&owner, "POST", &format!("/api/f/{name}/refresh"), Some(&json!({ "ref": "main" })))?;
+    s.ok(
+        "a refresh naming main moves main and reads live not at all",
+        r.status == 200 && r.body["refs"]["main"]["moved"] == true && r.body["refs"].get("live").is_none() && read(api, &owner, &name, "named.md").as_deref() == Some("main alone"),
+        &r,
+    );
+    let r = api.signed(&owner, "POST", &format!("/api/f/{name}/refresh"), Some(&json!({ "ref": "dev" })))?;
+    s.ok("a ref the platform does not pin is 400", r.status == 400, &r);
     s.fake.external_commit(&repo, "main", &[("polled.md", Some(b"found by the poll"))], "unannounced");
     let polled = s.eventually(Duration::from_secs(u64::from(crate::POLL_S) * 5), || read(api, &owner, &name, "polled.md").is_some());
     s.ok("the poll backstop finds a commit no one refreshed, a storage token since", polled, "");

crates/proto/src/lib.rs

@@ -1708,6 +1708,31 @@ pub struct SetVisibility {
     pub visibility: Visibility,
 }
 
+/// The refs a fragment's git has that the platform follows (its pins).
+pub const PINNED_REFS: [&str; 2] = ["main", "live"];
+
+/// `POST /api/f/<name>/refresh`'s body: the ref its writer moved, which is
+/// all the platform reads again (a sync moved main, a rollback live).
+/// Without one (an older CLI's `{}`, or no body), both.
+#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
+pub struct Refresh {
+    #[serde(rename = "ref", default, skip_serializing_if = "Option::is_none")]
+    pub moved: Option<String>,
+}
+
+impl Refresh {
+    /// The refs a refresh with this body reads again. A body that names
+    /// no ref, or is not one (the route once read none), reads both; a ref
+    /// the platform does not pin is refused.
+    pub fn refs_of(body: &[u8]) -> Result<Vec<&'static str>, String> {
+        let asked: Refresh = serde_json::from_slice(body).unwrap_or_default();
+        match asked.moved.as_deref() {
+            None => Ok(PINNED_REFS.to_vec()),
+            Some(r) => PINNED_REFS.iter().find(|p| **p == r).map(|p| vec![*p]).ok_or_else(|| format!("ref is main or live, not {r:?}")),
+        }
+    }
+}
+
 /// `GET /api/f/<name>/storage-token` (editor and up): a code.storage JWT
 /// scoped to this fragment's repo, `git:read` + `git:write`.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -2256,6 +2281,21 @@ pub mod wipe;
 mod tests {
     use super::*;
 
+    /// Goal: a refresh reads again the ref its writer names, and an older
+    /// writer's body (`{}`, none, or not JSON: the route read no body
+    /// before) both. Method: each body, and a ref the platform does not pin.
+    #[test]
+    fn a_refresh_reads_the_ref_it_names_or_both() {
+        assert_eq!(Refresh::refs_of(br#"{"ref":"main"}"#), Ok(vec!["main"]));
+        assert_eq!(Refresh::refs_of(br#"{"ref":"live"}"#), Ok(vec!["live"]));
+        for older in [&b"{}"[..], b"", b"not json", br#"{"other":1}"#] {
+            assert_eq!(Refresh::refs_of(older), Ok(vec!["main", "live"]), "{}", String::from_utf8_lossy(older));
+        }
+        assert!(Refresh::refs_of(br#"{"ref":"dev"}"#).is_err());
+        let sent = serde_json::to_value(Refresh { moved: Some("main".into()) }).unwrap();
+        assert_eq!(sent, serde_json::json!({ "ref": "main" }), "as a CLI sends it");
+    }
+
     /// Goal: the typed identity and sign-in reads the same JSON the
     /// registry and `GET /api/identities/me` carried before (T4: the wire
     /// is unchanged). Method: the literal old forms, decoded and encoded.

docs/api.md

@@ -676,7 +676,7 @@ inbox's, which takes its token. `{name}` is a fragment's name in full.
 | `GET /api/f/{name}/door` | its owner | → `{bound: {door, agent} \| null}`: whose door this connector is (An agent's doors) |
 | `POST /api/f/{name}/door` | its owner, in person (never an agent) | `{act: "link", agent}` → `{door, agent, relayed}`, or `{act: "unlink"}` → `{ok}` (An agent's doors). No connector, one the deployment does not allow, or an agent not the owner's, 403; another `act` 400 |
 | `GET /api/f/{name}/storage-token` | editor | → `{token, repo, api, expiresAt}`: ES256, this repo, `git:read`+`git:write`, `STORAGE_TOKEN_TTL_S` |
-| `POST /api/f/{name}/refresh` | editor | → `{ok, refs: {main: {pin, moved} \| {absent}, live: …}}` once a moved live is installed. A writer to the repo asks this after its push; the platform follows its own moves, and a busy fragment's poll is the backstop (`FRAGMENT_POLL_INTERVAL_S`) |
+| `POST /api/f/{name}/refresh` | editor | `{ref?: "main" \| "live"}`, the ref the writer moved (none, or no body: both; another ref 400) → `{ok, refs: {main: {pin, moved} \| {absent}, live: …}}`, the refs it read, once a moved live is installed. A writer to the repo asks this after its push; the platform follows its own moves, and a busy fragment's poll is the backstop (`FRAGMENT_POLL_INTERVAL_S`) |
 | `POST /api/f/{name}/files` | editor | `{files: [{path, text \| base64} \| {path, delete: true}], message?, key?}` → `{commit}`: one commit to main (at most `FILE_WRITES_MAX` files, 400; `API_FILES_WRITE_MAX_BYTES` decoded in a body of `BODY_MAX_BYTES`, 413; relative paths, no `.` or `..`). The same `key` from the same person answers the first commit again; a write of what main holds commits nothing. Main moves at once; live does not |
 | `POST /api/f/{name}/deploy` | editor | `{note?}` → `{live, canonical}`: live to main's tip, guarded against a live that moved meanwhile; the app installs at once |
 | `GET /api/f/{name}/files` | viewer | → `{ref, files: [{path, size, mode, lastCommitSha, machinery, blob?, sha256?, release?}]}` at main; a pointer's `sha256` is its blob. A blessed template's data from the release is listed beneath (`release: true`) |