From 56f19552cd2688670df3c87f48ca4134789d5a54 Mon Sep 17 00:00:00 2001
From: mabel <mabel@nexthop.ai>
Date: Thu, 14 May 2026 09:47:59 +0530
Subject: [PATCH] ConfigSession: always create initial commit on fresh
 /etc/coop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

- On a fresh box with no git history, `initializeGit()` only created
an initial commit if `cli/agent.conf` already existed. On first-ever
use that file doesn't exist yet, so both concurrent users end up with
`base_ = ""`.
- When User1 commits and HEAD moves, User2's stale-session check
(`!base_.empty() && currentHead != base_`) short-circuits on the empty
string, letting User2 silently overwrite User1's changes.
- Fix: always create an initial commit when the repo has no commits.
Commit `agent.conf` if it exists (as a copy — we can't rename because
`/etc/coop/agent.conf` must stay in place; `commit()` replaces it
with a symlink later), falling back to an empty `{}` JSON object. A
try/catch handles the concurrent-init race.

## Test plan

- [x] Run `ConfigConcurrentSessionsTest.ConflictAndRebase` on a fresh
box (no prior git history in `/etc/coop`) — should now fail User2's
commit with "system configuration has changed"
- [x] Confirm existing `ConfigSession` unit/integration tests still
pass

Co-Authored-By: Vybhav Acharya <vybhav@nexthop.ai>
---
 fboss/cli/fboss2/session/ConfigSession.cpp | 48 +++++++++++++++++++---
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/fboss/cli/fboss2/session/ConfigSession.cpp b/fboss/cli/fboss2/session/ConfigSession.cpp
index 03e0f42b7d7f2..26ee8402867ae 100644
--- a/fboss/cli/fboss2/session/ConfigSession.cpp
+++ b/fboss/cli/fboss2/session/ConfigSession.cpp
@@ -670,13 +670,49 @@ void ConfigSession::initializeGit() {
     git_->init();
   }
 
-  // If the repository has no commits but the config file exists,
-  // create an initial commit to track the existing configuration
-  std::string cliConfigPath = getCliConfigPath();
-  if (!git_->hasCommits() && fs::exists(cliConfigPath)) {
+  // Always ensure an initial commit exists so base_ is never empty.
+  // Replicates the exact structure that commit() produces:
+  //   cli/agent.conf  — the actual config content
+  //   agent.conf      — a symlink to cli/agent.conf
+  // This prevents two concurrent users from both seeing base_="" and silently
+  // overwriting each other's commits, and ensures rebase() can always call
+  // git show <base>:cli/agent.conf without hitting exit 128.
+  if (!git_->hasCommits()) {
     std::string systemConfigPath = getSystemConfigPath();
-    git_->commit(
-        {cliConfigPath, systemConfigPath}, "Initial commit", username_, "");
+    std::string cliConfigPath = getCliConfigPath();
+
+    ensureDirectoryExists(getCliConfigDir());
+
+    // Populate cli/agent.conf if it doesn't exist yet.
+    // If agent.conf is currently a plain file (pre-symlink state), copy from
+    // it so we don't lose the running config.  We copy rather than rename
+    // because /etc/coop/agent.conf must remain in place (the running system
+    // reads it, and commit() will later replace it with a symlink).
+    // Fall back to an empty JSON object so the file is always valid JSON.
+    if (!fs::exists(cliConfigPath)) {
+      std::error_code ec;
+      bool systemIsRegularFile =
+          fs::is_regular_file(fs::path(systemConfigPath), ec) && !ec;
+      std::string seedContent = "{}";
+      if (systemIsRegularFile) {
+        folly::readFile(systemConfigPath.c_str(), seedContent);
+        if (seedContent.empty()) {
+          seedContent = "{}";
+        }
+      }
+      folly::writeFileAtomic(
+          cliConfigPath, seedContent, 0644, folly::SyncType::WITH_SYNC);
+    }
+
+    try {
+      git_->commit({cliConfigPath}, "Initial commit", username_, "");
+    } catch (const std::exception&) {
+      // Another process may have raced us to the initial commit.
+      // If commits now exist, swallow the error; otherwise re-throw.
+      if (!git_->hasCommits()) {
+        throw;
+      }
+    }
   }
 }