Harden local RunIntent target observation

lib/components/fabro-manifest/src/lib.rs

@@ -26,11 +26,10 @@ use fabro_types::graph::ReferenceKind;
 use fabro_types::settings::interp::InterpString;
 use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode};
 use fabro_types::{
-    DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, WorkflowSettings,
-};
-use fabro_workflow::git::{
-    GitSyncStatus, branch_needs_push, push_branch_noninteractive, sync_status,
+    DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, RunTarget,
+    WorkflowSettings,
 };
+use fabro_workflow::git::{self, GitSyncStatus};
 
 pub use crate::local_workflow_package::{
     LocalWorkflowPackageError, ResolvedLocalWorkflowPackage, resolve_local_workflow_package,
@@ -216,8 +215,7 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManifest> {
     )?;
 
     let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings);
-    let git = observe_git_run_target(&working_directory, configured_repo_origin_url.as_deref())
-        .map(|observation| observation.legacy_git_context);
+    let git = build_legacy_git_context(&working_directory, configured_repo_origin_url.as_deref());
     let args = input.args.filter(|args| !manifest_args_is_empty(args));
 
     Ok(BuiltManifest {
@@ -309,11 +307,11 @@ fn resolved_goal_to_manifest(resolved: ResolvedRunGoal) -> types::ManifestGoal {
 
 /// Facts observed from one usable attached local Git checkout.
 ///
-/// The optional target is absent when the checkout's effective origin is not
-/// a canonical GitHub repository. Its SHA is present only when local tracking
-/// state or the existing safe best-effort push proves that exact commit is
-/// remotely available. The legacy projection retains the historical local
-/// SHA and normalized-origin behavior independently.
+/// The optional target is absent when the checkout cannot be represented as a
+/// valid GitHub run target. Its SHA is present only when a successful push or
+/// a direct query of the remote proves that exact commit is available. The
+/// legacy projection retains the historical local SHA and normalized-origin
+/// behavior independently.
 #[derive(Clone, Debug)]
 pub struct GitRunTargetObservation {
     pub run_target:         Option<GitRunTarget>,
@@ -324,19 +322,59 @@ pub struct GitRunTargetObservation {
 ///
 /// Outer `None` means `repo_path` is not a usable attached checkout. A
 /// returned observation with no target means Git facts were available but the
-/// effective origin was not a canonical GitHub repository.
+/// effective origin or attached branch cannot be represented by a valid
+/// GitHub run target. For a valid target, this operation may contact the
+/// remote and may make one noninteractive best-effort push of the attached
+/// branch so clone-based execution can resolve the observed commit.
 #[must_use]
 pub fn observe_git_run_target(
     repo_path: &Path,
     configured_repo_origin_url: Option<&str>,
 ) -> Option<GitRunTargetObservation> {
+    let local = inspect_local_git(repo_path, configured_repo_origin_url)?;
+    let legacy_git_context = local.legacy_git_context;
+    let mut run_target = github_run_target(
+        &legacy_git_context.origin_url,
+        &legacy_git_context.branch,
+        None,
+    );
+    if let Some(target) = run_target.as_mut() {
+        let publish_status = publish_manifest_branch_best_effort(
+            repo_path,
+            &legacy_git_context.branch,
+            local.push_origin_url.as_deref(),
+            configured_repo_origin_url,
+        );
+        target.sha = remotely_available_sha(
+            repo_path,
+            &legacy_git_context.branch,
+            legacy_git_context.sha.as_deref(),
+            publish_status,
+        );
+    }
+
+    Some(GitRunTargetObservation {
+        run_target,
+        legacy_git_context,
+    })
+}
+
+struct LocalGitObservation {
+    push_origin_url:    Option<String>,
+    legacy_git_context: GitContext,
+}
+
+fn inspect_local_git(
+    repo_path: &Path,
+    configured_repo_origin_url: Option<&str>,
+) -> Option<LocalGitObservation> {
     let ManifestRepoInfo {
         origin_url,
         push_origin_url,
         branch,
         sha,
     } = detect_manifest_repo_info(repo_path)?;
-    let dirty = match sync_status(repo_path, "origin", Some(&branch)) {
+    let dirty = match git::sync_status(repo_path, "origin", Some(&branch)) {
         GitSyncStatus::Dirty => DirtyStatus::Dirty,
         GitSyncStatus::Synced | GitSyncStatus::Unsynced => DirtyStatus::Clean,
     };
@@ -350,19 +388,9 @@ pub fn observe_git_run_target(
                 .filter(|url| !url.is_empty())
         })
         .unwrap_or_default();
-    let remotely_available = push_manifest_branch_best_effort(
-        repo_path,
-        &branch,
-        push_origin_url.as_deref(),
-        configured_repo_origin_url,
-    );
-    let run_target = github_run_target(
-        &repo_origin_url,
-        &branch,
-        sha.clone().filter(|_| remotely_available),
-    );
-    Some(GitRunTargetObservation {
-        run_target,
+
+    Some(LocalGitObservation {
+        push_origin_url,
         legacy_git_context: GitContext {
             origin_url: repo_origin_url,
             branch,
@@ -372,15 +400,35 @@ pub fn observe_git_run_target(
     })
 }
 
+fn build_legacy_git_context(
+    repo_path: &Path,
+    configured_repo_origin_url: Option<&str>,
+) -> Option<GitContext> {
+    let local = inspect_local_git(repo_path, configured_repo_origin_url)?;
+    publish_manifest_branch_best_effort(
+        repo_path,
+        &local.legacy_git_context.branch,
+        local.push_origin_url.as_deref(),
+        configured_repo_origin_url,
+    );
+    Some(local.legacy_git_context)
+}
+
 fn github_run_target(origin_url: &str, branch: &str, sha: Option<String>) -> Option<GitRunTarget> {
     let (owner, repository) = fabro_github::parse_github_owner_repo(origin_url).ok()?;
     let slug = GitHubRepositorySlug::try_new(&format!("{owner}/{repository}"))?;
-    Some(GitRunTarget {
+    let validated = RunTarget::Git(GitRunTarget {
         repo: slug.to_string(),
         branch: branch.to_owned(),
         tag: None,
         sha,
     })
+    .validate()
+    .ok()?;
+    let RunTarget::Git(target) = validated.target else {
+        unreachable!("a validated Git target must remain a Git target")
+    };
+    Some(target)
 }
 
 fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option<String> {
@@ -444,18 +492,25 @@ fn detect_manifest_repo_info(repo_path: &Path) -> Option<ManifestRepoInfo> {
     })
 }
 
-/// Best-effort push of the local branch so clone-based execution can see
-/// local commits. A failed push must not fail manifest creation, and the
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum BranchPublishStatus {
+    TrackingRefMatches,
+    Pushed,
+    Unavailable,
+}
+
+/// Best-effort publication of the local branch so clone-based execution can
+/// see local commits. A failed push must not fail manifest creation, and the
 /// discarded push error may contain raw Git stderr, so it is deliberately
 /// neither returned nor logged here.
-fn push_manifest_branch_best_effort(
+fn publish_manifest_branch_best_effort(
     repo_path: &Path,
     branch: &str,
     origin_url: Option<&str>,
     configured_repo_origin_url: Option<&str>,
-) -> bool {
+) -> BranchPublishStatus {
     let Some(origin_url) = origin_url else {
-        return false;
+        return BranchPublishStatus::Unavailable;
     };
 
     if let Some(repo_origin_url) = configured_repo_origin_url
@@ -464,15 +519,39 @@ fn push_manifest_branch_best_effort(
     {
         let remote = fabro_github::normalize_repo_origin_url(origin_url);
         if remote != repo_origin_url {
-            return false;
+            return BranchPublishStatus::Unavailable;
         }
     }
 
-    if !branch_needs_push(repo_path, "origin", branch) {
-        return true;
+    if !git::branch_needs_push(repo_path, "origin", branch) {
+        return BranchPublishStatus::TrackingRefMatches;
     }
 
-    push_branch_noninteractive(repo_path, "origin", branch).is_ok()
+    if git::push_branch_noninteractive(repo_path, "origin", branch).is_ok() {
+        BranchPublishStatus::Pushed
+    } else {
+        BranchPublishStatus::Unavailable
+    }
+}
+
+fn remotely_available_sha(
+    repo_path: &Path,
+    branch: &str,
+    local_sha: Option<&str>,
+    publish_status: BranchPublishStatus,
+) -> Option<String> {
+    let local_sha = local_sha?;
+    match publish_status {
+        BranchPublishStatus::Pushed => Some(local_sha.to_owned()),
+        BranchPublishStatus::TrackingRefMatches => {
+            git::remote_branch_sha_noninteractive(repo_path, "origin", branch)
+                .ok()
+                .flatten()
+                .filter(|remote_sha| remote_sha == local_sha)
+                .map(|_| local_sha.to_owned())
+        }
+        BranchPublishStatus::Unavailable => None,
+    }
 }
 
 /// Resolve a workflow reference and reject it when neither its config nor
@@ -1722,10 +1801,18 @@ exit 1
         let temp = tempfile::tempdir().unwrap();
         let workspace = temp.path().join("workspace");
         std::fs::create_dir_all(&workspace).unwrap();
+        let bare_origin = init_bare_origin(temp.path());
         init_git_repo(&workspace, "feature", "https://github.com/acme/widgets.git");
-        mark_origin_branch_synced(&workspace, "feature");
+        let local_url = format!("file://{}", bare_origin.display());
+        run_git(&workspace, &[
+            "config",
+            &format!("url.{local_url}.insteadOf"),
+            "https://github.com/acme/widgets.git",
+        ]);
+        run_git(&workspace, &["push", "origin", "feature"]);
 
-        let observation = observe_git_run_target(&workspace, None).unwrap();
+        let observation =
+            observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap();
         let target = observation.run_target.as_ref().unwrap();
         let legacy = &observation.legacy_git_context;
 
@@ -1759,6 +1846,66 @@ exit 1
         assert_eq!(bare_remote_branch_sha(&bare_origin, "feature"), target.sha);
     }
 
+    #[test]
+    fn stale_matching_tracking_ref_produces_a_branch_only_target() {
+        let temp = tempfile::tempdir().unwrap();
+        let workspace = temp.path().join("workspace");
+        std::fs::create_dir_all(&workspace).unwrap();
+        let bare_origin = init_bare_origin(temp.path());
+        init_git_repo(&workspace, "feature", "https://github.com/acme/widgets");
+        let local_url = format!("file://{}", bare_origin.display());
+        run_git(&workspace, &[
+            "config",
+            &format!("url.{local_url}.insteadOf"),
+            "https://github.com/acme/widgets",
+        ]);
+        run_git(&workspace, &["push", "origin", "feature"]);
+        run_git(&workspace, &[
+            "-c",
+            "user.name=test",
+            "-c",
+            "user.email=test@example.com",
+            "commit",
+            "--allow-empty",
+            "--quiet",
+            "-m",
+            "local-only",
+        ]);
+        mark_origin_branch_synced(&workspace, "feature");
+
+        let observation =
+            observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap();
+        let target = observation.run_target.as_ref().unwrap();
+
+        assert_eq!(target.sha, None);
+        assert!(observation.legacy_git_context.sha.is_some());
+        assert_ne!(
+            bare_remote_branch_sha(&bare_origin, "feature"),
+            observation.legacy_git_context.sha,
+        );
+    }
+
+    #[test]
+    fn branches_that_are_invalid_run_selectors_do_not_produce_git_targets() {
+        let temp = tempfile::tempdir().unwrap();
+        let invalid_branches = [
+            "heads/topic",
+            "tags/release",
+            "0123456789abcdef0123456789abcdef01234567",
+        ];
+
+        for (index, branch) in invalid_branches.into_iter().enumerate() {
+            let workspace = temp.path().join(format!("workspace-{index}"));
+            std::fs::create_dir_all(&workspace).unwrap();
+            init_git_repo(&workspace, branch, "https://github.com/acme/widgets");
+
+            let observation = observe_git_run_target(&workspace, None).unwrap();
+
+            assert_eq!(observation.run_target, None, "branch {branch}");
+            assert_eq!(observation.legacy_git_context.branch, branch);
+        }
+    }
+
     #[test]
     fn failed_push_and_origin_mismatch_produce_branch_only_targets() {
         let temp = tempfile::tempdir().unwrap();

lib/components/fabro-manifest/src/local_workflow_package.rs

@@ -107,7 +107,10 @@ pub fn resolve_local_workflow_package(
 }
 
 fn is_workflow_name(workflow: &Path) -> bool {
-    workflow.extension().is_none() && workflow.components().count() == 1
+    workflow.extension().is_none()
+        && workflow
+            .file_name()
+            .is_some_and(|name| workflow.as_os_str() == name)
 }
 
 fn resolve_named_workflow(
@@ -364,6 +367,28 @@ mod tests {
         assert!(matches!(error, LocalWorkflowPackageError::Resolve { .. }));
     }
 
+    #[test]
+    fn dot_relative_directory_is_an_explicit_path_even_when_name_exists() {
+        let temp = tempfile::tempdir().unwrap();
+        let project = temp.path().join("project");
+        fs::create_dir_all(&project).unwrap();
+        init_repo(&project);
+        write_workflow(&project, ".fabro/workflows/hello", "digraph Named {}");
+        write_workflow(&project, "hello", "digraph Explicit {}");
+
+        let package = resolve_local_workflow_package(Path::new("./hello"), &project, None).unwrap();
+
+        assert_eq!(package.source_root(), project.canonicalize().unwrap());
+        assert_eq!(
+            package.workflow_location().graph,
+            project.join("hello/workflow.fabro").canonicalize().unwrap(),
+        );
+        assert_eq!(
+            root_version(&package).entrypoint().as_str(),
+            "hello/workflow.fabro",
+        );
+    }
+
     #[test]
     fn explicit_workflow_uses_its_own_checkout_and_has_location_independent_bytes() {
         let temp = tempfile::tempdir().unwrap();

lib/components/fabro-manifest/src/workflow_version_collector.rs

@@ -84,19 +84,15 @@ pub fn collect_workflow_versions(
 ) -> Result<CollectedWorkflowClosure, WorkflowVersionCollectError> {
     let repository_workflow = repository_workflow_path(workflow);
     let location = crate::resolve_existing_workflow_location(&repository_workflow, checkout_root)
-        .map_err(|source| {
-            match source {
-                fabro_config::Error::WorkflowNotFound(_) => {
-                    WorkflowVersionCollectError::WorkflowNotFound {
-                        path: workflow.to_path_buf(),
-                    }
-                }
-                source => WorkflowVersionCollectError::Collect {
-                    path:   workflow.to_path_buf(),
-                    source: source.into(),
-                },
-            }
-        })?;
+        .map_err(|source| match source {
+        fabro_config::Error::WorkflowNotFound(_) => WorkflowVersionCollectError::WorkflowNotFound {
+            path: workflow.to_path_buf(),
+        },
+        source => WorkflowVersionCollectError::Collect {
+            path:   workflow.to_path_buf(),
+            source: source.into(),
+        },
+    })?;
 
     let package_root =
         checkout_root

lib/components/fabro-workflow/src/git.rs

@@ -234,6 +234,40 @@ pub fn push_branch_noninteractive(repo: &Path, remote: &str, branch: &str) -> Re
     )
 }
 
+/// Read the exact commit currently advertised for a remote branch without
+/// allowing Git to prompt for credentials.
+///
+/// This queries the remote itself rather than trusting the checkout's local
+/// remote-tracking ref, which may be stale or may have been rewritten locally.
+pub fn remote_branch_sha_noninteractive(
+    repo: &Path,
+    remote: &str,
+    branch: &str,
+) -> Result<Option<String>> {
+    let branch_ref = format!("refs/heads/{branch}");
+    let output = git_cmd(repo)
+        .env("GIT_TERMINAL_PROMPT", "0")
+        .args(["ls-remote", "--refs", remote, &branch_ref])
+        .output()
+        .map_err(|e| Error::engine_with_source("git ls-remote failed", e))?;
+    if !output.status.success() {
+        return Err(git_error("git ls-remote failed"));
+    }
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    for line in stdout.lines() {
+        let mut fields = line.split_whitespace();
+        let (Some(sha), Some(observed_ref), None) = (fields.next(), fields.next(), fields.next())
+        else {
+            continue;
+        };
+        if observed_ref == branch_ref {
+            return Ok(Some(sha.to_owned()));
+        }
+    }
+    Ok(None)
+}
+
 /// Push run and metadata branches to origin if a remote tracking branch exists.
 ///
 /// Callers supply pre-built refspecs so they control force-push (`+` prefix).

lib/components/fabro-workflow/tests/it/git_integration.rs

@@ -12,7 +12,7 @@ use fabro_agent::Sandbox;
 use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
 use fabro_types::{RunEvent, WorkflowSettings, fixtures};
 use fabro_workflow::event::Emitter;
-use fabro_workflow::git::{branch_needs_push, push_branch, push_ref};
+use fabro_workflow::git;
 use fabro_workflow::handler::HandlerRegistry;
 use fabro_workflow::handler::exit::ExitHandler;
 use fabro_workflow::handler::start::StartHandler;
@@ -178,7 +178,7 @@ fn push_ref_to_bare_remote() {
 
     rename_branch(&repo_dir, "test-push");
     let url = format!("file://{}", remote_dir.display());
-    push_ref(&repo_dir, &url, "refs/heads/test-push").unwrap();
+    git::push_ref(&repo_dir, &url, "refs/heads/test-push").unwrap();
 
     assert!(list_branch(&remote_dir, "test-push").contains("test-push"));
 }
@@ -194,7 +194,7 @@ fn push_branch_to_remote() {
     add_origin(&repo_dir, &remote_dir);
     rename_branch(&repo_dir, "main");
 
-    push_branch(&repo_dir, "origin", "main").unwrap();
+    git::push_branch(&repo_dir, "origin", "main").unwrap();
 
     assert!(list_branch(&remote_dir, "main").contains("main"));
 }
@@ -210,10 +210,10 @@ fn branch_needs_push_when_ahead() {
     add_origin(&repo_dir, &remote_dir);
     rename_branch(&repo_dir, "main");
 
-    push_branch(&repo_dir, "origin", "main").unwrap();
+    git::push_branch(&repo_dir, "origin", "main").unwrap();
     empty_commit(&repo_dir, "second");
 
-    assert!(branch_needs_push(&repo_dir, "origin", "main"));
+    assert!(git::branch_needs_push(&repo_dir, "origin", "main"));
 }
 
 #[test]
@@ -227,9 +227,39 @@ fn branch_needs_push_when_in_sync() {
     add_origin(&repo_dir, &remote_dir);
     rename_branch(&repo_dir, "main");
 
-    push_branch(&repo_dir, "origin", "main").unwrap();
+    git::push_branch(&repo_dir, "origin", "main").unwrap();
 
-    assert!(!branch_needs_push(&repo_dir, "origin", "main"));
+    assert!(!git::branch_needs_push(&repo_dir, "origin", "main"));
+}
+
+#[test]
+fn remote_branch_sha_ignores_a_locally_rewritten_tracking_ref() {
+    let dir = tempfile::tempdir().unwrap();
+    let repo_dir = dir.path().join("repo");
+    let remote_dir = dir.path().join("remote.git");
+
+    init_bare_remote(&remote_dir);
+    init_repo(&repo_dir);
+    add_origin(&repo_dir, &remote_dir);
+    rename_branch(&repo_dir, "main");
+    git::push_branch(&repo_dir, "origin", "main").unwrap();
+    let remote_sha = git::head_sha(&repo_dir).unwrap();
+
+    empty_commit(&repo_dir, "local-only");
+    let local_sha = git::head_sha(&repo_dir).unwrap();
+    let update_tracking = Command::new("git")
+        .args(["update-ref", "refs/remotes/origin/main", "HEAD"])
+        .current_dir(&repo_dir)
+        .output()
+        .expect("git update-ref should run");
+    assert_success(&update_tracking, "git update-ref");
+    assert!(!git::branch_needs_push(&repo_dir, "origin", "main"));
+
+    assert_eq!(
+        git::remote_branch_sha_noninteractive(&repo_dir, "origin", "main").unwrap(),
+        Some(remote_sha.clone()),
+    );
+    assert_ne!(local_sha, remote_sha);
 }
 
 #[tokio::test]