feat: implement stat point allocation on level-up#141
Conversation
stat_points_available was already a column in the DB schema but was never wired up. This activates it end-to-end: - awardXp now grants 10 stat points per level gained - new buddy_allocate tool lets you spend points on a specific stat (DEBUGGING, PATIENCE, CHAOS, WISDOM, SNARK), capped at 100 - buddy_status appends a notice when unspent points are available
|
thank you @TheMichaelFeng for the submission! Per codex: The allocation path is currently broken: |
…, status refresh
Four bugs addressed per maintainer review:
1. Wrong column names: handler read/wrote `debugging` instead of `stat_debugging`.
Fix: extract applyStatAllocation to lib/allocate.ts which uses the correct
stat_ prefix throughout.
2. NULL arithmetic: rescued companions have NULL stat columns; SQL `NULL + n = NULL`
silently wiped points. Fix: resolve current value via loadCompanion() (which
applies the bones fallback), then write a concrete integer.
3. Non-positive/fractional points: schema used type:"number" allowing 1.5 or -3.
Fix: schema uses type:"integer" + minimum:1; applyStatAllocation also validates
and returns { ok:false, reason:'invalid_points' } for early rejection.
4. Status file stale after allocation: writeBuddyStatus() was never called.
Fix: called immediately after the companion is reloaded post-update.
Adds 16 tests in src/__tests__/allocate.test.ts covering fresh companions,
rescued companions with NULL stat columns, stat caps, clamping, invalid point
amounts, and multi-level point gains.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ite atomic
Follow-up to the previous commit after a closer look:
- applyStatAllocation() now re-validates `stat` against STAT_NAMES itself
instead of trusting the caller. It interpolates the stat name into a SQL
column reference (stat_${stat.toLowerCase()}), so leaving that check only
in the MCP handler meant any other future caller of this now-exported lib
function could hand it an unvalidated string straight into SQL. Adds
'invalid_stat' to AllocateResult.
- Wraps the SELECT -> compute -> UPDATE -> SELECT sequence in db.transaction(),
matching the existing repairDuplicateCompanions convention in db/schema.ts.
The maintainer's review asked for the NULL fallback to be applied
"atomically"; the previous commit fixed the NULL bug but left the four
statements unwrapped. The MCP server and the UserPromptSubmit hook both
hold this DB file open, so this closes a real (if narrow) cross-process
race window, not just a theoretical one.
- index.ts: removed the now-duplicated stat/points validation ahead of the
call (applyStatAllocation does it internally) and replaced the two-if
fallback with an exhaustive switch over all 5 AllocateResult reasons.
- Tests: added invalid-stat-name coverage, including a SQL-injection-shaped
stat string, verified to be rejected without mutating the row or dropping
the table. Also fixed a latent flakiness bug in the original test file —
several tests picked an arbitrary stat and asserted success without
pinning its starting value, which breaks deterministically for any userId
whose rarity roll happens to land its peak stat at the 100 cap (this is
exactly what "multilevel-jump" did on first run). Fresh/multi-level tests
now pin the target stat before allocating; rescued-companion tests dynamically
pick a stat with headroom instead, since pinning would defeat the point of
testing the NULL-column fallback.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…DIATE for allocation Completes the "after awarding" half of the review's point 4 — the previous commits only fixed the allocating half: - awardXpAndRefresh built the companion by patching xp/level/mood onto the caller's pre-award row snapshot, so on the turn a level-up granted stat points, companion.availablePoints was stale (usually 0). observe and pet then wrote that stale value into the status file. Now re-reads the row after awarding so the returned companion — and the status file written from it — reflect the freshly granted points. Exported for testing (same precedent as recalcMood / self-healing.test.ts). - applyStatAllocation now runs its transaction as BEGIN IMMEDIATE. With two processes on one WAL DB (MCP server + UserPromptSubmit hook), a deferred read-then-write transaction can fail its write upgrade with SQLITE_BUSY_SNAPSHOT instead of waiting on busy_timeout; taking the write lock up front avoids that. Also reworded the transaction comment, which referenced a function that only exists on the fix/hatch branch. - Tests: two awardXpAndRefresh cases (multi-level jump grants levelsGained*10 fresh points; pre-existing unspent points survive a no-level-up award), plus an afterAll wipe so the xp_events rows these tests insert don't leak into later test files that clean up with a bare DELETE FROM companions (foreign_keys is ON; leaking broke doctor-insight, mode-orthogonality, and respawn-cleanup in the shared-DB suite run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Pushed a fix for all 4 points. Thanks for the detailed review. :-)
Also went back and hardened a couple things you didn't call out but seemed worth closing while I was in there: 22 new tests in Let me know if anything's still off :) |
|
Hey!
Thanks for taking the time to re examine it, that was a
much better review than I gave myself the first time around. So I pushed a
fix for all 4 things you flagged:
1. The stat_ column prefix mismatch (this was the "no such column:
debugging"
crash), fixed, and I moved the allocation logic into its own file so
it's unit-testable on its own.
2. The NULL + 1 = NULL bug for rescued companions, now resolves the real
current value through the existing bones-fallback logic before writing,
instead of doing a blind SQL increment.
3. points is now validated as a positive integer, both in the tool schema
and at runtime.
4. The stale status file, turned out to be two bugs, not one. Fixed the
allocate side, but then noticed the *award* side (observe/pet leveling
someone up) had the same staleness: on the exact turn a level-up handed
out new stat points, the status file got written with the old point
count because it was built from a pre-award snapshot instead of a fresh
read. Fixed that too.
Added 22 tests covering everything you asked for plus a couple of
edge cases I found while double-checking my own work (a SQL-injection-
shaped input to the stat parameter, and making the point-spending
transaction actually atomic across processes rather than just
assuming Node's single-threadedness would save it).
Full writeup and diff is on the PR:
#141
Let me know if you want anything else changed!
…On Thu, 2 Jul 2026 at 11:29, Steven(Jieli) Wu ***@***.***> wrote:
*fiorastudio* left a comment (fiorastudio/buddy#141)
<#141 (comment)>
thank you @TheMichaelFeng <https://github.com/TheMichaelFeng> for the
submission!
Per codex:
The allocation path is currently broken:
The schema uses stat_debugging, stat_patience, etc., but the handler
reads/updates debugging, patience, etc. A valid allocation throws no such
column: debugging.
Rescued/legacy companions have NULL persisted stat columns. Even after
fixing the prefix, NULL + 1 remains NULL while the point is deducted.
Initialize from loadCompanion(...).stats[stat] or use an appropriate
fallback atomically.
points must be validated as a positive integer; the current number schema
permits fractional and non-positive values.
Reload the companion and update the status file after awarding or
allocating points; current refresh logic uses stale point data.
Please add tests covering fresh companions, rescued companions with null
stat columns, stat caps, invalid point amounts, and multi-level gains.
—
Reply to this email directly, view it on GitHub
<#141?email_source=notifications&email_token=BNNZVQYU4WLTUTEITXJNNUD5CXJH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOBWGE4TCMZUGYYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4861913460>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BNNZVQ4XEUJC2DWUWFHWE4T5CXJH7AVCNFSNUABGKJSXA33TNF2G64TZHMYTEMBWGE4DMOJWHE5US43TOVSTWNBXG4ZTINJSG4YTDILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/BNNZVQ2ZM5GSWNVRTXNA6Z35CXJH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOBWGE4TCMZUGYYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/BNNZVQ5YIWSGHPR3G4W3WU35CXJH7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOBWGE4TCMZUGYYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
2 participants
stat_points_available was already a column in the DB schema but was never wired up. This activates it end-to-end: