feat: international daily-activity turn over the console agent channel - #52
Merged
maiphucgiang merged 7 commits intoOct 4, 2026
Merged
Conversation
WorkBuddy's international daily-activity reward is not granted for a /v2/chat/completions call: that endpoint is a bare inference path with no product-side usage record. It is granted for real client use, so this drives the same console conversation the web app uses, over ACP. app/acp_client.py is the only long-lived connection in the project: one SSE GET carrying the Acp-Connection-Id, plus one short JSON-RPC POST per method. It uses http.client rather than httpx on purpose, so the channel can stay open while console polling proceeds on separate connections and a read can never block the caller. Client capabilities are declared off deliberately — advertising fs or terminal support would make the sandbox wait for callbacks it will never get. app/daily_chat.py runs one turn per account per local day under a durable reservation. The commit point sits immediately before the prompt POST, so a failure that spent nothing (an unready sandbox, a channel that never connected) stays retryable while a sent-but-unconfirmed turn is never replayed. Cost is read from the protocol-native usage_update rather than inferred from a balance delta. The switch is opt-in and defaults off for international accounts, matching check-in and travel: a turn can spend credits on the account.
Five fixes found while verifying the feature against live accounts: - An AcpError escaping perform() left the reservation at 'reserved', and the next call treats any phase other than 'cancelled' as an unretryable 'pending'. A channel that never delivered the prompt therefore cost the whole day even though it spent nothing. One cancel_unless_sent() helper now serves every failure branch; only a record already 'sent' is preserved. - The day key came from time.strftime, which follows the process timezone. Where that is UTC the once-per-day guard turned over at 08:00 Beijing, so a Beijing day could be visited twice and a restart near UTC midnight could skip one. One today() helper now serves the sweep, the manual action and the inventory view. - The sweep fired every opted-in account in the same minute. due_at() derives a stable per-account slot inside a six-hour window from identity+day, so the accounts never go out together and the order still changes daily. A hash beats a random offset, which would move on every sweep and could starve an account. - prune_daily_chats was defined and tested but never called, so the table grew one row per account per day forever. - The sandbox is provisioned asynchronously and answers with a link before it is ready, so a single read raced provisioning. It is now polled briefly, and the conversation id is recorded before the sandbox is used so a later failure can be told apart from one that never reached the agent.
The daily guards (check-in, travel, daily-activity) all key on a local calendar day, but a container without TZ runs on UTC, so those guards turn over at 08:00 Beijing rather than at local midnight — a local day can be visited twice and a restart near UTC midnight can skip one. The default stays UTC, so an existing deployment is unchanged; operators in another zone set TZ once.
Reviewer's GuideAdds an opt-in international WorkBuddy daily-activity turn that reproduces a console agent session over a bounded ACP channel, persists a conservative account/day state machine to prevent double-spending, schedules and surfaces the task through maintenance and the admin UI, and verifies the safety-critical paths with offline tests. Sequence diagram for the international daily activity turnsequenceDiagram
participant Sweep as MaintenanceSweep
participant DailyChat as daily_chat.perform
participant Store as ControlStore
participant Console as WorkBuddyConsole
participant ACP as AcpChannel
Sweep->>DailyChat: perform(token, profile, ...)
DailyChat->>Store: daily_chat_record(identity, day)
Store-->>DailyChat: available or retryable reservation
DailyChat->>Store: reserve_daily_chat(identity, day)
DailyChat->>Console: create_conversation()
Console-->>DailyChat: conversation_id
DailyChat->>Store: daily_chat_checkpoint(conversation_id)
loop sandbox provisioning
DailyChat->>Console: sandbox_of(conversation_id)
Console-->>DailyChat: link, token, session_id
end
DailyChat->>ACP: run_turn(link, token, session_id, cwd, prompt)
ACP->>ACP: post(initialize)
ACP->>ACP: post(session/load)
DailyChat->>Store: transition_daily_chat(..., sent)
ACP->>ACP: post(session/prompt)
loop bounded completion polling
ACP-->>DailyChat: drain() usage_update
DailyChat->>Console: status_of(conversation_id)
Console-->>DailyChat: completed
end
DailyChat->>Store: transition_daily_chat(..., confirmed)
State diagram for the daily chat reservation lifecyclestateDiagram-v2
[*] --> reserved: reserve_daily_chat
reserved --> cancelled: unsent failure
reserved --> sent: transition_daily_chat
sent --> confirmed: completed console status
sent --> sent: unconfirmed failure
confirmed --> reconciled: reconcile
cancelled --> reserved: retry same day
sent --> [*]: no automatic replay
confirmed --> [*]
reconciled --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 security issue, and 1 other issue
Security issues:
- Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/control_store.py" line_range="26-31" />
<code_context>
return value
+def _day(value, label="日期"):
+ """Accept only a YYYY-MM-DD local day; never a timestamp or free text."""
+ if (not isinstance(value, str) or len(value) != 10 or value[4] != "-" or value[7] != "-"
+ or not value.replace("-", "").isdigit()):
+ raise ValueError(f"{label} 无效")
+ return value
+
+
</code_context>
<issue_to_address>
**nitpick (bug_risk):** `_day` accepts impossible calendar dates such as `2026-99-99` and `2026-02-31`; those values can be stored as daily-chat keys and later treated as distinct valid days.
**Triggers:** When a caller passes a syntactically valid but calendar-invalid day to a ControlStore daily-chat method.
**Suggested fix:** Parse the value with `datetime.date.fromisoformat` and reject values that are not real calendar dates.
```suggestion
def _day(value, label="日期"):
"""Accept only a YYYY-MM-DD local day; never a timestamp or free text."""
import datetime
if (not isinstance(value, str) or len(value) != 10 or value[4] != "-" or value[7] != "-"
or not value.replace("-", "").isdigit()):
raise ValueError(f"{label} 无效")
try:
datetime.date.fromisoformat(value)
except ValueError:
raise ValueError(f"{label} 无效") from None
return value
```
</issue_to_address>
### Comment 2
<location path="app/control_store.py" line_range="509-514" />
<code_context>
updated = self._db.execute(
"UPDATE daily_chats SET phase=CASE WHEN phase='reconciled' AND ?='confirmed' THEN phase ELSE ? END, "
"confirmed_at=CASE WHEN ? IN ('confirmed','reconciled') THEN COALESCE(confirmed_at,?) ELSE confirmed_at END, "
"updated_at=? WHERE account_key=? AND day=? AND attempt_id=? AND phase IN ("
+ ",".join("?" for _ in expected) + ")",
(phase, phase, phase, time.time(), time.time(), identity, day, attempt, *expected))
</code_context>
<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
*Source: opengrep*
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this adds an opt-in automated console agent turn that creates an external conversation, sends a prompt, and may consume account credits; a sent turn or spent credits cannot be undone by reverting the change. The daily reservation bounds the impact to one attempt per account per day, so the harm is bounded but still requires human review.
Blocking findings: app/control_store.py:514
The daily-turn turn landed two defects that only surface when the image is built rather than merged: - The save notice was reworded from "保存不会立即领取" to "保存不会立即执行" because claiming a reward is not what opting into the turn does, but automation.test.tsx kept asserting the old wording and went red. - web/src/api.ts and web/src/pages/Credentials.tsx were never run through oxfmt, so the Dockerfile's `vp check` gate failed and blocked the build. The switch itself also had no coverage. The fixture now carries the daily-turn fields, and the test asserts the domestic account's switch is shown but disabled while the international one toggles, persists, and issues no POST.
…f the call Two reviewer notes on app/control_store.py: - _day checked shape and digits only, so 2026-99-99 and 2026-02-31 were accepted. A day that never happened would become a distinct daily-chats key, and the once-per-day guard could then be satisfied against a day that is not the real one. It now also parses with datetime.date.fromisoformat. - transition_daily_chat rebuilt its transition dict and its IN (...) fragment on every call, concatenating the fragment into the statement text. That was never an injection surface -- the interpolated characters are only "?" placeholders, and the phase names are bound as parameters -- but the composition is now done once at import time from literals, so the method interpolates nothing. test_day_must_be_a_plain_date grows the calendar-invalid cases, and was verified to fail on '2026-99-99' before the fix.
Author
|
Adds two follow-up fixes found while deploying this feature. Both only surface at image-build time rather than in CI on the feature branch. fix: make the frontend gate pass again after the daily-turn turn
The assertion is updated, the fixture carries the daily-turn fields, and the switch now has coverage: the domestic account's switch is rendered but disabled (the turn is international-only), while the international one toggles, persists, and issues no POST. fix: reject impossible day keys, and hoist the transition table out of the call
The same commit hoists the Verification
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
An automated daily-activity turn for international WorkBuddy (
intl-work)accounts.
WorkBuddy grants a daily activity reward (30 credits, settled next day) for real
client use. A gateway
/v2/chat/completionscall does not earn it — that is abare inference path with no product-side usage record. The web console drives an
agent session instead, so this reproduces that path: create a console
conversation, attach to its sandbox over ACP, drive one turn to completion.
Why it needs a dedicated path
app/acp_client.py(227 lines) is the only long-lived connection in the project:one SSE
GETthat yields theAcp-Connection-Id, plus one short JSON-RPCPOSTper method. Two deliberate choices worth reviewing:
http.client, nothttpx. The channel has to stay open for the whole turnwhile console polling proceeds on separate connections, and its bytes are
drained opportunistically so a read can never block the caller.
fsread/writefalse,terminalfalse). Advertising them would make the sandbox wait for callbacksit will never receive, and the session would never finish. This is invisible
from the outside, so it is worth a comment rather than a guess.
app/daily_chat.py(410 lines) is the orchestration: one turn per account perlocal day under a durable reservation.
Safety properties
spent nothing — an unready sandbox, a channel that never connected — stays
retryable for the rest of the day; a sent-but-unconfirmed turn is never
replayed, because replaying it could spend credits twice.
auto_checkin/auto_travel: a turn canspend credits on the account.
spec returns a
stopReasonon the prompt response, but this implementationanswers the HTTP call before the turn ends, so protocol state is not business
state.
usage_update,not from a balance delta.
Verified live
Three accounts ran a turn; all three were credited +30 the next day (official
CreateTimeon the reward segment, 04:49 / 04:49 / 05:07 the following morning).These were the first
code_007segments those accounts had ever received, so thegrant cannot be confused with pre-existing credit. Measured cost per turn:
0.0.Tests
tests/test_daily_chat.py— 47 offline tests, no upstream calls. They pin thesafety properties rather than the happy path: one turn per day, no replay of an
unconfirmed send, an unsent day stays retryable, no upstream text or credential
path escapes into a result, and the ACP client declares no capabilities.
Full suite after these changes: 1434 passed, 11 failed. Those 11 are
test_sqlite_state.py/test_terminal_output.pyPTY/TTY tests that failidentically on unmodified
mainin a container without a controlling terminal —they are environmental, not related to this change.
Five defects fixed while verifying
Each has a regression test:
AcpErrorstranded the day. It bypassed the reservation cancel, so therecord stayed
reservedwhile the next call treats anything other thancancelledas unretryablepending— a channel that never delivered theprompt still cost the whole day. One
cancel_unless_sent()helper now servesevery failure branch.
once-per-day guard turned over at 08:00 Beijing, so a local day could be
visited twice and a restart near UTC midnight could skip one. One
today()helper now serves the sweep, the manual action and the inventory view.
due_at()derives a stableper-account slot inside a six-hour window from
identity + day. A hash beats arandom offset, which would move on every sweep and could starve an account.
prune_daily_chatswas never called, so the table grew one row per accountper day forever.
is ready, so a single read raced provisioning. It is now polled briefly, and
the conversation id is recorded before the sandbox is used so a later failure
can be told apart from one that never reached the agent.
Notes for reviewers
http.clientandselectare stdlib.intl-cli,cn-cliandcn-workare explicitly unsupported and returnunsupportedwithout touching the network.checkin-activity-statusendpoint still reportsactive: falseafter the rewardhas been paid, so it cannot be used as a success signal. The reward segment
appearing in
get-user-resourceis the only trustworthy check.Summary by Sourcery
Implement an opt-in international WorkBuddy daily activity turn that runs through the console agent channel and tracks each account’s outcome safely.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: