Skip to content

HTTP plans list: unbounded ListGoals then slice; GetPlan errors silently dropped #54

Description

@mrchatam
  • Label: bug
  • Severity: degrades quality
  • Location: internal/httpapi/handlers_p1.go (~defaultPlanListGoalLimit, plan list handler):
    goals, err := st.ListGoals()   // unbounded — store.ListGoals → ListGoalsLimited(-1)
    ...
    if len(goals) > limit {
        truncated = true
        goals = goals[:limit]
    }
    for _, g := range goals {
        view, err := ps.GetPlan(r.Context(), g.ID)
        if err != nil {
            continue   // swallow
        }
        items = append(items, view)
    }
    writeJSON(..., {"items": items, "count": len(items), "truncated": truncated})
  • Expected: SQL/keyset-limited goal page; plan load failures surface as errors (or explicit skipped[]), and count/truncated reflect honesty.
  • Actual: Residual load-all-then-slice. Any GetPlan failure omits that goal with no error, so clients can see count < requested slice with truncated: false and believe the graph is complete. Prior QA residual confirmed still present on this tip.

Audited tip: 6afe0fd (main). Filed from independent Phase 1 code audit 2026-09-19.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions