Skip to content

feat(workloads): add agent state#49

Merged
rowan-stein merged 2 commits intomainfrom
noa/issue-48
Apr 30, 2026
Merged

feat(workloads): add agent state#49
rowan-stein merged 2 commits intomainfrom
noa/issue-48

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • add agent_state persistence and proto mapping
  • update touch/sweep logic with workload.updated notifications
  • reset last_activity_at on starting->running and add sweep config

Testing

  • go test ./...
  • go vet ./...

Closes #48

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • add agent_state persistence and proto mapping
  • update touch/sweep logic with workload.updated notifications
  • reset last_activity_at on starting->running and add sweep config

Tests

  • go test ./... (passed: all, failed: 0, skipped: 0)
  • go vet ./... (no issues)

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 — core behavior is close, but a couple gaps will break the activity-sweep semantics and acceptance criteria:

  1. starting→running must reset last_activity_at for all status update paths
  • Today the reset is only triggered via UpdateWorkload’s ResetLastActivity; UpdateWorkloadStatus can also promote to running but never resets.
  • This can cause the sweep to immediately mark agent_state=idle right after a workload starts.
  • Please centralize in updateWorkload (SQL CASE based on previous status) or compute/pass the reset from UpdateWorkloadStatus too.
  1. Tests: assert agent_state is surfaced in API responses
  • Acceptance criteria calls out GetWorkload + ListWorkloadsByThread include agent_state.
  • Please add assertions (and a GetWorkload success-path test) to lock down DB-string → proto-enum mapping.
  1. (Minor) Add a TouchWorkload test for the processing (no-event) path with NotificationsClient enabled.

Comment thread internal/server/workloads.go
Comment thread internal/server/workloads_test.go
Comment thread internal/server/workloads_test.go
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • reset last_activity_at when UpdateWorkloadStatus moves starting -> running
  • add GetWorkload/ListWorkloadsByThread agent_state assertions
  • add TouchWorkload no-publish coverage for processing state

Tests

  • go test ./... (passed: all, failed: 0, skipped: 0)
  • go vet ./... (no issues)

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: requested changes are addressed.

  • UpdateWorkloadStatus now resets last_activity_at on starting→running (consistent with UpdateWorkload), preventing the activity sweep from prematurely marking workloads idle.
  • Added assertions/tests to ensure agent_state is returned by ListWorkloadsByThread + GetWorkload (DB-string → proto-enum mapping covered).
  • Added TouchWorkload coverage to ensure no workload.updated publish when already processing.

@rowan-stein rowan-stein merged commit 41c762c into main Apr 30, 2026
2 checks 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.

Implement Workload.agent_state (processing/idle) + activity sweep + workload.updated emits

3 participants