Skip to content

feat: route thread agents through instances - #40

Merged
rowan-stein merged 5 commits into
mainfrom
noa/issue-161-threads-agent-instances
Jul 16, 2026
Merged

feat: route thread agents through instances#40
rowan-stein merged 5 commits into
mainfrom
noa/issue-161-threads-agent-instances

Conversation

@casey-brooks

Copy link
Copy Markdown
Contributor

Summary

  • Rewrites agent-class thread participants to fresh agent instances through Agents CreateInstance and persists instance IDs.
  • Resolves existing agent_instance participants through Agents GetInstance for class-scoped can_initiate checks.
  • Sends message_recipients only for user/app-style recipients and fans out agent-instance recipients through Agents FanoutInboxItem.
  • Rejects passive participant requests and keeps initiator participants active.
  • Includes nickname instance suffix rendering in organization thread responses.

Closes #161

Validation

  • PATH=/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • go test ./... - passed: 3 packages passed, 6 packages had no test files
  • go vet ./... - passed with no errors
  • go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors
  • devspace run test:e2e - blocked before tests: DevSpace rejects command name test:e2e with regex ^(([a-z0-9][a-z0-9\-]*[a-z0-9])|([a-z0-9]))$
  • devspace run-pipeline test:e2e - blocked before tests: no valid kube config for context incluster
  • go test -tags e2e ./test/e2e - compiled and ran, failed dialing default threads:50051 with context deadline exceeded because no local Threads service is running

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Validation Summary

  • PATH=/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • go test ./... - passed: 3 packages passed, 6 packages had no test files
  • go vet ./... - passed with no errors
  • go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors
  • devspace run test:e2e - blocked before tests: DevSpace rejects command name test:e2e with regex ^(([a-z0-9][a-z0-9\-]*[a-z0-9])|([a-z0-9]))$
  • devspace run-pipeline test:e2e - blocked before tests: no valid kube config for context incluster
  • go test -tags e2e ./test/e2e - compiled and ran, failed dialing default threads:50051 with context deadline exceeded because no local Threads service is running

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the update. I found two blocking correctness issues: agent fanout is not recoverable/atomic with message persistence, and missing identity-type results are silently downgraded to UNSPECIFIED. Please address these before merge.

Comment thread internal/server/server.go Outdated
Comment thread internal/server/server.go Outdated
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Review update

Addressed Noa's two blocking comments:

  • Added a durable agent_inbox_deliveries outbox table written in the same DB transaction as message persistence, user message_recipients, and message_count update.
  • Added synchronous best-effort drain for newly-created outbox rows plus a startup/background retry worker that drains pending rows and uses Agents FanoutInboxItem idempotency before marking deliveries delivered.
  • FanoutInboxItem failures now leave durable pending rows and record attempts/last_error instead of making message persistence unrecoverable.
  • BatchGetIdentityTypes responses now reject missing, duplicate, wrong-count, nil, invalid, or unexpected entries instead of silently treating missing entries as UNSPECIFIED.

Validation Summary

  • PATH=/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • go test ./... - passed: 3 packages passed, 6 packages had no test files
  • go vet ./... - passed with no errors
  • go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors
  • devspace run test:e2e - blocked before tests: DevSpace rejects command name test:e2e with regex ^(([a-z0-9][a-z0-9\-]*[a-z0-9])|([a-z0-9]))$
  • devspace run-pipeline test:e2e - blocked before tests: no valid kube config for context incluster
  • go test -tags e2e ./test/e2e - compiled and ran, failed dialing default threads:50051 with context deadline exceeded because no local Threads service is running

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the updates. The previous two issues are addressed and I resolved those threads, but the new durable outbox path has two blocking retry/concurrency issues that need fixes before merge.

Comment thread internal/server/server.go
Comment thread internal/store/agent_inbox_deliveries.go Outdated
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Review update

Addressed Noa's two outbox retry/concurrency blockers:

  • Replaced pending reads with atomic claim/update using FOR UPDATE SKIP LOCKED, claimed_at, and claim_id ownership.
  • Mark-delivered/mark-failed now require the matching claim_id, preventing concurrent workers from completing the same row.
  • Delivery drain now processes every claimed row and returns joined errors after later rows are attempted, so one failed fanout does not starve subsequent deliveries in the same batch.
  • Failed deliveries now set next_attempt_at, so immediately retryable failed rows do not monopolize future claim batches ahead of later pending rows.
  • Added server tests for continuing after a failed delivery and store tests covering atomic claim SQL, claim ownership, and deferred retries.

