mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): strip legacy baked team instructions from stored prompts (#3035)
Agents in a team receive two `Team Instructions` blocks per turn, and
the observer feed renders two Team Instructions cards for them.
There are two producers. `with_team()` in `crates/buzz-acp/src/pool.rs`
appends the LIVE `[Team Instructions]` block from the runtime
`TeamRecord` — that one is correct. The second is baked into the stored
`system_prompt` itself: records written before the runtime framing
landed were composed by the now-removed `compose_prompt()` in
`buzz-persona`, which appended `"\n\n---\n# Team Instructions\n"` plus a
frozen copy of the team instructions. So an affected agent is fed a
stale roster ahead of the current one, and the transcript parser —
correctly — reports both.
Fixing this in `parseSystemPromptSections` would hide the symptom while
the agent kept receiving the stale bytes, so the suffix is removed at
rest by a boot migration.
`strip_baked_team_instructions` splits each stored `system_prompt` at
the LAST occurrence of the exact delimiter and keeps the text before it.
Last-occurrence matches the parser's own `lastIndexOf` guard: a persona
body may quote a delimiter-shaped passage, and only the final one is the
producer boundary. The match is byte-exact — a bare `---`, a `# Team
Instructions` heading at a different position, or a single preceding
newline are author content and are left alone. It applies to every
record regardless of `team_id` / `persona_id` / `pubkey`: the key-less
definition records carry the suffix exactly as the instances minted from
them do. A prompt that was nothing but the suffix becomes `None`, not
`Some("")`, matching the absent-prompt convention in
`AgentDefinition::into_agent_record`.
Stripping a definition's prompt changes its `persona_content_hash`,
which is the drift basis behind the Agents-menu "out of date" badge.
Left alone, every linked instance would light up stale for a change the
user never made. The migration therefore advances the pin of instances
whose `persona_source_version` still equals the definition's PRE-strip
hash — the same conditional `refresh_builtin_agent_avatars` already
uses. An instance that had genuinely drifted keeps its stale pin, and
its badge.
The migration runs after `fold_personas_into_agent_store` so definitions
lifted out of the legacy `personas.json` are cleaned in the same boot,
and before `backfill_standalone_agents` so a manufactured definition
never snapshots a suffix about to be removed. It writes only when at
least one record changed, so a second boot is a true no-op, and takes a
create-if-absent backup at
`managed-agents.json.pre-team-suffix-strip.bak` following the
`pre-backfill.bak` contract — a re-run after a partial failure cannot
replace the pristine backup with a half-migrated snapshot. An
unparseable store errors without writing and without taking a backup,
leaving the file for manual recovery.
Pass 5's legacy branch in `agentSessionTranscriptHelpers.ts` is
deliberately untouched: un-migrated installs and snapshot imports still
need it.
---------
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
co-authored by
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent
871a3b3772
commit
aee6314484
@@ -176,6 +176,11 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) {
|
||||
// Post-fold readers of the runtime map (`load_persona_runtimes`) fall
|
||||
// back to the unified store's definitions.
|
||||
fold_personas_into_agent_store(app);
|
||||
// Clean the legacy baked team-instructions suffix out of stored prompts
|
||||
// AFTER the fold (so definitions lifted out of personas.json are cleaned in
|
||||
// the same boot) and BEFORE backfill_standalone_agents (so a manufactured
|
||||
// definition never snapshots a suffix this strips).
|
||||
strip_baked_team_instructions(app);
|
||||
refresh_builtin_agent_avatars(app);
|
||||
// B5: manufacture definitions for standalone agents AFTER the fold (so
|
||||
// pre-existing definition slugs are present for collision checks) and
|
||||
@@ -1362,7 +1367,6 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) {
|
||||
}
|
||||
rename_provider_to_runtime_in_personas(&path);
|
||||
}
|
||||
|
||||
mod materialize;
|
||||
pub use materialize::materialize_agent_runtimes;
|
||||
mod fold;
|
||||
@@ -1372,6 +1376,8 @@ mod backfill;
|
||||
pub use backfill::backfill_standalone_agents;
|
||||
mod detach;
|
||||
pub use detach::detach_directory_backed_teams;
|
||||
mod team_suffix;
|
||||
pub use team_suffix::strip_baked_team_instructions;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "migration_test_support.rs"]
|
||||
|
||||
@@ -65,12 +65,13 @@ fn backfill_standalone_agents_in_dir(base_dir: &Path) -> Result<usize, String> {
|
||||
}
|
||||
|
||||
// Pre-migration backup, taken ONCE: a re-run after a partial failure must
|
||||
// not overwrite the pristine backup with a half-migrated snapshot.
|
||||
let bak_path = base_dir.join("managed-agents.json.pre-backfill.bak");
|
||||
if !bak_path.exists() {
|
||||
std::fs::write(&bak_path, &content)
|
||||
.map_err(|e| format!("failed to write pre-backfill backup: {e}"))?;
|
||||
}
|
||||
// not overwrite the pristine backup with a half-migrated snapshot. Owner-only
|
||||
// from the initial open, and sited next to the resolved store — see
|
||||
// `create_restricted_backup_once` and `resolved_backup_path`.
|
||||
let bak_path =
|
||||
crate::util::resolved_backup_path(&agents_path, "managed-agents.json.pre-backfill.bak");
|
||||
crate::util::create_restricted_backup_once(&bak_path, content.as_bytes())
|
||||
.map_err(|e| format!("failed to write pre-backfill backup: {e}"))?;
|
||||
|
||||
let existing_slugs: std::collections::HashSet<String> =
|
||||
all.iter().filter_map(|r| r.slug.clone()).collect();
|
||||
|
||||
@@ -299,3 +299,34 @@ fn slug_collision_fails_loudly_per_record_and_continues() {
|
||||
let clean_rec = records.iter().find(|r| r.pubkey == clean).unwrap();
|
||||
assert_eq!(clean_rec.persona_id.as_deref(), Some(clean.as_str()));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn backfill_creates_the_backup_owner_only() {
|
||||
// Same contract as the live store writer: this backup is a verbatim copy of
|
||||
// a file that carries plaintext agent nsecs when the keyring is unreachable,
|
||||
// so it is owner-only from the initial open rather than via a post-write
|
||||
// chmod that would leave a umask window.
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let pubkey = "e".repeat(64);
|
||||
let mut record = standalone_agent_json("Solo", &pubkey, Some("P"));
|
||||
record["private_key_nsec"] = serde_json::json!("nsec1exampleplaintextkey");
|
||||
write_agents_json(dir.path(), &serde_json::json!([record]));
|
||||
|
||||
assert_eq!(
|
||||
backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
|
||||
let bak = base(dir.path()).join("managed-agents.json.pre-backfill.bak");
|
||||
let mode = std::fs::metadata(&bak).unwrap().permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o600, "backup of an inline nsec must be owner-only");
|
||||
assert!(
|
||||
std::fs::read_to_string(&bak)
|
||||
.unwrap()
|
||||
.contains("nsec1exampleplaintextkey"),
|
||||
"the fixture really did carry an inline key"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
//! Strip the legacy baked team-instructions suffix from stored prompts.
|
||||
//!
|
||||
//! Records written before the runtime team framing landed have their team
|
||||
//! instructions BAKED into `system_prompt` by the now-removed
|
||||
//! `compose_prompt()` in buzz-persona:
|
||||
//!
|
||||
//! ```text
|
||||
//! {persona_prompt}\n\n---\n# Team Instructions\n{instructions}
|
||||
//! ```
|
||||
//!
|
||||
//! `with_team()` in `buzz-acp/src/pool.rs` now appends the LIVE
|
||||
//! `[Team Instructions]` section on top of that stored value, so an affected
|
||||
//! agent receives two team-instruction blocks per turn — the frozen copy first,
|
||||
//! the live one second — and the observer feed renders two Team Instructions
|
||||
//! cards. The frozen copy is not merely redundant: it carries whatever the team
|
||||
//! roster said the day it was written, so the agent is fed a stale roster ahead
|
||||
//! of the current one.
|
||||
//!
|
||||
//! The fix has to happen at rest. Suppressing the duplicate in the observer
|
||||
//! parser would hide the symptom while the agent kept receiving the stale bytes.
|
||||
//!
|
||||
//! Stripping a definition's prompt also changes its `persona_content_hash`, the
|
||||
//! drift basis behind the Agents-menu "out of date" badge, so the migration
|
||||
//! advances the pin of instances that were current before the strip (see
|
||||
//! [`repin_current_instances`]).
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::managed_agents::{
|
||||
persona_events::{persona_content_hash, persona_event_content},
|
||||
ManagedAgentRecord,
|
||||
};
|
||||
|
||||
/// The exact producer boundary emitted by the removed `compose_prompt()`.
|
||||
/// Matched byte-for-byte: a bare `---`, a `# Team Instructions` heading at a
|
||||
/// different position, or a single preceding newline are author content, not a
|
||||
/// producer boundary, and are left alone.
|
||||
const TEAM_DELIMITER: &str = "\n\n---\n# Team Instructions\n";
|
||||
|
||||
/// Strip the baked team-instructions suffix from every stored `system_prompt`.
|
||||
///
|
||||
/// Ordering (see `run_boot_migrations`): runs AFTER
|
||||
/// `fold_personas_into_agent_store` so definitions folded out of the legacy
|
||||
/// `personas.json` are cleaned in the same boot, and BEFORE
|
||||
/// `backfill_standalone_agents` so a manufactured definition never snapshots
|
||||
/// a suffix this migration is about to remove.
|
||||
pub fn strip_baked_team_instructions(app: &tauri::AppHandle) {
|
||||
let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else {
|
||||
return;
|
||||
};
|
||||
match strip_baked_team_instructions_in_dir(&base_dir) {
|
||||
Ok(0) => {}
|
||||
Ok(stripped) => eprintln!(
|
||||
"buzz-desktop: team-suffix-strip: removed the baked team-instructions suffix from \
|
||||
{stripped} record(s)"
|
||||
),
|
||||
Err(e) => eprintln!("buzz-desktop: team-suffix-strip: {e}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Core logic, decoupled from the Tauri `AppHandle` for testing.
|
||||
///
|
||||
/// `base_dir` is the managed-agents base directory (`<AppDataDir>/agents/`).
|
||||
/// Returns the number of records changed; `Ok(0)` means nothing to do and
|
||||
/// nothing was written, so a second boot is a clean no-op.
|
||||
pub(super) fn strip_baked_team_instructions_in_dir(base_dir: &Path) -> Result<usize, String> {
|
||||
let agents_path = base_dir.join("managed-agents.json");
|
||||
if !agents_path.exists() {
|
||||
return Ok(0);
|
||||
}
|
||||
let content = std::fs::read_to_string(&agents_path)
|
||||
.map_err(|e| format!("failed to read managed-agents.json: {e}"))?;
|
||||
let mut all: Vec<ManagedAgentRecord> = serde_json::from_str(&content)
|
||||
.map_err(|e| format!("failed to parse managed-agents.json: {e}"))?;
|
||||
|
||||
// Definition hashes BEFORE the strip: stripping a definition's
|
||||
// `system_prompt` changes its `persona_content_hash`, which is the drift
|
||||
// basis the Agents menu compares each linked instance's pinned
|
||||
// `persona_source_version` against. Captured here so instances that were
|
||||
// current before the strip can be re-pinned after it.
|
||||
let pre_strip_hashes = definition_hashes(&all);
|
||||
|
||||
// Applies to every record: the definition records carry the suffix as
|
||||
// surely as the instances minted from them, and neither `team_id` nor
|
||||
// `persona_id` nor a key is evidence either way.
|
||||
let mut stripped = 0usize;
|
||||
for record in all.iter_mut() {
|
||||
let Some(prompt) = record.system_prompt.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
// Last occurrence only, mirroring the observer parser's own
|
||||
// `lastIndexOf` guard (`agentSessionTranscriptHelpers.ts`): a persona
|
||||
// body may legitimately quote a delimiter-shaped passage, and only the
|
||||
// final one is the boundary `compose_prompt()` appended.
|
||||
let Some(at) = prompt.rfind(TEAM_DELIMITER) else {
|
||||
continue;
|
||||
};
|
||||
let head = &prompt[..at];
|
||||
// An empty head means the record held nothing but the baked suffix.
|
||||
// Store `None` rather than `Some("")`, matching the empty-prompt
|
||||
// convention in `AgentDefinition::into_agent_record`, so no phantom
|
||||
// empty prompt is left behind.
|
||||
record.system_prompt = (!head.is_empty()).then(|| head.to_string());
|
||||
stripped += 1;
|
||||
}
|
||||
|
||||
if stripped == 0 {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
repin_current_instances(&mut all, &pre_strip_hashes);
|
||||
|
||||
// Pre-migration backup, taken ONCE (same contract as the B5 backfill): a
|
||||
// re-run after a partial failure must not replace the pristine backup with
|
||||
// a half-migrated snapshot. Owner-only from the initial open, and sited next
|
||||
// to the resolved store — see `create_restricted_backup_once` and
|
||||
// `resolved_backup_path`.
|
||||
let bak_path = crate::util::resolved_backup_path(
|
||||
&agents_path,
|
||||
"managed-agents.json.pre-team-suffix-strip.bak",
|
||||
);
|
||||
crate::util::create_restricted_backup_once(&bak_path, content.as_bytes())
|
||||
.map_err(|e| format!("failed to write pre-strip backup: {e}"))?;
|
||||
|
||||
let payload = serde_json::to_vec_pretty(&all)
|
||||
.map_err(|e| format!("failed to serialize managed-agents.json: {e}"))?;
|
||||
crate::managed_agents::atomic_write_json_restricted(&agents_path, &payload)?;
|
||||
Ok(stripped)
|
||||
}
|
||||
|
||||
/// `slug → persona_content_hash` for every definition record, computed through
|
||||
/// the same projection the drift indicator uses (`persona_drift_state` in
|
||||
/// `managed_agents/runtime.rs`).
|
||||
fn definition_hashes(records: &[ManagedAgentRecord]) -> HashMap<String, String> {
|
||||
records
|
||||
.iter()
|
||||
.filter(|record| record.pubkey.is_empty())
|
||||
.filter_map(|record| {
|
||||
let definition = record.to_definition_view()?;
|
||||
let hash = persona_content_hash(&persona_event_content(&definition));
|
||||
Some((definition.id, hash))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Advance the drift pin of instances that were current before the strip.
|
||||
///
|
||||
/// Stripping a definition's prompt changes its `persona_content_hash`, so every
|
||||
/// linked instance would otherwise light up "out of date" in the Agents menu
|
||||
/// for a change the user never made — a badge that only clears on the next
|
||||
/// start, when `apply_persona_snapshot` re-pins. Same conditional as
|
||||
/// `refresh_builtin_agent_avatars`: move the pin only when it still equals the
|
||||
/// definition's PRE-strip hash. An instance that had genuinely drifted keeps
|
||||
/// its stale pin, and its badge.
|
||||
fn repin_current_instances(
|
||||
records: &mut [ManagedAgentRecord],
|
||||
pre_strip_hashes: &HashMap<String, String>,
|
||||
) {
|
||||
let post_strip_hashes = definition_hashes(records);
|
||||
for record in records.iter_mut() {
|
||||
if record.pubkey.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let Some(persona_id) = record.persona_id.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
let (Some(old), Some(new)) = (
|
||||
pre_strip_hashes.get(persona_id),
|
||||
post_strip_hashes.get(persona_id),
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
if old != new && record.persona_source_version.as_deref() == Some(old.as_str()) {
|
||||
record.persona_source_version = Some(new.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "team_suffix_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,449 @@
|
||||
use super::strip_baked_team_instructions_in_dir;
|
||||
use crate::migration::test_support::{read_agents_json, write_agents_json};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
const DELIMITER: &str = "\n\n---\n# Team Instructions\n";
|
||||
|
||||
fn base(dir: &Path) -> PathBuf {
|
||||
dir.join("agents")
|
||||
}
|
||||
|
||||
fn agent_json(name: &str, prompt: Option<&str>) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"name": name,
|
||||
"pubkey": "",
|
||||
"relay_url": "ws://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"parallelism": 4,
|
||||
"system_prompt": prompt,
|
||||
"model": "gpt-x",
|
||||
"provider": "openai",
|
||||
"env_vars": { "API_KEY": "secret" },
|
||||
"start_on_app_launch": true,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"last_started_at": null,
|
||||
"last_stopped_at": null,
|
||||
"last_exit_code": null,
|
||||
"last_error": null
|
||||
})
|
||||
}
|
||||
|
||||
fn prompt_of(dir: &Path, name: &str) -> Option<String> {
|
||||
read_agents_json(dir)
|
||||
.into_iter()
|
||||
.find(|r| r["name"] == name)
|
||||
.and_then(|r| r["system_prompt"].as_str().map(str::to_string))
|
||||
}
|
||||
|
||||
/// Live `persona_content_hash` of the definition with `slug`, computed off the
|
||||
/// store on disk through the same projection the drift badge uses.
|
||||
fn definition_hash(dir: &Path, slug: &str) -> String {
|
||||
let records: Vec<crate::managed_agents::ManagedAgentRecord> =
|
||||
serde_json::from_value(serde_json::Value::Array(read_agents_json(dir))).unwrap();
|
||||
super::definition_hashes(&records)
|
||||
.remove(slug)
|
||||
.expect("definition present")
|
||||
}
|
||||
|
||||
fn definition_json(slug: &str, prompt: &str) -> serde_json::Value {
|
||||
let mut record = agent_json(slug, Some(prompt));
|
||||
record["slug"] = serde_json::json!(slug);
|
||||
record["display_name"] = serde_json::json!("Paul");
|
||||
record
|
||||
}
|
||||
|
||||
fn instance_json(slug: &str, prompt: &str, pinned: &str) -> serde_json::Value {
|
||||
let mut record = agent_json("Paul", Some(prompt));
|
||||
record["pubkey"] = serde_json::json!("p".repeat(64));
|
||||
record["persona_id"] = serde_json::json!(slug);
|
||||
record["persona_source_version"] = serde_json::json!(pinned);
|
||||
record
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_removes_the_baked_suffix_and_keeps_the_persona_body() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json(
|
||||
"Paul",
|
||||
Some(&format!("You are Paul.{DELIMITER}| Agent | Role |"))
|
||||
)]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
prompt_of(dir.path(), "Paul").as_deref(),
|
||||
Some("You are Paul.")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_splits_on_the_last_delimiter_occurrence() {
|
||||
// A persona body may quote a delimiter-shaped passage; only the final
|
||||
// occurrence is the boundary the removed compose_prompt() appended.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let quoted = format!("Header.{DELIMITER}quoted example");
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json(
|
||||
"Duncan",
|
||||
Some(&format!("{quoted}{DELIMITER}live roster"))
|
||||
)]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(prompt_of(dir.path(), "Duncan").as_deref(), Some(&*quoted));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_leaves_near_miss_lookalikes_untouched() {
|
||||
// Each variant differs from the producer boundary by exactly one detail:
|
||||
// no heading, heading on a different line, a single preceding newline,
|
||||
// and a trailing-space heading.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let lookalikes = [
|
||||
("bare-rule", "Body.\n\n---\nMore body."),
|
||||
("heading-inline", "Body.\n\n--- # Team Instructions\nMore."),
|
||||
("single-newline", "Body.\n---\n# Team Instructions\nMore."),
|
||||
("heading-only", "Body.\n\n# Team Instructions\nMore."),
|
||||
(
|
||||
"trailing-space",
|
||||
"Body.\n\n---\n# Team Instructions \nMore.",
|
||||
),
|
||||
];
|
||||
let records: Vec<serde_json::Value> = lookalikes
|
||||
.iter()
|
||||
.map(|(name, prompt)| agent_json(name, Some(prompt)))
|
||||
.collect();
|
||||
write_agents_json(dir.path(), &serde_json::Value::Array(records));
|
||||
let before = std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
0
|
||||
);
|
||||
let after = std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap();
|
||||
assert_eq!(before, after, "no lookalike may be rewritten");
|
||||
for (name, prompt) in lookalikes {
|
||||
assert_eq!(prompt_of(dir.path(), name).as_deref(), Some(prompt));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_is_a_no_op_on_the_second_run() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json(
|
||||
"Paul",
|
||||
Some(&format!("You are Paul.{DELIMITER}roster"))
|
||||
)]),
|
||||
);
|
||||
let path = base(dir.path()).join("managed-agents.json");
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
let after_first = std::fs::read_to_string(&path).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
0,
|
||||
"second run finds nothing to strip"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&path).unwrap(),
|
||||
after_first,
|
||||
"second run must not write"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_preserves_every_other_field() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut record = agent_json("Thufir", Some(&format!("Prompt.{DELIMITER}roster")));
|
||||
record["pubkey"] = serde_json::json!("t".repeat(64));
|
||||
record["persona_id"] = serde_json::json!("sietch-tabr:thufir");
|
||||
record["team_id"] = serde_json::json!("team-uuid");
|
||||
record["persona_source_version"] = serde_json::json!("a".repeat(64));
|
||||
record["auth_tag"] = serde_json::json!("{\"tag\":\"value\"}");
|
||||
write_agents_json(dir.path(), &serde_json::json!([record]));
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
let stored = read_agents_json(dir.path()).remove(0);
|
||||
assert_eq!(stored["system_prompt"], "Prompt.");
|
||||
assert_eq!(stored["pubkey"], "t".repeat(64));
|
||||
assert_eq!(stored["persona_id"], "sietch-tabr:thufir");
|
||||
assert_eq!(stored["team_id"], "team-uuid");
|
||||
assert_eq!(stored["persona_source_version"], "a".repeat(64));
|
||||
assert_eq!(stored["auth_tag"], "{\"tag\":\"value\"}");
|
||||
assert_eq!(stored["env_vars"]["API_KEY"], "secret");
|
||||
assert_eq!(stored["model"], "gpt-x");
|
||||
assert_eq!(stored["provider"], "openai");
|
||||
assert_eq!(stored["parallelism"], 4);
|
||||
assert_eq!(stored["created_at"], "2026-01-01T00:00:00Z");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_writes_a_backup_once_and_never_clobbers_it() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let original = format!("Alia prompt.{DELIMITER}roster");
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json("Alia", Some(&original))]),
|
||||
);
|
||||
let bak = base(dir.path()).join("managed-agents.json.pre-team-suffix-strip.bak");
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
let bak_content = std::fs::read_to_string(&bak).unwrap();
|
||||
assert!(
|
||||
bak_content.contains("---\\n# Team Instructions"),
|
||||
"backup holds the pre-migration bytes"
|
||||
);
|
||||
|
||||
// A later record acquiring the suffix must not replace the pristine backup.
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json("Alia", Some(&format!("Edited.{DELIMITER}new")))]),
|
||||
);
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&bak).unwrap(),
|
||||
bak_content,
|
||||
"the first backup is never clobbered"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_nulls_a_prompt_that_was_only_the_baked_suffix() {
|
||||
// Head is empty: the record held nothing but the delimiter and the frozen
|
||||
// instructions. Storing Some("") would leave a phantom empty prompt that
|
||||
// reads as "the author wrote a blank prompt"; None is the absent-prompt
|
||||
// spelling the rest of the store uses.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json("Ghost", Some(&format!("{DELIMITER}roster")))]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
read_agents_json(dir.path())[0]["system_prompt"].is_null(),
|
||||
"no phantom empty prompt"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_skips_records_without_a_prompt() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([agent_json("Promptless", None)]),
|
||||
);
|
||||
let before = std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
0
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(),
|
||||
before
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_on_a_missing_store_is_a_no_op() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::create_dir_all(base(dir.path())).unwrap();
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_on_an_unparseable_store_errors_without_writing() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::create_dir_all(base(dir.path())).unwrap();
|
||||
let path = base(dir.path()).join("managed-agents.json");
|
||||
std::fs::write(&path, "{ not json").unwrap();
|
||||
|
||||
let err = strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap_err();
|
||||
assert!(err.contains("failed to parse"), "unexpected error: {err}");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&path).unwrap(),
|
||||
"{ not json",
|
||||
"a corrupt store is left for manual recovery"
|
||||
);
|
||||
assert!(
|
||||
!base(dir.path())
|
||||
.join("managed-agents.json.pre-team-suffix-strip.bak")
|
||||
.exists(),
|
||||
"no backup is taken when nothing can be migrated"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_repins_an_instance_that_was_current_before_the_strip() {
|
||||
// The definition's content hash changes when its prompt is stripped. An
|
||||
// instance pinned to the pre-strip hash was current, so it must stay
|
||||
// current — the user changed nothing and must not see a drift badge.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let baked = format!("You are Paul.{DELIMITER}frozen roster");
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([definition_json("sietch-tabr:paul", &baked)]),
|
||||
);
|
||||
let pre_hash = definition_hash(dir.path(), "sietch-tabr:paul");
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition_json("sietch-tabr:paul", &baked),
|
||||
instance_json("sietch-tabr:paul", &baked, &pre_hash),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
2
|
||||
);
|
||||
|
||||
let post_hash = definition_hash(dir.path(), "sietch-tabr:paul");
|
||||
assert_ne!(pre_hash, post_hash, "stripping must change the drift basis");
|
||||
let instance = read_agents_json(dir.path())
|
||||
.into_iter()
|
||||
.find(|r| r["pubkey"].as_str() == Some(&"p".repeat(64)))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
instance["persona_source_version"].as_str(),
|
||||
Some(post_hash.as_str()),
|
||||
"a previously-current instance must not start showing drift"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_leaves_an_already_drifted_instance_pin_alone() {
|
||||
// An instance pinned to something other than the definition's pre-strip
|
||||
// hash had genuinely drifted. Silently re-pinning it would erase a real
|
||||
// signal, so its stale pin — and its badge — survive.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let baked = format!("You are Paul.{DELIMITER}frozen roster");
|
||||
let stale = "d".repeat(64);
|
||||
write_agents_json(
|
||||
dir.path(),
|
||||
&serde_json::json!([
|
||||
definition_json("sietch-tabr:paul", &baked),
|
||||
instance_json("sietch-tabr:paul", &baked, &stale),
|
||||
]),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
2
|
||||
);
|
||||
|
||||
let instance = read_agents_json(dir.path())
|
||||
.into_iter()
|
||||
.find(|r| r["pubkey"].as_str() == Some(&"p".repeat(64)))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
instance["persona_source_version"].as_str(),
|
||||
Some(stale.as_str()),
|
||||
"real drift must keep its signal"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn strip_creates_the_backup_owner_only() {
|
||||
// The backup is a verbatim copy of a store that carries plaintext agent
|
||||
// nsecs whenever the keyring is unreachable, so it must be owner-only from
|
||||
// the initial open — a post-write chmod would leave a umask window in which
|
||||
// the secret is world-readable.
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut record = agent_json("Alia", Some(&format!("Alia prompt.{DELIMITER}roster")));
|
||||
record["private_key_nsec"] = serde_json::json!("nsec1exampleplaintextkey");
|
||||
write_agents_json(dir.path(), &serde_json::json!([record]));
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(dir.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
|
||||
let bak = base(dir.path()).join("managed-agents.json.pre-team-suffix-strip.bak");
|
||||
let mode = std::fs::metadata(&bak).unwrap().permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o600, "backup of an inline nsec must be owner-only");
|
||||
assert!(
|
||||
std::fs::read_to_string(&bak)
|
||||
.unwrap()
|
||||
.contains("nsec1exampleplaintextkey"),
|
||||
"the fixture really did carry an inline key"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn strip_backs_up_beside_the_symlink_target() {
|
||||
// Dev worktrees symlink `agents/managed-agents.json` to a shared data dir
|
||||
// (`sync_shared_agent_data`). The sole recovery copy must land next to the
|
||||
// real data, not in whichever worktree booted first.
|
||||
let shared = tempfile::tempdir().unwrap();
|
||||
let worktree = tempfile::tempdir().unwrap();
|
||||
write_agents_json(
|
||||
shared.path(),
|
||||
&serde_json::json!([agent_json("Alia", Some(&format!("Alia.{DELIMITER}roster")))]),
|
||||
);
|
||||
std::fs::create_dir_all(base(worktree.path())).unwrap();
|
||||
std::os::unix::fs::symlink(
|
||||
base(shared.path()).join("managed-agents.json"),
|
||||
base(worktree.path()).join("managed-agents.json"),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
strip_baked_team_instructions_in_dir(&base(worktree.path())).unwrap(),
|
||||
1
|
||||
);
|
||||
|
||||
assert!(
|
||||
base(shared.path())
|
||||
.join("managed-agents.json.pre-team-suffix-strip.bak")
|
||||
.exists(),
|
||||
"backup belongs beside the shared store"
|
||||
);
|
||||
assert!(
|
||||
!base(worktree.path())
|
||||
.join("managed-agents.json.pre-team-suffix-strip.bak")
|
||||
.exists(),
|
||||
"no orphan backup in the worktree data dir"
|
||||
);
|
||||
assert_eq!(prompt_of(shared.path(), "Alia").as_deref(), Some("Alia."));
|
||||
}
|
||||
@@ -220,6 +220,67 @@ pub(crate) fn configure_no_window(command: &mut std::process::Command) {
|
||||
let _ = command;
|
||||
}
|
||||
|
||||
/// Copy `source_bytes` to `path` owner-only, exactly once.
|
||||
///
|
||||
/// For the pre-migration backups taken by the migrations in `crate::migration` before they
|
||||
/// rewrite `managed-agents.json`. That store carries plaintext agent nsecs
|
||||
/// whenever the keyring is unreachable (`SECURITY.md`), so the backup must be
|
||||
/// owner-only from the initial open — `fs::write` then `set_permissions` leaves
|
||||
/// exactly the umask window that [`atomic_write_json_restricted`] sets the mode
|
||||
/// on the handle to close.
|
||||
///
|
||||
/// `create_new` also subsumes the create-if-absent check: `AlreadyExists` is the
|
||||
/// idempotent success case, so a re-run after a partial failure cannot replace a
|
||||
/// pristine backup with a half-migrated snapshot, and there is no exists-then-
|
||||
/// write window between the two. `Ok(false)` means a backup was already there
|
||||
/// and was left untouched. Every other error propagates, so a caller that fails
|
||||
/// to back up never proceeds to the live write.
|
||||
///
|
||||
/// Non-Unix platforms get `create_new` alone.
|
||||
///
|
||||
/// [`atomic_write_json_restricted`]: crate::managed_agents::atomic_write_json_restricted
|
||||
pub(crate) fn create_restricted_backup_once(
|
||||
path: &std::path::Path,
|
||||
source_bytes: &[u8],
|
||||
) -> Result<bool, String> {
|
||||
use std::io::Write as _;
|
||||
|
||||
let mut options = std::fs::OpenOptions::new();
|
||||
options.write(true).create_new(true);
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
options.mode(0o600);
|
||||
}
|
||||
let mut file = match options.open(path) {
|
||||
Ok(file) => file,
|
||||
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => return Ok(false),
|
||||
Err(e) => return Err(format!("create {}: {e}", path.display())),
|
||||
};
|
||||
file.write_all(source_bytes)
|
||||
.map_err(|e| format!("write {}: {e}", path.display()))?;
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
/// Derive the sibling backup path for a store, resolving symlinks first.
|
||||
///
|
||||
/// Dev worktrees symlink `agents/managed-agents.json` into the shared canonical
|
||||
/// dev data dir (`sync_shared_agent_data`). [`atomic_write_json_restricted`]
|
||||
/// canonicalizes before writing, so without the same resolution here the sole
|
||||
/// recovery copy would land beside the symlink while the data it protects lives
|
||||
/// elsewhere. Falls back to the given path when `canonicalize` fails — the store
|
||||
/// may legitimately not exist yet.
|
||||
///
|
||||
/// [`atomic_write_json_restricted`]: crate::managed_agents::atomic_write_json_restricted
|
||||
pub(crate) fn resolved_backup_path(
|
||||
store_path: &std::path::Path,
|
||||
backup_name: &str,
|
||||
) -> std::path::PathBuf {
|
||||
std::fs::canonicalize(store_path)
|
||||
.unwrap_or_else(|_| store_path.to_path_buf())
|
||||
.with_file_name(backup_name)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::slugify;
|
||||
|
||||
Reference in New Issue
Block a user