[ccf-lisa]

Task: IMPROVEMENTS item 5(b), stack 5 PR 7 of 7: fx/imp5b-7-supervise-poll on fx/imp5a-object-store-design. Design: P3 (#223).

Built on #223's branch (it touches only AGT's files) and to be restacked on top of PR 6.

Summary

Implements P3 §Agents poll briefly (supervise), Decision 6, the Rollout's PR 7 and §Tests "Brief polling".

• agent.yaml gets a poll block (internal/agent/config): config.Poll{Wait, Interval string}, decoded strictly like the rest of the file.
  • wait: whole seconds from 0s to 60s, default 30s (filled only when poll.wait is absent). 500ms, 1.5s, 61s, -1s, a bare number and a quoted, unit-less "0" are refused.
  • interval: default 0s, never negative, with a unit.
  • wait: 0s needs an interval of at least 1s, or the agent would poll in a hot loop.
  • The config parses the strings (its own job: the duration syntax) and checks them with the worker's worker.Poll.Validate, reporting its *worker.PollError under the key (poll.wait: "61s" must be whole seconds from 0s to 60s). The rules and their bounds live only in the worker.
  • Poll.WaitDuration() / IntervalDuration() give the parsed values (the pattern of WorkspaceSize.MemoryGiB).
• The worker (internal/agent/worker): Options.LeaseWait is replaced by Options.Poll, a worker.Poll{Wait, Interval time.Duration}.
  • A zero Wait means zero, so wait=0s reaches the wire. There is no default inside the worker: worker.DefaultPoll() is the long poll ({Wait: 30s}), and New refuses the zero Poll as a hot loop. Poll.Validate holds the rules (whole-second wait from 0s to 60s, an interval that isn't negative, at least 1s of interval with a zero wait) and returns a typed *PollError{Field, Value, Reason}.
  • After a 204 the loop releases its slot and sleeps Interval on loopCtx, so a drain or an abort ends the sleep. A poll that gets a lease polls again at once, so free slots fill. Throttling, the failure backoff, heartbeats, completes and abandons are unchanged.
• supervise passes the block through (agentloop.go) and logs pollWait/pollInterval at start.
• deploy/poc/agent.yaml.example: the block is commented out, with a note that older agents refuse the key, so rolling an agent back means removing it. deploykit_test.go walks config.Poll too and checks that the example keeps the long poll.
• No dispatcher change. Checked against internal/server/agents.go:136-145: waitRE (^[0-9]{1,2}s$) accepts 0s. Scheduler.Lease (internal/scheduler/lease.go:31-58) with a zero wait makes one tryLease pass and returns 204 when nothing matches.
• Docs: supervise.md has a new §Polling, plus the config example, the validation bullet, the lease-loop pseudocode, the drain (the interval's sleep is cancelled too) and the loop's tests.

Verification

• make reviewable (with a golangci-lint built with go1.26, since the one on PATH is built with go1.25 and refuses the module's Go version): exit 0. Lint reports 0 issues, every package is ok, and openapi-lint scores 100.
• New tests:
  • internal/agent/config: TestParsePoll (defaults per key, a zero wait stays zero, the shortest interval for it, a minute wait, a sub-second interval with a long poll); TestParseDefaults (no block: the long poll); TestParseInvalid (wait over 60s, negative, 500ms, 1.5s, not a duration, a bare number; a negative interval and one that isn't a duration; wait: 0s with no interval or with 500ms; a unit-less "0" for either key; an unknown poll key).
  • internal/agent/worker:
    • TestNewValidates, now a table over Poll (the zero Poll, a sub-second interval with a zero wait, a wait over 60s, a negative wait, a wait not in whole seconds, a negative interval).
    • TestPollWait: wait=0s and wait=30s on the wire. The defaults poll again at once on an unmoved clock.Fake; with an interval, no poll comes before it ends.
    • TestBriefPollLeasePollsAgainAtOnce: a lease polls again at once, the next 204 sleeps, and a drain ends the sleep.
    • TestBriefPollAgainstRealDispatcher: factory serve --dev-fake-providers as a subprocess. Idle polls get 204 and come an interval apart. A task created by a webhook while the agent sleeps is leased on the next poll, and every poll sent wait=0s.
  • cmd/factory-agent: TestSupervisePoll (no block sends wait=30s; poll: {wait: 0s, interval: 1s} sends wait=0s with the polls at least 1s apart); TestDeployAgentExample.
• Fixed tests: every worker test that built Options (fake_test.go's harness, TestRegisterRefused, the mock, real-dispatcher and effector tests) now sets Poll. The fake dispatcher records each poll's wait and answers wait=0s at once.
• Flakiness: the new worker tests passed -race -count=8, TestSupervisePoll -count=5, and the real-dispatcher tests -count=3 three times over after the served.stop fix below.
• Review: ccf-review's Should-fix CORE-DUP-001 (the poll rules duplicated in config and worker, thread PRRT_kwDOVK0lgs6wxG1y) and its held Nit (a unit-less "0" accepted) are fixed in c5d2e85; make reviewable passes again (exit 0, 0 lint issues).
• Self-review (lisa, ccf-review rules) took two passes. Pass 1 found a should-fix and a nit, both fixed in d9b3490. Pass 2 found nothing.

No bucket is involved: nothing here needs a real cloud bucket. Whether brief polls keep Cloud Run's bill near zero can only be checked on a real deployment (IMPROVEMENTS item 9).

Ownership

AGT owns internal/agent/ and cmd/factory-agent. The spec also names two files outside that line, and this PR edits both: deploy/poc/agent.yaml.example (the run kit's example, which cmd/factory-agent/deploykit_test.go checks) and design/workstreams/agent/LLD/supervise.md (AGT's own LLD).

For triage

• The real-dispatcher tests could hit serve's shutdown timeout. Go's http.Server.Shutdown treats a connection that never carried a request as active for 5 s. The tests run serve with --shutdown-timeout 5s (internal/agent/worker/dispatcher_test.go's startServe, and cmd/factory/serve_app.go:404-422's drain). So a connection the default transport dialled but never used failed served.stop with drain requests: context deadline exceeded, about 1 run in 3 of the new test.
  • Fixed in this PR on the test side: served.stop closes the default transport's idle connections before SIGTERM, for every real-dispatcher test (AGT's file).
  • In production, the same shutdown waits up to 5 s for such a connection and then reports "shutdown incomplete". That is RT's to decide. A possible fix: serve's drain closes connections still in StateNew (via ConnState) once the listeners stop accepting.

Deviations from the spec

None in behaviour. Two choices the spec leaves open:

• poll.interval has no upper bound and no whole-second rule, since P3 sets neither. A long poll may carry an interval too: it then sleeps after each 204.
• The worker refuses the zero Poll instead of defaulting it. A zero wait has to mean zero, and the zero Poll would be a hot loop, so every caller passes DefaultPoll() or the config's settings.

🤖 Generated with Claude Code

[gusfcarvalho]

ccf-review: COMMENT

1 Should-fix.

Stack (base-chain): #223 → #237