From 1121c5c35aa613b984671a31a935b7545a7b0961 Mon Sep 17 00:00:00 2001 From: Gary Basin Date: Thu, 16 Jul 2026 11:30:40 -0400 Subject: [PATCH 1/2] chore(node-sync): pin the git-identity wire contract --- runtime/node-sync/CONTRACT.md | 14 ++++++++++++++ .../node-sync/contract/fixtures/git-identity.json | 8 ++++++++ 2 files changed, 22 insertions(+) create mode 100644 runtime/node-sync/contract/fixtures/git-identity.json diff --git a/runtime/node-sync/CONTRACT.md b/runtime/node-sync/CONTRACT.md index 5a9b7fee..fcf92907 100644 --- a/runtime/node-sync/CONTRACT.md +++ b/runtime/node-sync/CONTRACT.md @@ -128,6 +128,20 @@ daemon's tests): | `GET sessions/:viewerId/atrium/changes` | `atrium-changes.json` | | `GET sessions/:viewerId/atrium/channels` | `atrium-channels.json` | | `GET sessions/:id/profile-bundles` | `profile-bundles.json` (element shape daemon-side; live check pins the envelope — seeding a real bundle needs the whole profile-writeback pipeline) | +| `GET sessions/:id/git-identity` | `git-identity.json` (200 with the identity, or **204** when none is resolvable) | + +The `git-identity` lane deserves a note, because its shape invites the wrong +home. It looks like a profile bundle and is deliberately not one: profile +bundles are user-authored files that get captured and **written back**, whereas +the git identity is server-derived per claim and must never round-trip (that is +the clobber class fixed in #97). It exists because commit authorship is the one +per-user value this architecture cannot inject at the HTTP boundary — the +iron-proxy rewrites headers, and authorship lives *inside* the payload, below +that seam. A warm pod's env is baked before the claiming principal is known, so +env cannot carry it either; per-session file materialization is the only channel +that reaches a claimed warm pod. On 204 the daemon writes nothing and the +image's baked `Centaur AI` identity stands — that fallback is the pre-existing +behavior, which is what makes this lane safe to ship dark. Lanes covered by route-existence + daemon-side parsing only (their payloads are opaque blobs or one-way writes): `artifacts/raw`, `harness-transcript`, diff --git a/runtime/node-sync/contract/fixtures/git-identity.json b/runtime/node-sync/contract/fixtures/git-identity.json new file mode 100644 index 00000000..f7e0fc5c --- /dev/null +++ b/runtime/node-sync/contract/fixtures/git-identity.json @@ -0,0 +1,8 @@ +{ + "_comment": "GET {session_prefix}/:threadKey/git-identity — the git author identity the daemon materializes into the sandbox's ~/.config/git/atrium-identity. /opt/centaur/gitconfig includes that path AFTER its own [user] block, so these values win (git config is last-wins); when the daemon writes nothing the image's baked 'Centaur AI' identity stands unchanged. A 204 means surface could not resolve an identity for this session — the daemon MUST then leave the file absent rather than write a partial one. This payload is SERVER-DERIVED per claim: it is deliberately NOT a profile bundle, because profile bundles are user-authored, captured, and written back, and an identity that round-trips through writeback would re-introduce the clobber class fixed in #97. Never capture, never write back, never let it be restored from a resumed session's overlay ahead of a fresh fetch.", + "authorName": "Allan Niemerg", + "authorEmail": "123+aniemerg@users.noreply.github.com", + "source": "github_noreply", + "sessionId": "de230f34-b9d9-42df-bce3-9270f2184294", + "harness": "claude" +} From 882f4567f0b7f73439b26cd24b6b5961fed746c8 Mon Sep 17 00:00:00 2001 From: Gary Basin Date: Thu, 16 Jul 2026 12:37:56 -0400 Subject: [PATCH 2/2] feat(surface,node-sync,centaur): agent commits are authored by the human who drove them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commits made by agents in Atrium sandboxes were authored `Centaur AI ` — an identity baked into the sandbox image. GitHub attributes commits by author EMAIL, so those commits linked to no GitHub user and the CLA bot reported "seems not to be a GitHub user". The contributor could not sign: signing was unreachable, not merely undone (#536). The gap was structural. Every per-user value in this architecture is injected at the HTTP boundary by the iron-proxy, keyed by a principal bound at claim time. Commit authorship is the one per-user value that lives BELOW that boundary — inside the payload — so the proxy pattern that solves everything else here cannot solve it. Env cannot carry it either: a warm pod's env is baked before the claiming principal is known, and the warm-hit path returns before execute-env is applied. Per-session file materialization is the only channel that reaches a claimed warm pod. - surface: capture GitHub's numeric account id from the /user responses we already parse (no new API calls) and resolve a session's author identity — `+@users.noreply.github.com` (the ID form always links; the plain form only links for pre-2017 accounts), falling back to the Atrium account email. Served on GET /api/internal/sessions/:id/git-identity, or 204 when unresolvable. - node-sync: fetch per claim and materialize ~/.config/git/atrium-identity via `git config --file` so git owns the escaping (a display name is user-controlled and is otherwise a config-injection vector). On 204, remove any stale file. - sandbox: include that path AFTER the baked [user] block, so an identity wins when present and the image identity stands when absent — making this safe to ship dark. commit-msg stamps Atrium-Session/Atrium-Harness trailers for provenance. Identity is sourced from our own records, never from the proxy's token: a user with no personal grant gets a SHARED fallback credential, so deriving authorship from `gh api user` would silently author their commits as a bot. Authorship and transport are different concerns and are kept separate here. --- centaur/services/sandbox/Dockerfile | 3 +- centaur/services/sandbox/git-hooks/commit-msg | 20 ++ .../services/sandbox/test_commit_msg_hook.py | 106 ++++++++++ .../centaur-node-syncd/linux_daemon/mod.rs | 42 +++- runtime/node-sync/src/http_client.rs | 52 +++++ runtime/node-sync/src/materializer.rs | 183 ++++++++++++++++++ runtime/node-sync/src/session_manifest.rs | 98 +++++++++- runtime/node-sync/tests/contract.rs | 16 +- .../083_git_identity_account_id.sql | 5 + surface/server/src/connections.ts | 39 ++-- surface/server/src/git-identity.ts | 79 ++++++++ .../src/routes/internal-session-runtime.ts | 11 ++ surface/server/src/routes/me.ts | 29 ++- surface/server/test/gitIdentity.test.ts | 167 ++++++++++++++++ surface/server/test/internalContract.test.ts | 28 +++ 15 files changed, 849 insertions(+), 29 deletions(-) create mode 100644 centaur/services/sandbox/test_commit_msg_hook.py create mode 100644 surface/server/migrations/083_git_identity_account_id.sql create mode 100644 surface/server/src/git-identity.ts create mode 100644 surface/server/test/gitIdentity.test.ts diff --git a/centaur/services/sandbox/Dockerfile b/centaur/services/sandbox/Dockerfile index c1f117c8..de8d68b6 100644 --- a/centaur/services/sandbox/Dockerfile +++ b/centaur/services/sandbox/Dockerfile @@ -249,7 +249,8 @@ RUN mkdir -p ~/.config/amp ~/.codex ~/.pi/agent ~/repos ~/workspace \ && git config --global core.hooksPath /opt/centaur/git-hooks \ && git config --global --add safe.directory '*' \ && git config --global user.name "Centaur AI" \ - && git config --global user.email "ai@centaur.local" + && git config --global user.email "ai@centaur.local" \ + && git config --global include.path /home/agent/.config/git/atrium-identity # ============================================================================== # Final thin stage: only frequently-changing local files below. diff --git a/centaur/services/sandbox/git-hooks/commit-msg b/centaur/services/sandbox/git-hooks/commit-msg index de1ad606..c0f394c3 100755 --- a/centaur/services/sandbox/git-hooks/commit-msg +++ b/centaur/services/sandbox/git-hooks/commit-msg @@ -27,3 +27,23 @@ Remove the generated attribution footer and commit again. EOF exit 1 fi + +session_id="$(git config --get atrium.sessionId || true)" +harness="$(git config --get atrium.harness || true)" +trailers=() + +if [[ -n "$session_id" ]]; then + trailers+=(--trailer "Atrium-Session: $session_id") +fi +if [[ -n "$harness" ]]; then + trailers+=(--trailer "Atrium-Harness: $harness") +fi + +if [[ ${#trailers[@]} -gt 0 ]]; then + git interpret-trailers \ + --in-place \ + --if-exists=replace \ + --if-missing=add \ + "${trailers[@]}" \ + "$message_file" +fi diff --git a/centaur/services/sandbox/test_commit_msg_hook.py b/centaur/services/sandbox/test_commit_msg_hook.py new file mode 100644 index 00000000..bb98a055 --- /dev/null +++ b/centaur/services/sandbox/test_commit_msg_hook.py @@ -0,0 +1,106 @@ +from __future__ import annotations + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + + +HOOK = Path(__file__).with_name("git-hooks") / "commit-msg" + + +class CommitMsgHookTest(unittest.TestCase): + def setUp(self) -> None: + temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(temp_dir.cleanup) + self.repo = Path(temp_dir.name) / "repo" + self.repo.mkdir() + self.env = { + **os.environ, + "BASH_ENV": "/dev/null", + "GIT_CONFIG_GLOBAL": "/dev/null", + "GIT_CONFIG_NOSYSTEM": "1", + } + self.git("init", "-q", "-b", "main", check=True) + self.git("config", "user.name", "Test User", check=True) + self.git("config", "user.email", "test@example.com", check=True) + self.git("config", "core.hooksPath", str(HOOK.parent), check=True) + + def git( + self, *args: str, check: bool = False, input: str | None = None + ) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *args], + cwd=self.repo, + env=self.env, + input=input, + text=True, + capture_output=True, + check=check, + ) + + def commit_message(self) -> str: + return self.git("log", "-1", "--pretty=%B", check=True).stdout.rstrip("\n") + + def test_appends_trailers_once_and_amend_is_idempotent(self) -> None: + self.git( + "config", + "atrium.sessionId", + "de230f34-b9d9-42df-bce3-9270f2184294", + check=True, + ) + self.git("config", "atrium.harness", "claude", check=True) + (self.repo / "example.txt").write_text("content\n") + self.git("add", "example.txt", check=True) + + self.git("commit", "-m", "feat: add provenance", check=True) + message = self.commit_message() + self.assertEqual(message.count("Atrium-Session:"), 1) + self.assertEqual(message.count("Atrium-Harness:"), 1) + self.assertTrue( + message.endswith( + "\n\nAtrium-Session: de230f34-b9d9-42df-bce3-9270f2184294\n" + "Atrium-Harness: claude" + ) + ) + + self.git("commit", "--amend", "--no-edit", check=True) + amended = self.commit_message() + self.assertEqual(amended.count("Atrium-Session:"), 1) + self.assertEqual(amended.count("Atrium-Harness:"), 1) + + def test_missing_atrium_config_leaves_message_unchanged(self) -> None: + original = "feat: preserve this message\n\nBody remains unchanged." + + self.git("commit", "--allow-empty", "-F", "-", input=original, check=True) + + self.assertEqual(self.commit_message(), original) + + def test_atrium_harness_claude_trailer_is_allowed(self) -> None: + message = "feat: record harness\n\nAtrium-Harness: claude" + + result = self.git("commit", "--allow-empty", "-F", "-", input=message) + + self.assertEqual(result.returncode, 0, result.stderr) + + def test_claude_coauthor_is_still_rejected(self) -> None: + message = ( + "feat: forbidden attribution\n\n" + "Co-authored-by: Claude " + ) + + result = self.git("commit", "--allow-empty", "-F", "-", input=message) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("AI co-author attribution", result.stderr) + + def test_non_conventional_subject_is_still_rejected(self) -> None: + result = self.git("commit", "--allow-empty", "-m", "not conventional") + + self.assertNotEqual(result.returncode, 0) + self.assertIn("conventional commit subject", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/runtime/node-sync/src/bin/centaur-node-syncd/linux_daemon/mod.rs b/runtime/node-sync/src/bin/centaur-node-syncd/linux_daemon/mod.rs index e6128bc7..b0d2ffe2 100644 --- a/runtime/node-sync/src/bin/centaur-node-syncd/linux_daemon/mod.rs +++ b/runtime/node-sync/src/bin/centaur-node-syncd/linux_daemon/mod.rs @@ -42,7 +42,7 @@ use centaur_node_sync::eviction::{ }; use centaur_node_sync::fs_linux; use centaur_node_sync::http_client::HttpAtriumClient; -use centaur_node_sync::materializer::write_mount_readme; +use centaur_node_sync::materializer::{materialize_git_identity, write_mount_readme}; use centaur_node_sync::overlay::RawEntry; use centaur_node_sync::overlay_mount::{OverlayMountPlan, mount_overlay, plan_overlay_mount}; use centaur_node_sync::quiesce::{LeaseGate, apply_quiesced_writes}; @@ -427,6 +427,46 @@ fn run_multi_session(global: GlobalConfig, overlays_root: PathBuf, once: bool, i }; let wip_remounted = !has_active_mount || mounted_overlays.get(&discovered.session) != Some(&plan); + // Git author identity is server-derived per CLAIM, and a claim is a + // mount — so refresh it only when the mount is being (re)established, + // never on every tick. An unconditional per-session GET per tick is + // exactly the steady-state cost the atrium delta protocol exists to + // avoid. Writing into the upper before mount_overlay is deliberate: + // the agent must never observe a home without its identity. + // + // Failures here must NOT skip the mount. The image bakes a + // "Centaur AI" identity and /opt/centaur/gitconfig includes this file + // only if it exists, so an absent identity degrades to exactly the + // pre-existing behavior. Gating the mount on identity would turn a + // surface blip into "the agent's home never mounts" — trading a + // recoverable misattributed commit for an unrecoverable dead session. + if wip_remounted { + let identity_client = HttpAtriumClient::new( + &global.base_url, + &global.api_key, + &discovered.atrium_session, + ); + match identity_client.get_git_identity() { + Ok(identity) => { + if let Err(error) = materialize_git_identity( + &discovered.upper, + identity.as_ref(), + Some(discovered.manifest.agent_uid), + ) { + eprintln!( + "session {}: git identity materialize: {error} (falling back to the image identity)", + discovered.session + ); + } + } + Err(error) => { + eprintln!( + "session {}: git identity fetch: {error} (falling back to the image identity)", + discovered.session + ); + } + } + } let mounted = match mount_overlay(plan, Some(discovered.manifest.agent_uid)) { Ok(plan) => plan, Err(e) => { diff --git a/runtime/node-sync/src/http_client.rs b/runtime/node-sync/src/http_client.rs index baf642d8..8d3421f1 100644 --- a/runtime/node-sync/src/http_client.rs +++ b/runtime/node-sync/src/http_client.rs @@ -94,6 +94,58 @@ impl HttpAtriumClient { .map_err(|e| HttpFeedError::Parse(e.to_string()))?; parse_profile_bundles(&value, harness).map_err(HttpFeedError::Parse) } + + pub fn get_git_identity(&self) -> Result, HttpFeedError> { + let resp = self + .agent + .get(&self.url("/git-identity")) + .set(AUTH_HEADER, &self.api_key) + .call() + .map_err(|e| http_feed_error("get git identity", e))?; + if resp.status() == 204 { + return Ok(None); + } + let value: serde_json::Value = resp + .into_json() + .map_err(|e| HttpFeedError::Parse(e.to_string()))?; + parse_git_identity(value) + .map(Some) + .map_err(HttpFeedError::Parse) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitIdentity { + #[serde(default)] + pub author_name: String, + #[serde(default)] + pub author_email: String, + #[serde(default)] + pub session_id: String, + #[serde(default)] + pub harness: String, +} + +pub fn parse_git_identity(value: serde_json::Value) -> Result { + let identity = serde_json::from_value::(value) + .map_err(|e| format!("parse git-identity response: {e}"))?; + let missing = [ + ("authorName", identity.author_name.as_str()), + ("authorEmail", identity.author_email.as_str()), + ("sessionId", identity.session_id.as_str()), + ("harness", identity.harness.as_str()), + ] + .into_iter() + .filter_map(|(field, value)| value.is_empty().then_some(field)) + .collect::>(); + if !missing.is_empty() { + return Err(format!( + "parse git-identity response: missing fields: {}", + missing.join(", ") + )); + } + Ok(identity) } #[derive(Debug)] diff --git a/runtime/node-sync/src/materializer.rs b/runtime/node-sync/src/materializer.rs index ef8f2b74..eb4dd23e 100644 --- a/runtime/node-sync/src/materializer.rs +++ b/runtime/node-sync/src/materializer.rs @@ -1,9 +1,11 @@ use std::io::Write; use std::path::{Path, PathBuf}; +use std::process::Command; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; use std::time::{Duration, Instant}; +use crate::http_client::GitIdentity; use crate::runtime::{AtriumChannel, AtriumClient, ContextDeltaRequest, ContextDocResponse}; use crate::state::{ContextDocState, DaemonState}; @@ -83,6 +85,94 @@ If a listed path is missing, it is still materializing — wait a few seconds an "#; pub const CONTEXT_READY_MARKER: &str = ".atrium-context-ready"; +pub const GIT_IDENTITY_RELATIVE_PATH: &str = ".config/git/atrium-identity"; + +/// Authoritatively materialize the server-derived identity into an overlay upper. +/// `None` is the 204 fallback and removes a prior identity so the image default wins. +pub fn materialize_git_identity( + home: &Path, + identity: Option<&GitIdentity>, + agent_uid: Option, +) -> Result<(), String> { + let dst = home.join(GIT_IDENTITY_RELATIVE_PATH); + let Some(identity) = identity else { + remove_file_if_present(&dst)?; + return Ok(()); + }; + let parent = dst + .parent() + .ok_or_else(|| format!("git identity path has no parent: {}", dst.display()))?; + std::fs::create_dir_all(parent) + .map_err(|e| format!("create git identity dir {}: {e}", parent.display()))?; + set_owner_if_requested(parent, agent_uid)?; + + let tmp = dst.with_extension("nodesync.tmp"); + remove_file_if_present(&tmp)?; + let result = (|| { + for (key, value) in [ + ("user.name", identity.author_name.as_str()), + ("user.email", identity.author_email.as_str()), + ("atrium.sessionId", identity.session_id.as_str()), + ("atrium.harness", identity.harness.as_str()), + ] { + // `git config --file` still performs repository DISCOVERY from the cwd, and + // hard-fails (exit 128) if it lands on a broken gitdir — even though writing + // a named file needs no repo at all. The daemon's cwd is ambient, and under + // the flat-~ layout the agent's home is itself a workspace, so discovery here + // is never something we want. Pin cwd to `/` to make this pure file I/O. + let output = Command::new("git") + .current_dir("/") + .args(["config", "--file"]) + .arg(&tmp) + .arg(key) + .arg(value) + .output() + .map_err(|e| format!("spawn git config for {key}: {e}"))?; + if !output.status.success() { + return Err(format!( + "git config for {key} failed (status {}): {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + )); + } + } + set_owner_if_requested(&tmp, agent_uid)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600)) + .map_err(|e| format!("chmod git identity {}: {e}", tmp.display()))?; + } + std::fs::rename(&tmp, &dst) + .map_err(|e| format!("rename {} -> {}: {e}", tmp.display(), dst.display())) + })(); + if result.is_err() { + let _ = std::fs::remove_file(&tmp); + } + result +} + +fn remove_file_if_present(path: &Path) -> Result<(), String> { + match std::fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(format!("remove {}: {error}", path.display())), + } +} + +#[cfg(unix)] +fn set_owner_if_requested(path: &Path, agent_uid: Option) -> Result<(), String> { + use std::os::unix::fs::chown; + let Some(uid) = agent_uid else { + return Ok(()); + }; + chown(path, Some(uid), Some(uid)).map_err(|e| format!("chown {} to {uid}: {e}", path.display())) +} + +#[cfg(not(unix))] +fn set_owner_if_requested(_path: &Path, _agent_uid: Option) -> Result<(), String> { + Ok(()) +} pub const ATRIUM_DOCS: &[(&str, &str)] = &[ ("transcript", "transcript.md"), @@ -759,6 +849,7 @@ fn staging_path(atrium_root: &Path, dst: &Path) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::http_client::GitIdentity; use std::collections::HashSet; use std::sync::Mutex; use std::sync::atomic::{AtomicU64, Ordering}; @@ -888,6 +979,98 @@ mod tests { format!("{session_id}/{doc}").into_bytes() } + fn git_identity(author_name: &str) -> GitIdentity { + GitIdentity { + author_name: author_name.to_string(), + author_email: "123+aniemerg@users.noreply.github.com".to_string(), + session_id: "de230f34-b9d9-42df-bce3-9270f2184294".to_string(), + harness: "claude".to_string(), + } + } + + // Pinned to `/` for the same reason the writer is: git discovers a repository from + // the cwd even for `--file` reads, so running the suite from inside a linked + // worktree (whose .git is a gitfile) would fail these on repo discovery, not on + // anything they are meant to assert. + fn git_config_get(path: &Path, key: &str) -> std::process::Output { + Command::new("git") + .current_dir("/") + .args(["config", "--file"]) + .arg(path) + .args(["--get", key]) + .output() + .unwrap() + } + + #[test] + fn git_identity_204_removes_a_stale_file() { + let temp = tempfile::tempdir().unwrap(); + let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH); + std::fs::create_dir_all(dst.parent().unwrap()).unwrap(); + std::fs::write(&dst, b"stale").unwrap(); + + materialize_git_identity(temp.path(), None, None).unwrap(); + + assert!(!dst.exists()); + } + + #[test] + fn git_identity_200_writes_exact_config() { + let temp = tempfile::tempdir().unwrap(); + let identity = git_identity("Allan Niemerg"); + let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH); + + materialize_git_identity(temp.path(), Some(&identity), None).unwrap(); + + assert_eq!( + std::fs::read_to_string(&dst).unwrap(), + "[user]\n\tname = Allan Niemerg\n\temail = 123+aniemerg@users.noreply.github.com\n\ + [atrium]\n\tsessionId = de230f34-b9d9-42df-bce3-9270f2184294\n\ + \tharness = claude\n" + ); + let output = git_config_get(&dst, "user.email"); + assert!(output.status.success()); + assert_eq!( + String::from_utf8(output.stdout).unwrap().trim_end(), + identity.author_email + ); + } + + #[test] + fn git_identity_hostile_name_cannot_inject_config() { + let temp = tempfile::tempdir().unwrap(); + let hostile = "Eve \"quoted\" \\\\ path\n[include]\n\tpath = /etc/passwd"; + let identity = git_identity(hostile); + let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH); + + materialize_git_identity(temp.path(), Some(&identity), None).unwrap(); + + let output = git_config_get(&dst, "user.name"); + assert!(output.status.success()); + assert_eq!( + String::from_utf8(output.stdout) + .unwrap() + .trim_end_matches('\n'), + hostile + ); + let include = git_config_get(&dst, "include.path"); + assert!(!include.status.success()); + assert!(include.stdout.is_empty()); + } + + #[test] + fn git_identity_materialization_is_idempotent() { + let temp = tempfile::tempdir().unwrap(); + let identity = git_identity("Allan Niemerg"); + let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH); + + materialize_git_identity(temp.path(), Some(&identity), None).unwrap(); + let first = std::fs::read(&dst).unwrap(); + materialize_git_identity(temp.path(), Some(&identity), None).unwrap(); + + assert_eq!(std::fs::read(&dst).unwrap(), first); + } + #[test] fn materializes_changed_sessions_docs() { let temp = tempfile::tempdir().unwrap(); diff --git a/runtime/node-sync/src/session_manifest.rs b/runtime/node-sync/src/session_manifest.rs index 4bdc9bbd..c5298d9c 100644 --- a/runtime/node-sync/src/session_manifest.rs +++ b/runtime/node-sync/src/session_manifest.rs @@ -4,7 +4,7 @@ //! The per-node daemon scans direct child directories of `` and //! only runs sessions that have a readable sidecar manifest. -use crate::overlay_mount::DEFAULT_AGENT_UID; +use crate::overlay_mount::{DEFAULT_AGENT_UID, READY_MARKER_FILE}; use std::path::{Component, Path, PathBuf}; #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] @@ -173,11 +173,44 @@ pub fn write_manifest(overlays_root: &Path, manifest: &SessionManifest) -> Resul let dir = sessions_dir(overlays_root); std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; let path = manifest_path(overlays_root, &manifest.session); + let invalidate_ready = match std::fs::read(&path) { + Ok(bytes) => serde_json::from_slice::(&bytes).map_or(true, |previous| { + claim_identity(&previous) != claim_identity(manifest) + }), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => false, + Err(error) => return Err(format!("read {}: {error}", path.display())), + }; let tmp = path.with_extension("json.tmp"); let bytes = serde_json::to_vec_pretty(manifest).map_err(|e| e.to_string())?; std::fs::write(&tmp, bytes).map_err(|e| format!("write {}: {e}", tmp.display()))?; std::fs::rename(&tmp, &path) - .map_err(|e| format!("rename {} -> {}: {e}", tmp.display(), path.display())) + .map_err(|e| format!("rename {} -> {}: {e}", tmp.display(), path.display()))?; + if invalidate_ready { + // A claim rewrites the manifest of an already-mounted warm sandbox. + // Publish the claimed identity first, then invalidate the old readiness + // handshake. node-sync recreates the marker only after per-claim + // materialization. An unchanged post-mount canonicalization write must + // leave the freshly-created marker alone. + let ready_marker = overlays_root + .join(&manifest.session) + .join(READY_MARKER_FILE); + match std::fs::remove_file(&ready_marker) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(format!("remove {}: {error}", ready_marker.display())), + } + } + Ok(()) +} + +fn claim_identity(manifest: &SessionManifest) -> (&str, Option<&str>, &str, &str, bool) { + ( + &manifest.atrium_session, + manifest.harness.as_deref(), + &manifest.harness_thread_id, + &manifest.harness_home, + manifest.flat_home, + ) } pub fn discover_sessions(overlays_root: &Path) -> Result { @@ -425,6 +458,67 @@ mod tests { assert_eq!(round_trip, manifest); } + #[test] + fn manifest_publication_invalidates_existing_ready_marker() { + let temp = tempfile::tempdir().unwrap(); + let upper = temp.path().join("sess-1"); + std::fs::create_dir_all(&upper).unwrap(); + let mut manifest = SessionManifest { + session: "sess-1".to_string(), + atrium_session: "sess-1".to_string(), + merged: temp.path().join("merged/sess-1"), + harness: None, + harness_thread_id: String::new(), + harness_home: String::new(), + flat_home: true, + generic_home_lower: PathBuf::new(), + context_source: PathBuf::new(), + repo: String::new(), + repos: Vec::new(), + agent_uid: 1001, + }; + write_manifest(temp.path(), &manifest).unwrap(); + let ready = upper.join(READY_MARKER_FILE); + std::fs::write(&ready, b"ready\n").unwrap(); + manifest.atrium_session = "surface:sess-1".to_string(); + manifest.harness = Some("claude".to_string()); + manifest.harness_thread_id = "thread-123".to_string(); + manifest.harness_home = ".claude".to_string(); + + write_manifest(temp.path(), &manifest).unwrap(); + + assert!(!ready.exists()); + assert_eq!(read_manifest(temp.path(), "sess-1").unwrap(), manifest); + } + + #[test] + fn unchanged_manifest_write_preserves_ready_marker() { + let temp = tempfile::tempdir().unwrap(); + let upper = temp.path().join("sess-1"); + std::fs::create_dir_all(&upper).unwrap(); + let manifest = SessionManifest { + session: "sess-1".to_string(), + atrium_session: "surface:sess-1".to_string(), + merged: temp.path().join("merged/sess-1"), + harness: Some("claude".to_string()), + harness_thread_id: "thread-123".to_string(), + harness_home: ".claude".to_string(), + flat_home: true, + generic_home_lower: PathBuf::new(), + context_source: PathBuf::new(), + repo: String::new(), + repos: Vec::new(), + agent_uid: 1001, + }; + write_manifest(temp.path(), &manifest).unwrap(); + let ready = upper.join(READY_MARKER_FILE); + std::fs::write(&ready, b"ready\n").unwrap(); + + write_manifest(temp.path(), &manifest).unwrap(); + + assert!(ready.is_file()); + } + #[test] fn missing_repos_deserializes_as_empty_for_back_compat() { let manifest: SessionManifest = serde_json::from_value(serde_json::json!({ diff --git a/runtime/node-sync/tests/contract.rs b/runtime/node-sync/tests/contract.rs index 9590cf13..248c890a 100644 --- a/runtime/node-sync/tests/contract.rs +++ b/runtime/node-sync/tests/contract.rs @@ -13,7 +13,7 @@ use centaur_node_sync::feeds::{ }; use centaur_node_sync::http_client::{ AUTH_HEADER, HEADER_EPOCH, HEADER_MODE, HEADER_NEXT_EVENT_ID, HEADER_NEXT_SEQ, QUERY_EPOCH, - QUERY_SINCE_EVENT_ID, QUERY_SINCE_SEQ, SESSION_PREFIX, + QUERY_SINCE_EVENT_ID, QUERY_SINCE_SEQ, SESSION_PREFIX, parse_git_identity, }; use centaur_node_sync::materializer::CONTEXT_READY_MARKER; use centaur_node_sync::overlay_mount::{ @@ -229,6 +229,20 @@ fn profile_bundles_fixture_parses() { assert!(!bundles[0].executable); } +#[test] +fn git_identity_fixture_parses() { + let fixture: serde_json::Value = + serde_json::from_str(include_str!("../contract/fixtures/git-identity.json")).unwrap(); + let identity = parse_git_identity(fixture).expect("git identity fixture must parse"); + assert_eq!(identity.author_name, "Allan Niemerg"); + assert_eq!( + identity.author_email, + "123+aniemerg@users.noreply.github.com" + ); + assert_eq!(identity.session_id, "de230f34-b9d9-42df-bce3-9270f2184294"); + assert_eq!(identity.harness, "claude"); +} + #[test] fn warmcache_hydration_fixture_parses() { let fixture: serde_json::Value = serde_json::from_str(include_str!( diff --git a/surface/server/migrations/083_git_identity_account_id.sql b/surface/server/migrations/083_git_identity_account_id.sql new file mode 100644 index 00000000..eb0de088 --- /dev/null +++ b/surface/server/migrations/083_git_identity_account_id.sql @@ -0,0 +1,5 @@ +ALTER TABLE user_connections + ADD COLUMN IF NOT EXISTS account_id text; + +ALTER TABLE user_connection_identities + ADD COLUMN IF NOT EXISTS account_id text; diff --git a/surface/server/src/connections.ts b/surface/server/src/connections.ts index 373f7ee8..38317461 100644 --- a/surface/server/src/connections.ts +++ b/surface/server/src/connections.ts @@ -50,6 +50,7 @@ interface ConnectionRow { status: ConnectionStatusValue; token_kind: string | null; account_login: string | null; + account_id: string | null; account_label: string | null; scopes: string[] | null; capabilities: unknown; @@ -67,6 +68,7 @@ interface ConnectionIdentityRow { status: Exclude; token_kind: Exclude; account_login: string | null; + account_id: string | null; account_label: string | null; scopes: string[] | null; capabilities: unknown; @@ -109,14 +111,14 @@ export class Connections { async list(userId: string, workspaceId: string, client: Queryable = this.pool): Promise { const connectionsRes = await client.query( - `SELECT workspace_id, user_id, provider, status, token_kind, account_login, account_label, + `SELECT workspace_id, user_id, provider, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, last_validated_at, last_error, updated_at FROM user_connections WHERE workspace_id = $1 AND user_id = $2`, [workspaceId, userId], ); const identitiesRes = await client.query( - `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_label, + `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, active, last_validated_at, last_error, updated_at FROM user_connection_identities WHERE workspace_id = $1 AND user_id = $2 @@ -148,6 +150,7 @@ export class Connections { status: Exclude; tokenKind: Exclude; accountLogin?: string | null; + accountId?: string | null; accountLabel?: string | null; scopes?: readonly string[]; capabilities?: Record; @@ -166,14 +169,15 @@ export class Connections { ); const res = await client.query( `INSERT INTO user_connections - (workspace_id, user_id, provider, status, token_kind, account_login, account_label, + (workspace_id, user_id, provider, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, last_validated_at, last_error) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8::text[], $9::jsonb, $10::jsonb, - CASE WHEN $4 = 'connected' THEN now() ELSE NULL END, $11) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9::text[], $10::jsonb, $11::jsonb, + CASE WHEN $4 = 'connected' THEN now() ELSE NULL END, $12) ON CONFLICT (workspace_id, user_id, provider) DO UPDATE SET status = EXCLUDED.status, token_kind = EXCLUDED.token_kind, account_login = EXCLUDED.account_login, + account_id = EXCLUDED.account_id, account_label = EXCLUDED.account_label, scopes = EXCLUDED.scopes, capabilities = EXCLUDED.capabilities, @@ -181,7 +185,7 @@ export class Connections { last_validated_at = CASE WHEN EXCLUDED.status = 'connected' THEN now() ELSE user_connections.last_validated_at END, last_error = EXCLUDED.last_error, updated_at = now() - RETURNING workspace_id, user_id, provider, status, token_kind, account_login, account_label, + RETURNING workspace_id, user_id, provider, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, last_validated_at, last_error, updated_at`, [ args.workspaceId, @@ -190,6 +194,7 @@ export class Connections { args.status, args.tokenKind, args.accountLogin ?? null, + args.accountId ?? null, args.accountLabel ?? args.accountLogin ?? null, normalizeScopes(args.scopes ?? []), JSON.stringify(args.capabilities ?? {}), @@ -199,14 +204,15 @@ export class Connections { ); await client.query( `INSERT INTO user_connection_identities - (workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_label, + (workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, active, last_validated_at, last_error) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9::text[], $10::jsonb, $11::jsonb, true, - CASE WHEN $5 = 'connected' THEN now() ELSE NULL END, $12) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10::text[], $11::jsonb, $12::jsonb, true, + CASE WHEN $5 = 'connected' THEN now() ELSE NULL END, $13) ON CONFLICT (workspace_id, user_id, provider, identity_id) DO UPDATE SET status = EXCLUDED.status, token_kind = EXCLUDED.token_kind, account_login = EXCLUDED.account_login, + account_id = EXCLUDED.account_id, account_label = EXCLUDED.account_label, scopes = EXCLUDED.scopes, capabilities = EXCLUDED.capabilities, @@ -223,6 +229,7 @@ export class Connections { args.status, args.tokenKind, args.accountLogin ?? null, + args.accountId ?? null, args.accountLabel ?? args.accountLogin ?? null, normalizeScopes(args.scopes ?? []), JSON.stringify(args.capabilities ?? {}), @@ -265,7 +272,7 @@ export class Connections { client: Queryable = this.pool, ): Promise { const selected = await client.query( - `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_label, + `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, active, last_validated_at, last_error, updated_at FROM user_connection_identities WHERE workspace_id = $1 AND user_id = $2 AND provider = $3 AND identity_id = $4 @@ -293,14 +300,15 @@ export class Connections { ); const active = await client.query( `INSERT INTO user_connections - (workspace_id, user_id, provider, status, token_kind, account_login, account_label, + (workspace_id, user_id, provider, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, last_validated_at, last_error) - VALUES ($1, $2, $3, 'connected', $4, $5, $6, $7::text[], $8::jsonb, $9::jsonb, - COALESCE($10::timestamptz, now()), NULL) + VALUES ($1, $2, $3, 'connected', $4, $5, $6, $7, $8::text[], $9::jsonb, $10::jsonb, + COALESCE($11::timestamptz, now()), NULL) ON CONFLICT (workspace_id, user_id, provider) DO UPDATE SET status = 'connected', token_kind = EXCLUDED.token_kind, account_login = EXCLUDED.account_login, + account_id = EXCLUDED.account_id, account_label = EXCLUDED.account_label, scopes = EXCLUDED.scopes, capabilities = EXCLUDED.capabilities, @@ -308,7 +316,7 @@ export class Connections { last_validated_at = EXCLUDED.last_validated_at, last_error = NULL, updated_at = now() - RETURNING workspace_id, user_id, provider, status, token_kind, account_login, account_label, + RETURNING workspace_id, user_id, provider, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, last_validated_at, last_error, updated_at`, [ workspaceId, @@ -316,6 +324,7 @@ export class Connections { GITHUB_CONNECTION_PROVIDER, identity.token_kind, identity.account_login, + identity.account_id, identity.account_label, normalizeScopes(identity.scopes ?? []), JSON.stringify(plainRecord(identity.capabilities)), @@ -448,7 +457,7 @@ async function listConnectionIdentities( provider: ConnectionProvider, ): Promise { const res = await client.query( - `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_label, + `SELECT workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, account_label, scopes, capabilities, metadata, active, last_validated_at, last_error, updated_at FROM user_connection_identities WHERE workspace_id = $1 AND user_id = $2 AND provider = $3 diff --git a/surface/server/src/git-identity.ts b/surface/server/src/git-identity.ts new file mode 100644 index 00000000..fe8a4856 --- /dev/null +++ b/surface/server/src/git-identity.ts @@ -0,0 +1,79 @@ +import type { Db, DbClient } from './db.js'; + +export interface GitIdentity { + authorName: string; + authorEmail: string; + source: 'github_noreply' | 'atrium_account'; + sessionId: string; + harness: string; +} + +interface GitIdentityRow { + session_id: string; + harness: string; + display_name: string; + account_login: string | null; + account_id: string | null; + email: string | null; +} + +type Queryable = Pick; + +export async function resolveGitIdentity(pool: Queryable, sessionId: string): Promise { + const result = await pool.query( + `SELECT s.id AS session_id, + s.harness, + u.display_name, + i.account_login, + i.account_id, + u.email + FROM sessions s + JOIN users u ON u.id = COALESCE(s.provider_credential_user_id, s.spawned_by) + LEFT JOIN user_connection_identities i + ON i.workspace_id = s.workspace_id + AND i.user_id = u.id + AND i.provider = 'github' + AND i.active + AND i.status = 'connected' + WHERE s.id = $1 + LIMIT 1`, + [sessionId], + ); + const row = result.rows[0]; + if (!row) return null; + + const accountLogin = nonEmpty(row.account_login); + const accountId = nonEmpty(row.account_id); + const atriumEmail = nonEmpty(row.email); + const authorName = nonEmpty(row.display_name) ?? accountLogin ?? emailLocalPart(atriumEmail) ?? 'Atrium User'; + + if (accountLogin && accountId) { + return { + authorName, + authorEmail: `${accountId}+${accountLogin}@users.noreply.github.com`, + source: 'github_noreply', + sessionId: row.session_id, + harness: row.harness, + }; + } + if (atriumEmail) { + return { + authorName, + authorEmail: atriumEmail, + source: 'atrium_account', + sessionId: row.session_id, + harness: row.harness, + }; + } + return null; +} + +function nonEmpty(value: string | null): string | null { + const trimmed = value?.trim(); + return trimmed ? trimmed : null; +} + +function emailLocalPart(email: string | null): string | null { + if (!email) return null; + return nonEmpty(email.split('@', 1)[0] ?? null); +} diff --git a/surface/server/src/routes/internal-session-runtime.ts b/surface/server/src/routes/internal-session-runtime.ts index 403f1708..cfa23e1a 100644 --- a/surface/server/src/routes/internal-session-runtime.ts +++ b/surface/server/src/routes/internal-session-runtime.ts @@ -1,5 +1,6 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; import type { Db } from '../db.js'; +import { resolveGitIdentity } from '../git-identity.js'; import { isHarness, loadHarnessStateBundle, @@ -150,6 +151,16 @@ export async function registerInternalSessionRuntimeRoutes( return reply.send({ bundles }); }); + app.get('/api/internal/sessions/:id/git-identity', async (req, reply) => { + if (!requireCaptureKey(req, reply)) return; + const { id } = req.params as { id: string }; + const session = await resolveInternalSessionRef(id); + if (!session) return reply.code(404).send({ error: 'session_not_found' }); + const identity = await resolveGitIdentity(pool, session.id); + if (!identity) return reply.code(204).send(); + return reply.send(identity); + }); + app.get('/api/internal/sessions/:id/profile-bundle-blob', async (req, reply) => { if (!requireCaptureKey(req, reply)) return; const { id } = req.params as { id: string }; diff --git a/surface/server/src/routes/me.ts b/surface/server/src/routes/me.ts index 19764090..bfbd930f 100644 --- a/surface/server/src/routes/me.ts +++ b/surface/server/src/routes/me.ts @@ -185,7 +185,7 @@ export function registerMeRoutes(app: FastifyInstance, deps: MeRouteDeps): void } const token = typeof body.token === 'string' ? body.token.trim() : ''; let brokerCredentialId = typeof body.brokerCredentialId === 'string' ? body.brokerCredentialId.trim() : ''; - let verifiedPatIdentity: { accountLogin: string; scopes: string[] } | null = null; + let verifiedPatIdentity: { accountLogin: string; accountId: string | null; scopes: string[] } | null = null; let verifiedInstallation: GitHubAppInstallationInfo | null = null; const installationId = typeof body.installationId === 'string' @@ -254,6 +254,7 @@ export function registerMeRoutes(app: FastifyInstance, deps: MeRouteDeps): void tokenKind, accountLogin: verifiedPatIdentity?.accountLogin ?? verifiedInstallation?.accountLogin ?? stringOrNull(body.accountLogin), + accountId: verifiedPatIdentity?.accountId, accountLabel: verifiedPatIdentity?.accountLogin ?? verifiedInstallation?.accountLogin ?? stringOrNull(body.accountLabel), scopes: verifiedPatIdentity?.scopes ?? stringArray(body.scopes), @@ -413,7 +414,7 @@ export function registerMeRoutes(app: FastifyInstance, deps: MeRouteDeps): void message: 'GitHub OAuth exchange returned no access token', }); } - const accountLogin = await fetchGitHubTokenLogin(token.accessToken); + const { accountLogin, accountId } = await fetchGitHubTokenLogin(token.accessToken); const brokerCredentialId = githubAppUserBrokerCredentialForeignId(workspaceId, user.id); await ironControl.upsertBrokerCredential({ foreignId: brokerCredentialId, @@ -445,6 +446,7 @@ export function registerMeRoutes(app: FastifyInstance, deps: MeRouteDeps): void status: 'connected', tokenKind: 'app_user', accountLogin, + accountId, accountLabel: accountLogin, scopes: token.scopes, capabilities: {}, @@ -861,7 +863,7 @@ async function exchangeGitHubAppUserCode(code: string): Promise { +async function fetchGitHubTokenLogin(token: string): Promise<{ accountLogin: string; accountId: string | null }> { const res = await fetch('https://api.github.com/user', { method: 'GET', headers: { @@ -873,15 +875,17 @@ async function fetchGitHubTokenLogin(token: string): Promise { if (!res.ok) { throw new RouteResponse(502, 'github_user_lookup_failed', 'Could not fetch GitHub user identity'); } - const body = (await res.json().catch(() => null)) as { login?: unknown } | null; - const login = stringOrNull(body?.login); - if (!login) { + const body = (await res.json().catch(() => null)) as { login?: unknown; id?: unknown } | null; + const accountLogin = stringOrNull(body?.login); + if (!accountLogin) { throw new RouteResponse(502, 'github_user_lookup_failed', 'GitHub user lookup returned no login'); } - return login; + return { accountLogin, accountId: githubAccountId(body?.id) }; } -async function validateGitHubPatToken(token: string): Promise<{ accountLogin: string; scopes: string[] }> { +async function validateGitHubPatToken( + token: string, +): Promise<{ accountLogin: string; accountId: string | null; scopes: string[] }> { const res = await fetch('https://api.github.com/user', { method: 'GET', headers: { @@ -896,17 +900,24 @@ async function validateGitHubPatToken(token: string): Promise<{ accountLogin: st if (!res.ok) { throw new RouteResponse(502, 'github_token_validation_failed', 'Could not validate GitHub token'); } - const body = (await res.json().catch(() => null)) as { login?: unknown } | null; + const body = (await res.json().catch(() => null)) as { login?: unknown; id?: unknown } | null; const accountLogin = stringOrNull(body?.login); if (!accountLogin) { throw new RouteResponse(502, 'github_token_validation_failed', 'GitHub token validation returned no login'); } return { accountLogin, + accountId: githubAccountId(body?.id), scopes: scopesHeaderArray(res.headers.get('x-oauth-scopes')), }; } +function githubAccountId(value: unknown): string | null { + if (typeof value === 'number' && Number.isSafeInteger(value) && value > 0) return String(value); + if (typeof value === 'string' && /^[1-9]\d*$/.test(value.trim())) return value.trim(); + return null; +} + function stringOrNull(value: unknown): string | null { return typeof value === 'string' && value.trim() ? value.trim() : null; } diff --git a/surface/server/test/gitIdentity.test.ts b/surface/server/test/gitIdentity.test.ts new file mode 100644 index 00000000..6a8ae1e6 --- /dev/null +++ b/surface/server/test/gitIdentity.test.ts @@ -0,0 +1,167 @@ +import { randomUUID } from 'node:crypto'; +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import type pg from 'pg'; +import { buildApp } from '../src/app.js'; +import { Connections } from '../src/connections.js'; +import { resolveGitIdentity } from '../src/git-identity.js'; +import { createTestPool, seedFixture, seedMember, truncateAll, type Fixture } from './helpers.js'; + +let pool: pg.Pool; +let fx: Fixture; + +beforeAll(async () => { + pool = await createTestPool(); +}); + +afterAll(async () => { + await pool.end(); +}); + +beforeEach(async () => { + await truncateAll(pool); + fx = await seedFixture(pool); +}); + +async function setUser(userId: string, displayName: string, email: string | null): Promise { + await pool.query('UPDATE users SET display_name = $2, email = $3 WHERE id = $1', [userId, displayName, email]); +} + +async function addGitHubIdentity(userId: string, accountLogin: string, accountId: string | null): Promise { + await pool.query( + `INSERT INTO user_connection_identities + (workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, active) + VALUES ($1, $2, 'github', 'github:pat', 'connected', 'pat', $3, $4, true)`, + [fx.workspaceId, userId, accountLogin, accountId], + ); +} + +async function addSession(args: { + spawnedBy: string; + credentialOwner?: string | null; + harness?: string; +}): Promise<{ id: string; threadKey: string }> { + const threadKey = `git-identity-${randomUUID()}`; + const result = await pool.query<{ id: string }>( + `INSERT INTO sessions + (workspace_id, channel_id, centaur_thread_key, harness, title, status, spawned_by, + provider_credential_user_id) + VALUES ($1, $2, $3, $4, 'git identity', 'running', $5, $6) + RETURNING id`, + [fx.workspaceId, fx.channelId, threadKey, args.harness ?? 'codex', args.spawnedBy, args.credentialOwner ?? null], + ); + return { id: result.rows[0]!.id, threadKey }; +} + +describe('resolveGitIdentity', () => { + it('uses the active GitHub login and id for an ID-prefixed noreply address', async () => { + await setUser(fx.userId, 'Allan Niemerg', 'allan@example.com'); + await addGitHubIdentity(fx.userId, 'aniemerg', '123'); + const session = await addSession({ spawnedBy: fx.userId }); + + await expect(resolveGitIdentity(pool, session.id)).resolves.toEqual({ + authorName: 'Allan Niemerg', + authorEmail: '123+aniemerg@users.noreply.github.com', + source: 'github_noreply', + sessionId: session.id, + harness: 'codex', + }); + }); + + it('falls back to the Atrium email when an existing GitHub identity has no account id', async () => { + await setUser(fx.userId, 'Allan Niemerg', 'allan@example.com'); + await addGitHubIdentity(fx.userId, 'aniemerg', null); + const session = await addSession({ spawnedBy: fx.userId }); + + await expect(resolveGitIdentity(pool, session.id)).resolves.toMatchObject({ + authorEmail: 'allan@example.com', + source: 'atrium_account', + }); + }); + + it('uses the Atrium email when no GitHub connection exists', async () => { + await setUser(fx.userId, 'Allan Niemerg', 'allan@example.com'); + const session = await addSession({ spawnedBy: fx.userId }); + + await expect(resolveGitIdentity(pool, session.id)).resolves.toMatchObject({ + authorName: 'Allan Niemerg', + authorEmail: 'allan@example.com', + source: 'atrium_account', + }); + }); + + it('returns no identity when neither GitHub nor Atrium supplies an email', async () => { + await setUser(fx.userId, '', null); + const session = await addSession({ spawnedBy: fx.userId }); + + await expect(resolveGitIdentity(pool, session.id)).resolves.toBeNull(); + }); + + it('uses provider_credential_user_id ahead of spawned_by', async () => { + await setUser(fx.userId, 'Spawner', 'spawner@example.com'); + await addGitHubIdentity(fx.userId, 'spawner', '111'); + const credentialOwnerId = await seedMember(pool, fx.workspaceId, 'owner', 'Credential Owner'); + await pool.query('UPDATE users SET email = $2 WHERE id = $1', [credentialOwnerId, 'owner@example.com']); + await addGitHubIdentity(credentialOwnerId, 'credential-owner', '456'); + const session = await addSession({ spawnedBy: fx.userId, credentialOwner: credentialOwnerId, harness: 'claude' }); + + await expect(resolveGitIdentity(pool, session.id)).resolves.toEqual({ + authorName: 'Credential Owner', + authorEmail: '456+credential-owner@users.noreply.github.com', + source: 'github_noreply', + sessionId: session.id, + harness: 'claude', + }); + }); + + it('backfills account_id on both live connection tables during the next validation upsert', async () => { + await setUser(fx.userId, 'Allan Niemerg', 'allan@example.com'); + await addGitHubIdentity(fx.userId, 'aniemerg', null); + await new Connections(pool).upsertGitHubMetadata({ + workspaceId: fx.workspaceId, + userId: fx.userId, + status: 'connected', + tokenKind: 'pat', + accountLogin: 'aniemerg', + accountId: '123', + }); + + const stored = await pool.query<{ table_name: string; account_id: string | null }>( + `SELECT 'user_connections' AS table_name, account_id + FROM user_connections + WHERE workspace_id = $1 AND user_id = $2 AND provider = 'github' + UNION ALL + SELECT 'user_connection_identities' AS table_name, account_id + FROM user_connection_identities + WHERE workspace_id = $1 AND user_id = $2 AND provider = 'github' AND active + ORDER BY table_name`, + [fx.workspaceId, fx.userId], + ); + expect(stored.rows).toEqual([ + { table_name: 'user_connection_identities', account_id: '123' }, + { table_name: 'user_connections', account_id: '123' }, + ]); + }); +}); + +describe('GET /api/internal/sessions/:id/git-identity', () => { + it('returns 204 with no body when identity resolution is unavailable', async () => { + await setUser(fx.userId, '', null); + const session = await addSession({ spawnedBy: fx.userId }); + const app = await buildApp({ + pool, + artifactCaptureApiKey: 'git-identity-test-key', + sessionRuns: { baseUrl: 'http://127.0.0.1:1', apiKey: 'test', autoResume: false }, + }); + try { + const response = await app.inject({ + method: 'GET', + url: `/api/internal/sessions/${session.threadKey}/git-identity`, + headers: { 'x-api-key': 'git-identity-test-key' }, + }); + expect(response.statusCode).toBe(204); + expect(response.body).toBe(''); + } finally { + await app.close(); + } + }); +}); diff --git a/surface/server/test/internalContract.test.ts b/surface/server/test/internalContract.test.ts index 33ff5e0f..0ee00f61 100644 --- a/surface/server/test/internalContract.test.ts +++ b/surface/server/test/internalContract.test.ts @@ -283,4 +283,32 @@ describe('node-sync wire contract (fixtures)', () => { expect(res.statusCode).toBe(200); expect(Array.isArray(res.json<{ bundles: unknown[] }>().bundles)).toBe(true); }); + + it('git identity carries the contract shape', async () => { + await pool.query(`UPDATE users SET display_name = 'Allan Niemerg', email = 'allan@example.com' WHERE id = $1`, [ + fx.userId, + ]); + await pool.query( + `INSERT INTO user_connection_identities + (workspace_id, user_id, provider, identity_id, status, token_kind, account_login, account_id, active) + VALUES ($1, $2, 'github', 'github:pat', 'connected', 'pat', 'aniemerg', '123', true)`, + [fx.workspaceId, fx.userId], + ); + const sid = await session(); + const res = await app.inject({ + method: 'GET', + url: `/api/internal/sessions/${sid}/git-identity`, + headers: { 'x-api-key': KEY }, + }); + expect(res.statusCode).toBe(200); + const { _comment, ...template } = loadFixture('git-identity.json') as Record; + expect(typeof _comment).toBe('string'); + expectShape(res.json(), template); + expect(res.json()).toMatchObject({ + authorName: 'Allan Niemerg', + authorEmail: '123+aniemerg@users.noreply.github.com', + source: 'github_noreply', + sessionId: sid, + }); + }); });