Make client CodeForge deserialization forward-compatible (fixes repo-less run "environment not found") - #15456
Conversation
A repo-less (code_forge: NONE) environment's task run failed with a misleading "Environment not found" error on staging, even though the GSO plainly existed server-side. The real defect: the client's CodeForge enum only recognized GITHUB and GITLAB, so an unrecognized forge value present on the wire (NONE, or any future forge) failed deserialization of the whole environment object instead of just that field. The sync layer never materialized the environment locally, so the later lookup came back empty and got reported as "not found" - pointing at entirely the wrong layer. Adds an explicit None variant for the repo-less case, plus a #[serde(other)] Unknown catch-all (the pattern already used elsewhere in this crate, e.g. ActionPermission) so a forge value newer than this client build degrades to "no usable forge" rather than failing deserialization or silently defaulting to GitHub, which would send an old client off to authenticate against the wrong host.
|
This PR was generated with Warp. Comment |
|
@warp-factory is this ready for review? |
|
Not yet — review found the new Responding as wilson: Open session · View in factory |
Review caught that the panic in repository_forge_for_repo was reachable: effective_repos() copies the container forge onto every repository that omits its own, so a future server forge value on an environment WITH repositories deserializes fine (via CodeForge::Unknown) and then panics while building clone requests, whenever the run has any repository HEAD override at all - moving the failure from deserialization to run preparation instead of actually tolerating it. CodeForgeNone is enforced as a container-only value by the server today, but Unknown specifically represents a value a newer server assigned before this client recognized it, and no invariant of today's server can bound what a future one does. Historical or corrupt payloads land in the same place. A panic can't rely on an invariant a future version is free to violate. repository_forge_for_repo now returns Option<RepositoryForge>, so an override can no longer match a repository with an unsupported forge. repository_clone_requests fails clearly with a new UnsupportedRepositoryForge error for such a repository, rather than silently building a clone request with an empty host.
|
Ready now — out of draft, CI green. The panic is gone: Worth knowing for rollout: cloud-dispatched runs pick this up once the Responding as wilson: Open session · View in factory |



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 misleadingEnvironment '<id>' not founderror, even though the environment demonstrably existed server-side (visible in the legacy Oz web UI).Root cause:
cloud_object_models::CodeForgeonly recognizesGITHUBandGITLAB.AmbientAgentEnvironment.code_forgeisOption<CodeForge>, and serde'sdefaultonly 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 laterCloudAmbientAgentEnvironment::get_by_idlookup inAgentDriver::resolve_environmentcomes back empty and gets reported as "not found" — a client-side sync failure, not a server 404.The actual defect isn't that
NONEwas 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:CodeForge::Nonevariant for the repo-less case.#[serde(other)] Unknowncatch-all, the pattern this crate already uses for the same problem elsewhere (seeActionPermission,WriteToPtyPermission,ComputerUsePermission,RunAgentsPermissioninai_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 returnsOption<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 carryNone/Unknowntoday (repo-less environment) or in the future (a forge value newer than this client).repository_clone_requestsnow returns a newPrepareEnvironmentError::UnsupportedRepositoryForgefor 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_repocorrespondingly 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 emptysource_reposlist 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
deserialize_environment_with_unrecognized_forge_still_succeedsfails without theUnknowncatch-all;clone_requests_reject_a_repository_with_an_unrecognized_forgepanicked against an earlier revision of this PR that treated an unresolvable repository forge as impossible), then fixed each.crates/cloud_object_models/src/cloud_environment_tests.rs:deserialize_repo_less_environment_resolves_to_none_forge— acode_forge: "NONE"environment deserializes and resolves toCodeForge::Nonewith no repositories.deserialize_environment_with_unrecognized_forge_still_succeeds— a made-up future forge value ("BITBUCKET") still lets the whole environment deserialize, resolving toCodeForge::Unknownrather than failing or silently becomingGitHub.none_and_unknown_forges_have_no_clonable_host— pins that neither degrades togithub.com.app/src/ai/agent_sdk/driver/environment_tests.rs:clone_requests_reject_a_repository_with_an_unrecognized_forge— building clone requests for anUnknown-forge repository returnsUnsupportedRepositoryForgeinstead 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_overridesreports 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 warningsandcargo clippy -p warp --lib -- -D warnings— clean../script/format— clean../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 levelrust-unit-testsrecommends for a deterministic, non-UI bug like this one.Agent Mode
CHANGELOG-NONE