Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

fix(admin): batch param is unclamped and batch=0 spins 50 no-op batches in all=true mode

Abierto Apto para principiantes
  • ¿Algo no está bien?
#28 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
typescript
Área
api, backend

Línea de trabajo

Empieza en apps/web/app/routes/api.admin.recompute.ts, en parseIntSafe, la asignación de batchSize y el bucle all=true; compara la validación con el comportamiento descrito en recompute-all-skills.ts:177. Usa la reproducción proporcionada con curl de ADMIN_SECRET y los bindings D1 y KV requeridos; después verifica que los valores de batch estén acotados y que batch=0 ya no produzca 50 batches sin operación.

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

Descripción

bug

Summary

The batch query parameter is neither clamped nor range-checked on POST /api/admin/recompute. parseIntSafe rejects only NaN and negative values, so batch=0 is accepted and there is no upper bound either. With all=true, batch=0 makes one request burn all 50 permitted batch iterations while processing nothing.

Current behavior

  • apps/web/app/routes/api.admin.recompute.ts:28-32 — the only validation, which lets 0 through and caps nothing: 
    function parseIntSafe(value: string | null, fallback: number): number {
  if (value == null) return fallback;
  const n = parseInt(value, 10);
  return Number.isNaN(n) || n < 0 ? fallback : n;
}
  • apps/web/app/routes/api.admin.recompute.ts:58 — that value becomes batchSize and reaches .limit(batchSize) in the query builder, so batch=0 becomes LIMIT 0.
  • apps/web/app/lib/leaderboard/recompute-all-skills.ts:177 — with an empty batch the completion test is 0 < 0, which is false: 
    const isComplete = batch.length < batchSize;
    The batch is reported as incomplete, and nextOffset is set to offset unchanged, so the all=true loop at apps/web/app/routes/api.admin.recompute.ts:81 keeps calling with the same offset until MAX_BATCHES_PER_REQUEST (50) is reached, then returns "Paused after 50 batches" with nothing processed.
  • There is also no upper bound: batch=100000 is accepted and would defeat the guard added at line 21 to keep the Worker under its CPU limit.

Expected behavior

Clamp the value to a sane range, as the review recommended, and reject nonsense input explicitly:

const batchSize = Math.min(Math.max(parseIntSafe(batch, 100), 1), 500);

Reproduction

  1. Set batch to 0 and select all-at-once mode: 
    ADMIN_SECRET=... curl -sS -X POST "https://skillx.sh/api/admin/recompute?all=true&batch=0"
  2. The response reports batches: 50, totalProcessed: 0, and the "Paused after 50 batches" message, while GET /api/admin/recompute shows the checkpoint unchanged.

Conditions: requires ADMIN_SECRET plus the D1 and KV bindings; admin-only endpoint. Behavior read from main at 9d0f9e9; the exact request was not executed against production.

Flow

flowchart LR
  A["batch=0"] --> B[parseIntSafe returns 0]
  B --> C["LIMIT 0 -> empty batch"]
  C --> D{{"isComplete = 0 < 0 = false"}}
  D -->|nextOffset unchanged| E[all=true loop retries same offset]
  E --> F[50 no-op batches, then Paused]

Evidence

  • Affected code: apps/web/app/routes/api.admin.recompute.ts:28, apps/web/app/routes/api.admin.recompute.ts:58, apps/web/app/routes/api.admin.recompute.ts:81, apps/web/app/lib/leaderboard/recompute-all-skills.ts:177
  • Introduced by: 88a5bff, shipped on main via 9d0f9e9 (PR #9)
  • Related: plans/reports/code-reviewer-260306-1531-admin-recompute-endpoint.md, section "8. Input validation for batch and offset params"

Open questions

  • Upper bound: 500 as suggested in the review, or align it with the existing default of 100?
  • Should batch=0 return HTTP 400 instead of being silently clamped to 1?

Notes

  • offset has the same missing upper clamp, but an out-of-range offset only yields an empty batch and a correct completion, so it is far less harmful than batch=0.
  • MAX_BATCHES_PER_REQUEST at line 21 already handles the Worker-timeout risk from the review; the missing piece is bounding the work each iteration can request.
Lenguaje dominante
TypeScript
Estrellas
189
Forks
40
Merge medio
6 h 54 min
PR fusionados (30 d)
24

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

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 nextlevelbuilder/skillx

Todos los issues de nextlevelbuilder/skillx

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.