fix(mcp): range-check NDC arguments on the wire value before narrowing to f32

Aperta
  • Qualcosa non va?
#419 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@cristim ci sta già lavorando.

Dal 9/11/2026.

  • #427 di @cristim — aperta

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
25/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
rust
Ambito
api, backend-api-design

Direzione di ricerca

Read src/mcp/features/selection.rs, src/viewport/rect_pick.rs, and .project-docs/mcp.md, then check the linked pull request #427 for work already underway. First settle whether the range contract applies to the f64 wire value or the f32 value; done means documenting that decision and adding consistent tool descriptions and tests for every affected argument.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

area/mcp effort/s impact/few priority/p3 severity/low triaged type/bug urgency/eventually

Problem

Raised by a CodeRabbit review of #404 and checked against the code. MCP tools that take normalised screen coordinates deserialise them as [f32; 2] (for example select_in_rect, src/mcp/features/selection.rs). serde rounds a value such as 1.00000001 to 1.0 before the range check (NdcRect::new in src/viewport/rect_pick.rs), so a value that is 1e-8 out of range is accepted as the boundary instead of returning invalid_params. The accepted rectangle is the same as the boundary, so the effect is negligible, but the contract says out-of-range values are refused.

This is the f32 convention across the MCP surface, so fixing it for one tool alone would be inconsistent.

Change

  • Decide whether the range contract is on the f64 wire value or the f32 value, and document it in .project-docs/mcp.md.
  • If the wire value: parse these fields as f64, check the range, then narrow, in one shared helper used by every tool with NDC or screen-fraction arguments; add a test with 1.00000001 for each.
  • If the f32 value: state that in the tool descriptions and the docs, and pin it with a test so it is a decision and not an accident.

Related: #404, #390.

Lingua principale
Rust
Stelle
4
Fork
2
Merge medio
1g 1h
PR unite (30g)
225

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di cristim/AztecCraft

Tutte le issue di cristim/AztecCraft

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.