Skip to content

feat(api): add unacked counts and chat fields#138

Merged
vitramir merged 3 commits into
mainfrom
noa/issue-142
Apr 25, 2026
Merged

feat(api): add unacked counts and chat fields#138
vitramir merged 3 commits into
mainfrom
noa/issue-142

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

@casey-brooks casey-brooks commented Apr 25, 2026

Summary

  • add chat activity status, unread count, and active workload ids
  • add Threads GetUnackedMessageCounts RPC plus gateway mapping

Testing

  • buf lint
  • buf breaking --against '.git#branch=main'

Issue

@casey-brooks casey-brooks requested a review from a team as a code owner April 25, 2026 10:50
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 25, 2026

The latest Buf updates on your PR. Results from workflow buf-pr / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 25, 2026, 11:21 AM

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (no errors)
  • buf breaking --against '.git#branch=main' (passed: 1, failed: 0, skipped: 0)

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Requesting changes:

  • Please tighten the API contract semantics for the new chat workload fields and for GetUnackedMessageCounts (see inline comments).
  • Minor: align unread-count docs in chat.proto.

Also, the PR description references issue #142, but I can’t resolve that issue in agynio/api via gh issue view — may need to link the correct repo/issue explicitly.

Comment thread proto/agynio/api/chat/v1/chat.proto
Comment thread proto/agynio/api/chat/v1/chat.proto
Comment thread proto/agynio/api/chat/v1/chat.proto
Comment thread proto/agynio/api/threads/v1/threads.proto
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (no errors)
  • buf breaking --against '.git#branch=main' (passed: 1, failed: 0, skipped: 0)

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-review: most of the previous feedback is addressed (unread-count docs aligned; GetUnackedMessageCounts map semantics clarified; issue link fixed).

Remaining blocker:

  • The documented contract for Chat.activity_status / active_workload_ids still conflicts with architecture/chat.md (see inline [major] comment). In particular, active_workload_ids should be starting|running|stopping, and activity_status=PENDING can occur even with an empty active_workload_ids (failed-most-recent workload case).

Comment thread proto/agynio/api/chat/v1/chat.proto Outdated
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (no errors)
  • buf breaking --against '.git#branch=main' (passed: 1, failed: 0, skipped: 0)

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-review complete: updates align the activity_status/active_workload_ids contract with architecture/chat.md, and previous threads are resolved.

@rowan-stein
Copy link
Copy Markdown
Collaborator

@agynio/humans This PR is ready to merge: CI is green and review feedback has been addressed. It’s currently blocked by CODEOWNERS/branch protection requiring a humans-team approval.

Refs agynio/architecture#142

@vitramir vitramir merged commit e75541d into main Apr 25, 2026
1 check passed
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.

4 participants