## Summary
- Convert `bin/lib/onboard-session.js` (440 lines) to
`src/lib/onboard-session.ts`
- Typed interfaces: `Session`, `StepState`, `SessionFailure`,
`LockResult`, `SessionUpdates`, `LockInfo`, `SessionMetadata`
- Full migration including fs I/O (session persistence, file locking)
and pure helpers (redaction, validation, normalization)
- Co-locate tests (14 tests) with updated cache-clearing for dist/ path

Stacked on #1240. 615 CLI tests pass. Coverage ratchet passes.

Relates to #924 (shell consolidation).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Reorganized onboarding session management module structure and
improved distribution of the codebase. Session tracking, step-level
state management, lock handling, and sensitive data redaction
functionality have been consolidated and optimized for better
maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

## Summary
- Convert `bin/lib/onboard-session.js` (440 lines) to
`src/lib/onboard-session.ts`
- Typed interfaces: `Session`, `StepState`, `SessionFailure`,
`LockResult`, `SessionUpdates`, `LockInfo`, `SessionMetadata`
- Full migration including fs I/O (session persistence, file locking)
and pure helpers (redaction, validation, normalization)
- Co-locate tests (14 tests) with updated cache-clearing for dist/ path

Stacked on NVIDIA#1240. 615 CLI tests pass. Coverage ratchet passes.

Relates to NVIDIA#924 (shell consolidation).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Reorganized onboarding session management module structure and
improved distribution of the codebase. Session tracking, step-level
state management, lock handling, and sensitive data redaction
functionality have been consolidated and optimized for better
maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The acquireOnboardLock stale-cleanup path read a stale lock, decided
the holder was dead, and unconditionally unlinked LOCK_FILE before
retrying writeFileSync(wx). Two concurrent processes that both observe
the same stale lock will both try to clean it up — and the slower of
the two can unlink the *fresh* lock the faster process just claimed,
breaking mutual exclusion: both processes end up holding 'their' lock
and onboard runs in parallel against the same shared session state.

Reported as NVIDIA#1281, originally surfaced by CodeRabbit on NVIDIA#1272.

Fix: capture the stale file's inode via fs.statSync({ bigint: true })
at the same time we read its contents, then in a new
unlinkIfInodeMatches() helper, re-stat right before fs.unlinkSync and
bail if the inode has changed. The dual stat-then-unlink is the only
portable POSIX primitive Node exposes for this — there is no atomic
"unlink-if-inode" syscall — so a sufficiently unlucky race can still
slip through. The window is orders of magnitude smaller than the
unconditional unlink it replaces, and the outer retry loop will detect
a wrong unlink on its next writeFileSync(wx) attempt because either
we re-create the file or we observe a new lock with a different inode.

Also bumps MAX_ATTEMPTS from 2 to 5 because the inode-verified cleanup
can take a few more spins under contention before one cleaner wins.

Adds a regression test that simulates the race deterministically by
wrapping fs.statSync so the first stat succeeds against the original
stale inode, then atomically swaps the lock file (unlink + recreate)
to give it a new inode before unlinkIfInodeMatches re-stats it. The
test asserts the fresh claim survives the race and is the file on
disk after acquireOnboardLock returns.

Verified by stashing the source fix and re-running: the new test
fails on the unguarded code as expected, and passes with the inode
guard in place.

Closes NVIDIA#1281

## Summary
- Convert `bin/lib/onboard-session.js` (440 lines) to
`src/lib/onboard-session.ts`
- Typed interfaces: `Session`, `StepState`, `SessionFailure`,
`LockResult`, `SessionUpdates`, `LockInfo`, `SessionMetadata`
- Full migration including fs I/O (session persistence, file locking)
and pure helpers (redaction, validation, normalization)
- Co-locate tests (14 tests) with updated cache-clearing for dist/ path

Stacked on NVIDIA#1240. 615 CLI tests pass. Coverage ratchet passes.

Relates to NVIDIA#924 (shell consolidation).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Reorganized onboarding session management module structure and
improved distribution of the codebase. Session tracking, step-level
state management, lock handling, and sensitive data redaction
functionality have been consolidated and optimized for better
maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The acquireOnboardLock stale-cleanup path read a stale lock, decided
the holder was dead, and unconditionally unlinked LOCK_FILE before
retrying writeFileSync(wx). Two concurrent processes that both observe
the same stale lock will both try to clean it up — and the slower of
the two can unlink the *fresh* lock the faster process just claimed,
breaking mutual exclusion: both processes end up holding 'their' lock
and onboard runs in parallel against the same shared session state.

Reported as NVIDIA#1281, originally surfaced by CodeRabbit on NVIDIA#1272.

Fix: capture the stale file's inode via fs.statSync({ bigint: true })
at the same time we read its contents, then in a new
unlinkIfInodeMatches() helper, re-stat right before fs.unlinkSync and
bail if the inode has changed. The dual stat-then-unlink is the only
portable POSIX primitive Node exposes for this — there is no atomic
"unlink-if-inode" syscall — so a sufficiently unlucky race can still
slip through. The window is orders of magnitude smaller than the
unconditional unlink it replaces, and the outer retry loop will detect
a wrong unlink on its next writeFileSync(wx) attempt because either
we re-create the file or we observe a new lock with a different inode.

Also bumps MAX_ATTEMPTS from 2 to 5 because the inode-verified cleanup
can take a few more spins under contention before one cleaner wins.

Adds a regression test that simulates the race deterministically by
wrapping fs.statSync so the first stat succeeds against the original
stale inode, then atomically swaps the lock file (unlink + recreate)
to give it a new inode before unlinkIfInodeMatches re-stats it. The
test asserts the fresh claim survives the race and is the file on
disk after acquireOnboardLock returns.

Verified by stashing the source fix and re-running: the new test
fails on the unguarded code as expected, and passes with the inode
guard in place.

Closes NVIDIA#1281

The acquireOnboardLock stale-cleanup path read a stale lock, decided
the holder was dead, and unconditionally unlinked LOCK_FILE before
retrying writeFileSync(wx). Two concurrent processes that both observe
the same stale lock will both try to clean it up — and the slower of
the two can unlink the *fresh* lock the faster process just claimed,
breaking mutual exclusion: both processes end up holding 'their' lock
and onboard runs in parallel against the same shared session state.

Reported as NVIDIA#1281, originally surfaced by CodeRabbit on NVIDIA#1272.

Fix: capture the stale file's inode via fs.statSync({ bigint: true })
at the same time we read its contents, then in a new
unlinkIfInodeMatches() helper, re-stat right before fs.unlinkSync and
bail if the inode has changed. The dual stat-then-unlink is the only
portable POSIX primitive Node exposes for this — there is no atomic
"unlink-if-inode" syscall — so a sufficiently unlucky race can still
slip through. The window is orders of magnitude smaller than the
unconditional unlink it replaces, and the outer retry loop will detect
a wrong unlink on its next writeFileSync(wx) attempt because either
we re-create the file or we observe a new lock with a different inode.

Also bumps MAX_ATTEMPTS from 2 to 5 because the inode-verified cleanup
can take a few more spins under contention before one cleaner wins.

Adds a regression test that simulates the race deterministically by
wrapping fs.statSync so the first stat succeeds against the original
stale inode, then atomically swaps the lock file (unlink + recreate)
to give it a new inode before unlinkIfInodeMatches re-stats it. The
test asserts the fresh claim survives the race and is the file on
disk after acquireOnboardLock returns.

Verified by stashing the source fix and re-running: the new test
fails on the unguarded code as expected, and passes with the inode
guard in place.

Closes NVIDIA#1281

The acquireOnboardLock stale-cleanup path read a stale lock, decided
the holder was dead, and unconditionally unlinked LOCK_FILE before
retrying writeFileSync(wx). Two concurrent processes that both observe
the same stale lock will both try to clean it up — and the slower of
the two can unlink the *fresh* lock the faster process just claimed,
breaking mutual exclusion: both processes end up holding 'their' lock
and onboard runs in parallel against the same shared session state.

Reported as NVIDIA#1281, originally surfaced by CodeRabbit on NVIDIA#1272.

Fix: capture the stale file's inode via fs.statSync({ bigint: true })
at the same time we read its contents, then in a new
unlinkIfInodeMatches() helper, re-stat right before fs.unlinkSync and
bail if the inode has changed. The dual stat-then-unlink is the only
portable POSIX primitive Node exposes for this — there is no atomic
"unlink-if-inode" syscall — so a sufficiently unlucky race can still
slip through. The window is orders of magnitude smaller than the
unconditional unlink it replaces, and the outer retry loop will detect
a wrong unlink on its next writeFileSync(wx) attempt because either
we re-create the file or we observe a new lock with a different inode.

Also bumps MAX_ATTEMPTS from 2 to 5 because the inode-verified cleanup
can take a few more spins under contention before one cleaner wins.

Adds a regression test that simulates the race deterministically by
wrapping fs.statSync so the first stat succeeds against the original
stale inode, then atomically swaps the lock file (unlink + recreate)
to give it a new inode before unlinkIfInodeMatches re-stats it. The
test asserts the fresh claim survives the race and is the file on
disk after acquireOnboardLock returns.

Verified by stashing the source fix and re-running: the new test
fails on the unguarded code as expected, and passes with the inode
guard in place.

Closes NVIDIA#1281

…#1656)

## Summary

Closes #1281.

`acquireOnboardLock` in `src/lib/onboard-session.ts` had a stale-cleanup
path that read a stale lock, decided the holder was dead, and
**unconditionally** called `fs.unlinkSync(LOCK_FILE)` before retrying
`writeFileSync(wx)`.

Two concurrent processes that both observe the same stale lock both try
to clean it up — and the slower of the two can unlink the *fresh* lock
the faster process just claimed. Both processes then end up holding
'their' lock and onboard runs in parallel against the same shared
session state.

Originally surfaced by CodeRabbit on #1272.

## Race walkthrough

```
A: writeFile(wx) → EEXIST (stale lock present)
B: writeFile(wx) → EEXIST (same stale lock)
A: read stale, isProcessAlive(stale.pid) → false
A: unlinkSync(LOCK_FILE)                                     ← deletes stale
A: loop, writeFile(wx) → SUCCESS, A holds the lock
B: read stale (still has its own copy from before)
B: isProcessAlive(stale.pid) → false (B is reasoning about
   the OLD pid, not the fresh lock A just wrote)
B: unlinkSync(LOCK_FILE)                                     ← DELETES A's FRESH LOCK
B: loop, writeFile(wx) → SUCCESS, B also "holds" the lock
```

After this, both A and B believe they own the lock and proceed with
onboard.

## Fix

Capture the stale file's **inode** via `fs.statSync({ bigint: true })`
at the same time we read its contents. Then in a new
`unlinkIfInodeMatches()` helper, re-stat right before `fs.unlinkSync`
and bail if the inode has changed.

The dual stat-then-unlink is the only portable POSIX primitive Node
exposes for this — there is no atomic "unlink-if-inode" syscall — so a
sufficiently unlucky race can still slip through. **However** the window
is orders of magnitude smaller than the unconditional unlink it
replaces, and the outer retry loop will detect a wrong unlink on its
next `writeFileSync(wx)` attempt because either we re-create the file or
we observe a new lock with a different inode and retry.

Also bumps `MAX_ATTEMPTS` from 2 to 5 because the inode-verified cleanup
can take a few more spins under contention before one cleaner wins.

## Behavior preserved

- Malformed lock files are still left on disk (the existing "treats
unreadable or transient lock contents as a retry" test still passes — I
explicitly do NOT call `unlinkIfInodeMatches` on malformed locks, only
on parseable-but-stale ones).
- A holder PID that is still alive is still reported correctly to the
caller.
- `releaseOnboardLock()` semantics unchanged.

## Test plan

- [x] Added regression test `regression #1281: stale-cleanup race does
not unlink a fresh lock claimed by another process` that simulates the
race deterministically by wrapping `fs.statSync` so the first stat
(inside acquireOnboardLock) succeeds against the original stale inode,
then atomically swaps the lock file (unlink + recreate) to give it a new
inode before `unlinkIfInodeMatches` re-stats it. The test asserts the
fresh claim survives the race and is the file on disk after
`acquireOnboardLock` returns.
- [x] **Negative case verified**: stashed the source fix, re-ran the
test against the unguarded code. The new regression test correctly
**fails** because the unconditional `unlinkSync` deletes the fresh claim
and writes a new one with a different command string. The test's
`expect(onDisk.command).toContain("fresh claim from concurrent
process")` then fails with the actual content showing the wrong winner.
- [x] **Positive case verified**: re-applied the fix, re-ran. All 14
lock-related tests pass.
- [x] No regression on existing lock tests (`acquires and releases the
onboard lock`, `replaces a stale onboard lock`, `treats unreadable or
transient lock contents as a retry, not a stale lock`, `ignores
malformed lock files when releasing the onboard lock`).
- [x] One pre-existing Windows-only POSIX-permissions test (`creates and
persists a session with restrictive permissions` checking `mode & 0o777
=== 0o600`) fails on this branch with the same failure as `main` — this
is the umask/POSIX-mode-on-Windows issue and is unrelated to the lock
fix.

## Why an inode check rather than the issue's suggested `linkSync`
approach

The issue body suggests using `fs.linkSync(temp, LOCK_FILE)` which fails
atomically with EEXIST if the target exists. That's a valid alternative,
but:

1. The current `writeFileSync(LOCK_FILE, payload, { flag: "wx" })`
already provides the same atomic-create-or-fail semantics — there's no
need to introduce a tempfile + link dance for the *initial* claim. The
race is specifically in the *cleanup* of stale locks, not the initial
claim.
2. Inode-comparison preserves the existing happy-path code
(single-syscall `writeFileSync(wx)`) and only adds the check on the rare
stale-cleanup branch.
3. Tempfile + linkSync would add a second possible failure mode
(tempfile leak under crash) without solving the cleanup race any more
cleanly.

Both approaches are correct; this one is the smaller surgical change.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added a regression test that reproduces a stale-lock race and verifies
correct behavior without cross-test side effects.

* **Bug Fixes**
* Increased retry attempts for lock acquisition to reduce transient
failures.
  * More robust, atomic lock creation to avoid partial writes.
  * Safer stale-lock cleanup using on-disk verification before removal.
* More reliable lock release that verifies ownership before removing
lock files.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: ColinM-sys <cmcdonough@50words.com>

---------

Signed-off-by: ColinM-sys <cmcdonough@50words.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>