Repository navigation
chore(repo): land unpushed local engine work - #87
Conversation
apply_effect accepted mechanics[].type as freeform z.string(), but the read-back CustomEffectSchema.parse requires the closed MechanicTypeSchema enum. A bad type passed input validation, INSERTed, then threw on read-back, leaving an unreadable row keyed on (target,name). Every later apply() with that name re-parsed the poison and re-threw the SAME error verbatim — looked like a name-keyed error cache, but was a live parse of the stored row. - input mechanics[].type -> MechanicTypeSchema enum (+ .describe() on all effect fields so load_tool_schema/flat schema self-document the shape) - repo.apply(): insert+read-back wrapped in a transaction (rollback on bad read-back so no poison can persist) - repo.apply(): findRawByTargetAndName + try/parse self-heals a corrupt same-name row instead of re-throwing forever - regression test: tests/storage/repos/custom-effects.repo.test.ts NOTE: test suite not run locally (better-sqlite3 ABI mismatch); tsc --noEmit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tter-sqlite3 ABI mismatch Root cause: fnm default is Node 22 (ABI 127) but the harness-launched MCP server (dist/server/index.js) and the better-sqlite3 prebuilt both run on system Node 20 (ABI 115). Running 'npm test' from an fnm-22 shell failed with NODE_MODULE_VERSION 115 vs 127 every time. Rebuilding for 22 would break the running Node-20 server, so we align dev/test on Node 20 instead. After 'fnm install 20.18.1', cd into the repo auto-selects Node 20 and the suite runs clean (verified: 41/41 in two repo+tool test files). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s stop returning empty content
Root cause of silent agents (e.g. Vesh): agent.maxTokens defaults to 800,
but for INT>=11 the competency ladder selects a reasoning model (o3/gpt-5.x).
For those, max_completion_tokens caps reasoning tokens AND output together —
800 is consumed entirely by reasoning, so the model stops at
finish_reason=length with EMPTY content, deterministically, every call. The
empty-content ProviderError ('malformed') even trips the circuit after 3, so
the agent then flips to circuit_open and is skipped.
- OpenAIProvider floors max_completion_tokens for reasoning models, scaled by
reasoning_effort (low 4k / medium 8k / high 16k / xhigh 32k). Only ever
RAISES the caller value; billed on actual usage so short replies stay cheap.
- empty-content + finish_reason=length now throws a self-diagnosing error
naming the exhausted budget instead of an opaque 'empty message content'.
- tests: floor by effort, no-effort default, no-lowering, non-reasoning
passthrough, and the length-error message. 70/70 agent tests green (Node 20).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, player sheet, docs
Phased plan extending ADR-005: campaign-scoped tenancy, Postgres port, OAuth (GitHub + Google) over Streamable HTTP, Railway deploy, PixiJS VTT, free-tier metering + BYOK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da01a82cb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| upsertCls.run(SUNDAR, 'Brawler', 2, 600); // grandfathered: earned by training (by-use), not XP | ||
| upsertCls.run(SUNDAR, 'Deepsense', 1, 300); // bought with 300 XP | ||
| db.prepare('UPDATE characters SET xp=? WHERE id=?').run(200, SUNDAR); // 200 banked in the pool |
There was a problem hiding this comment.
Despite the script being documented as safe to rerun, these unconditional upserts reset Sundar's class tracks to Brawler 2 / Deepsense 1 and then overwrite the XP pool with 200. If the script is rerun after any subsequent level-up or XP gain, it silently rolls back that live progression; make the backfill conditional on missing rows/state rather than overwriting existing values.
Useful? React with 👍 / 👎.
| Safe to re-run (idempotent). Run: node scripts/migrate-class-progression.cjs */ | ||
| const Database = require('better-sqlite3'); | ||
| const path = require('path'); | ||
| const db = new Database(path.join(process.env.APPDATA, 'rpg-mcp', 'rpg.db')); |
There was a problem hiding this comment.
This bypasses the repository's database resolver and always targets %APPDATA%/rpg-mcp/rpg.db. Consequently, it throws at startup on typical macOS/Linux environments where APPDATA is unset, ignores RPG_MCP_DB_PATH, RPG_DATA_DIR, and --db-path in configured single-user installs, and cannot select a per-campaign database in hosted mode; the migration may therefore fail or modify a database the server never reads.
Useful? React with 👍 / 👎.
| - **XP is fuel.** Earned by play, banked in the pool. Leveling a class **spends** | ||
| `CLASS_LEVEL_XP_COST` (default **300**, flat, tunable) from the pool. |
There was a problem hiding this comment.
The progression rules state that every class level costs a flat 300 XP, while levelUpCost() charges 300 * currentEffectiveLevel (for example, 900 XP at effective level 3). The included level-up script also spends only 300 in that scenario, so callers following the documentation and callers using ClassProgressionRepo.costToLevelUp() will update the same character differently; establish one formula and update the other paths to match it.
Useful? React with 👍 / 👎.
* fix(improvisation): close apply_effect poisoned-row loop
apply_effect accepted mechanics[].type as freeform z.string(), but the
read-back CustomEffectSchema.parse requires the closed MechanicTypeSchema
enum. A bad type passed input validation, INSERTed, then threw on read-back,
leaving an unreadable row keyed on (target,name). Every later apply() with
that name re-parsed the poison and re-threw the SAME error verbatim — looked
like a name-keyed error cache, but was a live parse of the stored row.
- input mechanics[].type -> MechanicTypeSchema enum (+ .describe() on all
effect fields so load_tool_schema/flat schema self-document the shape)
- repo.apply(): insert+read-back wrapped in a transaction (rollback on bad
read-back so no poison can persist)
- repo.apply(): findRawByTargetAndName + try/parse self-heals a corrupt
same-name row instead of re-throwing forever
- regression test: tests/storage/repos/custom-effects.repo.test.ts
NOTE: test suite not run locally (better-sqlite3 ABI mismatch); tsc --noEmit clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: pin Node to 20.18.1 (.node-version/.nvmrc) to fix recurring better-sqlite3 ABI mismatch
Root cause: fnm default is Node 22 (ABI 127) but the harness-launched MCP
server (dist/server/index.js) and the better-sqlite3 prebuilt both run on
system Node 20 (ABI 115). Running 'npm test' from an fnm-22 shell failed with
NODE_MODULE_VERSION 115 vs 127 every time. Rebuilding for 22 would break the
running Node-20 server, so we align dev/test on Node 20 instead.
After 'fnm install 20.18.1', cd into the repo auto-selects Node 20 and the
suite runs clean (verified: 41/41 in two repo+tool test files).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(agent): floor reasoning-model completion budget so high-INT agents stop returning empty content
Root cause of silent agents (e.g. Vesh): agent.maxTokens defaults to 800,
but for INT>=11 the competency ladder selects a reasoning model (o3/gpt-5.x).
For those, max_completion_tokens caps reasoning tokens AND output together —
800 is consumed entirely by reasoning, so the model stops at
finish_reason=length with EMPTY content, deterministically, every call. The
empty-content ProviderError ('malformed') even trips the circuit after 3, so
the agent then flips to circuit_open and is skipped.
- OpenAIProvider floors max_completion_tokens for reasoning models, scaled by
reasoning_effort (low 4k / medium 8k / high 16k / xhigh 32k). Only ever
RAISES the caller value; billed on actual usage so short replies stay cheap.
- empty-content + finish_reason=length now throws a self-diagnosing error
naming the exhausted budget instead of an opaque 'empty message content'.
- tests: floor by effort, no-effort default, no-lowering, non-reasoning
passthrough, and the length-error message. 70/70 agent tests green (Node 20).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(class-progression): migrations + repo, levelup/migration scripts, player sheet, docs
* docs: cloud migration plan (Railway, OAuth, Postgres, web VTT)
Phased plan extending ADR-005: campaign-scoped tenancy, Postgres port,
OAuth (GitHub + Google) over Streamable HTTP, Railway deploy, PixiJS VTT,
free-tier metering + BYOK.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Consolidates the two local working copies of this repo onto one source of truth.
There was only ever one GitHub remote —
mnehmos.rpg.mcp. The duplication was local: two clones, one of which (mnehmos-rpg-mcp-railway) received all the recent hosted-tenancy work while the other (mnehmos.rpg.mcp, the one the stdio MCP config actually runs) carried five commits that had never been pushed.This lands those five so nothing is stranded on a laptop.
What was unpushed
feat(class-progression)— migrations, repository, level-up and migration scripts, generated player sheet, docs. The substantial one, ~1,000 lines.fix(improvisation)— closes theapply_effectpoisoned-row loop.fix(agent)— floors the reasoning-model completion budget so high-INT agents stop returning empty content.chore— pins Node to 20.18.1 via.node-version/.nvmrc, fixing the recurring better-sqlite3 ABI mismatch.docs— cloud migration plan.A sixth commit, the
scene_manage action:'current'transposed-args fix, was dropped during rebase as already upstream — it had been cherry-picked into main earlier. No duplicate.Rebase notes
Rebased onto
origin/mainrather than merged, so the five sit on top of the tenancy work as ordinary commits.No conflicts. Worth confirming explicitly, since the two lines both touched
ensureDb(): the tenancy work rewrote those helpers to callgetDb()with no path, and the improvisation fix touched a different region of the same file. The structural guard confirms nothing regressed — zero path-basedgetDbcalls remain outsidestorage/index.ts.Risk
Low. Every commit here already ran locally for days; this is about custody, not new behaviour.
Acceptance criteria
getDbcall is reintroduced.Verification
npm test— 2,373 passed / 7 skipped, no failures. Up from 2,364 on main: the custom-effects and openai provider tests these commits bring with them.npx tsc --noEmit— exit 0.🤖 Generated with Claude Code