fix: preserve selector retries and workdir observations

This commit is contained in:
2026-08-26 16:05:14 +09:00
parent 471db64bcc
commit 3cdcbb47bf
5 changed files with 213 additions and 39 deletions
+21
View File
@@ -111,6 +111,8 @@ pub struct WorkingDirectoryProvenance {
pub creation_selector: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub creation_ref: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub creation_tree: Option<String>,
pub materializer_kind: MaterializerKind,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cleanup_target: Option<WorkingDirectoryCleanupTarget>,
@@ -124,6 +126,10 @@ pub struct WorkingDirectoryCurrentObservation {
pub current_selector: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub current_ref: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub current_tree: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub observed_at_epoch_seconds: Option<u64>,
pub status: WorkingDirectoryStatusKind,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cleanliness: Option<String>,
@@ -143,9 +149,15 @@ pub struct WorkingDirectorySummary {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub creation_ref: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub creation_tree: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub current_selector: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub current_ref: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub current_tree: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub observed_at_epoch_seconds: Option<u64>,
pub materializer_kind: MaterializerKind,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cleanup_target: Option<WorkingDirectoryCleanupTarget>,
@@ -168,6 +180,7 @@ impl WorkingDirectorySummary {
WorkingDirectoryProvenance {
creation_selector: self.creation_selector.clone(),
creation_ref: self.creation_ref.clone(),
creation_tree: self.creation_tree.clone(),
materializer_kind: self.materializer_kind.clone(),
cleanup_target: self.cleanup_target.clone(),
}
@@ -177,6 +190,8 @@ impl WorkingDirectorySummary {
WorkingDirectoryCurrentObservation {
current_selector: self.current_selector.clone(),
current_ref: self.current_ref.clone(),
current_tree: self.current_tree.clone(),
observed_at_epoch_seconds: self.observed_at_epoch_seconds,
status: self.status.clone(),
cleanliness: self.cleanliness.clone(),
primary_worker_id: self.primary_worker_id.clone(),
@@ -249,8 +264,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: Some("develop".to_string()),
creation_ref: Some("abc123".to_string()),
creation_tree: Some("tree123".to_string()),
current_selector: Some("work/ticket".to_string()),
current_ref: Some("def456".to_string()),
current_tree: Some("tree456".to_string()),
observed_at_epoch_seconds: Some(1_777_777_777),
materializer_kind: MaterializerKind::LocalGitWorktree,
cleanup_target: Some(WorkingDirectoryCleanupTarget {
kind: "git_worktree".to_string(),
@@ -271,8 +289,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: Some("987fed".to_string()),
current_tree: None,
observed_at_epoch_seconds: None,
materializer_kind: MaterializerKind::LocalGitWorktree,
cleanup_target: None,
status: WorkingDirectoryStatusKind::Active,
+37 -4
View File
@@ -69,8 +69,11 @@ impl WorkingDirectory {
repository_id: self.repository_id.clone(),
creation_selector: self.evidence.requested_selector.clone(),
creation_ref: Some(self.evidence.resolved_commit.clone()),
creation_tree: self.evidence.resolved_tree.clone(),
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materializer_kind: self.materializer_kind.clone(),
cleanup_target: Some(self.cleanup_target.clone()),
status: self.status.clone(),
@@ -126,9 +129,16 @@ impl WorkingDirectoryBinding {
}
let mut summary = working_directory.status_summary();
summary.cleanliness = if summary.status == WorkingDirectoryStatusKind::Active {
let (current_selector, current_ref) = binding_current_revision(self);
let (current_selector, current_ref, current_tree) = binding_current_revision(self);
summary.current_selector = current_selector;
summary.current_ref = current_ref;
summary.current_tree = current_tree;
summary.observed_at_epoch_seconds = Some(
SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or_default()
.as_secs(),
);
Some(binding_cleanliness(self))
} else {
Some("unknown".to_string())
@@ -217,12 +227,14 @@ fn binding_paths_are_available(binding: &WorkingDirectoryBinding) -> bool {
source_repository_path.is_dir()
}
fn binding_current_revision(binding: &WorkingDirectoryBinding) -> (Option<String>, Option<String>) {
fn binding_current_revision(
binding: &WorkingDirectoryBinding,
) -> (Option<String>, Option<String>, Option<String>) {
let current_ref = git_stdout(binding.root(), ["rev-parse", "HEAD"])
.ok()
.filter(|value| !value.is_empty());
if current_ref.is_none() {
return (None, None);
return (None, None, None);
}
let current_selector = git_stdout(
binding.root(),
@@ -230,7 +242,10 @@ fn binding_current_revision(binding: &WorkingDirectoryBinding) -> (Option<String
)
.ok()
.filter(|value| !value.is_empty());
(current_selector, current_ref)
let current_tree = git_stdout(binding.root(), ["rev-parse", "HEAD^{tree}"])
.ok()
.filter(|value| !value.is_empty());
(current_selector, current_ref, current_tree)
}
fn binding_cleanliness(binding: &WorkingDirectoryBinding) -> String {
@@ -303,8 +318,11 @@ impl RuntimeGitCacheMaterializer {
repository_id: "unknown".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materializer_kind: MaterializerKind::RuntimeGitCache,
cleanup_target: Some(WorkingDirectoryCleanupTarget {
kind: "runtime_git_cache_worktree".to_string(),
@@ -2299,6 +2317,21 @@ mod tests {
assert_eq!(summary.creation_ref.as_deref(), Some(initial_ref.as_str()));
assert_eq!(summary.current_selector.as_deref(), Some("observed-branch"));
assert_ne!(summary.current_ref.as_deref(), Some(initial_ref.as_str()));
assert!(summary.creation_tree.is_some());
assert!(summary.current_tree.is_some());
assert_ne!(summary.current_tree, summary.creation_tree);
assert!(summary.observed_at_epoch_seconds.is_some());
assert_eq!(summary.cleanliness.as_deref(), Some("clean"));
fs::write(bound.root.join("dirty.txt"), "dirty\n").unwrap();
let dirty = materializer.list_working_directories().unwrap()[0]
.summary
.clone();
assert_eq!(dirty.creation_ref, summary.creation_ref);
assert_eq!(dirty.current_ref, summary.current_ref);
assert_eq!(dirty.current_tree, summary.current_tree);
assert_eq!(dirty.cleanliness.as_deref(), Some("dirty"));
assert!(dirty.observed_at_epoch_seconds.is_some());
}
#[test]
+76 -25
View File
@@ -8477,30 +8477,39 @@ async fn create_workspace_working_directory(
if let Some(existing) = api
.config_store
.load_workdir_create_operation(workspace_id, &operation_id)?
&& let (Some(kind), Some(uri), Some(revision), Some(fingerprint)) = (
existing.source_kind.as_deref(),
existing.source_uri,
existing.source_revision,
existing.source_fingerprint,
)
{
let kind = match kind {
"local_path" => workspace_api::RepositorySourceKind::LocalPath,
"file" => workspace_api::RepositorySourceKind::File,
"https" => workspace_api::RepositorySourceKind::Https,
"http" => workspace_api::RepositorySourceKind::Http,
"ssh" => workspace_api::RepositorySourceKind::Ssh,
"invalid" => workspace_api::RepositorySourceKind::Invalid,
_ => {
return Err(settings_bad_request(
"working_directory_repository_source_invalid",
"persisted Workdir create Repository source kind is invalid",
));
}
};
working_directory_request.repository.source = workspace_api::RepositorySource { kind, uri };
working_directory_request.repository.source_revision = revision;
working_directory_request.repository.source_fingerprint = fingerprint;
working_directory_request.repository.selector =
crate::workdir_create_operations::selector_for_retry(
request.selector.as_deref(),
existing.selector.as_deref(),
working_directory_request.repository.selector.as_deref(),
)
.map(RuntimeRepositorySelector::from);
if let (Some(kind), Some(uri), Some(revision), Some(fingerprint)) = (
existing.source_kind.as_deref(),
existing.source_uri.clone(),
existing.source_revision,
existing.source_fingerprint.clone(),
) {
let kind = match kind {
"local_path" => workspace_api::RepositorySourceKind::LocalPath,
"file" => workspace_api::RepositorySourceKind::File,
"https" => workspace_api::RepositorySourceKind::Https,
"http" => workspace_api::RepositorySourceKind::Http,
"ssh" => workspace_api::RepositorySourceKind::Ssh,
"invalid" => workspace_api::RepositorySourceKind::Invalid,
_ => {
return Err(settings_bad_request(
"working_directory_repository_source_invalid",
"persisted Workdir create Repository source kind is invalid",
));
}
};
working_directory_request.repository.source =
workspace_api::RepositorySource { kind, uri };
working_directory_request.repository.source_revision = revision;
working_directory_request.repository.source_fingerprint = fingerprint;
}
}
let selector = working_directory_request
.repository
@@ -13704,8 +13713,11 @@ fn upsert_pending_backend_workdir(
.as_ref()
.map(|selector| selector.as_ref().to_string()),
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "pending".to_string(),
cleanliness: "unknown".to_string(),
created_at: timestamp.clone(),
@@ -13853,8 +13865,11 @@ fn workdir_record_from_summary(
repository_id: summary.repository_id.clone(),
creation_selector: summary.creation_selector.clone(),
creation_ref: summary.creation_ref.clone(),
creation_tree: summary.creation_tree.clone(),
current_selector: summary.current_selector.clone(),
current_ref: summary.current_ref.clone(),
current_tree: summary.current_tree.clone(),
observed_at_epoch_seconds: summary.observed_at_epoch_seconds,
materialization_status: match summary.status {
WorkingDirectoryStatusKind::Active => "present",
WorkingDirectoryStatusKind::CleanupPending => "pending",
@@ -13891,6 +13906,21 @@ fn preserve_workdir_identity_for_corrupted_summary(
if record.creation_ref.is_none() {
record.creation_ref = existing.creation_ref.clone();
}
if record.creation_tree.is_none() {
record.creation_tree = existing.creation_tree.clone();
}
if record.current_selector.is_none() {
record.current_selector = existing.current_selector.clone();
}
if record.current_ref.is_none() {
record.current_ref = existing.current_ref.clone();
}
if record.current_tree.is_none() {
record.current_tree = existing.current_tree.clone();
}
if record.observed_at_epoch_seconds.is_none() {
record.observed_at_epoch_seconds = existing.observed_at_epoch_seconds;
}
}
fn workdir_summary_from_record(record: &WorkdirRegistryRecord) -> WorkingDirectorySummary {
@@ -13907,11 +13937,14 @@ fn workdir_summary_from_record(record: &WorkdirRegistryRecord) -> WorkingDirecto
repository_id: record.repository_id.clone(),
creation_selector: record.creation_selector.clone(),
creation_ref: record.creation_ref.clone(),
creation_tree: record.creation_tree.clone(),
current_selector: record.current_selector.clone(),
current_ref: record.current_ref.clone(),
materializer_kind: MaterializerKind::LocalGitWorktree,
current_tree: record.current_tree.clone(),
observed_at_epoch_seconds: record.observed_at_epoch_seconds,
materializer_kind: MaterializerKind::RuntimeGitCache,
cleanup_target: Some(WorkingDirectoryCleanupTarget {
kind: "local_git_worktree".to_string(),
kind: "runtime_git_cache_worktree".to_string(),
working_directory_id: record.workdir_id.clone(),
repository_id: record.repository_id.clone(),
}),
@@ -15109,8 +15142,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: Some("develop".to_string()),
creation_ref: Some("abcdef".to_string()),
creation_tree: Some("tree-creation".to_string()),
current_selector: None,
current_ref: Some("fedcba".to_string()),
current_tree: Some("tree-current".to_string()),
observed_at_epoch_seconds: Some(3),
materialization_status: "missing".to_string(),
cleanliness: "clean".to_string(),
created_at: "1".to_string(),
@@ -16168,8 +16204,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "present".to_string(),
cleanliness: "clean".to_string(),
created_at: "1".to_string(),
@@ -16184,8 +16223,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "present".to_string(),
cleanliness: "unknown".to_string(),
created_at: "1".to_string(),
@@ -16257,8 +16299,11 @@ mod tests {
repository_id: "repo".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "present".to_string(),
cleanliness: "unknown".to_string(),
created_at: "1".to_string(),
@@ -20586,8 +20631,11 @@ mod tests {
repository_id: "repo-test".to_string(),
creation_selector: Some("HEAD".to_string()),
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: status.to_string(),
cleanliness: cleanliness.to_string(),
created_at: now.clone(),
@@ -23967,8 +24015,11 @@ VALUES ('0192f0e8-4d84-7d6e-a000-000000000001', 'ticket', 3);
repository_id: "main".to_string(),
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: Some("work/ticket".to_string()),
current_ref: Some("abc123".to_string()),
current_tree: Some("tree123".to_string()),
observed_at_epoch_seconds: Some(1_777_777_777),
materializer_kind: MaterializerKind::LocalGitWorktree,
cleanup_target: None,
status: WorkingDirectoryStatusKind::Active,
+52 -10
View File
@@ -620,8 +620,11 @@ pub struct WorkdirRegistryRecord {
pub repository_id: String,
pub creation_selector: Option<String>,
pub creation_ref: Option<String>,
pub creation_tree: Option<String>,
pub current_selector: Option<String>,
pub current_ref: Option<String>,
pub current_tree: Option<String>,
pub observed_at_epoch_seconds: Option<u64>,
pub materialization_status: String,
pub cleanliness: String,
pub created_at: String,
@@ -4627,16 +4630,20 @@ impl ControlPlaneStore for SqliteWorkspaceStore {
conn.execute(
r#"INSERT INTO workdir_registry (
workspace_id, workdir_id, runtime_id, repository_id,
creation_selector, creation_ref, current_selector, current_ref,
creation_selector, creation_ref, creation_tree,
current_selector, current_ref, current_tree, observed_at_epoch_seconds,
materialization_status, cleanliness, created_at, updated_at
) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12)
) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)
ON CONFLICT(workspace_id, workdir_id) DO UPDATE SET
runtime_id = excluded.runtime_id,
repository_id = excluded.repository_id,
creation_selector = excluded.creation_selector,
creation_ref = excluded.creation_ref,
creation_tree = excluded.creation_tree,
current_selector = excluded.current_selector,
current_ref = excluded.current_ref,
current_tree = excluded.current_tree,
observed_at_epoch_seconds = excluded.observed_at_epoch_seconds,
materialization_status = excluded.materialization_status,
cleanliness = excluded.cleanliness,
updated_at = excluded.updated_at"#,
@@ -4647,8 +4654,11 @@ impl ControlPlaneStore for SqliteWorkspaceStore {
record.repository_id,
record.creation_selector,
record.creation_ref,
record.creation_tree,
record.current_selector,
record.current_ref,
record.current_tree,
record.observed_at_epoch_seconds.map(|value| value as i64),
record.materialization_status,
record.cleanliness,
record.created_at,
@@ -5868,7 +5878,8 @@ fn require_expected_ticket_assignment(
fn workdir_registry_select_sql(where_clause: &str) -> String {
format!(
"SELECT workspace_id, workdir_id, runtime_id, repository_id, \
creation_selector, creation_ref, current_selector, current_ref, \
creation_selector, creation_ref, creation_tree, \
current_selector, current_ref, current_tree, observed_at_epoch_seconds, \
materialization_status, cleanliness, created_at, updated_at \
FROM workdir_registry {where_clause}"
)
@@ -5884,12 +5895,15 @@ fn read_workdir_registry_record(
repository_id: row.get(3)?,
creation_selector: row.get(4)?,
creation_ref: row.get(5)?,
current_selector: row.get(6)?,
current_ref: row.get(7)?,
materialization_status: row.get(8)?,
cleanliness: row.get(9)?,
created_at: row.get(10)?,
updated_at: row.get(11)?,
creation_tree: row.get(6)?,
current_selector: row.get(7)?,
current_ref: row.get(8)?,
current_tree: row.get(9)?,
observed_at_epoch_seconds: row.get::<_, Option<i64>>(10)?.map(|value| value as u64),
materialization_status: row.get(11)?,
cleanliness: row.get(12)?,
created_at: row.get(13)?,
updated_at: row.get(14)?,
})
}
@@ -6830,6 +6844,9 @@ fn create_repository_ssh_secret_authority(conn: &Connection) -> Result<()> {
fn bind_workdir_create_repository_access_evidence(conn: &Connection) -> Result<()> {
conn.execute_batch(
r#"
ALTER TABLE workdir_registry ADD COLUMN creation_tree TEXT;
ALTER TABLE workdir_registry ADD COLUMN current_tree TEXT;
ALTER TABLE workdir_registry ADD COLUMN observed_at_epoch_seconds INTEGER;
ALTER TABLE workdir_create_operations ADD COLUMN source_kind TEXT;
ALTER TABLE workdir_create_operations ADD COLUMN source_uri TEXT;
ALTER TABLE workdir_create_operations ADD COLUMN source_revision INTEGER;
@@ -11643,6 +11660,9 @@ INSERT INTO worker_registry (
DROP TABLE repository_ssh_host_trust_revisions;
DROP TABLE repository_ssh_host_trusts;
DROP TABLE workdir_create_operations;
ALTER TABLE workdir_registry DROP COLUMN creation_tree;
ALTER TABLE workdir_registry DROP COLUMN current_tree;
ALTER TABLE workdir_registry DROP COLUMN observed_at_epoch_seconds;
DELETE FROM __yoi_schema_migrations WHERE version IN (45, 46, 47);",
)
.unwrap();
@@ -11681,6 +11701,9 @@ INSERT INTO worker_registry (
DROP TABLE repository_ssh_credentials;
DROP TABLE repository_ssh_host_trust_revisions;
DROP TABLE repository_ssh_host_trusts;
ALTER TABLE workdir_registry DROP COLUMN creation_tree;
ALTER TABLE workdir_registry DROP COLUMN current_tree;
ALTER TABLE workdir_registry DROP COLUMN observed_at_epoch_seconds;
ALTER TABLE workdir_create_operations DROP COLUMN source_kind;
ALTER TABLE workdir_create_operations DROP COLUMN source_uri;
ALTER TABLE workdir_create_operations DROP COLUMN source_revision;
@@ -11722,7 +11745,10 @@ INSERT INTO worker_registry (
configure_sqlite(&conn).unwrap();
apply_migrations(&conn).unwrap();
conn.execute_batch(
"ALTER TABLE workdir_create_operations DROP COLUMN source_kind;
"ALTER TABLE workdir_registry DROP COLUMN creation_tree;
ALTER TABLE workdir_registry DROP COLUMN current_tree;
ALTER TABLE workdir_registry DROP COLUMN observed_at_epoch_seconds;
ALTER TABLE workdir_create_operations DROP COLUMN source_kind;
ALTER TABLE workdir_create_operations DROP COLUMN source_uri;
ALTER TABLE workdir_create_operations DROP COLUMN source_revision;
ALTER TABLE workdir_create_operations DROP COLUMN source_fingerprint;
@@ -11757,6 +11783,13 @@ INSERT INTO worker_registry (
"missing column {required}"
);
}
let workdir_columns = table_columns(&conn, "workdir_registry").unwrap();
for required in ["creation_tree", "current_tree", "observed_at_epoch_seconds"] {
assert!(
workdir_columns.iter().any(|column| column == required),
"missing column {required}"
);
}
}
#[test]
@@ -12537,6 +12570,9 @@ WHERE workspace_id = 'workspace-a'
"updated_at",
"current_selector",
"current_ref",
"creation_tree",
"current_tree",
"observed_at_epoch_seconds",
],
);
assert_columns(
@@ -13028,8 +13064,11 @@ CREATE TABLE ticket_assignment_operations (
repository_id: "repo".to_string(),
creation_selector: Some("develop".to_string()),
creation_ref: Some("abcdef".to_string()),
creation_tree: Some("tree-creation".to_string()),
current_selector: None,
current_ref: Some("abcdef".to_string()),
current_tree: Some("tree-current".to_string()),
observed_at_epoch_seconds: Some(1_777_777_777),
materialization_status: "not_found".to_string(),
cleanliness: "clean".to_string(),
created_at: "2".to_string(),
@@ -13043,8 +13082,11 @@ CREATE TABLE ticket_assignment_operations (
repository_id: "repo".to_string(),
creation_selector: Some("feature".to_string()),
creation_ref: Some("123456".to_string()),
creation_tree: None,
current_selector: Some("feature".to_string()),
current_ref: Some("123456".to_string()),
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "present".to_string(),
cleanliness: "unknown".to_string(),
created_at: "3".to_string(),
@@ -4,6 +4,17 @@ use sha2::{Digest, Sha256};
use crate::store::WorkdirCreateOperationRecord;
use crate::{Error, Result, SqliteWorkspaceStore};
pub fn selector_for_retry(
explicit_selector: Option<&str>,
persisted_selector: Option<&str>,
current_default_selector: Option<&str>,
) -> Option<String> {
explicit_selector
.or(persisted_selector)
.or(current_default_selector)
.map(str::to_string)
}
pub fn request_fingerprint(
repository_id: &str,
selector: Option<&str>,
@@ -267,6 +278,22 @@ mod tests {
use super::*;
use crate::store::{ControlPlaneStore, RepositoryRecord, WorkspaceRecord};
#[test]
fn retry_selector_keeps_persisted_default_but_honors_explicit_input() {
assert_eq!(
selector_for_retry(None, Some("develop"), Some("main")),
Some("develop".to_string())
);
assert_eq!(
selector_for_retry(Some("release"), Some("develop"), Some("main")),
Some("release".to_string())
);
assert_eq!(
selector_for_retry(None, None, Some("main")),
Some("main".to_string())
);
}
#[test]
fn retry_keeps_resolved_config_evidence_and_rejects_changed_input() {
let store = SqliteWorkspaceStore::in_memory().unwrap();