From 336e3394533f806d2c2c13e7bf34712d392d3d21 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Fri, 30 Oct 2026 12:59:57 -0700 Subject: [PATCH 1/2] fix(writes): tell a write never sent from one whose answer was lost --- crates/agent_panel/src/gateway.rs | 248 ++++++++++++++++++++++-- crates/graph_client/src/client.rs | 180 +++++++++++++---- crates/graph_client/src/graph_client.rs | 23 ++- crates/graph_store/src/registry.rs | 28 +++ crates/graph_store/src/session.rs | 17 ++ crates/query_editor/src/view.rs | 19 ++ crates/review/src/branches.rs | 6 + crates/review/src/view.rs | 25 ++- crates/type_table/src/blob_write.rs | 23 +++ crates/type_table/src/model.rs | 17 ++ crates/type_table/src/write.rs | 5 +- specs/004-type-tables/plan.md | 4 +- specs/007-query-editor/plan.md | 10 +- specs/008-branches-review-merge/plan.md | 6 +- specs/013-agent-cli/plan.md | 10 +- specs/016-oauth-client/plan.md | 2 + 16 files changed, 552 insertions(+), 71 deletions(-) diff --git a/crates/agent_panel/src/gateway.rs b/crates/agent_panel/src/gateway.rs index 5bf7d7c1..e8facb31 100644 --- a/crates/agent_panel/src/gateway.rs +++ b/crates/agent_panel/src/gateway.rs @@ -21,8 +21,8 @@ use futures_rustls::TlsAcceptor; use futures_rustls::rustls::ServerConfig; use graph_client::api::CommitKind; use graph_client::{ - AsyncBody, CONTRACT, CONTRACT_HEADER, ForwardRequest, GraphClient, HeaderMap, Method, - StatusCode, WriteOutcome, + Answered, AsyncBody, CONTRACT, CONTRACT_HEADER, ForwardRequest, GraphClient, HeaderMap, Method, + NoAnswer, StatusCode, }; use serde::Deserialize as _; use serde_json::{Value, json}; @@ -430,6 +430,13 @@ async fn handle( stream_back(stream, &method, response).await; } Verdict::Write(write) => { + // A target already retired sends nothing: the agent is told so at once, as a read + // is, rather than the person being asked to approve a write that cannot go. One that + // retires while the person decides is refused when sent (`NoAnswer::Retired`). + if let Some(why) = shared.client.retired() { + answer(stream, 403, "forbidden", &retired_refusal(why)).await; + return; + } // Whoever ran the command, as they are now (`requester`): on Windows the CLI can // outlive a command stopped by its process id, its connection open. let starters = Starters::of(client, server).await; @@ -519,8 +526,14 @@ async fn handle( .detach(); // When the server's answer was lost, the CLI's connection closes without one, and // the CLI reports the outcome as unknown, as it is. - if let Ok(Some((status, headers, body))) = reply_rx.await { - write_buffered(stream, status, &headers, &body).await; + match reply_rx.await { + Ok(Some(Reply::Server(status, headers, body))) => { + write_buffered(stream, status, &headers, &body).await + } + Ok(Some(Reply::NotSent(unsent))) => { + answer(stream, unsent.status, unsent.code, &unsent.message).await + } + _ => {} } } } @@ -606,12 +619,22 @@ fn forward_request(request: Request, body: Option>) -> ForwardRequest { /// Reads a write's answer whole, and what it says happened to a write of `kind`. async fn read_write_answer( - response: Result, + response: Result, kind: WriteKind, -) -> (Seen, Option<(StatusCode, HeaderMap, Vec)>) { +) -> (Seen, Option) { let mut response = match response { Ok(response) => response, - Err(reason) => return (Seen::Unknown(reason), None), + Err(no) => { + return match unsent_refusal(&no) { + // Never sent (the target retired, the connection refused, or a request Studio + // would not make): nothing was written, and the CLI is told so. + Some(unsent) => ( + Seen::Refused(nothing_sent(&String::from(no))), + Some(Reply::NotSent(unsent)), + ), + None => (Seen::Unknown(String::from(no)), None), + }; + } }; let mut body = Vec::new(); // One byte past the limit tells a whole answer from a cut one. @@ -642,14 +665,72 @@ async fn read_write_answer( // Judged as Studio's own writes are, a success whose body cannot be read included: it // confirms nothing. let outcome = match graph_client::judge_write(response.status, contract, &body) { - WriteOutcome::Committed(body) => match serde_json::from_slice::(body) { + Answered::Committed(body) => match serde_json::from_slice::(body) { Ok(json) => seen_written(&json, kind), Err(error) => Seen::Unknown(format!("the success response could not be read: {error}")), }, - WriteOutcome::Refused(error) => Seen::Refused(error.body.error), - WriteOutcome::Unknown { reason } => Seen::Unknown(reason), + Answered::Refused(error) => Seen::Refused(error.body.error), + Answered::Unknown { reason } => Seen::Unknown(reason), + }; + ( + outcome, + Some(Reply::Server(response.status, response.headers, body)), + ) +} + +/// The CLI's refusal of a request to a retired target ([`NoAnswer::Retired`]). +fn retired_refusal(why: &str) -> String { + format!("Studio: {why}. Tell the person what this says. Nothing was sent.") +} + +/// That a request was never sent, and why ([`NoAnswer::Retired`], [`NoAnswer::Unsendable`], +/// [`NoAnswer::Unreached`]), as its card says it. +fn nothing_sent(why: &str) -> String { + format!("{why}, so nothing was sent.") +} + +/// How the CLI is answered for a request never sent ([`answer`]): its status, code and message. +#[derive(Debug)] +struct Unsent { + status: u16, + code: &'static str, + message: String, +} + +/// The CLI's answer to a request never sent, or `None` for one whose answer was lost: as every +/// gateway refusal does (spec 013), it says what to do instead and ends "Nothing was sent.", +/// which the agent reads as nothing written. A request Studio refused is a 403 `forbidden`, to +/// be fixed first; one whose server refused the connection is a 503 `service_unavailable`, to be +/// tried again once the server answers. +fn unsent_refusal(no: &NoAnswer) -> Option { + let forbidden = |message| Unsent { + status: 403, + code: "forbidden", + message, }; - (outcome, Some((response.status, response.headers, body))) + match no { + NoAnswer::Retired(why) => Some(forbidden(retired_refusal(why))), + NoAnswer::Unsendable(why) => Some(forbidden(format!( + "Studio: {why}. Change the request, or tell the person what this says. Nothing was \ + sent." + ))), + NoAnswer::Unreached(why) => Some(Unsent { + status: 503, + code: "service_unavailable", + message: format!( + "Studio: the server could not be reached ({why}). Try again once it answers, or \ + tell the person. Nothing was sent." + ), + }), + NoAnswer::Lost(_) => None, + } +} + +/// What the CLI is answered with once a forwarded write ends: the server's answer, or the +/// refusal of one never sent. +enum Reply { + Server(StatusCode, HeaderMap, Vec), + NotSent(Unsent), } /// What a success answer to a write of `kind` says it did. A write's receipt names its commit @@ -1077,18 +1158,24 @@ async fn write_buffered( async fn stream_back( stream: &mut (impl AsyncWrite + Unpin), method: &str, - response: Result, + response: Result, ) { let mut response = match response { Ok(response) => response, - Err(reason) => { - answer( - stream, - 502, - "internal", - &format!("Studio could not reach the graph's server: {reason}"), - ) - .await; + Err(no) => { + match unsent_refusal(&no) { + Some(unsent) => answer(stream, unsent.status, unsent.code, &unsent.message).await, + None => { + let reason = String::from(no); + answer( + stream, + 502, + "internal", + &format!("Studio could not reach the graph's server: {reason}"), + ) + .await + } + } return; } }; @@ -1543,6 +1630,56 @@ mod tests { /// or a body that cannot be read, confirms nothing. #[test] fn a_writes_success_answer_says_what_it_did() { + // A write the client never sent wrote nothing: its outcome says so, and the CLI is + // answered that nothing was sent and what to do, rather than left to report it unknown. + // One whose answer was lost stays unknown, with nothing to answer. + let (seen, reply) = smol::block_on(super::read_write_answer( + Err(NoAnswer::Retired("the target is gone".into())), + WriteKind::Mutate, + )); + assert_eq!( + seen, + super::Seen::Refused("the target is gone, so nothing was sent.".into()) + ); + assert!(matches!(reply, Some(super::Reply::NotSent(unsent)) + if unsent.status == 403 + && unsent.message + == "Studio: the target is gone. Tell the person what this says. Nothing was sent.")); + // One Studio would not make as asked says to change it. + let (seen, reply) = smol::block_on(super::read_write_answer( + Err(NoAnswer::Unsendable("refused path \"../x\"".into())), + WriteKind::Mutate, + )); + assert_eq!( + seen, + super::Seen::Refused("refused path \"../x\", so nothing was sent.".into()) + ); + assert!(matches!(reply, Some(super::Reply::NotSent(unsent)) + if unsent.status == 403 + && unsent.message == "Studio: refused path \"../x\". Change the request, or tell \ + the person what this says. Nothing was sent.")); + // One whose server refused the connection may be tried again once it answers: a 503 + // whose disposition says so (`effects: none`, `action: retry`), nothing having been sent. + let (seen, reply) = smol::block_on(super::read_write_answer( + Err(NoAnswer::Unreached("the connection was refused".into())), + WriteKind::Mutate, + )); + assert_eq!( + seen, + super::Seen::Refused("the connection was refused, so nothing was sent.".into()) + ); + assert!(matches!(reply, Some(super::Reply::NotSent(unsent)) + if unsent.status == 503 + && unsent.code == "service_unavailable" + && unsent.message.ends_with("Try again once it answers, or tell the person. \ + Nothing was sent."))); + let (seen, reply) = smol::block_on(super::read_write_answer( + Err(NoAnswer::Lost("the connection closed".into())), + WriteKind::Mutate, + )); + assert_eq!(seen, super::Seen::Unknown("the connection closed".into())); + assert!(reply.is_none()); + let answer = |kind: WriteKind, body: &str| { let mut headers = HeaderMap::new(); headers.append(CONTRACT_HEADER, CONTRACT.parse().unwrap()); @@ -1922,6 +2059,7 @@ mod tests { fn the_gateway_checks_the_session_then_forwards_reads_and_approved_writes() { smol::block_on(async { let (client, seen) = upstream(); + let upstream_client = client.clone(); let mut gateway = Gateway::bind().unwrap(); let (cli, token) = (Cli::of(&gateway), gateway.token.clone()); let (events_tx, mut events) = mpsc::unbounded(); @@ -2171,6 +2309,76 @@ mod tests { assert_eq!(status, 200); assert_eq!(body, r#"{"rows":[{"n":200}]}"#); assert_eq!(seen.lock().unwrap().len(), 4); + + // The target retired (the CLI no longer lists it) while the person decides a write: + // approved, it is answered that nothing was sent, as the gateway's own refusals + // are, and its outcome is a refusal, not unknown. A read, and a write asked after, + // are refused at once, the person not asked. + let mutation = + r#"{"query":"query m() { … }","name":"m","branch":"agent/20261013-101500"}"#; + let allowed = smol::spawn(call( + cli.clone(), + post("/graphs/knowledge/mutate", &token, Some(CONTRACT), mutation), + )); + let Some(GatewayEvent::Write { decide, .. }) = events.next().await else { + panic!("no write event") + }; + upstream_client.retire("the target is gone".into()); + let refused_so = |status: u16, body: &str| { + assert_eq!(status, 403, "{body}"); + let refused: Value = serde_json::from_str(body).unwrap(); + assert_eq!(refused["code"], "forbidden", "{body}"); + assert_eq!( + refused["disposition"], + json!({ "effects": "none", "action": "fix" }), + "{body}" + ); + assert!( + body.contains( + "Studio: the target is gone. Tell the person what this says. Nothing was \ + sent." + ), + "{body}" + ); + }; + let (status, _, body) = call( + cli.clone(), + post( + "/graphs/knowledge/query", + &token, + Some(CONTRACT), + r#"{"query":"q","branch":"main"}"#, + ), + ) + .await; + refused_so(status, &body); + let after = smol::spawn(call( + cli.clone(), + post("/graphs/knowledge/mutate", &token, Some(CONTRACT), mutation), + )); + let futures::future::Either::Left(((status, _, body), _)) = + futures::future::select(after, events.next()).await + else { + panic!("the person was asked to approve a write to a retired target") + }; + refused_so(status, &body); + let recorded: Arc>> = Arc::default(); + let record = recorded.clone(); + decide + .send(Some(Box::new(move |seen: &super::Seen| { + *record.lock().unwrap() = Some(seen.clone()); + }))) + .ok() + .unwrap(); + let (status, _, body) = allowed.await; + refused_so(status, &body); + assert_eq!( + recorded.lock().unwrap().clone(), + Some(super::Seen::Refused( + "the target is gone, so nothing was sent.".into() + )) + ); + assert_eq!(seen.lock().unwrap().len(), 4, "nothing reached the server"); }); } diff --git a/crates/graph_client/src/client.rs b/crates/graph_client/src/client.rs index a85ba8e7..3847efde 100644 --- a/crates/graph_client/src/client.rs +++ b/crates/graph_client/src/client.rs @@ -23,7 +23,8 @@ use crate::api::{ QueriesCatalogOutput, QueryRequest, ReadOutput, ReadinessOutput, SchemaOutput, SnapshotOutput, }; use crate::{ - ApiError, CONTRACT, CONTRACT_HEADER, IF_GRAPH_COMMIT_HEADER, WriteOutcome, speaks_contract, + Answered, ApiError, CONTRACT, CONTRACT_HEADER, IF_GRAPH_COMMIT_HEADER, WriteOutcome, + speaks_contract, }; const READ_TIMEOUT: Duration = Duration::from_secs(60); @@ -499,6 +500,34 @@ pub enum FeedAt<'a> { Page(&'a str), } +/// Why a request has no answer. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum NoAnswer { + /// Never sent, as its client is retired ([`GraphClient::retire`]): nothing it asked for + /// was done, and nothing that client sends will be. + Retired(String), + /// Never sent, as Studio would not make it as asked: a path that leaves the server's root, + /// or a request that could not be built. Nothing it asked for was done. + Unsendable(String), + /// Never reached the server, as it refused the connection: a request is written only on a + /// connection made (an earlier attempt a client retries is one the server did not process), + /// so nothing it asked for was done. + Unreached(String), + /// It was sent, or may have been, and its answer was lost: a write may have landed. + Lost(String), +} + +impl From for String { + fn from(no: NoAnswer) -> Self { + match no { + NoAnswer::Retired(reason) + | NoAnswer::Unsendable(reason) + | NoAnswer::Unreached(reason) + | NoAnswer::Lost(reason) => reason, + } + } +} + /// The server's transport, which a changed target replaces ([`GraphClient::set_http`]): a client /// that sends each request through the one current when it is sent, so everything holding it /// moves to a new one together, and a request under way finishes through the one it began with. @@ -718,18 +747,18 @@ impl GraphClient { &self, url: &Url, request: Request, - ) -> Result, String> { + ) -> Result, NoAnswer> { // A retired client sends nothing: each request fails with why it was retired, as given, // not read as a transport's failure. if let Some(why) = self.retired.get() { - return Err(why.clone()); + return Err(NoAnswer::Retired(why.clone())); } let response = self .http .current() .send(request) .await - .map_err(|error| describe_transport(&error))?; + .map_err(|error| unanswered_by_transport(&error))?; self.note_refusal(url, response.status(), response.headers()); Ok(response) } @@ -762,6 +791,11 @@ impl GraphClient { .fetch_add(1, std::sync::atomic::Ordering::SeqCst); } + /// Why the client was retired ([`Self::retire`]), once it was: it sends nothing. + pub fn retired(&self) -> Option<&str> { + self.retired.get().map(String::as_str) + } + /// Stops the client and every clone of it for good, as when its CLI target is gone or names /// a transport Studio cannot use: every request after fails with `why`, and nothing is sent, /// a request waiting for the next token, a read slot or out a 429 included, at once. What @@ -1044,7 +1078,7 @@ impl GraphClient { body: Option>, headers: &[(&str, &str)], timeout: Duration, - ) -> Result { + ) -> Result { self.fresh_token().await; self.send_now( method, @@ -1069,6 +1103,7 @@ impl GraphClient { BodyLimit::First(READ_ANSWER_MAX), ) .await + .map_err(String::from) } /// Sends with the token held now, keeping only what `limit` says of the body: the rest is @@ -1083,7 +1118,7 @@ impl GraphClient { headers: &[(&str, &str)], timeout: Duration, limit: BodyLimit, - ) -> Result { + ) -> Result { let head = method == Method::HEAD; let (builder, carried) = self.request(method, &url); let mut builder = builder.timeout(timeout); @@ -1100,7 +1135,7 @@ impl GraphClient { } let request = builder .body(body.map(AsyncBody::from).unwrap_or_else(AsyncBody::empty)) - .map_err(|error| error.to_string())?; + .map_err(|error| NoAnswer::Unsendable(error.to_string()))?; let response = self.dispatch(&url, request).await?; let status = response.status(); let contract = response @@ -1128,7 +1163,7 @@ impl GraphClient { let length = body_length(head, status, &headers); let bytes = read_body(response.into_body(), skip, keep, length) .await - .map_err(|error| describe_transport(&error.into()))?; + .map_err(|error| NoAnswer::Lost(describe_transport(&error.into())))?; Ok(RawResponse { carried, status, @@ -1201,7 +1236,7 @@ impl GraphClient { limit, ) .await - .map_err(ReadError::Transport)? + .map_err(|no| ReadError::Transport(no.into()))? }; // A read changes nothing, so a rate-limited one waits as asked and goes again. if raw.status == StatusCode::TOO_MANY_REQUESTS && retries < READ_RETRIES { @@ -1483,7 +1518,7 @@ impl GraphClient { ) -> WriteOutcome { match self.send(method, url, body, headers, WRITE_TIMEOUT).await { Ok(raw) => classify_write(raw), - Err(reason) => WriteOutcome::Unknown { reason }, + Err(no) => unanswered(no), } } @@ -1498,7 +1533,7 @@ impl GraphClient { let url = self.url(segments, query); let raw = match self.send(method, url, body, headers, WRITE_TIMEOUT).await { Ok(raw) => raw, - Err(reason) => return WriteOutcome::Unknown { reason }, + Err(no) => return unanswered(no), }; classify_write(raw) } @@ -1510,7 +1545,7 @@ impl GraphClient { /// ways and there is no total timeout, as the CLI sets none; a write is sent once and never /// retried (constitution III). `path_and_query` is below the server root, without a leading /// slash; a path that would leave the root is refused. - pub async fn forward(&self, request: ForwardRequest) -> Result { + pub async fn forward(&self, request: ForwardRequest) -> Result { let path = request .path_and_query .split(['?', '#']) @@ -1523,18 +1558,24 @@ impl GraphClient { .any(|segment| segment == ".." || segment == ".") || path.contains('\\') { - return Err(format!("refused path {:?}", request.path_and_query)); + return Err(NoAnswer::Unsendable(format!( + "refused path {:?}", + request.path_and_query + ))); } let url = self .base .join(&request.path_and_query) - .map_err(|error| error.to_string())?; + .map_err(|error| NoAnswer::Unsendable(error.to_string()))?; // The server receives exactly the path judged here: a join that resolves `%2e%2e`, // drops a tab or encodes a byte would send another. if !url.as_str().starts_with(self.base.as_str()) || url.path() != format!("{}{path}", self.base.path()) { - return Err(format!("refused path {:?}", request.path_and_query)); + return Err(NoAnswer::Unsendable(format!( + "refused path {:?}", + request.path_and_query + ))); } self.fresh_token().await; let (mut builder, _) = self.request(request.method, &url); @@ -1543,7 +1584,7 @@ impl GraphClient { } let request = builder .body(request.body) - .map_err(|error| error.to_string())?; + .map_err(|error| NoAnswer::Unsendable(error.to_string()))?; let response = self.dispatch(&url, request).await?; Ok(ForwardResponse { status: response.status(), @@ -2029,15 +2070,15 @@ pub fn judge_write<'a>( status: StatusCode, contract: Option<&str>, body: &'a [u8], -) -> WriteOutcome<&'a [u8]> { +) -> Answered<&'a [u8]> { let contract_ok = speaks_contract(contract); if status.is_success() { if !contract_ok { - return WriteOutcome::Unknown { + return Answered::Unknown { reason: "the response did not carry the contract header".into(), }; } - return WriteOutcome::Committed(body); + return Answered::Committed(body); } if contract_ok && let Ok(stated) = serde_json::from_slice::(body) { let effects = stated.disposition.map(|disposition| disposition.effects); @@ -2050,22 +2091,20 @@ pub fn judge_write<'a>( ..ErrorOutput::default() }); return match effects { - Some(DispositionEffects::None) => WriteOutcome::Refused(ApiError { + Some(DispositionEffects::None) => Answered::Refused(ApiError { status: status.as_u16(), body, }), - Some(DispositionEffects::Unknown | DispositionEffects::Partial) => { - WriteOutcome::Unknown { - reason: format!( - "the server answered {status} and could not say whether the change was \ + Some(DispositionEffects::Unknown | DispositionEffects::Partial) => Answered::Unknown { + reason: format!( + "the server answered {status} and could not say whether the change was \ made: {}", - body.error - ), - } - } + body.error + ), + }, // An effect this build does not know, or none: the contract states one on every // error, so an answer without one cannot rule the change out. - Some(DispositionEffects::Other) | None => WriteOutcome::Unknown { + Some(DispositionEffects::Other) | None => Answered::Unknown { reason: format!( "the server answered {status} without saying whether the change was made: {}", body.error @@ -2073,7 +2112,7 @@ pub fn judge_write<'a>( }, }; } - WriteOutcome::Unknown { + Answered::Unknown { reason: format!("the server answered {status}"), } } @@ -2093,14 +2132,41 @@ struct Stated { /// unknown. fn classify_write(raw: RawResponse) -> WriteOutcome { match judge_write(raw.status, raw.contract.as_deref(), &raw.body) { - WriteOutcome::Committed(body) => match serde_json::from_slice(body) { + Answered::Committed(body) => match serde_json::from_slice(body) { Ok(value) => WriteOutcome::Committed(value), Err(error) => WriteOutcome::Unknown { reason: format!("the success response could not be read: {error}"), }, }, - WriteOutcome::Refused(error) => WriteOutcome::Refused(error), - WriteOutcome::Unknown { reason } => WriteOutcome::Unknown { reason }, + Answered::Refused(error) => WriteOutcome::Refused(error), + Answered::Unknown { reason } => WriteOutcome::Unknown { reason }, + } +} + +/// Why a request the transport failed has no answer, by the error's kind and never its words: +/// a connection the server refused carried no request ([`NoAnswer::Unreached`]); any other +/// failure may have come after the request went out ([`NoAnswer::Lost`]). +fn unanswered_by_transport(error: &anyhow::Error) -> NoAnswer { + let refused = error.chain().any(|layer| { + layer + .downcast_ref::() + .is_some_and(|io| io.kind() == std::io::ErrorKind::ConnectionRefused) + }); + let why = describe_transport(error); + if refused { + NoAnswer::Unreached(why) + } else { + NoAnswer::Lost(why) + } +} + +/// A write with no answer: never sent, so nothing was written, or lost, so it may have been. +fn unanswered(no: NoAnswer) -> WriteOutcome { + match no { + NoAnswer::Retired(reason) | NoAnswer::Unsendable(reason) | NoAnswer::Unreached(reason) => { + WriteOutcome::NotSent { reason } + } + NoAnswer::Lost(reason) => WriteOutcome::Unknown { reason }, } } @@ -2379,9 +2445,9 @@ mod tests { Some(crate::CONTRACT), body.as_bytes(), ) { - WriteOutcome::Committed(_) => "committed", - WriteOutcome::Refused(_) => "refused", - WriteOutcome::Unknown { .. } => "unknown", + Answered::Committed(_) => "committed", + Answered::Refused(_) => "refused", + Answered::Unknown { .. } => "unknown", } }; assert_eq!(judge(500, Some("none")), "refused", "nothing was done"); @@ -2414,14 +2480,14 @@ mod tests { Some(crate::CONTRACT), unread.as_bytes(), ) { - WriteOutcome::Refused(refusal) => assert_eq!(refusal.body.error, "no"), + Answered::Refused(refusal) => assert_eq!(refusal.body.error, "no"), _ => panic!("a refusal whose details this build does not read"), } // Without the contract header the server did not answer, whatever the body says. let body = error(Some("none")); assert!(matches!( judge_write(StatusCode::INTERNAL_SERVER_ERROR, None, body.as_bytes()), - WriteOutcome::Unknown { .. } + Answered::Unknown { .. } )); } @@ -3176,6 +3242,11 @@ mod tests { held.retire("the target is gone".into()); assert_eq!(gateway.credential_changes(), changes + 2); let refused = block_on(gateway.schema("g")).unwrap_err(); + // A write is not sent either, and says so: nothing was written. + assert!(matches!( + block_on(gateway.mutate("g", &ChangeRequest::default(), None)), + WriteOutcome::NotSent { reason } if reason == "the target is gone" + )); assert!( format!("{refused:?}").contains("the target is gone"), "{refused:?}" @@ -3188,6 +3259,34 @@ mod tests { ); } + /// A write whose connection the server refused was never sent, and says so: nothing was + /// written. One whose connection failed otherwise (reset) may have landed after it went out, + /// and so may one whose failure only says "connection refused" in words: the error's kind + /// decides, not its text. Before, every transport failure was an answer lost. + #[test] + fn a_write_the_server_refused_to_connect_to_is_not_sent() { + let failing = |kind: Option| { + client(FakeHttpClient::create(move |_| async move { + Err(match kind { + Some(kind) => anyhow::Error::from(std::io::Error::from(kind)) + .context("error sending request"), + None => anyhow::anyhow!("connection refused, in words alone"), + }) + })) + }; + let write = + |client: GraphClient| block_on(client.mutate("g", &ChangeRequest::default(), None)); + assert!(matches!( + write(failing(Some(std::io::ErrorKind::ConnectionRefused))), + WriteOutcome::NotSent { reason } if reason == "the connection was refused" + )); + assert!(matches!( + write(failing(Some(std::io::ErrorKind::ConnectionReset))), + WriteOutcome::Unknown { .. } + )); + assert!(matches!(write(failing(None)), WriteOutcome::Unknown { .. })); + } + /// Stored queries: the catalog decodes (a kind Studio does not know is `Other`), a read /// says it expects a read, and a write says it expects a write, drops any snapshot and, bound /// to a commit, goes to the conditional route with the commit in its header. The name is one @@ -3359,7 +3458,10 @@ mod tests { headers: Vec::new(), body: AsyncBody::empty(), })); - assert!(refused.is_err(), "{path} was sent"); + assert!( + matches!(refused, Err(NoAnswer::Unsendable(_))), + "{path} was sent" + ); } assert!(seen.lock().unwrap().is_empty()); } diff --git a/crates/graph_client/src/graph_client.rs b/crates/graph_client/src/graph_client.rs index 82ff00d4..70274794 100644 --- a/crates/graph_client/src/graph_client.rs +++ b/crates/graph_client/src/graph_client.rs @@ -14,8 +14,8 @@ mod client; pub use api::{CommitChanges, ErrorOutput}; pub use client::{ ApiErrorSummary, BlobAt, BlobCell, BlobMeta, CHANGE_READ_TIMEOUT, Challenge, FeedAt, - ForwardRequest, ForwardResponse, GraphClient, HistoricalGap, MergePreview, PropertyThen, - ReadError, ServerHealth, judge_write, server_root, + ForwardRequest, ForwardResponse, GraphClient, HistoricalGap, MergePreview, NoAnswer, + PropertyThen, ReadError, ServerHealth, judge_write, server_root, }; pub use http_client::{ AsyncBody, @@ -49,11 +49,30 @@ pub enum WriteOutcome { Committed(T), /// The server refused the change; nothing was written. Refused(ApiError), + /// Studio never sent the write: its client was retired ([`GraphClient::retire`]), the + /// server refused the connection, or the request could not be made. A refusal in the + /// constitution's sense (III): nothing was written, and there is nothing to settle, as with + /// [`Self::Refused`], though no server answered. + NotSent { reason: String }, /// The response was lost, timed out, or lacked the contract header. Settle it by reading /// history, never by sending the write again. Unknown { reason: String }, } +/// What a server's answer to a write says of it ([`judge_write`]): an answer means the write was +/// sent, so it was made, refused, or may have been made. [`WriteOutcome`] adds the write never +/// sent. +#[expect(clippy::large_enum_variant)] +#[derive(Debug)] +pub enum Answered { + /// The server published the change. + Committed(T), + /// The server refused the change; nothing was written. + Refused(ApiError), + /// The answer cannot say whether the change was made. + Unknown { reason: String }, +} + /// A refusal from the server: the HTTP status and the parsed `ErrorOutput` body. #[derive(Debug)] pub struct ApiError { diff --git a/crates/graph_store/src/registry.rs b/crates/graph_store/src/registry.rs index 348262b7..279b1ca2 100644 --- a/crates/graph_store/src/registry.rs +++ b/crates/graph_store/src/registry.rs @@ -1897,6 +1897,34 @@ mod tests { } } + /// A write to a server that refuses the connection, through Studio's own transport, is not + /// sent: the refusal reaches the client as a refused connection, which it judges by the + /// error's kind, so nothing was written and nothing is to be settled. + #[test] + fn a_write_to_a_server_refusing_connections_is_not_sent() { + // A port nothing listens on: bound, then let go. + let port = std::net::TcpListener::bind("127.0.0.1:0") + .unwrap() + .local_addr() + .unwrap() + .port(); + let http: Arc = + Arc::new(reqwest_client::ReqwestClient::user_agent(crate::USER_AGENT).unwrap()); + let client = + graph_client::GraphClient::new(&format!("http://127.0.0.1:{port}"), None, http) + .unwrap(); + let outcome = + smol::block_on(client.mutate("g", &graph_client::api::ChangeRequest::default(), None)); + assert!( + matches!( + &outcome, + graph_client::WriteOutcome::NotSent { reason } + if reason == "the connection was refused" + ), + "{outcome:?}" + ); + } + /// A CLI target that trusts its own certificate authority is reached with it, as the CLI /// reaches it: trusted with the system's roots (`append`) or alone (`replace`). Studio's own /// client, which trusts the system's roots, is refused at the handshake, and so is a bundle diff --git a/crates/graph_store/src/session.rs b/crates/graph_store/src/session.rs index 4d7a262f..6dcaec13 100644 --- a/crates/graph_store/src/session.rs +++ b/crates/graph_store/src/session.rs @@ -4271,6 +4271,9 @@ fn outcome_kind( WriteOutcome::Refused(error) => OutcomeKind::Refused { message: error.body.error.clone(), }, + WriteOutcome::NotSent { reason } => OutcomeKind::Refused { + message: reason.clone(), + }, WriteOutcome::Unknown { reason } => OutcomeKind::Unknown { reason: reason.clone(), }, @@ -4330,6 +4333,20 @@ mod tests { use crate::cli_tokens::{NoToken, Token, TokenSource}; use crate::connection::{ConnectionState, ServerEntry, ServerSource}; + /// A write never sent is logged as refused, with why: nothing was written. + #[test] + fn a_write_never_sent_is_logged_as_refused() { + let outcome: WriteOutcome = WriteOutcome::NotSent { + reason: "the target is gone".into(), + }; + assert_eq!( + outcome_kind(&outcome, |commit: &CommitOutput| Some(commit)), + OutcomeKind::Refused { + message: "the target is gone".into() + } + ); + } + /// A scope on main showing answers that depend on these node and edge types. fn watching(session: &mut GraphSession, nodes: &[&str], edges: &[&str]) -> ReadScope { let scope = session.read_scope("main"); diff --git a/crates/query_editor/src/view.rs b/crates/query_editor/src/view.rs index df2c8e25..dedb9575 100644 --- a/crates/query_editor/src/view.rs +++ b/crates/query_editor/src/view.rs @@ -281,6 +281,11 @@ enum WriteResult { message: String, diagnostic: Option, }, + /// The write was never sent (its connection retired, the server refusing the connection, + /// or the request not made): nothing was written. + NotSent { + reason: String, + }, Unknown { reason: String, }, @@ -3048,6 +3053,10 @@ impl QueryEditor { text: "Refused · nothing written".into(), failed: true, }, + WriteOutcome::NotSent { .. } => RunNote { + text: "Not sent · nothing written".into(), + failed: true, + }, WriteOutcome::Unknown { .. } => RunNote { text: "Outcome unknown · check History".into(), failed: true, @@ -3074,6 +3083,7 @@ impl QueryEditor { .and_then(|d| server_diagnostic(d, &sent_text, map)), message: error.body.error, }, + WriteOutcome::NotSent { reason } => WriteResult::NotSent { reason }, WriteOutcome::Unknown { reason } => WriteResult::Unknown { reason }, }); this.replace_output(written, cx); @@ -4518,6 +4528,15 @@ impl QueryEditor { diagnostic.clone(), cx, ), + WriteResult::NotSent { reason } => self.render_message( + IconName::XCircle, + Color::Error, + format!("Not sent: {reason}"), + Some("Nothing was written.".into()), + Vec::new(), + None, + cx, + ), WriteResult::Unknown { reason } => self.render_message( IconName::Warning, Color::Warning, diff --git a/crates/review/src/branches.rs b/crates/review/src/branches.rs index 74608299..a0dde30d 100644 --- a/crates/review/src/branches.rs +++ b/crates/review/src/branches.rs @@ -598,6 +598,9 @@ impl CreateBranchModal { this.error = Some(format!("Not created: {}", error.body.error).into()) } }, + WriteOutcome::NotSent { reason } => { + this.error = Some(format!("Not created: {reason}").into()) + } WriteOutcome::Unknown { .. } => { this.error = Some( "The outcome is unknown. Check the branch list before trying again." @@ -895,6 +898,9 @@ impl DeleteBranchModal { WriteOutcome::Refused(error) => { this.error = Some(format!("Not deleted: {}", error.body.error).into()) } + WriteOutcome::NotSent { reason } => { + this.error = Some(format!("Not deleted: {reason}").into()) + } WriteOutcome::Unknown { .. } => this.error = Some( "The outcome is unknown. Check the branch list before trying again." .into(), diff --git a/crates/review/src/view.rs b/crates/review/src/view.rs index 2b58006b..9fdfb935 100644 --- a/crates/review/src/view.rs +++ b/crates/review/src/view.rs @@ -67,12 +67,19 @@ const FORK_BOUND: usize = FORK_PAGE as usize * FORK_PAGES; enum FixState { Idle, Running, - Reverted { commit: String }, + Reverted { + commit: String, + }, Saving, - Edited { commit: String }, + Edited { + commit: String, + }, Unchanged, Moved, Refused(String), + /// Never sent (its connection retired, the server refusing the connection, or the request + /// not made): nothing was written. + NotSent(String), Unknown, } @@ -152,6 +159,9 @@ enum MergeState { }, Conflicts(Vec), Refused(String), + /// Never sent (its connection retired, the server refusing the connection, or the request + /// not made): nothing was merged, and nothing is to be settled. + NotSent(String), Unknown { reason: String, /// What the target's history shows of it. @@ -1808,6 +1818,7 @@ impl ReviewView { } WriteOutcome::Refused(error) if error.status == 412 => FixState::Moved, WriteOutcome::Refused(error) => FixState::Refused(error.body.error), + WriteOutcome::NotSent { reason } => FixState::NotSent(reason), WriteOutcome::Unknown { .. } => FixState::Unknown, }; let landed = matches!( @@ -2135,6 +2146,7 @@ impl ReviewView { Some(conflicts) if !conflicts.is_empty() => MergeState::Conflicts(conflicts), _ => MergeState::Refused(error.body.error), }, + WriteOutcome::NotSent { reason } => MergeState::NotSent(reason), WriteOutcome::Unknown { reason } => { let sent = MergeSent { source_head: bound, @@ -2761,6 +2773,11 @@ impl ReviewView { format!("The merge was refused: {message}. Nothing was merged."), None, ), + MergeState::NotSent(reason) => ( + Severity::Error, + format!("The merge was not sent: {reason}. Nothing was merged."), + None, + ), MergeState::Unknown { reason, landed, .. } => ( Severity::Warning, unknown_merge_words(reason, *landed, &self.target), @@ -3213,6 +3230,10 @@ fn fix_status(fix: &FixState) -> Option<(String, Color)> { Color::Warning, ), FixState::Refused(message) => (format!("Refused: {message}"), Color::Error), + FixState::NotSent(reason) => ( + format!("Not sent: {reason}. Nothing was written."), + Color::Error, + ), FixState::Unknown => ( "Outcome unknown. Check History before trying again.".to_owned(), Color::Warning, diff --git a/crates/type_table/src/blob_write.rs b/crates/type_table/src/blob_write.rs index 73199908..0a710627 100644 --- a/crates/type_table/src/blob_write.rs +++ b/crates/type_table/src/blob_write.rs @@ -530,6 +530,10 @@ async fn written( }; BlobWritten::Refused(Notice::new(Tone::Refused, message)) } + WriteOutcome::NotSent { reason } => BlobWritten::Refused(Notice::new( + Tone::Refused, + format!("Not sent: {reason}. Nothing was written."), + )), WriteOutcome::Unknown { .. } => BlobWritten::Unknown(Notice::new( Tone::Unknown, "The server's answer was lost, so the value may have changed. Check History \ @@ -787,6 +791,25 @@ mod tests { #[gpui::test] async fn a_blob_write_held_off_its_branch_is_not_saved(cx: &mut gpui::TestAppContext) { let (session, _) = crate::test_support::session_with_blobs(NOTES, cx); + // A write never sent wrote nothing, and says why. + let task = cx.update(|cx| { + let session = session.clone(); + cx.spawn(async move |cx| { + let outcome = WriteOutcome::NotSent { + reason: "the target is gone".into(), + }; + written(outcome, &session, "main", &cell(false), true, cx).await + }) + }); + cx.run_until_parked(); + let BlobWritten::Refused(notice) = futures::executor::block_on(task) else { + panic!("a write never sent is not saved"); + }; + assert_eq!( + notice.message, + "Not sent: the target is gone. Nothing was written." + ); + let receipt = |kind: &str| -> BlobWriteOutput { serde_json::from_value(serde_json::json!({ "selector": {"entity": "node", "type": "Note", "id": "n1", "property": "attachment"}, diff --git a/crates/type_table/src/model.rs b/crates/type_table/src/model.rs index 3b650ea2..14e12f62 100644 --- a/crates/type_table/src/model.rs +++ b/crates/type_table/src/model.rs @@ -953,6 +953,10 @@ pub fn save_result(outcome: &WriteOutcome, branch: &str) -> SaveRe error.body.precondition_failure.is_some(), branch, )), + WriteOutcome::NotSent { reason } => SaveResult::Refused(Notice::new( + Tone::Refused, + format!("Not sent: {reason}. Nothing was written."), + )), WriteOutcome::Unknown { .. } => { SaveResult::Unknown(Notice::new(Tone::Unknown, UNKNOWN_MESSAGE)) } @@ -1666,6 +1670,19 @@ mod tests { ), SaveResult::Unknown(Notice::new(Tone::Unknown, UNKNOWN_MESSAGE)) ); + // Never sent (the target retired): nothing was written, said as a refusal, not unknown. + assert_eq!( + save_result( + &WriteOutcome::NotSent { + reason: "The CLI no longer lists the target".into() + }, + "main" + ), + SaveResult::Refused(Notice::new( + Tone::Refused, + "Not sent: The CLI no longer lists the target. Nothing was written." + )) + ); assert_eq!( changed_elsewhere(None, Some(&json!("CEO"))).message, "Changed elsewhere to “CEO” while you were editing" diff --git a/crates/type_table/src/write.rs b/crates/type_table/src/write.rs index 215307d8..6da54e80 100644 --- a/crates/type_table/src/write.rs +++ b/crates/type_table/src/write.rs @@ -27,8 +27,9 @@ pub(crate) fn change_refusal( .refusal(AccessAction::Change, &AccessScope::branch(branch)) } -/// How a write bound to the values it was read at ended: sent, with the server's outcome, or -/// not sent. +/// How a write bound to the values it was read at ended: the write's own outcome, never sent +/// among them (`Sent`), or held back before it was sent, as the values it is bound to moved or +/// could not be read. // Made once per write and matched at once, into the outcome: boxing it saves nothing. #[expect(clippy::large_enum_variant)] pub(crate) enum Bound { diff --git a/specs/004-type-tables/plan.md b/specs/004-type-tables/plan.md index 552ead16..4d4e4c5d 100644 --- a/specs/004-type-tables/plan.md +++ b/specs/004-type-tables/plan.md @@ -431,7 +431,9 @@ its open section. a proposal or a snapshot, is not a save: "main did not change" with the commit, the draft kept; a kind Studio does not know is an unknown outcome (both judged by `OutcomeKind::written`, for cell saves, drafts, bulk sets and Blob writes, since 2026-10-21). A 4xx with `ErrorOutput`: -refused, "Nothing was written." plus the reason. A lost response, a timeout or a response +refused, "Nothing was written." plus the reason. A write never sent (its connection retired, +the server refusing the connection, or the request not made): "Not sent:" and why, "Nothing +was written." (since 2026-10-30). A lost response, a timeout or a response without the contract header: unknown, "Check History before trying again; your changes are still here", with a button that opens the branch's recent commits. diff --git a/specs/007-query-editor/plan.md b/specs/007-query-editor/plan.md index 103d0ab6..33bca493 100644 --- a/specs/007-query-editor/plan.md +++ b/specs/007-query-editor/plan.md @@ -155,7 +155,8 @@ a commit), VII (mutations never run by accident), XII (spike S4 decides the edit declares any more goes once a check reads every declaration. - A run line above each declaration (a Zed editor block anchored to the declaration, rebuilt when the declarations change): ▶ Run, then that declaration's last outcome ("25 rows · 96 - ms", "Committed · 1 nodes, 0 edges", "Refused", "Outcome unknown · check History"). A line, + ms", "Committed · 1 nodes, 0 edges", "Refused", "Not sent · nothing written", "Outcome unknown · + check History"). A line, not a gutter glyph: Zed's gutter run indicators belong to its task system, which needs a project. A declaration's range excludes its end, so the line above a declaration runs it even when the one before it lacks its `}` and runs on to it. @@ -208,8 +209,11 @@ a commit), VII (mutations never run by accident), XII (spike S4 decides the edit on no branch" for `as unattached branch`; "Created branch x at commit ", "Deleted branch x", "Merged a into b" (or already up to date), "Published commit to b" and "Dropped commit " for statements, which report no counts; "Nothing changed" for no - commit. Refused ("Nothing was written", with the server's diagnostic) or unknown ("Check - History before trying again") as before. The text is the draft and is never cleared. + commit. Refused ("Nothing was written", with the server's diagnostic), not sent ("Not sent:" + and why, "Nothing was written.", when the connection is retired (its target gone, or a + transport Studio cannot use), the server refused the connection, or the request could not be + made; the run reads "Not sent · nothing written") or unknown ("Check History before + trying again") as before. The text is the draft and is never cleared. Closing the tab does not cancel a write in flight; the session logs its outcome. - Parameters of the query under the cursor get an input each; values are typed by `gq_language::param_value` and sent as JSON (`null` for an empty nullable). diff --git a/specs/008-branches-review-merge/plan.md b/specs/008-branches-review-merge/plan.md index ebca9c60..b489c179 100644 --- a/specs/008-branches-review-merge/plan.md +++ b/specs/008-branches-review-merge/plan.md @@ -164,7 +164,11 @@ place), X (designed on 0.13; the better routes are deferred). fast-forward"; conflicts show in a banner with Show conflicts, the same list a merge's 409 gives. A dry run that cannot be read says nothing; Merge stays available either way, since the merge itself decides. -- Outcome shown plainly: fast-forwarded, merged (with commit), already up to date. +- Outcome shown plainly: fast-forwarded, merged (with commit), already up to date. A merge or a + fix never sent (its connection retired, the server refusing the connection, or the request + not made) says so, "The merge was not sent: . Nothing was merged.", and leaves nothing + to settle (since 2026-10-30; before, a merge or a fix that failed to send read as an unknown + outcome, and a merge's was settled by reading the target's history). - A 409 opens a conflicts view: one row per conflict with its kind in words ("Changed on both branches", "Deleted on main, changed here", "Relationship points to a deleted node"), the entity, and the message. Resolution happens on the source branch with ordinary edits; then diff --git a/specs/013-agent-cli/plan.md b/specs/013-agent-cli/plan.md index 4db80ec9..fbba9241 100644 --- a/specs/013-agent-cli/plan.md +++ b/specs/013-agent-cli/plan.md @@ -438,7 +438,15 @@ ones, each let go within 10 s and opened again, locked the CLI out). Writes are checked on the conversation's graph only. A refusal is a 403 `ErrorOutput` with `code: forbidden` and the contract header; its message ends "Nothing was sent." and says what to do instead. A CLI with E22 (Omnigraph #981) reports it as not started; an older one still says -`effects: unknown`, so `AGENTS.md` says a Studio refusal never wrote anything. +`effects: unknown`, so `AGENTS.md` says a Studio refusal never wrote anything. A request Studio +never sends is refused the same way: one whose connection is retired (its CLI target gone, or a +transport Studio cannot use) says to tell the person, and one Studio would not make as asked (a +path that leaves the server's root) says to change it, or to tell the person. One whose server +refused the connection is answered 503 `service_unavailable` instead, with +`disposition: {effects: none, action: retry}`, and says to try again once the server answers. A write to a target already retired is refused so +before it is a card, as a rule's refusal is: the person is not asked about a write that cannot +go. One whose target retires while the person decides keeps its card, which then reads "..., so +nothing was sent." A forwarded write's answer is read whole (at most 8 MiB) so the card can show it, then sent on. When the server's answer is lost, the gateway closes the connection without one, and the CLI diff --git a/specs/016-oauth-client/plan.md b/specs/016-oauth-client/plan.md index c822f906..c518985a 100644 --- a/specs/016-oauth-client/plan.md +++ b/specs/016-oauth-client/plan.md @@ -559,6 +559,8 @@ same way. | 2026-10-30 | A legacy pass over the files the change touches | `graph_store` `connection::tests` (the probe's failure keeps its challenge; the test that gave it a refusal by hand gives a bare `Bearer` challenge). The workspace's tests pass unchanged | It found no unused function, field, variant or helper. Taken: whether a 401 refused the token sent was decided in two places, the probe's (`token_refused.unwrap_or(token_sent)`) and a graph read's (`is_none_or(token_refused)`, then the token held), the split an earlier row records a defect from; `Challenge::refuses` decides it for both, and the probe's failure keeps its challenge rather than two answers taken from it. Docs in connection.rs, cli_tokens.rs and registry.rs, and the rules of specs 003 and 016, said what the code no longer does: a 401 to an OAuth token refreshing whatever it refused, the source asked after any 401, a renewal always quiet, and a typed token standing until Refresh alone. The token endpoint's last busy arm could not match a server error, which two arms before it took; one arm, decided by `failed_for_now` and the error, keeps the same words. Left: a window's reads that failed under a 401 that refused no token stay failed, each with its own Try again, after the connection's Retry, as after any read the network failed; a sign-in's refresh no longer reads them again, since it would send its token the same way | | 2026-10-30 | A review of the rules' wording | `graph_store` `connection::tests::the_ask_after_a_probes_refusal_is_the_reads_one_more_try` (new). Fails without its fix (checked) | Stating how often a source is asked after 401s found the code asymmetric: the ask after a probe's 401 counted as the graph reads' one more try only when it changed the token, so a source that gave the refused token back was asked again by the next read's 401, one more CLI export or refresh for a token already asked about. Any ask after a refusal is now that one more try, and specs 003 and 016 state the one rule. The same rounds corrected the tooltips, which said a typed token lasts until Studio quits where the CLI is asked again over it, and the docs that still stated the old rules | | 2026-10-30 | A managed target signs in where its cluster offers it, in its profile's organization, and uses the CLI's login elsewhere | `graph_store` `sign_in::tests`: `a_managed_target_signs_in_where_its_cluster_offers_it`, `a_managed_target_uses_the_clis_login_where_its_cluster_offers_no_sign_in`, `a_sign_in_to_a_managed_target_must_be_in_its_organization`; `cli_tokens::tests`: `the_targets_are_listed_as_the_cli_describes_them` (extended), `a_managed_profile_the_cli_cannot_describe_is_said` (new). Live: `omnigraph credentials status --profile managed-og --json` (CLI `0.14.1-edge+1755782`); `GET /graphs` without a token on the production `modernrelay-og-cluster` and on the staging cluster | Pass. A kept sign-in is refreshed with the CLI's login never asked, and with none kept the cluster asks for a sign-in; a cluster whose 401 names no sign-in gets one request without a token, then the CLI's, and the CLI's at once on the next connect; a token in another organization is refused before it is sent. The CLI reports `owner.organization_id` with no secret read. Production answers `Bearer` alone, so it is reached on the CLI's login; staging names `resource_metadata` | +| 2026-10-30 | A write a retired connection refuses is not sent, and said so | `graph_client` `client::tests::a_replaced_http_client_reaches_every_clone` (extended: a write on a retired client is `NotSent`), `a_forward_carries_studios_credentials_and_stays_under_the_root` (its refused paths are `NoAnswer::Unsendable`), `agent_panel` `gateway::tests::a_writes_success_answer_says_what_it_did` (extended: a forwarded write never sent is refused; one whose answer was lost stays unknown), `the_gateway_checks_the_session_then_forwards_reads_and_approved_writes` (extended: once the target is retired, a read and an approved write are answered 403 with nothing sent, and nothing reaches the server), `type_table` `model::tests::classifies_write_outcomes` (extended). Each fails without its fix (checked) | Pass. A write a retired connection refused before sending was reported as of unknown outcome, to be settled by reading a history the same connection could not read, and the agent's CLI saw its connection close. The client now tells a request never sent (`NoAnswer::Retired`, or `NoAnswer::Unsendable`: a refused path, one that could not be made) from one whose answer was lost (`NoAnswer::Lost`); a write never sent is `WriteOutcome::NotSent`, which every window shows as nothing written, the refused outcome of constitution III, and the gateway answers the CLI 403 with nothing sent and what to do: tell the person for a retired target, change it, or tell the person, for one Studio would not make (specs 013 and 007 say so). A write whose answer was lost stays unknown. A review of the whole change found no write that may have reached the server reported as not sent. Taken: `judge_write` answers `Answered`, which has no case for a write never sent, so neither the client nor the gateway keeps an arm that cannot run; a write to a retired target is refused at once, as a read is, rather than the person being asked to approve a write that cannot go (`agent_panel` `gateway::tests::the_gateway_checks_the_session_then_forwards_reads_and_approved_writes`, extended, fails without it); `graph_store` `session::tests::a_write_never_sent_is_logged_as_refused` (new) and `type_table` `blob_write::tests::a_blob_write_held_off_its_branch_is_not_saved` (extended) hold the session's and a Blob write's words for one, each failing when it is mapped to unknown; spec 007 says what the query editor shows for one | +| 2026-10-30 | A review of that change on its final base | `graph_client` `client::tests::a_write_the_server_refused_to_connect_to_is_not_sent` (new). Fails without its fix (checked) | It found no write that may have landed said as not sent. Taken: a write whose connection the server refused (the server down) sent no byte, yet was said as an answer lost that may have changed the value, and a merge so kept Merge off until history could be read; it is not sent (`NoAnswer::Unreached`), judged by the error's kind (an io `ConnectionRefused`), never by its words, which a failure after sending can share. Specs 007 and 013 and the editor's doc said a write is not sent only when the target is gone; a connection is also retired for a transport Studio cannot use, and a refused connection sends nothing. The agent's gateway answers such a request 503 `service_unavailable`, to be tried again once the server answers, not 403 `forbidden`, which tells the agent to change it (`agent_panel` `gateway::tests::a_writes_success_answer_says_what_it_did`, extended). `Bound`'s doc said `Sent` holds a write sent, and a test asserted what every outcome but a commit gives | ## Corrections From cef06d0df56e08bb0fa918ae59287e2df340a592 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Fri, 30 Oct 2026 15:09:15 -0700 Subject: [PATCH 2/2] fix(writes): say a refused connection and a send never made as such everywhere A refused connection is described by its kind as well as judged by it, so Windows reads as other systems do; a fix dropped unsent and a merge whose head could not be read are not sent, not refused; a reason quoted into a sentence that goes on ends once (studio_ui::clause), the toast included; a never-sent write is logged with the gateway's words; a write and a Blob write share one send-and-judge step. --- crates/agent_panel/src/gateway.rs | 18 ++--- crates/graph_client/src/client.rs | 75 ++++++++++++------- crates/graph_store/src/graph_store.rs | 2 +- crates/graph_store/src/session.rs | 14 +++- crates/history/src/view.rs | 3 +- crates/review/src/branches.rs | 7 +- crates/review/src/view.rs | 44 +++++++---- crates/studio/src/activity.rs | 5 +- crates/studio_ui/src/studio_ui.rs | 7 ++ .../studio_workspace/src/studio_workspace.rs | 38 +++++++++- crates/type_table/src/blob_write.rs | 7 +- crates/type_table/src/model.rs | 16 +++- specs/008-branches-review-merge/plan.md | 9 ++- specs/016-oauth-client/plan.md | 1 + 14 files changed, 172 insertions(+), 74 deletions(-) diff --git a/crates/agent_panel/src/gateway.rs b/crates/agent_panel/src/gateway.rs index e8facb31..ff23cb27 100644 --- a/crates/agent_panel/src/gateway.rs +++ b/crates/agent_panel/src/gateway.rs @@ -629,7 +629,7 @@ async fn read_write_answer( // Never sent (the target retired, the connection refused, or a request Studio // would not make): nothing was written, and the CLI is told so. Some(unsent) => ( - Seen::Refused(nothing_sent(&String::from(no))), + Seen::Refused(graph_store::nothing_sent(&String::from(no))), Some(Reply::NotSent(unsent)), ), None => (Seen::Unknown(String::from(no)), None), @@ -680,13 +680,10 @@ async fn read_write_answer( /// The CLI's refusal of a request to a retired target ([`NoAnswer::Retired`]). fn retired_refusal(why: &str) -> String { - format!("Studio: {why}. Tell the person what this says. Nothing was sent.") -} - -/// That a request was never sent, and why ([`NoAnswer::Retired`], [`NoAnswer::Unsendable`], -/// [`NoAnswer::Unreached`]), as its card says it. -fn nothing_sent(why: &str) -> String { - format!("{why}, so nothing was sent.") + format!( + "Studio: {}. Tell the person what this says. Nothing was sent.", + studio_ui::clause(why) + ) } /// How the CLI is answered for a request never sent ([`answer`]): its status, code and message. @@ -711,8 +708,9 @@ fn unsent_refusal(no: &NoAnswer) -> Option { match no { NoAnswer::Retired(why) => Some(forbidden(retired_refusal(why))), NoAnswer::Unsendable(why) => Some(forbidden(format!( - "Studio: {why}. Change the request, or tell the person what this says. Nothing was \ - sent." + "Studio: {}. Change the request, or tell the person what this says. Nothing was \ + sent.", + studio_ui::clause(why) ))), NoAnswer::Unreached(why) => Some(Unsent { status: 503, diff --git a/crates/graph_client/src/client.rs b/crates/graph_client/src/client.rs index 3847efde..19b8627a 100644 --- a/crates/graph_client/src/client.rs +++ b/crates/graph_client/src/client.rs @@ -1488,7 +1488,7 @@ impl GraphClient { let url = self.blob_url(graph, cell, BlobAt::Branch(branch)); let mut headers = vec![("content-type", "application/octet-stream")]; headers.extend(if_match.map(|tag| ("if-match", tag))); - self.send_blob_write(Method::PUT, url, Some(bytes), &headers) + self.send_write(Method::PUT, url, Some(bytes), &headers) .await } @@ -1505,37 +1505,33 @@ impl GraphClient { let url = self.blob_url(graph, cell, BlobAt::Branch(branch)); let headers: Vec<(&str, &str)> = if_match.map(|tag| ("if-match", tag)).into_iter().collect(); - self.send_blob_write(Method::DELETE, url, None, &headers) - .await + self.send_write(Method::DELETE, url, None, &headers).await } - async fn send_blob_write( + async fn write( &self, method: Method, - url: Url, + segments: &[&str], + query: &[(&str, &str)], body: Option>, headers: &[(&str, &str)], - ) -> WriteOutcome { - match self.send(method, url, body, headers, WRITE_TIMEOUT).await { - Ok(raw) => classify_write(raw), - Err(no) => unanswered(no), - } + ) -> WriteOutcome { + self.send_write(method, self.url(segments, query), body, headers) + .await } - async fn write( + /// Sends a write once and judges its answer, or says why it has none. + async fn send_write( &self, method: Method, - segments: &[&str], - query: &[(&str, &str)], + url: Url, body: Option>, headers: &[(&str, &str)], ) -> WriteOutcome { - let url = self.url(segments, query); - let raw = match self.send(method, url, body, headers, WRITE_TIMEOUT).await { - Ok(raw) => raw, - Err(no) => return unanswered(no), - }; - classify_write(raw) + match self.send(method, url, body, headers, WRITE_TIMEOUT).await { + Ok(raw) => classify_write(raw), + Err(no) => unanswered(no), + } } // ---- public reads ---- @@ -2147,19 +2143,25 @@ fn classify_write(raw: RawResponse) -> WriteOutcome { /// a connection the server refused carried no request ([`NoAnswer::Unreached`]); any other /// failure may have come after the request went out ([`NoAnswer::Lost`]). fn unanswered_by_transport(error: &anyhow::Error) -> NoAnswer { - let refused = error.chain().any(|layer| { - layer - .downcast_ref::() - .is_some_and(|io| io.kind() == std::io::ErrorKind::ConnectionRefused) - }); let why = describe_transport(error); - if refused { + if connection_refused(error) { NoAnswer::Unreached(why) } else { NoAnswer::Lost(why) } } +/// Whether the server refused the connection, by the error's kind: an io `ConnectionRefused` +/// anywhere in its chain, whatever its text says (Windows words it "No connection could be made +/// because the target machine actively refused it"). +fn connection_refused(error: &anyhow::Error) -> bool { + error.chain().any(|layer| { + layer + .downcast_ref::() + .is_some_and(|io| io.kind() == std::io::ErrorKind::ConnectionRefused) + }) +} + /// A write with no answer: never sent, so nothing was written, or lost, so it may have been. fn unanswered(no: NoAnswer) -> WriteOutcome { match no { @@ -2251,7 +2253,9 @@ fn describe_transport(error: &anyhow::Error) -> String { } else { Some(REQUEST_TIMED_OUT) } - } else if all.contains("connection refused") { + } else if connection_refused(error) { + // By its kind alone: a failure that only says so in words may come after sending, and + // is not said as one that sent nothing. Some("the connection was refused") } else if all.contains("dns error") || all.contains("lookup address") @@ -3280,11 +3284,28 @@ mod tests { write(failing(Some(std::io::ErrorKind::ConnectionRefused))), WriteOutcome::NotSent { reason } if reason == "the connection was refused" )); + // Said the same where the system words it otherwise, as Windows does (error 10061). + let windows = client(FakeHttpClient::create(|_| async { + Err(anyhow::Error::from(std::io::Error::new( + std::io::ErrorKind::ConnectionRefused, + "No connection could be made because the target machine actively refused it. \ + (os error 10061)", + )) + .context("tcp connect error")) + })); + assert!(matches!( + write(windows), + WriteOutcome::NotSent { reason } if reason == "the connection was refused" + )); assert!(matches!( write(failing(Some(std::io::ErrorKind::ConnectionReset))), WriteOutcome::Unknown { .. } )); - assert!(matches!(write(failing(None)), WriteOutcome::Unknown { .. })); + // Said in its own words: nothing here says it was not sent. + assert!(matches!( + write(failing(None)), + WriteOutcome::Unknown { reason } if reason == "connection refused, in words alone" + )); } /// Stored queries: the catalog decodes (a kind Studio does not know is `Other`), a read diff --git a/crates/graph_store/src/graph_store.rs b/crates/graph_store/src/graph_store.rs index c874f477..af0aac92 100644 --- a/crates/graph_store/src/graph_store.rs +++ b/crates/graph_store/src/graph_store.rs @@ -63,7 +63,7 @@ pub mod cli { pub use session::{ BranchStart, DraftEntry, DraftKey, ExternalWrite, FollowUp, GraphSession, HeadWatch, HeldAs, Loadable, MergeEntry, MergeSent, MergeSettled, OutcomeEntry, OutcomeKind, ServerState, - SessionEvent, open_session, open_session_behind, + SessionEvent, nothing_sent, open_session, open_session_behind, }; /// The user agent of Studio's requests. diff --git a/crates/graph_store/src/session.rs b/crates/graph_store/src/session.rs index 6dcaec13..18613226 100644 --- a/crates/graph_store/src/session.rs +++ b/crates/graph_store/src/session.rs @@ -4272,7 +4272,7 @@ fn outcome_kind( message: error.body.error.clone(), }, WriteOutcome::NotSent { reason } => OutcomeKind::Refused { - message: reason.clone(), + message: nothing_sent(reason), }, WriteOutcome::Unknown { reason } => OutcomeKind::Unknown { reason: reason.clone(), @@ -4280,6 +4280,13 @@ fn outcome_kind( } } +/// What a write never sent says of itself where its outcome is logged, a refusal in the +/// constitution's sense (nothing was written): why, and that nothing was sent. The agent's +/// gateway says the same of the requests it never sends. +pub fn nothing_sent(why: &str) -> String { + format!("{}, so nothing was sent.", studio_ui::clause(why)) +} + /// The parent of the commit a write's answer names, which `commit` finds: the head below it /// when it is published. fn parent_of( @@ -4333,7 +4340,8 @@ mod tests { use crate::cli_tokens::{NoToken, Token, TokenSource}; use crate::connection::{ConnectionState, ServerEntry, ServerSource}; - /// A write never sent is logged as refused, with why: nothing was written. + /// A write never sent is logged as refused, with why and that nothing was sent: nothing + /// was written. #[test] fn a_write_never_sent_is_logged_as_refused() { let outcome: WriteOutcome = WriteOutcome::NotSent { @@ -4342,7 +4350,7 @@ mod tests { assert_eq!( outcome_kind(&outcome, |commit: &CommitOutput| Some(commit)), OutcomeKind::Refused { - message: "the target is gone".into() + message: "the target is gone, so nothing was sent.".into() } ); } diff --git a/crates/history/src/view.rs b/crates/history/src/view.rs index 11aff568..9d436cb1 100644 --- a/crates/history/src/view.rs +++ b/crates/history/src/view.rs @@ -2186,7 +2186,8 @@ impl Render for HistoryView { .min_w_0() .overflow_hidden() .tooltip(Tooltip::text(format!( - "{error}. The commits shown are as read before." + "{}. The commits shown are as read before.", + studio_ui::clause(error.as_ref()) ))) .child( studio_ui::ds::text::error("The newest commits could not be read") diff --git a/crates/review/src/branches.rs b/crates/review/src/branches.rs index a0dde30d..1b1601c7 100644 --- a/crates/review/src/branches.rs +++ b/crates/review/src/branches.rs @@ -866,9 +866,10 @@ impl DeleteBranchModal { this.error = Some( match &holds { Holds::Unknown(error) => format!( - "Studio could not check {} again: {error}. Type its \ - name to delete it.", - this.branch + "Studio could not check {} again: {}. Type its name to \ + delete it.", + this.branch, + studio_ui::clause(&error.to_string()) ), _ => format!( "{} has a commit {TARGET} does not since this opened; \ diff --git a/crates/review/src/view.rs b/crates/review/src/view.rs index 9fdfb935..721156bf 100644 --- a/crates/review/src/view.rs +++ b/crates/review/src/view.rs @@ -77,8 +77,9 @@ enum FixState { Unchanged, Moved, Refused(String), - /// Never sent (its connection retired, the server refusing the connection, or the request - /// not made): nothing was written. + /// Never sent (its connection retired, the server refusing the connection, the request not + /// made, or dropped because the fix before it did not land or has an unknown outcome): + /// nothing was written. NotSent(String), Unknown, } @@ -159,8 +160,9 @@ enum MergeState { }, Conflicts(Vec), Refused(String), - /// Never sent (its connection retired, the server refusing the connection, or the request - /// not made): nothing was merged, and nothing is to be settled. + /// Never sent (its connection retired, the server refusing the connection, the request not + /// made, or the branch's head not read before it): nothing was merged, and nothing is to be + /// settled. NotSent(String), Unknown { reason: String, @@ -1847,10 +1849,10 @@ impl ReviewView { /// perhaps still there. fn drop_fixes_after(&mut self, key: &str, unknown: bool) { let why = if unknown { - "Not sent: whether the relationship that replaced it was removed is not known. \ - Check History before moving it back." + "whether the relationship that replaced it was removed is not known; check History \ + before moving it back" } else { - "Not sent: the relationship that replaced it was not removed first." + "the relationship that replaced it was not removed first" }; let (dropped, kept): (Vec, Vec) = std::mem::take(&mut self.fix_queue) .into_iter() @@ -1860,7 +1862,7 @@ impl ReviewView { self.fixes_running = self.fixes_running.saturating_sub(1); self.fixes_in_flight.remove(&fix.key); if let Some(card) = self.card_of(&fix.key).and_then(|ix| self.cards.get_mut(ix)) { - card.fix = FixState::Refused(why.into()); + card.fix = FixState::NotSent(why.into()); } } } @@ -2041,7 +2043,7 @@ impl ReviewView { Ok(base) => (Some(base.source_commit_id), Some(base.target_commit_id)), Err(error) => { this.update(cx, |this, cx| { - this.merge = MergeState::Refused(format!( + this.merge = MergeState::NotSent(format!( "Studio could not read {branch}'s head: {error}" )); this.writes_settled(cx); @@ -2730,7 +2732,11 @@ impl ReviewView { let deletion = match deleted { None => String::new(), Some(Ok(())) => format!(" Deleted {}.", self.branch), - Some(Err(why)) => format!(" {} was not deleted: {why}.", self.branch), + Some(Err(why)) => format!( + " {} was not deleted: {}.", + self.branch, + studio_ui::clause(why) + ), }; let beyond = format!("{beyond}{deletion}"); ( @@ -2770,12 +2776,18 @@ impl ReviewView { ), MergeState::Refused(message) => ( Severity::Error, - format!("The merge was refused: {message}. Nothing was merged."), + format!( + "The merge was refused: {}. Nothing was merged.", + studio_ui::clause(message) + ), None, ), MergeState::NotSent(reason) => ( Severity::Error, - format!("The merge was not sent: {reason}. Nothing was merged."), + format!( + "The merge was not sent: {}. Nothing was merged.", + studio_ui::clause(reason) + ), None, ), MergeState::Unknown { reason, landed, .. } => ( @@ -3152,6 +3164,7 @@ fn merged_check_words(verified: Check, target: &str) -> String { /// History that does not show it yet is no proof it will not land: the merge may still be /// running on the server. fn unknown_merge_words(reason: &str, landed: Check, target: &str) -> String { + let reason = studio_ui::clause(reason); match landed { Check::Reading => format!( "The merge outcome is unknown ({reason}). Studio did not retry; reading {target}'s history…" @@ -3231,7 +3244,10 @@ fn fix_status(fix: &FixState) -> Option<(String, Color)> { ), FixState::Refused(message) => (format!("Refused: {message}"), Color::Error), FixState::NotSent(reason) => ( - format!("Not sent: {reason}. Nothing was written."), + format!( + "Not sent: {}. Nothing was written.", + studio_ui::clause(reason) + ), Color::Error, ), FixState::Unknown => ( @@ -4443,7 +4459,7 @@ pub(crate) mod tests { // The first's outcome is unknown, and the second says so: not that it was not // removed. assert!( - matches!(&view.cards[1].fix, FixState::Refused(why) if why.contains("is not known")), + matches!(&view.cards[1].fix, FixState::NotSent(why) if why.contains("is not known")), "{:?}", view.cards[1].fix ); diff --git a/crates/studio/src/activity.rs b/crates/studio/src/activity.rs index 6bc8777d..baccf907 100644 --- a/crates/studio/src/activity.rs +++ b/crates/studio/src/activity.rs @@ -241,7 +241,10 @@ fn said(entry: &OutcomeEntry) -> Said { verb: "has an unknown outcome", commit: None, place: Some(("on", branch)), - tooltip: Some(format!("{reason}. Check History before trying again.")), + tooltip: Some(format!( + "{}. Check History before trying again.", + studio_ui::clause(reason) + )), }, } } diff --git a/crates/studio_ui/src/studio_ui.rs b/crates/studio_ui/src/studio_ui.rs index 19cf44b2..4db129ce 100644 --- a/crates/studio_ui/src/studio_ui.rs +++ b/crates/studio_ui/src/studio_ui.rs @@ -144,6 +144,13 @@ pub fn grid_cell(width: Pixels, align_right: bool) -> Div { .when(align_right, |d| d.justify_end()) } +/// `text` without the one stop that ends it, for a sentence that goes on after it: a message +/// quoted into ". Nothing was written." ends once, whether or not it ended its own +/// sentence. An ellipsis ("...") keeps reading as one. +pub fn clause(text: &str) -> &str { + text.strip_suffix('.').unwrap_or(text) +} + /// How a JSON value from a query result reads in a cell. Null is an em dash; lists show the /// first items then "+N"; objects show their `@id` when they have one. pub fn format_value(value: &Value) -> String { diff --git a/crates/studio_workspace/src/studio_workspace.rs b/crates/studio_workspace/src/studio_workspace.rs index e56fb3ba..74175144 100644 --- a/crates/studio_workspace/src/studio_workspace.rs +++ b/crates/studio_workspace/src/studio_workspace.rs @@ -3058,7 +3058,8 @@ impl Workspace { let mut tooltip = connection_tooltip(&url, &state, actor.as_deref()); if let Some(reason) = &unavailable { tooltip = format!( - "This graph is unavailable: {reason}. Reads resume when it is ready. · {tooltip}" + "This graph is unavailable: {}. Reads resume when it is ready. · {tooltip}", + studio_ui::clause(reason) ); } let trigger = ButtonLike::new("graph-switcher") @@ -4421,8 +4422,9 @@ fn toast_text(entry: &graph_store::OutcomeEntry) -> String { studio_ui::short_commit_id(commit) ), OutcomeKind::Refused { message } => format!( - "{} · refused: {message}. Nothing was written.", - entry.description + "{} · refused: {}. Nothing was written.", + entry.description, + studio_ui::clause(message) ), // Its history is the branch's, which Open history shows. OutcomeKind::Unknown { .. } => format!( @@ -7080,6 +7082,36 @@ mod tests { cx.run_until_parked(); } + /// A refusal's toast ends its message once, whether or not the message ends its own + /// sentence, as a server's and a write never sent ("…, so nothing was sent.") do. Before, those + /// read "… sent.. Nothing was written." + #[test] + fn a_refusal_toast_ends_its_message_once() { + let entry = |message: &str| graph_store::OutcomeEntry { + at: std::time::SystemTime::now(), + branch: "main".into(), + description: "Revert Person p1".into(), + outcome: OutcomeKind::Refused { + message: message.into(), + }, + follow_up: None, + merge: None, + }; + assert_eq!( + toast_text(&entry("the target is gone, so nothing was sent.")), + "Revert Person p1 · refused: the target is gone, so nothing was sent. Nothing was \ + written." + ); + assert_eq!( + toast_text(&entry("the policy denies it")), + "Revert Person p1 · refused: the policy denies it. Nothing was written." + ); + assert_eq!( + toast_text(&entry("the server is busy...")), + "Revert Person p1 · refused: the server is busy... Nothing was written." + ); + } + /// A delete's Undo, which only its toast offers, is offered though another write of the /// graph (a cell's save, the agent's) is still out; so is every other commit. #[gpui::test] diff --git a/crates/type_table/src/blob_write.rs b/crates/type_table/src/blob_write.rs index 0a710627..46fb0fd2 100644 --- a/crates/type_table/src/blob_write.rs +++ b/crates/type_table/src/blob_write.rs @@ -530,10 +530,9 @@ async fn written( }; BlobWritten::Refused(Notice::new(Tone::Refused, message)) } - WriteOutcome::NotSent { reason } => BlobWritten::Refused(Notice::new( - Tone::Refused, - format!("Not sent: {reason}. Nothing was written."), - )), + WriteOutcome::NotSent { reason } => { + BlobWritten::Refused(crate::model::not_sent_notice(&reason)) + } WriteOutcome::Unknown { .. } => BlobWritten::Unknown(Notice::new( Tone::Unknown, "The server's answer was lost, so the value may have changed. Check History \ diff --git a/crates/type_table/src/model.rs b/crates/type_table/src/model.rs index 14e12f62..ab3757d8 100644 --- a/crates/type_table/src/model.rs +++ b/crates/type_table/src/model.rs @@ -934,6 +934,17 @@ pub enum SaveResult { pub const UNKNOWN_MESSAGE: &str = "Check History before trying again; your changes are still here"; +/// What a write never sent says: why, and that nothing was written. +pub(crate) fn not_sent_notice(reason: &str) -> Notice { + Notice::new( + Tone::Refused, + format!( + "Not sent: {}. Nothing was written.", + studio_ui::clause(reason) + ), + ) +} + /// Classifies a write's outcome. `branch` names the write target in a denial. pub fn save_result(outcome: &WriteOutcome, branch: &str) -> SaveResult { match outcome { @@ -953,10 +964,7 @@ pub fn save_result(outcome: &WriteOutcome, branch: &str) -> SaveRe error.body.precondition_failure.is_some(), branch, )), - WriteOutcome::NotSent { reason } => SaveResult::Refused(Notice::new( - Tone::Refused, - format!("Not sent: {reason}. Nothing was written."), - )), + WriteOutcome::NotSent { reason } => SaveResult::Refused(not_sent_notice(reason)), WriteOutcome::Unknown { .. } => { SaveResult::Unknown(Notice::new(Tone::Unknown, UNKNOWN_MESSAGE)) } diff --git a/specs/008-branches-review-merge/plan.md b/specs/008-branches-review-merge/plan.md index b489c179..2f9f05af 100644 --- a/specs/008-branches-review-merge/plan.md +++ b/specs/008-branches-review-merge/plan.md @@ -166,9 +166,12 @@ place), X (designed on 0.13; the better routes are deferred). the merge itself decides. - Outcome shown plainly: fast-forwarded, merged (with commit), already up to date. A merge or a fix never sent (its connection retired, the server refusing the connection, or the request - not made) says so, "The merge was not sent: . Nothing was merged.", and leaves nothing - to settle (since 2026-10-30; before, a merge or a fix that failed to send read as an unknown - outcome, and a merge's was settled by reading the target's history). + not made; a merge whose branch head could not be read; a fix dropped because the one + before it did not land or has an unknown outcome) says so ("The merge was not + sent: . Nothing was merged."; a fix's "Not sent: . Nothing was written."), and + leaves nothing to settle (since 2026-10-30; before, one that failed to send read as an + unknown outcome, a merge's settled by reading the target's history, and the others as + refused). - A 409 opens a conflicts view: one row per conflict with its kind in words ("Changed on both branches", "Deleted on main, changed here", "Relationship points to a deleted node"), the entity, and the message. Resolution happens on the source branch with ordinary edits; then diff --git a/specs/016-oauth-client/plan.md b/specs/016-oauth-client/plan.md index c518985a..93fe724f 100644 --- a/specs/016-oauth-client/plan.md +++ b/specs/016-oauth-client/plan.md @@ -561,6 +561,7 @@ same way. | 2026-10-30 | A managed target signs in where its cluster offers it, in its profile's organization, and uses the CLI's login elsewhere | `graph_store` `sign_in::tests`: `a_managed_target_signs_in_where_its_cluster_offers_it`, `a_managed_target_uses_the_clis_login_where_its_cluster_offers_no_sign_in`, `a_sign_in_to_a_managed_target_must_be_in_its_organization`; `cli_tokens::tests`: `the_targets_are_listed_as_the_cli_describes_them` (extended), `a_managed_profile_the_cli_cannot_describe_is_said` (new). Live: `omnigraph credentials status --profile managed-og --json` (CLI `0.14.1-edge+1755782`); `GET /graphs` without a token on the production `modernrelay-og-cluster` and on the staging cluster | Pass. A kept sign-in is refreshed with the CLI's login never asked, and with none kept the cluster asks for a sign-in; a cluster whose 401 names no sign-in gets one request without a token, then the CLI's, and the CLI's at once on the next connect; a token in another organization is refused before it is sent. The CLI reports `owner.organization_id` with no secret read. Production answers `Bearer` alone, so it is reached on the CLI's login; staging names `resource_metadata` | | 2026-10-30 | A write a retired connection refuses is not sent, and said so | `graph_client` `client::tests::a_replaced_http_client_reaches_every_clone` (extended: a write on a retired client is `NotSent`), `a_forward_carries_studios_credentials_and_stays_under_the_root` (its refused paths are `NoAnswer::Unsendable`), `agent_panel` `gateway::tests::a_writes_success_answer_says_what_it_did` (extended: a forwarded write never sent is refused; one whose answer was lost stays unknown), `the_gateway_checks_the_session_then_forwards_reads_and_approved_writes` (extended: once the target is retired, a read and an approved write are answered 403 with nothing sent, and nothing reaches the server), `type_table` `model::tests::classifies_write_outcomes` (extended). Each fails without its fix (checked) | Pass. A write a retired connection refused before sending was reported as of unknown outcome, to be settled by reading a history the same connection could not read, and the agent's CLI saw its connection close. The client now tells a request never sent (`NoAnswer::Retired`, or `NoAnswer::Unsendable`: a refused path, one that could not be made) from one whose answer was lost (`NoAnswer::Lost`); a write never sent is `WriteOutcome::NotSent`, which every window shows as nothing written, the refused outcome of constitution III, and the gateway answers the CLI 403 with nothing sent and what to do: tell the person for a retired target, change it, or tell the person, for one Studio would not make (specs 013 and 007 say so). A write whose answer was lost stays unknown. A review of the whole change found no write that may have reached the server reported as not sent. Taken: `judge_write` answers `Answered`, which has no case for a write never sent, so neither the client nor the gateway keeps an arm that cannot run; a write to a retired target is refused at once, as a read is, rather than the person being asked to approve a write that cannot go (`agent_panel` `gateway::tests::the_gateway_checks_the_session_then_forwards_reads_and_approved_writes`, extended, fails without it); `graph_store` `session::tests::a_write_never_sent_is_logged_as_refused` (new) and `type_table` `blob_write::tests::a_blob_write_held_off_its_branch_is_not_saved` (extended) hold the session's and a Blob write's words for one, each failing when it is mapped to unknown; spec 007 says what the query editor shows for one | | 2026-10-30 | A review of that change on its final base | `graph_client` `client::tests::a_write_the_server_refused_to_connect_to_is_not_sent` (new). Fails without its fix (checked) | It found no write that may have landed said as not sent. Taken: a write whose connection the server refused (the server down) sent no byte, yet was said as an answer lost that may have changed the value, and a merge so kept Merge off until history could be read; it is not sent (`NoAnswer::Unreached`), judged by the error's kind (an io `ConnectionRefused`), never by its words, which a failure after sending can share. Specs 007 and 013 and the editor's doc said a write is not sent only when the target is gone; a connection is also retired for a transport Studio cannot use, and a refused connection sends nothing. The agent's gateway answers such a request 503 `service_unavailable`, to be tried again once the server answers, not 403 `forbidden`, which tells the agent to change it (`agent_panel` `gateway::tests::a_writes_success_answer_says_what_it_did`, extended). `Bound`'s doc said `Sent` holds a write sent, and a test asserted what every outcome but a commit gives | +| 2026-10-30 | A final review of the write change, with a legacy pass | `graph_client` `client::tests::a_write_the_server_refused_to_connect_to_is_not_sent` (extended: a refusal worded as Windows words it), `review` tests (a fix dropped unsent is `NotSent`). The Windows case fails without its fix (checked) | Taken: a refused connection was told by its kind but described by its words, so on Windows (error 10061) its reason was the system's sentence and the new transport test would fail there; both now read the kind. A fix dropped because the one before it did not land, and a merge whose branch head could not be read, were said as refused; they are not sent. The not-sent notice is one helper in the type table, and a never-sent write is logged with the words the agent gateway uses (`graph_store::nothing_sent`), still as a refusal in the constitution's sense. A write and a Blob write share one send-and-judge step. Its confirmation found the toast doubled the stop of a message that ends its own sentence, a never-sent write's, the agent's and many a server's: it ends it once, as does every sentence that quotes a reason and goes on (the merge banner and its deletion, a fix's line, the type table's not-sent notice, the activity log's unknown tooltip, the log's never-sent words, the gateway's refusals, the graph switcher's unavailable tooltip, History's unread-commits tooltip and the branch delete check), through one `studio_ui::clause` (`studio_workspace` `a_refusal_toast_ends_its_message_once`, new, failing without its fix); and a failure saying "connection refused" in words alone was worded as refused though judged lost: the words follow the kind alone (`graph_client` `a_write_the_server_refused_to_connect_to_is_not_sent`, extended to pin its words, failing without its fix) | ## Corrections