mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): durable session-scope migration and rollback convergence
Two review findings from the PR #1981 session-scope setting: Legacy materialization (Mari, MAJOR 1): hydration read the legacy acp_top_level_sessions override but never persisted the translation, so removing the compatibility reader would silently flip explicit channel users to the thread default. hydrate_scope now atomically materializes an explicit legacy value into acp_session_scope during hydration — false→channel, true→thread — while the bare no-field default stays unwritten (unset remains distinguishable from an override) and an explicit new-field value is never rewritten. DesktopSettings gains a #[serde(flatten)] passthrough so any save preserves JSON fields owned by other features. A failed materialization write keeps the translated scope in memory and the legacy field on disk for retry — no divergence. Rollback convergence (Mari, MAJOR 2): on apply failure with a failed rollback write, the old path restarted processes and forced the UI to 'previous' while the authoritative backend still held the new value — false convergence. applyAcpSessionScopeSetting now establishes the authoritative scope first (confirmed rollback write, else re-read via getBackend), reconciles processes and UI to that actual value, and if the authority is unreadable fires onUnrecoverable — the card surfaces a hard recovery state and disables the toggle instead of claiming a scope. Tests: 7 new Tauri hydration/materialization tests (incl. read-only-dir write-failure injection) and 2 new rollback-matrix tests (authority re-read reconciliation; double-failure hard recovery). Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
eca05aa257
commit
ade79bf89a
@@ -43,6 +43,9 @@ struct DesktopSettings {
|
||||
// have had a release cycle to persist `acp_session_scope`.
|
||||
#[serde(skip_serializing)]
|
||||
acp_top_level_sessions: Option<bool>,
|
||||
// Fields owned by other features must survive our writes untouched.
|
||||
#[serde(flatten)]
|
||||
extra: serde_json::Map<String, serde_json::Value>,
|
||||
}
|
||||
|
||||
impl DesktopSettings {
|
||||
@@ -97,10 +100,8 @@ fn save_settings(path: &Path, settings: &DesktopSettings) -> Result<(), String>
|
||||
|
||||
pub(crate) fn acp_session_scope(app: &AppHandle) -> AcpSessionScope {
|
||||
HYDRATE.call_once(
|
||||
|| match settings_path(app).and_then(|path| load_settings(&path)) {
|
||||
Ok(settings) => {
|
||||
ACP_SESSION_SCOPE.store(settings.session_scope().atomic_value(), Ordering::Release)
|
||||
}
|
||||
|| match settings_path(app).and_then(|path| hydrate_scope(&path)) {
|
||||
Ok(scope) => ACP_SESSION_SCOPE.store(scope.atomic_value(), Ordering::Release),
|
||||
Err(error) => {
|
||||
eprintln!("buzz-desktop: failed to hydrate desktop settings: {error}");
|
||||
}
|
||||
@@ -112,6 +113,29 @@ pub(crate) fn acp_session_scope(app: &AppHandle) -> AcpSessionScope {
|
||||
}
|
||||
}
|
||||
|
||||
/// Load the persisted scope, materializing an explicit legacy override into
|
||||
/// the durable `acp_session_scope` field so the compatibility reader can be
|
||||
/// removed without flipping untouched legacy installs to the default.
|
||||
///
|
||||
/// The ordinary no-field default is deliberately NOT written: unset must stay
|
||||
/// distinguishable from a user/legacy override. A failed materialization
|
||||
/// write keeps the translated scope in memory and leaves the legacy field on
|
||||
/// disk, so the next launch retries — no divergence, no corruption.
|
||||
fn hydrate_scope(path: &Path) -> Result<AcpSessionScope, String> {
|
||||
let mut settings = load_settings(path)?;
|
||||
let scope = settings.session_scope();
|
||||
if settings.acp_session_scope.is_none() && settings.acp_top_level_sessions.is_some() {
|
||||
settings.acp_session_scope = Some(scope);
|
||||
if let Err(error) = save_settings(path, &settings) {
|
||||
eprintln!(
|
||||
"buzz-desktop: failed to materialize legacy ACP session scope \
|
||||
(kept in memory; legacy field retained on disk for retry): {error}"
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(scope)
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub fn get_acp_session_scope(app: AppHandle) -> AcpSessionScope {
|
||||
acp_session_scope(&app)
|
||||
@@ -130,7 +154,7 @@ pub fn set_acp_session_scope(scope: AcpSessionScope, app: AppHandle) -> Result<(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{load_settings, save_settings, AcpSessionScope, DesktopSettings};
|
||||
use super::{hydrate_scope, load_settings, save_settings, AcpSessionScope, DesktopSettings};
|
||||
|
||||
#[test]
|
||||
fn missing_store_defaults_to_thread_scope() {
|
||||
@@ -148,6 +172,7 @@ mod tests {
|
||||
&DesktopSettings {
|
||||
acp_session_scope: Some(AcpSessionScope::Channel),
|
||||
acp_top_level_sessions: None,
|
||||
extra: Default::default(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
@@ -175,4 +200,113 @@ mod tests {
|
||||
std::fs::write(&path, b"not json").unwrap();
|
||||
assert!(load_settings(&path).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hydration_materializes_legacy_opt_out_as_channel() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(&path, br#"{"acp_top_level_sessions":false}"#).unwrap();
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
let persisted: serde_json::Value =
|
||||
serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
|
||||
assert_eq!(persisted["acp_session_scope"], "channel");
|
||||
assert!(persisted.get("acp_top_level_sessions").is_none());
|
||||
// Restart: the durable field alone must reproduce the override.
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hydration_materializes_legacy_opt_in_as_thread() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(&path, br#"{"acp_top_level_sessions":true}"#).unwrap();
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Thread);
|
||||
let persisted: serde_json::Value =
|
||||
serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
|
||||
assert_eq!(persisted["acp_session_scope"], "thread");
|
||||
assert!(persisted.get("acp_top_level_sessions").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hydration_never_materializes_the_bare_default() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(&path, br#"{"other_feature":1}"#).unwrap();
|
||||
let before = std::fs::read(&path).unwrap();
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Thread);
|
||||
// Unset must remain distinguishable from an override: no write.
|
||||
assert_eq!(std::fs::read(&path).unwrap(), before);
|
||||
// Nor is a missing store created.
|
||||
let missing = dir.path().join("missing.json");
|
||||
assert_eq!(hydrate_scope(&missing).unwrap(), AcpSessionScope::Thread);
|
||||
assert!(!missing.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_new_field_wins_over_legacy_and_is_not_rewritten() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(
|
||||
&path,
|
||||
br#"{"acp_session_scope":"channel","acp_top_level_sessions":true}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let before = std::fs::read(&path).unwrap();
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
assert_eq!(std::fs::read(&path).unwrap(), before);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn materialization_preserves_unrelated_fields() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(
|
||||
&path,
|
||||
br#"{"acp_top_level_sessions":false,"other_feature":{"nested":true},"count":3}"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
let persisted: serde_json::Value =
|
||||
serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
|
||||
assert_eq!(persisted["acp_session_scope"], "channel");
|
||||
assert_eq!(persisted["other_feature"]["nested"], true);
|
||||
assert_eq!(persisted["count"], 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_preserves_unrelated_fields_round_trip() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
std::fs::write(
|
||||
&path,
|
||||
br#"{"acp_session_scope":"thread","other_feature":"keep-me"}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let mut settings = load_settings(&path).unwrap();
|
||||
settings.acp_session_scope = Some(AcpSessionScope::Channel);
|
||||
save_settings(&path, &settings).unwrap();
|
||||
let persisted: serde_json::Value =
|
||||
serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
|
||||
assert_eq!(persisted["acp_session_scope"], "channel");
|
||||
assert_eq!(persisted["other_feature"], "keep-me");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn failed_materialization_keeps_translated_scope_and_legacy_field() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("desktop-experiments.json");
|
||||
let legacy = br#"{"acp_top_level_sessions":false}"#;
|
||||
std::fs::write(&path, legacy).unwrap();
|
||||
// Read-only directory: the atomic temp-file write must fail.
|
||||
let readonly = std::fs::Permissions::from_mode(0o555);
|
||||
std::fs::set_permissions(dir.path(), readonly).unwrap();
|
||||
// In memory: translated override. On disk: legacy field untouched,
|
||||
// so the next launch retries the materialization. No divergence.
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
assert_eq!(std::fs::read(&path).unwrap(), legacy);
|
||||
assert_eq!(hydrate_scope(&path).unwrap(), AcpSessionScope::Channel);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ type SessionScope = "thread" | "channel";
|
||||
export function AcpSessionScopeSettingsCard() {
|
||||
const [scope, setScope] = useState<SessionScope>("thread");
|
||||
const [pending, setPending] = useState(true);
|
||||
const [unrecoverable, setUnrecoverable] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
@@ -37,10 +38,12 @@ export function AcpSessionScopeSettingsCard() {
|
||||
await applyAcpSessionScopeSetting(scope === "thread", threadScoped, {
|
||||
setBackend: (next) =>
|
||||
invokeTauri("set_acp_session_scope", { scope: next }),
|
||||
getBackend: () => invokeTauri<SessionScope>("get_acp_session_scope"),
|
||||
listAgents: listManagedAgents,
|
||||
stopAgent: stopManagedAgent,
|
||||
startAgent: startManagedAgent,
|
||||
setUi: (enabled) => setScope(enabled ? "thread" : "channel"),
|
||||
onUnrecoverable: () => setUnrecoverable(true),
|
||||
});
|
||||
} catch (error) {
|
||||
console.error("Failed to apply ACP session scope", error);
|
||||
@@ -64,12 +67,21 @@ export function AcpSessionScopeSettingsCard() {
|
||||
Run separate threads concurrently. Turn this off for one legacy
|
||||
session per channel.
|
||||
</p>
|
||||
{unrecoverable && (
|
||||
<p
|
||||
className="text-xs text-destructive"
|
||||
data-testid="acp-session-scope-recovery"
|
||||
>
|
||||
The session scope could not be applied or restored. Restart the
|
||||
app to recover a consistent state.
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
<Switch
|
||||
aria-labelledby="acp-session-scope-label"
|
||||
checked={scope === "thread"}
|
||||
data-testid="acp-session-scope-toggle"
|
||||
disabled={pending}
|
||||
disabled={pending || unrecoverable}
|
||||
onCheckedChange={(value) => void setThreadScoped(value)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -24,10 +24,15 @@ function harness(overrides = {}) {
|
||||
calls,
|
||||
deps: {
|
||||
setBackend: async (scope) => calls.push(["backend", scope]),
|
||||
getBackend: async () => {
|
||||
calls.push(["read-backend"]);
|
||||
return "thread";
|
||||
},
|
||||
listAgents: async () => [localRunning, remoteRunning, localStopped],
|
||||
stopAgent: async (pubkey) => calls.push(["stop", pubkey]),
|
||||
startAgent: async (pubkey) => calls.push(["start", pubkey]),
|
||||
setUi: (enabled) => calls.push(["ui", enabled]),
|
||||
onUnrecoverable: () => calls.push(["unrecoverable"]),
|
||||
...overrides,
|
||||
},
|
||||
};
|
||||
@@ -130,4 +135,81 @@ describe("ACP session scope setting", () => {
|
||||
]);
|
||||
assert.equal(secondStarts, 2);
|
||||
});
|
||||
|
||||
it("reconciles UI and processes to the re-read authoritative scope when rollback persistence fails", async () => {
|
||||
let backendCalls = 0;
|
||||
const { calls, deps } = harness({
|
||||
setBackend: async (scope) => {
|
||||
calls.push(["backend", scope]);
|
||||
backendCalls += 1;
|
||||
if (backendCalls === 1) return; // apply write succeeds (thread persisted)
|
||||
throw new Error("rollback persist failed");
|
||||
},
|
||||
startAgent: async (pubkey) => {
|
||||
calls.push(["start", pubkey]);
|
||||
if (calls.filter((c) => c[0] === "start").length === 1)
|
||||
throw new Error("restart failed");
|
||||
},
|
||||
// Authoritative backend remains the NEW value (thread): the apply
|
||||
// write landed and the rollback write failed.
|
||||
getBackend: async () => {
|
||||
calls.push(["read-backend"]);
|
||||
return "thread";
|
||||
},
|
||||
});
|
||||
await assert.rejects(
|
||||
applyAcpSessionScopeSetting(false, true, deps),
|
||||
/restart failed/,
|
||||
);
|
||||
// UI must land on the actual persisted scope (thread), never the
|
||||
// assumed previous (channel), and processes reconcile after the read.
|
||||
const readIndex = calls.findIndex((c) => c[0] === "read-backend");
|
||||
const uiIndex = calls.findIndex((c) => c[0] === "ui");
|
||||
assert.notEqual(readIndex, -1);
|
||||
assert.deepEqual(calls[uiIndex], ["ui", true]);
|
||||
assert.ok(readIndex < uiIndex, "authority read must precede UI commit");
|
||||
const restartsAfterRead = calls
|
||||
.slice(readIndex)
|
||||
.filter((c) => c[0] === "stop" || c[0] === "start");
|
||||
assert.deepEqual(restartsAfterRead, [
|
||||
["stop", "local"],
|
||||
["start", "local"],
|
||||
]);
|
||||
assert.ok(!calls.some((c) => c[0] === "unrecoverable"));
|
||||
});
|
||||
|
||||
it("surfaces hard recovery and touches nothing when rollback and authority read both fail", async () => {
|
||||
let backendCalls = 0;
|
||||
const { calls, deps } = harness({
|
||||
setBackend: async (scope) => {
|
||||
calls.push(["backend", scope]);
|
||||
backendCalls += 1;
|
||||
if (backendCalls === 1) return;
|
||||
throw new Error("rollback persist failed");
|
||||
},
|
||||
startAgent: async (pubkey) => {
|
||||
calls.push(["start", pubkey]);
|
||||
if (calls.filter((c) => c[0] === "start").length === 1)
|
||||
throw new Error("restart failed");
|
||||
},
|
||||
getBackend: async () => {
|
||||
calls.push(["read-backend"]);
|
||||
throw new Error("authority unreadable");
|
||||
},
|
||||
});
|
||||
await assert.rejects(
|
||||
applyAcpSessionScopeSetting(false, true, deps),
|
||||
/restart failed/,
|
||||
);
|
||||
assert.ok(calls.some((c) => c[0] === "unrecoverable"));
|
||||
// No UI claim and no process restarts under an unknown scope.
|
||||
const readIndex = calls.findIndex((c) => c[0] === "read-backend");
|
||||
assert.ok(!calls.some((c) => c[0] === "ui"));
|
||||
assert.deepEqual(
|
||||
calls
|
||||
.slice(readIndex + 1)
|
||||
.filter((c) => c[0] === "stop" || c[0] === "start"),
|
||||
[],
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,10 +6,14 @@ export type SessionScopeAgent = {
|
||||
|
||||
export type SessionScopeDependencies = {
|
||||
setBackend: (scope: "thread" | "channel") => Promise<void>;
|
||||
getBackend: () => Promise<"thread" | "channel">;
|
||||
listAgents: () => Promise<SessionScopeAgent[]>;
|
||||
stopAgent: (pubkey: string) => Promise<unknown>;
|
||||
startAgent: (pubkey: string) => Promise<unknown>;
|
||||
setUi: (threadScoped: boolean) => void;
|
||||
/** The authoritative backend scope could not be restored or read: the UI
|
||||
* must surface a hard recovery state instead of claiming any scope. */
|
||||
onUnrecoverable: () => void;
|
||||
};
|
||||
|
||||
async function restartRunningLocalAgents(
|
||||
@@ -25,8 +29,13 @@ async function restartRunningLocalAgents(
|
||||
|
||||
/**
|
||||
* Apply the Rust-owned session-scope setting and restart affected processes. The UI is
|
||||
* committed only after every restart succeeds. On failure, both persisted
|
||||
* backend state and already-restarted agents are restored best-effort.
|
||||
* committed only after every restart succeeds.
|
||||
*
|
||||
* Failure invariant: persisted setting, Rust in-memory value, UI, and affected
|
||||
* processes must converge on one authoritative scope. Rollback is only claimed
|
||||
* after the rollback write is confirmed; if that write fails, the authoritative
|
||||
* value is re-read and UI/processes reconcile to it. If the authority cannot be
|
||||
* read either, `onUnrecoverable` fires and nothing pretends to know the scope.
|
||||
*/
|
||||
export async function applyAcpSessionScopeSetting(
|
||||
previous: boolean,
|
||||
@@ -39,14 +48,31 @@ export async function applyAcpSessionScopeSetting(
|
||||
await restartRunningLocalAgents(agents, deps);
|
||||
deps.setUi(next);
|
||||
} catch (error) {
|
||||
// Establish the authoritative backend scope before touching UI or
|
||||
// processes: preferably by restoring `previous`, otherwise by reading
|
||||
// what actually persisted.
|
||||
let authoritative: boolean;
|
||||
try {
|
||||
await deps.setBackend(previous ? "thread" : "channel");
|
||||
authoritative = previous;
|
||||
} catch (rollbackError) {
|
||||
console.error(
|
||||
"Failed to roll back ACP session-scope backend state",
|
||||
rollbackError,
|
||||
);
|
||||
try {
|
||||
authoritative = (await deps.getBackend()) === "thread";
|
||||
} catch (readError) {
|
||||
console.error(
|
||||
"Failed to read authoritative ACP session scope after rollback failure",
|
||||
readError,
|
||||
);
|
||||
deps.onUnrecoverable();
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
// Reconcile every affected process under the confirmed authoritative
|
||||
// scope — never under an assumed one.
|
||||
for (const agent of agents) {
|
||||
if (agent.status !== "running" || agent.backend.type !== "local")
|
||||
continue;
|
||||
@@ -60,7 +86,7 @@ export async function applyAcpSessionScopeSetting(
|
||||
);
|
||||
}
|
||||
}
|
||||
deps.setUi(previous);
|
||||
deps.setUi(authoritative);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user