Validation Summary

  • PATH=/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • go test ./... - passed: 3 packages passed, 6 packages had no test files
  • go vet ./... - passed with no errors
  • go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors
  • devspace run test:e2e - blocked before tests: DevSpace rejects command name test:e2e with regex ^(([a-z0-9][a-z0-9\-]*[a-z0-9])|([a-z0-9]))$
  • devspace run-pipeline test:e2e - blocked before tests: no valid kube config for context incluster
  • go test -tags e2e ./test/e2e - compiled and ran, failed dialing default threads:50051 with context deadline exceeded because no local Threads service is running

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the follow-up. The prior starvation/concurrent-claim threads are addressed and I resolved them, but there is one remaining blocking issue: the immediate send-path delivery now bypasses claim ownership and can fan out before failing to mark delivered, causing duplicate later delivery.

Comment thread internal/server/server.go
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Review update

Addressed the remaining immediate send-path blocker:

  • The immediate send path now drains via DrainPendingAgentInboxDeliveries, which claims pending outbox rows before fanout and uses the same claim_id ownership semantics as the retry worker.
  • Tightened TestSendMessageFansOutAgentInstancesOnly so it proves send-path fanout uses the claimed outbox row, not the unclaimed SendMessageResult row, and marks delivered with the matching claim_id.
  • Installed missing local tooling (gcc, devspace) through Nix to run validation instead of skipping.

Validation Summary

  • PATH=/root/.nix-profile/bin:/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • PATH=/root/.nix-profile/bin:$PATH go test ./... - passed: 3 packages passed, 6 packages had no test files
  • PATH=/root/.nix-profile/bin:$PATH go vet ./... - passed with no errors
  • PATH=/root/.nix-profile/bin:$PATH go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors
  • PATH=/root/.nix-profile/bin:$PATH devspace run test:e2e - blocked before tests: DevSpace rejects command name test:e2e with regex ^(([a-z0-9][a-z0-9\-]*[a-z0-9])|([a-z0-9]))$
  • PATH=/root/.nix-profile/bin:$PATH devspace run-pipeline test:e2e - blocked before tests: no valid kube config for context incluster
  • PATH=/root/.nix-profile/bin:$PATH go test -tags e2e ./test/e2e - compiled and ran, failed dialing default threads:50051 with context deadline exceeded because no local Threads service is running

@casey-brooks

Copy link
Copy Markdown
Contributor Author

CI reliability update

Replaced bufbuild/buf-setup-action@v1 in this repo's CI with a minimal Go-toolchain install:

go install github.com/bufbuild/buf/cmd/buf@v1.66.0

This keeps Buf pinned to 1.66.0 while avoiding the setup action's GitHub release-resolution path that was repeatedly returning the Unicorn HTML page before tests started.

Validation Summary

  • PATH=/root/.nix-profile/bin:/root/go/bin:$PATH go install github.com/bufbuild/buf/cmd/buf@v1.66.0 - passed
  • PATH=/root/.nix-profile/bin:/root/go/bin:$PATH buf --version - passed: 1.66.0
  • PATH=/root/.nix-profile/bin:/root/go/bin:$PATH buf generate buf.build/agynio/api --path agynio/api/threads/v1 --path agynio/api/notifications/v1 --path agynio/api/identity/v1 --path agynio/api/metering/v1 --path agynio/api/agents/v1 --path agynio/api/authorization/v1 - passed
  • PATH=/root/.nix-profile/bin:$PATH go test ./... - passed: 3 packages passed, 6 packages had no test files
  • PATH=/root/.nix-profile/bin:$PATH go vet ./... - passed with no errors
  • PATH=/root/.nix-profile/bin:$PATH go build ./... - passed with no errors
  • git diff --check - passed with no whitespace errors

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Final re-review complete. The immediate send-path now drains claimed outbox rows instead of directly delivering unclaimed inserted rows, and the claimed-row test covers that behavior. Prior threads are resolved. Approved.

@rowan-stein
rowan-stein merged commit 6e52231 into main Jul 16, 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.

3 participants