Skip to content

fix(cron): stamp + replay creator sender/role across cron fires#1129

Open
mozaa-solana wants to merge 1 commit intonextlevelbuilder:devfrom
mozaa-solana:fix/cron-creator-sender-propagation
Open

fix(cron): stamp + replay creator sender/role across cron fires#1129
mozaa-solana wants to merge 1 commit intonextlevelbuilder:devfrom
mozaa-solana:fix/cron-creator-sender-propagation

Conversation

@mozaa-solana
Copy link
Copy Markdown

Summary

Cron jobs created in a group context (UserID like `group:telegram:-100…`) consistently fail group-scope permission checks the moment they fire — the scheduled agent ends up with an empty SenderID, and any `write_file` or `cron` mutation inside that turn trips:

permission denied: system context cannot ... in group chats. If this is a legitimate user action, ensure the acting sender is preserved through the tool chain.

Root cause: `gateway_cron.go` built the RunRequest with `UserID = job.UserID` but no `SenderID`. `loop_context.injectContext` skips `WithSenderID` when `req.SenderID` is empty, so the agent's resumed ctx had no human attribution. `UserID` alone is the group scope, not a real user.

Fix

Capture the human's sender + role at cron-create time; replay them at every fire.

  • `store.CronPayload`: new `CreatorSenderID` + `CreatorRole` fields (JSON-serialized into the existing `payload` column — no DB migration).
  • `CronStore.AddJob`: signature gains `creatorSenderID` + `creatorRole`. PG + sqlite impls thread them into the new payload fields.
  • `internal/tools/cron.go` `handleAdd`: reads `SenderIDFromContext` + `RoleFromContext`, drops synthetic prefixes (`subagent:`, `teammate:`, `ticker:`, `system:`, `notification:`, `session_send_tool`) so we never attribute future fires to a system component, then calls `AddJob` with the captured creator.
  • `cmd/gateway_cron.go`: passes `job.Payload.CreatorSenderID` + `CreatorRole` into `RunRequest.SenderID` + `Role` each fire. Empty values preserve prior behaviour — DM crons, system-installed crons, and pre-fix crons all keep the deny-on-empty guard at fire time. No security relaxation.
  • `gateway/methods/cron.go` (WS RPC): explicitly passes empty creator strings. There is no separate "Telegram numeric sender" upstream of the WS handler, so WS-installed crons that need to fire in group chats should be created via a Telegram-rooted flow (ctx-aware) instead.

Failure repro

  1. Human pings agent in a group → `@bot create cron neo-daily for the content cycle…`
  2. Agent calls `cron(action="add", ...)` → job stored with `UserID="group:..."`, no creator sender
  3. Cron fires → agent resumes, calls `write_file` to log cycle output
  4. Returns: `permission denied: system context cannot write files in group chats`

After this fix the same flow:
1-3. As above; job now stores `Payload.CreatorSenderID = ""`
4. Cron fires with `req.SenderID = ""` → `write_file` succeeds with attributed audit trail.

Test plan

  • `go build ./...` clean
  • `go test ./internal/store/... ./internal/tools/ ./internal/gateway/methods/ ./cmd/` passes (existing + new)
  • New tests:
    • `TestCronTool_handleAdd_PropagatesRealSender` — Telegram-rooted human dispatch survives roundtrip into `Payload.CreatorSenderID`.
    • `TestCronTool_handleAdd_DropsSyntheticSender` (table-driven, 6 synthetic prefixes) — synthetic ctx senders are deliberately dropped, never attributed.

Related

Companion fix to #1128 (team-task announce sender propagation). Together they close the "system context cannot write" denial across both re-ingress paths a Lead-style agent uses inside a daily-cycle workflow.

Cron jobs created in a group context (group-scope UserID like
"group:telegram:-100…") consistently fail group-scope permission
checks once they fire — the scheduled agent's turn ends up with an
empty SenderID, and any write_file / cron mutation inside that turn
trips:

    permission denied: system context cannot ... in group chats.
    If this is a legitimate user action, ensure the acting sender is
    preserved through the tool chain.

Root cause: gateway_cron.go built RunRequest with UserID=job.UserID
but no SenderID, so loop_context.injectContext skipped WithSenderID
and the agent ran with no human attribution. UserID alone is the
group scope, not a real user.

Fix: capture the human's sender + role at cron-create time, replay
them at every fire.

  - store.CronPayload: new CreatorSenderID + CreatorRole fields
    (JSON-serialized into existing payload column — no migration).
  - CronStore.AddJob: signature gains creatorSenderID + creatorRole;
    PG + sqlite impls thread them into the new payload fields.
  - tools/cron.go handleAdd: read SenderIDFromContext +
    RoleFromContext, drop synthetic prefixes (subagent:, teammate:,
    ticker:, system:, notification:, session_send_tool) so we never
    attribute future fires to a system component, then call AddJob
    with the captured creator.
  - cmd/gateway_cron.go: pass job.Payload.CreatorSenderID +
    CreatorRole into RunRequest.SenderID + Role each fire. Empty
    values preserve prior behaviour (DM crons, system-installed
    crons, pre-fix crons all keep the deny-on-empty guard at fire
    time — no security relaxation).
  - gateway/methods/cron.go (WS RPC): explicitly passes empty
    creator strings — there is no separate "Telegram numeric sender"
    upstream of the WS handler, so WS-installed crons that need to
    fire in group chats should be created via a Telegram-rooted
    flow (ctx-aware) instead.

Tests:
  - internal/tools/cron_creator_propagation_test.go: real-sender
    propagation + synthetic-sender drop coverage (table-driven).
  - internal/gateway/methods/cron_test.go: stub updated to match
    new AddJob signature, captures creator strings into Payload.

This is the cron-firing counterpart to the team-task announce
attribution fix — together they close the "system context cannot
write" denial across both re-ingress paths a Lead-style agent uses.
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.

1 participant