Repository navigation
fix(inventory): add authoritative light extinguish - #105
Conversation
|
✅ Health of changed files: 5.3 (unchanged) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (3)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_src_server_consolidated_inventory_manage_ts[".../consolidated/inventory-manage.ts 🔥"]:::changed
end
f_src_server_consolidated_batch_manage_ts[".../consolidated/batch-manage.ts"]
f_src_server_consolidated_inventory_manage_ts --> f_src_server_consolidated_batch_manage_ts
f_src_server_consolidated_index_ts[".../consolidated/index.ts"]
f_src_server_consolidated_inventory_manage_ts --> f_src_server_consolidated_index_ts
t_tests_server_consolidated_open5e_authority_test_ts(["✅ .../consolidated/open5e-authority.test.ts"]):::guard
t_tests_server_consolidated_open5e_authority_test_ts -.-> f_src_server_consolidated_inventory_manage_ts
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (2 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (2)
💀 Dead code (10 findings)
7 more
📊 See the full report for this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe consolidated inventory tool adds an ChangesInventory extinguish action
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant InventoryManage
participant effectsRepo
participant ResponseFormatter
Caller->>InventoryManage: request extinguish with characterId and itemId
InventoryManage->>InventoryManage: validate light-source item
InventoryManage->>effectsRepo: find active effect by target and name
effectsRepo-->>InventoryManage: active effect or no effect
InventoryManage->>effectsRepo: deactivate active effect
InventoryManage->>ResponseFormatter: return extinguish result
ResponseFormatter-->>Caller: render extinguished or already-extinguished status
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: d44dc1134b
ℹ️ 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".
| const effectName = `Light source: ${item.name}`; | ||
| const activeEffect = effectsRepo.findByTargetAndName(params.characterId, 'character', effectName); |
There was a problem hiding this comment.
Match the persisted light by source item ID
When two light-source templates share a name, extinguishing the never-lit template will find and deactivate the other template's effect; similarly, item_manage.update can rename a lit template, after which this lookup reports it as already extinguished while the authoritative effect remains active. The effect already persists the stable item identity in source_entity_id, so the lookup should match the target and that ID rather than the mutable, non-unique display name.
Useful? React with 👍 / 👎.
Summary\n- add inventory_manage action �xtinguish (aliases put_out and douse) for recognized torch/lantern sources\n- deactivate the authoritative custom effect while preserving source item provenance and inventory\n- make repeated extinguish calls idempotent and explicit\n\n## Evidence\n-
pm test (2,275 passed, 7 skipped)\n-
pm run build passes\n- focused inventory suite: 34 passed\n\nCloses #104\nRefs Mnehmos/rpg-mcp-live#268
Summary by CodeRabbit