Skip to content

Commit 9424410

Browse files
Make client CodeForge deserialization forward-compatible (fixes repo-less run "environment not found") (#15456)
<!-- warp:pr-description-artifacts start --> <!-- warp:pr-description-artifacts end --> ## Description Fixes a staging bug found while testing REMOTE-2965 (forge-less/repo-less factories, warp-server#15763): dispatching a run against a repo-less (`code_forge: NONE`) environment failed with a misleading `Environment '<id>' not found` error, even though the environment demonstrably existed server-side (visible in the legacy Oz web UI). **Root cause**: `cloud_object_models::CodeForge` only recognizes `GITHUB` and `GITLAB`. `AmbientAgentEnvironment.code_forge` is `Option<CodeForge>`, and serde's `default` only covers an *absent* field — a *present* but unrecognized string (`"NONE"`, which the server correctly sends for a repo-less environment) is a hard deserialization error for the whole environment object. The sync layer never materializes that environment locally, so the later `CloudAmbientAgentEnvironment::get_by_id` lookup in `AgentDriver::resolve_environment` comes back empty and gets reported as "not found" — a client-side sync failure, not a server 404. **The actual defect isn't that `NONE` was missing** — it's that the client cannot survive the server introducing any forge value it doesn't yet know about. Multi-forge support is coming and will add more values, so this exact failure mode would recur. The fix: - Adds an explicit `CodeForge::None` variant for the repo-less case. - Adds a `#[serde(other)] Unknown` catch-all, the pattern this crate already uses for the same problem elsewhere (see `ActionPermission`, `WriteToPtyPermission`, `ComputerUsePermission`, `RunAgentsPermission` in `ai_execution_profile.rs`), so a forge value newer than this client build degrades to "no usable forge" (`host()` returns `""`) instead of failing deserialization or silently defaulting to GitHub — which would send an old client off to authenticate against the wrong host. - `repository_forge_for_repo` (per-repository forge resolution) now returns `Option<RepositoryForge>` instead of assuming every repository's forge is resolvable. `AmbientAgentEnvironment::effective_repos()` copies the container's forge onto any repository that omits its own, so a repository can legitimately carry `None`/`Unknown` today (repo-less environment) or in the future (a forge value newer than this client). `repository_clone_requests` now returns a new `PrepareEnvironmentError::UnsupportedRepositoryForge` for such a repository — checked before head-override matching, so it fails fast with a clear error rather than either panicking or silently building a clone request with an empty host. `head_override_matches_repo` correspondingly never matches an override against a repository whose forge can't be resolved. The actual repo-cloning/setup-command logic (`prepare_environment_impl`) already handles an empty `source_repos` list correctly (the pre-existing legacy zero-repo path), so once the environment deserializes, run execution against a repo-less environment needs no further change. ## Linked Issue Linear REMOTE-2965 (no GitHub issue). Follow-up to warp-server#15763. ## Testing - [x] Reproduced the exact defects with unit tests first (`deserialize_environment_with_unrecognized_forge_still_succeeds` fails without the `Unknown` catch-all; `clone_requests_reject_a_repository_with_an_unrecognized_forge` panicked against an earlier revision of this PR that treated an unresolvable repository forge as impossible), then fixed each. - Added unit tests in `crates/cloud_object_models/src/cloud_environment_tests.rs`: - `deserialize_repo_less_environment_resolves_to_none_forge` — a `code_forge: "NONE"` environment deserializes and resolves to `CodeForge::None` with no repositories. - `deserialize_environment_with_unrecognized_forge_still_succeeds` — a made-up future forge value (`"BITBUCKET"`) still lets the whole environment deserialize, resolving to `CodeForge::Unknown` rather than failing or silently becoming `GitHub`. - `none_and_unknown_forges_have_no_clonable_host` — pins that neither degrades to `github.com`. - Added unit tests in `app/src/ai/agent_sdk/driver/environment_tests.rs`: - `clone_requests_reject_a_repository_with_an_unrecognized_forge` — building clone requests for an `Unknown`-forge repository returns `UnsupportedRepositoryForge` instead of panicking. - `clone_requests_reject_an_unrecognized_forge_repository_even_with_unrelated_overrides` — same, but with a head override present for a different, resolvable-forge repository in the same environment, proving this path is reachable via ordinary override handling. - `head_override_validation_treats_an_unrecognized_forge_repository_as_never_matching` — `validate_repository_head_overrides` reports such a repository as "not declared" instead of panicking when an override names it. - `cargo test -p cloud_object_models` — all green. `cargo test -p warp --lib ai::agent_sdk` — 529 of 530 pass; the one failure (`api_key::tests::resolve_api_key_identifier_errors_for_ambiguous_name_matches`) blocks on an interactive TTY prompt in this sandbox and is unrelated to this change. - `cargo clippy -p cloud_object_models --all-targets --all-features -- -D warnings` and `cargo clippy -p warp --lib -- -D warnings` — clean. `./script/format` — clean. - Not manually tested with `./script/run`: the observable failure only reproduces inside an actual dispatched cloud/local agent run against a repo-less environment, which requires live dispatch infrastructure this environment doesn't have. The unit tests above reproduce the actual defects (deserialization, then clone-request building) directly, which is the level `rust-unit-tests` recommends for a deterministic, non-UI bug like this one. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-NONE --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com>
1 parent 702aa10 commit 9424410

4 files changed

Lines changed: 168 additions & 11 deletions

File tree

app/src/ai/agent_sdk/driver/environment.rs

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ pub enum PrepareEnvironmentError {
5656
first_owner: String,
5757
second_owner: String,
5858
},
59+
#[error(
60+
"Repository {repo_name} has a code forge this client build doesn't support; update Warp to a version that does"
61+
)]
62+
UnsupportedRepositoryForge { repo_name: String },
5963
#[error("Terminal driver error while preparing environment: {source}")]
6064
TerminalDriver { source: AgentDriverError },
6165
}
@@ -299,7 +303,7 @@ async fn prepare_environment_impl(
299303
setup_events
300304
.record_result(SetupStep::EnvironmentRepoClone, async {
301305
clone_checkout_requests(
302-
&repository_clone_requests(source_repos, repository_head_overrides),
306+
&repository_clone_requests(source_repos, repository_head_overrides)?,
303307
working_dir,
304308
spawner,
305309
)
@@ -465,14 +469,19 @@ fn record_codebase_indexing(
465469
});
466470
}
467471

468-
fn repository_forge_for_repo(repo: &SourceRepo) -> RepositoryForge {
472+
// `None` covers both a repo-less container forge and one this client build
473+
// doesn't recognize. Unlike `None`, a future server can assign the latter to
474+
// a real repository before this client updates, so callers must treat it as
475+
// an ordinary "can't clone this" outcome rather than an invariant violation.
476+
fn repository_forge_for_repo(repo: &SourceRepo) -> Option<RepositoryForge> {
469477
match repo.code_forge.unwrap_or_default() {
470-
CodeForge::GitHub => RepositoryForge::GitHub,
471-
CodeForge::GitLab => RepositoryForge::GitLab,
478+
CodeForge::GitHub => Some(RepositoryForge::GitHub),
479+
CodeForge::GitLab => Some(RepositoryForge::GitLab),
480+
CodeForge::None | CodeForge::Unknown => None,
472481
}
473482
}
474483
fn head_override_matches_repo(head_override: &RepositoryHeadOverride, repo: &SourceRepo) -> bool {
475-
head_override.code_forge == repository_forge_for_repo(repo)
484+
Some(head_override.code_forge) == repository_forge_for_repo(repo)
476485
&& head_override.repo_owner == repo.owner
477486
&& head_override.repo_name == repo.repo
478487
}
@@ -495,16 +504,25 @@ struct RepositoryCloneRequest {
495504
fn repository_clone_requests(
496505
repos: &[SourceRepo],
497506
overrides: &[RepositoryHeadOverride],
498-
) -> Vec<RepositoryCloneRequest> {
507+
) -> Result<Vec<RepositoryCloneRequest>, PrepareEnvironmentError> {
499508
repos
500509
.iter()
501510
.cloned()
502511
.map(|repo| {
512+
// A repository this client can't identify a host for can never
513+
// clone; fail clearly here rather than attempt one with an empty
514+
// host, which would otherwise be the only signal something is
515+
// wrong.
516+
if repository_forge_for_repo(&repo).is_none() {
517+
return Err(PrepareEnvironmentError::UnsupportedRepositoryForge {
518+
repo_name: format!("{}/{}", repo.owner, repo.repo),
519+
});
520+
}
503521
let checkout = match head_override_for_repo(overrides, &repo) {
504522
Some(head_override) => Some(head_override.head.clone()),
505523
None => repo.checkout_ref.clone().map(RepositoryHeadRef::Branch),
506524
};
507-
RepositoryCloneRequest { repo, checkout }
525+
Ok(RepositoryCloneRequest { repo, checkout })
508526
})
509527
.collect()
510528
}
@@ -661,7 +679,12 @@ pub(super) async fn clone_repos(
661679
working_dir: &Path,
662680
spawner: &ModelSpawner<TerminalDriver>,
663681
) -> Result<(), PrepareEnvironmentError> {
664-
clone_checkout_requests(&repository_clone_requests(repos, &[]), working_dir, spawner).await
682+
clone_checkout_requests(
683+
&repository_clone_requests(repos, &[])?,
684+
working_dir,
685+
spawner,
686+
)
687+
.await
665688
}
666689

667690
async fn clone_checkout_requests(

app/src/ai/agent_sdk/driver/environment_tests.rs

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ fn head_overrides_replace_checkout_ref_only_for_matching_repos() {
344344
branch_head_override(RepositoryForge::GitHub, "warpdotdev", "unused", "develop"),
345345
];
346346

347-
let prepared = repository_clone_requests(&repos, &overrides);
347+
let prepared = repository_clone_requests(&repos, &overrides).unwrap();
348348

349349
assert_eq!(
350350
prepared[0].checkout,
@@ -358,6 +358,84 @@ fn head_overrides_replace_checkout_ref_only_for_matching_repos() {
358358
);
359359
}
360360

361+
#[test]
362+
fn clone_requests_reject_a_repository_with_an_unrecognized_forge() {
363+
// An environment forge value newer than this client build (see
364+
// CodeForge::Unknown) can still be assigned to a real repository by a
365+
// newer server. Building clone requests for it must fail clearly rather
366+
// than panic or silently attempt a clone with no host.
367+
let repos = vec![SourceRepo::new(
368+
CodeForge::Unknown,
369+
"warpdotdev".to_string(),
370+
"warp".to_string(),
371+
)];
372+
373+
let error = repository_clone_requests(&repos, &[]).unwrap_err();
374+
375+
assert!(matches!(
376+
error,
377+
PrepareEnvironmentError::UnsupportedRepositoryForge { repo_name }
378+
if repo_name == "warpdotdev/warp"
379+
));
380+
}
381+
382+
#[test]
383+
fn clone_requests_reject_an_unrecognized_forge_repository_even_with_unrelated_overrides() {
384+
// A head override targeting a different, supported-forge repository must
385+
// not mask the unsupported repository elsewhere in the same environment:
386+
// every repository is checked, not just the ones an override names.
387+
let repos = vec![
388+
SourceRepo::new(
389+
CodeForge::GitHub,
390+
"warpdotdev".to_string(),
391+
"warp".to_string(),
392+
),
393+
SourceRepo::new(
394+
CodeForge::Unknown,
395+
"warpdotdev".to_string(),
396+
"warp-server".to_string(),
397+
),
398+
];
399+
let overrides = vec![commit_head_override(
400+
RepositoryForge::GitHub,
401+
"warpdotdev",
402+
"warp",
403+
"0123456789abcdef0123456789abcdef01234567",
404+
)];
405+
406+
let error = repository_clone_requests(&repos, &overrides).unwrap_err();
407+
408+
assert!(matches!(
409+
error,
410+
PrepareEnvironmentError::UnsupportedRepositoryForge { repo_name }
411+
if repo_name == "warpdotdev/warp-server"
412+
));
413+
}
414+
415+
#[test]
416+
fn head_override_validation_treats_an_unrecognized_forge_repository_as_never_matching() {
417+
// No head override can target a repository whose forge this client can't
418+
// represent; validation must reject it as "not declared" (an override
419+
// that names a repository the environment doesn't have) rather than
420+
// panicking while checking whether it matches.
421+
let environment = environment_with_repos(vec![SourceRepo::new(
422+
CodeForge::Unknown,
423+
"warpdotdev".to_string(),
424+
"warp".to_string(),
425+
)]);
426+
let override_for_it = commit_head_override(
427+
RepositoryForge::GitHub,
428+
"warpdotdev",
429+
"warp",
430+
"0123456789abcdef0123456789abcdef01234567",
431+
);
432+
433+
let error =
434+
validate_repository_head_overrides(&environment.effective_repos(), &[override_for_it])
435+
.expect_err("an unrecognized-forge repository can never match an override");
436+
assert!(error.to_string().contains("not declared"));
437+
}
438+
361439
#[test]
362440
fn repository_head_override_validation_rejects_duplicates_and_mismatches() {
363441
let environment = environment_with_repos(vec![SourceRepo::new(
@@ -439,7 +517,7 @@ fn applied_head_overrides_are_threaded_through_the_existing_clone_command() {
439517
"develop",
440518
)];
441519
let command = build_parallel_clone_command(
442-
&repository_clone_requests(&repos, &overrides),
520+
&repository_clone_requests(&repos, &overrides).unwrap(),
443521
ShellType::Bash,
444522
);
445523

@@ -468,7 +546,7 @@ fn applied_commit_override_uses_sha_only_fetch() {
468546
"0123456789abcdef0123456789abcdef01234567",
469547
)];
470548
let command = build_parallel_clone_command(
471-
&repository_clone_requests(&repos, &overrides),
549+
&repository_clone_requests(&repos, &overrides).unwrap(),
472550
ShellType::Bash,
473551
);
474552

crates/cloud_object_models/src/cloud_environment.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,26 @@ pub enum CodeForge {
1616
GitHub,
1717
#[serde(rename = "GITLAB")]
1818
GitLab,
19+
/// Explicit "no code forge" container value: a repo-less environment
20+
/// that clones nothing and relies entirely on `setup_commands`.
21+
#[serde(rename = "NONE")]
22+
None,
23+
// Catches a forge value this client build doesn't recognize yet (e.g. the
24+
// server adds one before this client updates), so the rest of the
25+
// environment still deserializes instead of the whole object failing.
26+
#[serde(other)]
27+
Unknown,
1928
}
2029

2130
impl CodeForge {
31+
/// The clonable host for this forge, empty for `None`/`Unknown` since
32+
/// neither identifies one; callers must not fall back to `github.com`
33+
/// for either, which would authenticate against the wrong host.
2234
pub const fn host(self) -> &'static str {
2335
match self {
2436
CodeForge::GitHub => "github.com",
2537
CodeForge::GitLab => "gitlab.com",
38+
CodeForge::None | CodeForge::Unknown => "",
2639
}
2740
}
2841
}
@@ -32,6 +45,8 @@ impl fmt::Display for CodeForge {
3245
match self {
3346
CodeForge::GitHub => write!(f, "GitHub"),
3447
CodeForge::GitLab => write!(f, "GitLab"),
48+
CodeForge::None => write!(f, "None"),
49+
CodeForge::Unknown => write!(f, "Unknown"),
3550
}
3651
}
3752
}

crates/cloud_object_models/src/cloud_environment_tests.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,47 @@ fn source_repo_checkout_ref_round_trips_and_is_optional() {
109109
assert_eq!(parsed.checkout_ref, None);
110110
}
111111

112+
#[test]
113+
fn deserialize_repo_less_environment_resolves_to_none_forge() {
114+
let json = serde_json::json!({
115+
"name": "repo-less-env",
116+
"code_forge": "NONE",
117+
"github_repos": [],
118+
"source_repos": [],
119+
"setup_commands": ["echo hello"]
120+
});
121+
122+
let env: AmbientAgentEnvironment = serde_json::from_value(json).unwrap();
123+
124+
assert_eq!(env.effective_code_forge(), CodeForge::None);
125+
assert!(env.effective_repos().is_empty());
126+
}
127+
128+
#[test]
129+
fn deserialize_environment_with_unrecognized_forge_still_succeeds() {
130+
// A forge value this client build doesn't know about yet (e.g. the
131+
// server introduces a new one before this client updates) must not fail
132+
// deserialization of the whole environment.
133+
let json = serde_json::json!({
134+
"name": "future-forge-env",
135+
"code_forge": "BITBUCKET",
136+
"github_repos": [],
137+
"setup_commands": ["echo hello"]
138+
});
139+
140+
let env: AmbientAgentEnvironment = serde_json::from_value(json).unwrap();
141+
142+
assert_eq!(env.effective_code_forge(), CodeForge::Unknown);
143+
}
144+
145+
#[test]
146+
fn none_and_unknown_forges_have_no_clonable_host() {
147+
// Neither identifies a real host; a caller falling back to GitHub's host
148+
// for either would authenticate against the wrong one.
149+
assert_eq!(CodeForge::None.host(), "");
150+
assert_eq!(CodeForge::Unknown.host(), "");
151+
}
152+
112153
#[test]
113154
fn deserialize_gitlab_environment_uses_authoritative_source_repos() {
114155
let json = serde_json::json!({

0 commit comments

Comments
 (0)