Skip to content

fix: wrap board API responses in envelope for consistency with mail#108

Open
lucamorettibuilds wants to merge 2 commits intoopenseed-dev:mainfrom
lucamorettibuilds:fix/board-api-envelope
Open

fix: wrap board API responses in envelope for consistency with mail#108
lucamorettibuilds wants to merge 2 commits intoopenseed-dev:mainfrom
lucamorettibuilds:fix/board-api-envelope

Conversation

@lucamorettibuilds
Copy link
Copy Markdown
Contributor

Closes #105

Wraps all board read endpoints in envelope objects to match the mail API shape.

Changes

listPosts() — was bare BoardPost[], now:

{ "posts": [...], "total": 42 }

getPost() — was BoardPost & { replies }, now:

{ "post": {...}, "replies": [...], "reply_count": 3 }

getReplies() — was bare BoardPost[], now:

{ "replies": [...], "reply_count": 2 }

Dashboard — updated fetchBoardPosts and fetchBoardPost to unwrap the new envelopes so the UI is unaffected.


One question: you mentioned updating environment.md — I don't see an existing API doc file. Should I create one as part of this PR, or is there a different place you'd like the routes documented?

Closes openseed-dev#105

- listPosts() now returns { posts, total } instead of bare array
- getPost() now returns { post, replies, reply_count } instead of merged object
- getReplies() now returns { replies, reply_count } instead of bare array
- Dashboard updated to unwrap new envelope shapes

Every creature-facing endpoint now returns an object with named data keys,
matching the mail API pattern. This eliminates shape-guessing between endpoints
and provides pagination metadata (total) without extra calls.
Copy link
Copy Markdown

@openseed-reviews openseed-reviews bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup + dashboard additions. One UX nit: BoardPanel builds the author filter options from the currently loaded posts, so once you filter by an author the dropdown shrinks to only that author and you can’t switch to a different one without clearing first. Consider keeping a full author list (or fetching unfiltered authors separately) so the dropdown stays stable.

Also, the /api/board response now includes { posts, total }, but fetchBoardPosts discards total and the header uses posts.length, which will undercount when the limit is hit. Might be worth surfacing total in the API helper + UI.

Copy link
Copy Markdown

@openseed-reviews openseed-reviews bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The envelope shapes feel consistent and the dashboard updates keep UI behavior intact. On the docs question: I don’t see an existing API reference either; a short note in README or a new docs/api.md would be fine, but I don’t think it should block this PR if you want to keep it lean.

Copy link
Copy Markdown
Contributor

@openseed-patch openseed-patch bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall direction is good — the board API was inconsistently shaped compared to mail, and having total on the list response is useful. A few things to look at before merging:

Breaking change for creature callers

The environment template (src/shared/environment-template.md) documents GET /api/board for creature use, and previously that endpoint returned a raw array. After this PR it returns { posts: [...], total: N }. Any creature currently iterating the board response will break. The environment-template.md (injected into every creature's context) should be updated to show the new shape.

Similarly, GET /api/board/<id> previously returned { id, author, body, replies, ... } (flat merge). Now it returns { post: { ... }, replies: [...], reply_count: N }. Creatures reading .body or .title directly on the response will get undefined.

Spread params in better-sqlite3 .get()

const total = (d.prepare(countSql).get(...countParams) as any).c;

better-sqlite3's .get() accepts a single binding object or positional args as a rest parameter, so the spread works fine at runtime. But countParams is typed any[], and if it's empty (no opts.author), get() is called with no args — that's fine, the SQL has no ? placeholders in that case. Worth a quick check but this should be okay.

getReplies return type change is observable

GET /api/board/:id/replies now returns { replies: [...], reply_count: N } instead of a raw array. This endpoint isn't used by the dashboard directly (it uses fetchBoardPost which hits /:id), but if creatures use it they'll break. Same environment-template update needed.

Suggestion

Update src/shared/environment-template.md to show the new response shapes:

# Returns: { posts: [...], total: N }
curl http://$HOST_URL/api/board

# Returns: { post: {...}, replies: [...], reply_count: N }
curl http://$HOST_URL/api/board/<post-uuid>

The board.ts changes themselves look clean — the struct types (BoardListResult, BoardThreadResult, BoardRepliesResult) make the shape explicit. The reply_count: post.reply_count ?? replies.length fallback in getPost is a nice safety. Main blocker is the undocumented creature-facing breaking change.

…late

Addresses review feedback from openseed-patch — creatures reading the
environment template will now see the new response shapes for:
- GET /api/board → { posts: [...], total: N }
- GET /api/board/<id> → { post: {...}, replies: [...], reply_count: N }
@lucamorettibuilds
Copy link
Copy Markdown
Contributor Author

Good catch on the breaking change for creature callers — pushed fce6ebb which adds the new response shapes to environment-template.md:

  • GET /api/board{ posts: [...], total: N }
  • GET /api/board/<id>{ post: {...}, replies: [...], reply_count: N }

This ensures creatures see the updated shapes in their injected context. The /api/board/:id/replies endpoint shape change is also covered since the template already points creatures at the post-level endpoint for thread reading.

On the BoardPanel author-filter UX nit — that's fair but feels like a separate issue. Happy to file one if it's worth tracking.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API consistency: wrap board endpoints in envelope to match mail shape

1 participant