feat(17): speak SLA — caches, JSON-safe fetch, social fast lane#5
Merged
Conversation
…etch Add layered caches for worker-state, memory-context, and lore chunks; harden embed and HTTP JSON parsing to avoid empty-body decode errors. Introduce social edge fast lane and speak intent improvements on the worker path; update CONTRACTS and verify scripts accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR internalizes apply-actions, adds concurrency gates and caches (worker-state, memory, chunk lore) with per-player invalidation and stale fallbacks, instruments internal latency, and introduces a deterministic social-edge fast-lane plus related tests and scripts. ChangesCache Infrastructure and Concurrency Gating
Player-Scoped Operations and State Invalidation
Internal Endpoint Security and Routing
Memory Context and Latency Instrumentation
Speak Job Control Flow
Worker Resilience, Lazy Lore, and Collective Gate
Social-Edge Fast-Lane and Intent Routing
Tool parsing and invoke robustness
Test suite & scripts
Docs & Contracts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/game-server/src/routes/internal-memories.ts (1)
67-92:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMemory-context cache needs invalidation on memory mutations to avoid stale read-after-write responses.
This new cache is now authoritative for Line 67 onward, but write routes in the same router (
POST /:roomId/memories,POST /:roomId/reflect,POST /:roomId/summarize-bulk,DELETE /:roomId/memories) do not invalidate affected keys. Repeated prompts can return stale context until TTL expiry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/game-server/src/routes/internal-memories.ts` around lines 67 - 92, The memory-context cache (used via memoryContextCacheKey, getCachedMemoryContext, setCachedMemoryContext) is not being invalidated on writes, causing stale responses; add cache invalidation after any mutation by removing relevant keys (e.g., by roomId/playerId/npcId scope) — implement or expose a helper like invalidateMemoryContext(roomId, playerId?, npcId?) that deletes matching cache entries and call it at the end of each write handler: POST /:roomId/memories, POST /:roomId/reflect, POST /:roomId/summarize-bulk, and DELETE /:roomId/memories (use the same identifiers used to build keys so the fresh buildMemoryContext returns up-to-date data).
🧹 Nitpick comments (2)
workers/agent-worker/src/graph/social_edge_fast_lane.py (1)
49-50: 💤 Low valueConsider reusing a single
httpx.Clientcontext.The function creates two separate
httpx.Client()contexts for sequential synchronous calls. Since there's no threading here, a single client context would reduce overhead.♻️ Suggested refactor
- with httpx.Client() as client: - state = fetch_state_and_memory(state, settings=cfg, client=client) - - player_msg = player_message.strip() - reconciled = reconcile_social_perception(player_msg, preview.social) - turn = preview.model_copy(update={"social": reconciled}) - - state = { - **state, - "social_perception": turn.social.model_dump(), - "reply_draft": turn.reply, - "tool_calls": [], - "social_applied": False, - "collective_updated": False, - } - - state = apply_social_event(state) - state = refresh_collective_in_state(state) - - with httpx.Client() as client: - state = apply_tools(state, settings=cfg, client=client) + with httpx.Client() as client: + state = fetch_state_and_memory(state, settings=cfg, client=client) + + player_msg = player_message.strip() + reconciled = reconcile_social_perception(player_msg, preview.social) + turn = preview.model_copy(update={"social": reconciled}) + + state = { + **state, + "social_perception": turn.social.model_dump(), + "reply_draft": turn.reply, + "tool_calls": [], + "social_applied": False, + "collective_updated": False, + } + + state = apply_social_event(state) + state = refresh_collective_in_state(state) + state = apply_tools(state, settings=cfg, client=client)Also applies to: 68-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workers/agent-worker/src/graph/social_edge_fast_lane.py` around lines 49 - 50, The current code creates two separate httpx.Client() contexts for sequential calls; open a single httpx.Client() as client once and reuse it for both calls (wrap the block that calls fetch_state_and_memory(...) and fetch_social_edges(...) in one `with httpx.Client() as client:`), remove the second client context, and pass the same `client` instance into both functions (they already accept a client param: fetch_state_and_memory and fetch_social_edges) so you reduce overhead and reuse connections.apps/game-server/src/world/lore-chunk-cache.test.ts (1)
11-21: ⚡ Quick winAdd a concurrent same-key test to lock in stampede prevention behavior.
Current coverage verifies sequential hits, but not concurrent callers on an empty cache. Add a
Promise.all([...])case and assert repository fetch count stays1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/game-server/src/world/lore-chunk-cache.test.ts` around lines 11 - 21, Add a concurrent-request test to ensure stampede prevention: update the test that uses getChunkLoreCached and loreRepo.getChunkLore by adding a Promise.all([...]) call that invokes getChunkLoreCached("world-1", 1, 2) multiple times concurrently when the cache is empty, await the Promise.all result, assert all results equal the mocked row, and assert the spy on loreRepo.getChunkLore was called exactly once; keep the existing spy setup (vi.spyOn(loreRepo, "getChunkLore").mockResolvedValue(row)) and reuse the same arguments so the test verifies concurrent callers share a single repository fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/game-server/src/colyseus/bridge.ts`:
- Around line 200-204: The reset currently uses the singleton map.player as the
anchor which can desync multiplayer; instead, for each entry in state.players
use that player's own position or home/spawn coordinates when finding a reset
location. Replace the anchor = map.player usage in the loop with the per-player
anchor (e.g., use player.x/player.y or player.homeX/player.homeY if a spawn
field exists) before calling buildMoveGrid/findNearestWalkableCell so each
player's spawn is computed from their own state rather than the global
map.player.
In `@apps/game-server/src/index.test.ts`:
- Around line 307-311: The test currently asserts that "p-a" attitude is removed
but doesn't assert that "p-b" remains; update the reset test around
stateRes.body.attitudes to explicitly find the attitude for playerId "p-b"
(similar to how attitudeA is found) and assert it's present (not undefined)
and/or has expected properties, e.g., check that attitudeB !== undefined or
matches expected values; locate this logic near the existing expectations using
stateRes and attitudes in index.test.ts.
In `@apps/game-server/src/routes/internal-memories.ts`:
- Around line 65-68: The cache key is being computed using playerId that may not
consider the X-Player-Id header; before calling memoryContextCacheKey, read the
primary identity from req.header("x-player-id") and use that as playerId
(falling back to playerIdFromInternal(...) only if the header is missing) so
cache partitioning always prefers the X-Player-Id value; update the code around
the speakHotPath/cacheKey lines to derive playerId from
req.header("x-player-id") first and then pass that into memoryContextCacheKey.
In `@apps/game-server/src/world/lore-chunk-cache.ts`:
- Around line 24-31: Concurrent callers all call getChunkLore for the same cache
key causing stampedes; fix by adding an in-flight promise deduplication map
keyed by cacheKey(worldId, cx, cy): before awaiting
runWithLoreConcurrencyLimit(() => getChunkLore(...)) check and return an
existing in-flight promise result, otherwise create and store the promise (the
result of runWithLoreConcurrencyLimit(...)), await it, then cache.set(key, {
expiresAt: Date.now() + TTL_MS, row }) and remove the entry from the in-flight
map in both success and failure paths so later callers either reuse the
in-flight promise or trigger a fresh fetch.
In `@docs/CONTRACTS.md`:
- Line 20: Update the contract entry for POST /internal/rooms/:id/apply-actions
in docs/CONTRACTS.md to mention the optional body field jobId (used by
recordSuccessfulMutation for audit tracking); explicitly mark it optional and
show where it fits alongside actingNpcId and actions[] so integrators know it
can be supplied but is not required.
---
Outside diff comments:
In `@apps/game-server/src/routes/internal-memories.ts`:
- Around line 67-92: The memory-context cache (used via memoryContextCacheKey,
getCachedMemoryContext, setCachedMemoryContext) is not being invalidated on
writes, causing stale responses; add cache invalidation after any mutation by
removing relevant keys (e.g., by roomId/playerId/npcId scope) — implement or
expose a helper like invalidateMemoryContext(roomId, playerId?, npcId?) that
deletes matching cache entries and call it at the end of each write handler:
POST /:roomId/memories, POST /:roomId/reflect, POST /:roomId/summarize-bulk, and
DELETE /:roomId/memories (use the same identifiers used to build keys so the
fresh buildMemoryContext returns up-to-date data).
---
Nitpick comments:
In `@apps/game-server/src/world/lore-chunk-cache.test.ts`:
- Around line 11-21: Add a concurrent-request test to ensure stampede
prevention: update the test that uses getChunkLoreCached and
loreRepo.getChunkLore by adding a Promise.all([...]) call that invokes
getChunkLoreCached("world-1", 1, 2) multiple times concurrently when the cache
is empty, await the Promise.all result, assert all results equal the mocked row,
and assert the spy on loreRepo.getChunkLore was called exactly once; keep the
existing spy setup (vi.spyOn(loreRepo, "getChunkLore").mockResolvedValue(row))
and reuse the same arguments so the test verifies concurrent callers share a
single repository fetch.
In `@workers/agent-worker/src/graph/social_edge_fast_lane.py`:
- Around line 49-50: The current code creates two separate httpx.Client()
contexts for sequential calls; open a single httpx.Client() as client once and
reuse it for both calls (wrap the block that calls fetch_state_and_memory(...)
and fetch_social_edges(...) in one `with httpx.Client() as client:`), remove the
second client context, and pass the same `client` instance into both functions
(they already accept a client param: fetch_state_and_memory and
fetch_social_edges) so you reduce overhead and reuse connections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3522da14-d770-4d59-82cc-a1fce28187cc
📒 Files selected for processing (40)
apps/game-server/src/collective/service.tsapps/game-server/src/colyseus/GameRoom.tsapps/game-server/src/colyseus/bridge.tsapps/game-server/src/colyseus/workerStateCache.test.tsapps/game-server/src/colyseus/workerStateCache.tsapps/game-server/src/index.test.tsapps/game-server/src/memory/embed.tsapps/game-server/src/memory/memoryContextCache.tsapps/game-server/src/memory/service.tsapps/game-server/src/observability/internalLatency.tsapps/game-server/src/routes/chat.tsapps/game-server/src/routes/internal-memories.tsapps/game-server/src/routes/internal.tsapps/game-server/src/routes/rooms.tsapps/game-server/src/sse/hub.tsapps/game-server/src/util/concurrencyGate.tsapps/game-server/src/world/lore-chunk-cache.test.tsapps/game-server/src/world/lore-chunk-cache.tsdocs/CONTRACTS.mddocs/LLM-E2E-FLOW-AND-LATENCY.mdpackages/npc-memory/src/collective/repository.test.tspackages/npc-memory/src/collective/repository.tsscripts/uat-phase7-reset-snap.mjsscripts/verify-phase10.mjsscripts/verify-phase12.mjsscripts/verify-phase2.mjsscripts/verify-phase4.mjsscripts/verify-phase5.mjsworkers/agent-worker/src/collective/social_turn.pyworkers/agent-worker/src/graph/ambient_intent.pyworkers/agent-worker/src/graph/nodes/llm_social_turn.pyworkers/agent-worker/src/graph/npc_loop.pyworkers/agent-worker/src/graph/social_edge_fast_lane.pyworkers/agent-worker/src/graph/speak_intent.pyworkers/agent-worker/src/main.pyworkers/agent-worker/src/memory/client.pyworkers/agent-worker/tests/test_fetch_state_and_memory.pyworkers/agent-worker/tests/test_social_turn.pyworkers/agent-worker/tests/test_speak_intent.pyworkers/agent-worker/tests/test_tool_gate.py
Comment on lines
+200
to
+204
| const anchor = map.player; | ||
| state.players.forEach((player, sessionId) => { | ||
| const grid = buildMoveGrid(map, state, sessionId, loader); | ||
| const spawn = findNearestWalkableCell(anchor.x, anchor.y, grid); | ||
| player.x = spawn.x; |
There was a problem hiding this comment.
Use state.players/home spawn for resets; don’t anchor from map.player in multiplayer.
Line 200 reintroduces singleton RoomState.player as the position source for resetting all humans, which can desync multiplayer semantics and snap everyone from the wrong anchor.
Suggested fix
- } else {
- const anchor = map.player;
- state.players.forEach((player, sessionId) => {
- const grid = buildMoveGrid(map, state, sessionId, loader);
- const spawn = findNearestWalkableCell(anchor.x, anchor.y, grid);
+ } else {
+ state.players.forEach((player, sessionId) => {
+ const grid = buildMoveGrid(map, state, sessionId, loader);
+ const spawn = findNearestWalkableCell(
+ HOME_DEFAULT_PLAYER_SPAWN.x,
+ HOME_DEFAULT_PLAYER_SPAWN.y,
+ grid,
+ );
player.x = spawn.x;
player.y = spawn.y;
});
}As per coding guidelines: “Multiplayer: human positions live in players, not lone RoomState.player.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const anchor = map.player; | |
| state.players.forEach((player, sessionId) => { | |
| const grid = buildMoveGrid(map, state, sessionId, loader); | |
| const spawn = findNearestWalkableCell(anchor.x, anchor.y, grid); | |
| player.x = spawn.x; | |
| const grid = buildMoveGrid(map, state, sessionId, loader); | |
| state.players.forEach((player, sessionId) => { | |
| const grid = buildMoveGrid(map, state, sessionId, loader); | |
| const spawn = findNearestWalkableCell( | |
| HOME_DEFAULT_PLAYER_SPAWN.x, | |
| HOME_DEFAULT_PLAYER_SPAWN.y, | |
| grid, | |
| ); | |
| player.x = spawn.x; | |
| player.y = spawn.y; | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/colyseus/bridge.ts` around lines 200 - 204, The reset
currently uses the singleton map.player as the anchor which can desync
multiplayer; instead, for each entry in state.players use that player's own
position or home/spawn coordinates when finding a reset location. Replace the
anchor = map.player usage in the loop with the per-player anchor (e.g., use
player.x/player.y or player.homeX/player.homeY if a spawn field exists) before
calling buildMoveGrid/findNearestWalkableCell so each player's spawn is computed
from their own state rather than the global map.player.
Source: Coding guidelines
Comment on lines
+307
to
+311
| expect(stateRes.body.recentEvents).toHaveLength(1); | ||
| const attitudeA = stateRes.body.attitudes?.find( | ||
| (a: { playerId: string }) => a.playerId === "p-a", | ||
| ); | ||
| expect(attitudeA).toBeUndefined(); |
There was a problem hiding this comment.
Assert surviving player attitude explicitly in the reset test.
Line 307 and Line 311 verify event retention and p-a removal, but they do not prove p-b attitude still exists; this case can pass if attitudes are accidentally cleared wholesale.
Suggested test hardening
expect(stateRes.status).toBe(200);
expect(stateRes.body.recentEvents).toHaveLength(1);
const attitudeA = stateRes.body.attitudes?.find(
(a: { playerId: string }) => a.playerId === "p-a",
);
expect(attitudeA).toBeUndefined();
+ const attitudeB = stateRes.body.attitudes?.find(
+ (a: { playerId: string }) => a.playerId === "p-b",
+ );
+ expect(attitudeB).toBeDefined();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(stateRes.body.recentEvents).toHaveLength(1); | |
| const attitudeA = stateRes.body.attitudes?.find( | |
| (a: { playerId: string }) => a.playerId === "p-a", | |
| ); | |
| expect(attitudeA).toBeUndefined(); | |
| expect(stateRes.body.recentEvents).toHaveLength(1); | |
| const attitudeA = stateRes.body.attitudes?.find( | |
| (a: { playerId: string }) => a.playerId === "p-a", | |
| ); | |
| expect(attitudeA).toBeUndefined(); | |
| const attitudeB = stateRes.body.attitudes?.find( | |
| (a: { playerId: string }) => a.playerId === "p-b", | |
| ); | |
| expect(attitudeB).toBeDefined(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/index.test.ts` around lines 307 - 311, The test
currently asserts that "p-a" attitude is removed but doesn't assert that "p-b"
remains; update the reset test around stateRes.body.attitudes to explicitly find
the attitude for playerId "p-b" (similar to how attitudeA is found) and assert
it's present (not undefined) and/or has expected properties, e.g., check that
attitudeB !== undefined or matches expected values; locate this logic near the
existing expectations using stateRes and attitudes in index.test.ts.
Comment on lines
+24
to
+36
| export function getCachedMemoryContext(key: string): MemoryContext | null { | ||
| const entry = cache.get(key); | ||
| if (!entry) return null; | ||
| if (Date.now() > entry.expiresAt) { | ||
| cache.delete(key); | ||
| return null; | ||
| } | ||
| return entry.context; | ||
| } | ||
|
|
||
| export function setCachedMemoryContext(key: string, context: MemoryContext): void { | ||
| cache.set(key, { expiresAt: Date.now() + TTL_MS, context }); | ||
| } |
There was a problem hiding this comment.
Bound the memory-context cache to avoid unbounded growth.
Expired entries are only cleaned on reads (Line 27). Entries written once and never read again remain indefinitely, so unique playerMessage traffic can grow memory without bound.
Proposed fix
const TTL_MS = 5000;
+const MAX_ENTRIES = 2000;
type CacheEntry = {
expiresAt: number;
context: MemoryContext;
};
const cache = new Map<string, CacheEntry>();
+
+function pruneExpired(now = Date.now()): void {
+ for (const [k, v] of cache) {
+ if (now > v.expiresAt) cache.delete(k);
+ }
+}
export function setCachedMemoryContext(key: string, context: MemoryContext): void {
+ pruneExpired();
+ if (cache.size >= MAX_ENTRIES) {
+ const oldestKey = cache.keys().next().value as string | undefined;
+ if (oldestKey) cache.delete(oldestKey);
+ }
cache.set(key, { expiresAt: Date.now() + TTL_MS, context });
}
Comment on lines
+65
to
+68
| const speakHotPath = req.header("x-speak-hot-path") === "1"; | ||
|
|
||
| const cacheKey = memoryContextCacheKey(roomId, playerId, npcId, playerMessage, skipEmbed); | ||
| const started = Date.now(); |
There was a problem hiding this comment.
Use X-Player-Id as the primary identity source for this hot path before cache-keying.
On Line 67, cache partitioning depends on playerId, but playerIdFromInternal(...) does not read the player-id header. That can mis-scope context/caching when query/body values are absent or inconsistent.
As per coding guidelines, “spatial NL needs X-Player-Id header.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/routes/internal-memories.ts` around lines 65 - 68, The
cache key is being computed using playerId that may not consider the X-Player-Id
header; before calling memoryContextCacheKey, read the primary identity from
req.header("x-player-id") and use that as playerId (falling back to
playerIdFromInternal(...) only if the header is missing) so cache partitioning
always prefers the X-Player-Id value; update the code around the
speakHotPath/cacheKey lines to derive playerId from req.header("x-player-id")
first and then pass that into memoryContextCacheKey.
Source: Coding guidelines
Comment on lines
+24
to
+31
| const key = cacheKey(worldId, cx, cy); | ||
| const hit = cache.get(key); | ||
| if (hit && Date.now() <= hit.expiresAt) { | ||
| return hit.row; | ||
| } | ||
| const row = await runWithLoreConcurrencyLimit(() => getChunkLore(worldId, cx, cy)); | ||
| cache.set(key, { expiresAt: Date.now() + TTL_MS, row }); | ||
| return row; |
There was a problem hiding this comment.
Deduplicate in-flight fetches per chunk key to prevent cache-miss stampedes.
On Line 29, concurrent calls for the same key all execute getChunkLore(...) before any result is cached. In hot-path fan-out this can multiply DB load and latency under burst traffic.
Suggested fix
const cache = new Map<string, CacheEntry>();
+const inFlight = new Map<string, Promise<ChunkLoreRow | null>>();
export async function getChunkLoreCached(
worldId: string,
cx: number,
cy: number,
): Promise<ChunkLoreRow | null> {
const key = cacheKey(worldId, cx, cy);
const hit = cache.get(key);
if (hit && Date.now() <= hit.expiresAt) {
return hit.row;
}
- const row = await runWithLoreConcurrencyLimit(() => getChunkLore(worldId, cx, cy));
+ const existing = inFlight.get(key);
+ if (existing) return existing;
+ const pending = runWithLoreConcurrencyLimit(() => getChunkLore(worldId, cx, cy))
+ .then((row) => {
+ cache.set(key, { expiresAt: Date.now() + TTL_MS, row });
+ return row;
+ })
+ .finally(() => {
+ inFlight.delete(key);
+ });
+ inFlight.set(key, pending);
+ const row = await pending;
cache.set(key, { expiresAt: Date.now() + TTL_MS, row });
return row;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/game-server/src/world/lore-chunk-cache.ts` around lines 24 - 31,
Concurrent callers all call getChunkLore for the same cache key causing
stampedes; fix by adding an in-flight promise deduplication map keyed by
cacheKey(worldId, cx, cy): before awaiting runWithLoreConcurrencyLimit(() =>
getChunkLore(...)) check and return an existing in-flight promise result,
otherwise create and store the promise (the result of
runWithLoreConcurrencyLimit(...)), await it, then cache.set(key, { expiresAt:
Date.now() + TTL_MS, row }) and remove the entry from the in-flight map in both
success and failure paths so later callers either reuse the in-flight promise or
trigger a fresh fetch.
| | **Memory 热路径** | `GET .../memory-context`：`skipEmbed=1` 用于 CASUAL/SOCIAL_EDGE/NARRATIVE；RECALL 须 full embed；5s 进程内 cache；header `X-Speak-Hot-Path: 1` 优先 embed 队列 | | ||
| | **Prompt** | 「我 / 下方 / 旁边」仅相对 **本次** `state.player`（见 `workers/.../prompt.py`） | | ||
| | **执行** | `POST .../apply-actions` body: `actingNpcId`, `actions[]`, `initiatorPlayerId` | | ||
| | **执行** | 唯一 HTTP 写入口：`POST /internal/rooms/:id/apply-actions`（Bearer `INTERNAL_WORKER_TOKEN` + header `X-Player-Id`；body `initiatorPlayerId` 仅 header 缺失时 fallback）。公开 `/rooms/.../apply-actions` 已移除。body: `actingNpcId`, `actions[]` | |
There was a problem hiding this comment.
Document optional jobId body field for completeness.
The contract specifies body: actingNpcId, actions[], but the implementation also accepts an optional jobId field used for audit trail tracking (recordSuccessfulMutation). While jobId is optional and its omission doesn't break functionality, the contract should mention it for completeness and to prevent confusion during integration.
📝 Suggested contract clarification
-| **执行** | 唯一 HTTP 写入口：`POST /internal/rooms/:id/apply-actions`（Bearer `INTERNAL_WORKER_TOKEN` + header `X-Player-Id`；body `initiatorPlayerId` 仅 header 缺失时 fallback）。公开 `/rooms/.../apply-actions` 已移除。body: `actingNpcId`, `actions[]` |
+| **执行** | 唯一 HTTP 写入口：`POST /internal/rooms/:id/apply-actions`（Bearer `INTERNAL_WORKER_TOKEN` + header `X-Player-Id`；body `initiatorPlayerId` 仅 header 缺失时 fallback）。公开 `/rooms/.../apply-actions` 已移除。body: `actingNpcId`, `actions[]`, `jobId?`（可选，用于审计） |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | **执行** | 唯一 HTTP 写入口：`POST /internal/rooms/:id/apply-actions`（Bearer `INTERNAL_WORKER_TOKEN` + header `X-Player-Id`；body `initiatorPlayerId` 仅 header 缺失时 fallback）。公开 `/rooms/.../apply-actions` 已移除。body: `actingNpcId`, `actions[]` | | |
| | **执行** | 唯一 HTTP 写入口：`POST /internal/rooms/:id/apply-actions`（Bearer `INTERNAL_WORKER_TOKEN` + header `X-Player-Id`；body `initiatorPlayerId` 仅 header 缺失时 fallback）。公开 `/rooms/.../apply-actions` 已移除。body: `actingNpcId`, `actions[]`, `jobId?`（可选，用于审计） | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/CONTRACTS.md` at line 20, Update the contract entry for POST
/internal/rooms/:id/apply-actions in docs/CONTRACTS.md to mention the optional
body field jobId (used by recordSuccessfulMutation for audit tracking);
explicitly mark it optional and show where it fits alongside actingNpcId and
actions[] so integrators know it can be supplied but is not required.
Add JSON-safe fetch/tool-call fallbacks for parallel speak, gate hostile ambient enqueue on hub rejection, and adapt verify scripts for Phase 16 world plus phase8 NL staging with spawn crowd clear. Co-authored-by: Cursor <cursoragent@cursor.com>
Move the second browser tab within cached home chunk before cache-nav so pageA can reach (34,13) without triggering extra lore enqueues. Co-authored-by: Cursor <cursoragent@cursor.com>
Assert hostile NL move block on speak-path coords; allow ≤1 cell HTTP drift from ambient wander after the turn completes. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 17 speak SLA on top of merged Phase 16 (
main).Test plan
pnpm --filter @aetherlife/game-server test— 175 passedcd workers/agent-worker && LLM_MOCK=1 uv run pytest -q— 201 passedpnpm agent:verify --e2e --base— GF-01/03/04/05/06/07 + phase8 cross-layer (~7 min, real LLM, post-commit)pnpm agent:verify— OKNotes
assets/pet-cat/intentionally not included (untracked art assets).