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

Abierto
  • ¿Algo no está bien?
#419 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

@cristim ya está trabajando en esto.

Desde el 9/11/2026.

  • #427 de @cristim — abierto

Evaluación

Dificultad
5/5
Tiempo estimado
Más de una semana
Aptitud para principiantes
25/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Estancado
Stack tecnológico
rust
Área
api, backend-api-design

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Rust
Estrellas
4
Forks
2
Merge medio
1 d 1 h
PR fusionados (30 d)
225

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de cristim/AztecCraft

Todos los issues de cristim/AztecCraft

Issues similares

Más issues de Rust

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.