Skip to content

[Nexthop][fboss2-dev] ConfigSession: always create initial commit on fresh /etc/coop#1191

Open
vybhav-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:config-session-initial-commit
Open

[Nexthop][fboss2-dev] ConfigSession: always create initial commit on fresh /etc/coop#1191
vybhav-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:config-session-initial-commit

Conversation

@vybhav-nexthop
Copy link
Copy Markdown

@vybhav-nexthop vybhav-nexthop commented May 14, 2026

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

##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 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

  • 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"
  • Confirm existing ConfigSession unit/integration tests still
    pass

## 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>
@vybhav-nexthop vybhav-nexthop requested a review from a team as a code owner May 14, 2026 05:26
@meta-cla meta-cla Bot added the CLA Signed label May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant