Fix CI failure: DB Reopen race and install test drift

[wesm]

Summary

• Fix DB Reopen race (internal/db/db.go): reopenLocked() was closing old *sql.DB pools immediately after swapping atomic pointers, causing concurrent readers to hit sql: database is closed. Old pools are now retired into a list and closed in DB.Close(), eliminating the race deterministically.
• Fix install test drift (scripts/install.sh, scripts/install_test.sh): The test duplicated the grep/cut parsing logic from install.sh. Added a BASH_SOURCE guard to install.sh so the test can source it and call get_latest_version directly with a mocked curl. Also switched from echo to printf for reliable fixture piping.

Test plan

• TestConcurrentReadsWhileReopen passes 5x with -count=5
• All TestReopen*, TestCloseConnections, TestCloseRenameReopen pass
• Full internal/db test suite passes
• scripts/install_test.sh passes (6/6)
• install.sh syntax check passes (bash -n)
• go vet and go fmt clean

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (ad93891)

Summary: The changes introduce a high-impact regression in the installation script and significant concurrency and resource leak issues in the database lifecycle management that must be addressed.

High

1. Broken Install Script Execution (curl ... | bash)

• File: scripts/install.sh:20
  3
• Description: The new guard [[ "${BASH_SOURCE[0]}" == "${0}" ]] breaks the documented install path. During stdin execution
  (curl ... | bash), BASH_SOURCE is unset under set -u, causing the script to exit with an unbound variable error, and the installation fails to run.
• Suggested Remediation: Use a nounset-safe guard, such as if [[ "${BASH _SOURCE[0]-$0}" == "$0" ]]; then ... fi, which still prevents running main when sourced but allows piped execution.

2. Unbounded Resource Leak (File Descriptor Exhaustion / DoS)

• File: internal/db/db.go:42
  1-426
• Description: By changing the logic to retire old database pools instead of closing them, reopenLocked appends old
  *sql.DB connection pools to the db.retired slice indefinitely. Every reopened pool holds onto memory and file descriptors. If called repeatedly during the lifecycle of the service, this causes an unbounded resource leak, eventually crashing the application with a "too many open files" error.
• Suggested Remediation
  : Implement a safe cleanup mechanism for retired pools, such as a background goroutine that closes them after a safe timeout, or a reference counter to track active in-flight queries and close the pool safely once it drops to zero.

Medium

3. CloseConnections Leaks Retired Pools

• File:
  internal/db/db.go:380, [internal/db/db.go:426](/home/roborev/.rob
  orev/clones/wesm/agentsview/internal/db/db.go:426)
• Description: CloseConnections() currently closes only the active reader/writer pools. Retired pools accumulated in db.retired are not closed, leaving file handles open and violating the method
  ’s contract ("releasing file locks so the database file can be renamed").
• Suggested Remediation: Iterate, close, and clear the db.retired slice inside CloseConnections() (while holding db.mu).

4. Data Race on db.retired Slice in Close ()

• File: internal/db/db.go:365-374
• Description: The Close() method accesses, iterates over, and clears the db.retired slice without acquiring the db.mu mutex, while reopenLocked() appends to it under db.mu. Concurrent Close() and Reopen() calls will cause a Go data race, potentially resulting in missed closes,
  FD leaks, inconsistent state, or a runtime panic.
• Suggested Remediation: Acquire db.mu.Lock() before iterating over and clearing db.retired in the Close() method to safely synchronize with reopenLocked().

---

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (90432ce)

Summary Verdict: The code introduces concurrency issues related to the database connection lifecycle during
Close() and Reopen() operations.

Medium

• Concurrency / Resource Leak in Close()
  • File: [internal/db/db.go#L366](/home/roborev/.roborev/clones/wesm/agentsview/internal/
    db/db.go#L366) (lines 364-378)
  • Description: There is a race condition in the Close() method that can result in leaked database connections and inconsistent behavior. The method unlocks db.mu before capturing and closing the database
    pools via db.getWriter() and db.getReader(). A concurrent Reopen() can swap in fresh pools between the unlock and close, meaning Close() may close the newly reopened pools and leave the old ones (now in db.retired) orphaned and unclosed.

  ---

Suggested Remediation:** Keep db.mu held for the entire duration of the Close() method using a deferred unlock, or atomically capture the pointers to known values under lock and then close exactly those captured handles.

• Unsafe Handoff for In-Flight Readers (One-Cycle Retirement)
  *
  File: internal/db/db.go#L430
  • Description: reopenLocked() closes all previously
    retired pools at the start of the next reopen. A reader that loaded the old pointer before reopen N can still be preempted or slow, hitting a closed pool when reopen N+1 executes quickly. This is the same class of race condition, simply delayed by one cycle.
  • Suggested Remediation
    : Use a deterministic grace mechanism (e.g., refcounted reader leases or a time-based grace window independent of the reopen count) before closing retired pools.

---

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (41b5eb8)

Summary Verdict: One High severity issue found regarding a regression in
the install.sh execution guard; all other changes are clean and secure.

High

Guard in install.sh breaks the documented curl ... | bash install path and can abort under set -u
File: scripts/install.sh:203

Problem: if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then assumes BASH_SOURCE[0] is always
set. When the script is executed from stdin (... | bash), BASH_SOURCE[0] is unset, so with set -u this can fail with unbound variable (or skip main entirely depending shell context).
Suggested fix: Make the guard nounset-safe and
treat stdin execution as “executed directly”, e.g. use ${BASH_SOURCE[0]-} and include a stdin case.

---

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (c2b8756)

Summary Verdict: One medium severity issue found regarding a potential reader race condition during rapid database reopens; all other checks passed.

Medium

• Potential reader race across rapid consecutive Reopen() calls
  Files: internal/db/db.go:430, internal/db/db.go:449
  The old pools are only deferred by one reopen cycle (retired closed at next
  Reopen). A reader that captured the old pool pointer before the first swap, but executes later, can still hit a closed DB if a second reopen happens quickly. This weakens the intended “no read interruption during reopen” guarantee under bursty reopen sequences.
  Suggested fix: Use a real grace mechanism (time
  -based delayed close, generation count >1, or in-flight read tracking/refcount) before closing retired pools.

---

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

[wesm]

acceptable risk. merging