Handle Office Manager Slack volunteering

[Light-code11]

Summary

• handle the signed Volunteer for today Block Kit action on Public Roo only
• derive the volunteer identity only from the verified Slack payload, never from button values or model-generated parameters
• persist each verified member/date click in Roo's existing durable SQLite volume before returning 200 OK
• lease actions to one worker, recover pending or abandoned work after restarts, and drain in-flight actions during graceful shutdown
• call the backend atomic, idempotent claim endpoint and return private feedback with a DM fallback
• avoid claiming that no booking exists when a backend or gateway failure leaves the result uncertain
• add the no-food reminder only to coworking-related confirmations and queued coworking retries
• keep Start Here introduction awards focused on welcoming new members, without coworking rules
• run the Office Manager, primary coworking message, queued retry, and Start Here regression suites in the required PR workflow

Member flow

Members click Volunteer for today in Roo's announcement in the Cowork and Chill channel; they do not need to reply in the channel or thread. Roo durably records the click, then processes the backend claim asynchronously. The winning member receives private confirmation, while the backend updates the original request and posts a separate channel announcement tagging the Office Manager.

The backend announcement tells members using the space to book themselves with @Roo book me in today. Roo does not give the Office Manager permission to book other people and does not expose a named attendance roster.

The no-food reminder is scoped to the coworking and Office Manager experience. It is not shown to members posting introductions in Start Here.

Backend contract

The companion backend PR produces the Slack button. Contract tests in mlai-backend#649 verify:

• action ID office_manager_volunteer_today
• button value.date is the Melbourne-local Office Manager date in YYYY-MM-DD form
• success statuses claimed and already_claimed_by_you
• rejection codes already_claimed, claim_closed, member_not_eligible, and office_manager_day_not_found
• points_refunded remains present in successful claim responses
• repeated member/date claims are backend-idempotent, which makes outbox recovery safe

Dependency and rollout

1. Deploy MLAI-AUS-Inc/mlai-backend#649 first with the Office Manager feature disabled.
2. Set backend OFFICE_MANAGER_SLACK_BOT_TOKEN to the Public Roo app's Bot User OAuth token. Do not use the Jobs or Admin Roo token.
3. Deploy this Roo action handler.
4. Configure the Cowork and Chill channel in the backend and Roo channel allowlist.
5. Enable the backend scheduler only after both services and the token/channel configuration are in place.

Validation

• focused Office Manager/coworking/Start Here/security tests: 78 passed
• exact required PR workflow command: 543 passed
• full Roo suite: 843 passed, 20 failed
• current main independently reproduces the exact same 20 unrelated failures: 809 passed, 20 failed
• Python compilation and git diff --check passed

Coordination

• Roo #209 is still open. A current trial merge conflicts in roo/clients/mlai_backend.py and roo/tests/test_mlai_backend_client.py; the executor and skill instructions auto-merge. Whichever PR lands second must retain both account-linking and Office Manager client methods/tests.
• Roo #225 is still open. A current trial merge has one content conflict in roo/skills/executor.py; main.py, the workflow, and backend client auto-merge. Whichever PR lands second should retain the shared Slack action task helper and both feature formatters.

Tracking

• Linear: MLA-1760
• Related public issue: Let members volunteer as Office Manager of the Day through Roo mlai-au#1472
• Backend companion: MLAI-AUS-Inc/mlai-backend#649

[Light-code11]

@drsamdonegan @mesieou this is ready for review. Please focus on Slack signature verification, deriving the volunteer identity only from the signed payload, duplicate-delivery handling, and private response/DM fallback behavior.

Required backend PR: https://github.com/MLAI-AUS-Inc/mlai-backend/pull/649

The backend must be deployed first with the feature disabled; the scheduler should only be enabled after this Roo handler and channel configuration are deployed.

[Light-code11]

Rebased this branch onto current main (bfbb036) and resolved the conflict in roo-standalone/roo/start_here_introductions.py. The resolution preserves main's updated “introducing yourself or your startup” wording and the Office Manager PR's no-food reminder.

Validation after the rebase:

• 51 passed across the focused Office Manager action, coworking intent, backend client, start-here, and Slack security tests
• 768 passed, 22 failed in the full Roo suite; the 22 failures are the existing current-main failures in content-factory test settings, jobs scheduler fixtures, and stale points/model expectations, with no new Office Manager failures
• git diff --check passes
• GitHub security-checks passes

The PR is now mergeable. Review requests remain assigned to @drsamdonegan and @mesieou. The backend companion PR #649 still needs to be deployed first with the feature disabled, as described in the rollout section.

[Light-code11]

@drsamdonegan @mesieou This PR has been rebased onto current main and is mergeable again.

The new self-booking reminder is generated by the backend companion PR #649, so this Roo branch did not need extra message-generation logic. I retested the signed action handler, verified Slack actor binding, duplicate delivery handling, private feedback fallback, backend client, coworking intents, onboarding, and Slack security: 52/52 focused tests passed. The full Roo suite remains identical to current main (847 passing and the same 22 unrelated existing failures).

Please review this together with backend PR #649.

[callumholt]

Reviewed this alongside #209 and #225. The security core is sound: the handler sits after signature verification and the context allowlist, the actor comes only from payload["user"]["id"], and the UUNTRUSTED value in the test fixture proves the button's own slack_user_id is ignored. Duplicate delivery is already covered by _is_duplicate_slack_request on the signed body, and the ephemeral→DM fallback handles both None and ok: false.

Five inline comments above, two medium. Two things that did not fit on a line:

Merge order. This PR and #209 both insert a new client method at the same anchor in clients/mlai_backend.py (right after book_coworking_many), and both rewrite _format_coworking_booking_success plus skills/mlai_points/SKILL.md. Each is clean against main individually, but whichever lands second will conflict — and the resolution needs a deliberate call on how the no-food reminder and the account-link nudge sit together in one message, rather than whatever falls out of the merge. mlai-backend#649 is still open, so there is time.

The no-food reminder in post_award_notification. Adding it to coworking confirmations and queued retries is on-topic. Adding it to the introductions award notification means someone who introduces themselves in #start-here is told about food rules in the coworking space, which reads as a non-sequitur to a brand-new member. The PR says this was agreed, so flagging rather than objecting.

Minor: the second "text": ( block in test_coworking_booking_intents.py is indented four spaces deeper than its enclosing dict.

[Light-code11]

Addressed all five inline review comments in a2773cb after rebasing onto current main:

• separated backend claim execution from defensive response rendering
• whitelisted only claimed and already_claimed_by_you as winner states
• rejected non-object button values without a 500
• rejected stale/invalid dates against Melbourne current date before the backend call
• moved synchronous Slack feedback calls to asyncio.to_thread and kept the action acknowledgement immediate

Validation:

• final Office Manager action tests: 20/20 passed
• focused Office Manager/client/coworking/start-here/Slack suite: 64/64 passed before the final additional idempotent-status test
• exact CI security gate: 494/494 passed
• adjacent points/coworking suite: 110 passed, with the same 3 failures reproduced on current main
• full suite: 862 passed, 20 known baseline failures, with no Office Manager failures
• compileall and git diff --check passed

I replied to and resolved each addressed review thread. The companion backend PR #649 remains required, and backend-first rollout with the feature disabled still applies.

[Light-code11]

Updated the no-food reminder scope following the product clarification:

• removed the reminder from post_award_notification, so Start Here introductions remain a straightforward welcome and Roo Points award
• retained the reminder in coworking confirmations, queued coworking retries, and the Office Manager flow in the Cowork and Chill channel
• added a regression test asserting the exact Start Here notification and ensuring it contains no food-rule copy

Validation on the updated head:

• focused feature/security tests: 66 passed
• exact security gate: 494 passed
• full suite: 864 passed with the same 20 unrelated baseline failures
• Python compilation and diff checks passed

[Light-code11]

Addressed the latest merge-blocking and coordination feedback across this PR and backend #649:

• added roo/tests/test_office_manager_actions.py to the explicit deploy.yml security test list; the exact workflow command now passes 516/516
• restricted the volunteer action to ROO_SURFACE == "public", with an allowlisted Admin Roo regression proving it is ignored there
• replaced bare fire-and-forget tasks with a shared strong-reference helper and a lifecycle test proving tasks remain retained until completion
• verified the backend is the button producer and added backend contract tests for the exact action ID, Melbourne-local date payload, success statuses, rejection codes, and refund field
• trial-merged with feat: add private Small and Big Meeting Room booking #225; main.py now auto-merges again and only the previously known skills/executor.py conflict remains

Additional validation: 71 focused Roo tests passed; full Roo suite is 866 passed with the same 20 baseline failures. Backend Office Manager module is 42 passed on SQLite (three PostgreSQL-only tests skipped), in addition to the previously passing PostgreSQL concurrency suite.

@callumholt could you please take another look at the updated Roo and backend contract commits?

[Light-code11]

Addressed the adversarial review findings in 53f9208:

• verified volunteer clicks are now persisted before Slack receives 200 OK
• added single-worker leases, restart recovery, owner-fenced completion, and graceful shutdown draining
• changed uncertain backend/gateway failures so Roo no longer promises that no booking exists
• added direct CI assertions for self, admin, and batch coworking no-food reminders
• added the queued coworking and Start Here suites to the explicit workflow gate
• documented the required Public Roo bot token and current MLA-1758: Add private Roo account-linking flow #209/feat: add private Small and Big Meeting Room booking #225 merge coordination

Validation: 78 focused tests passed, 543 exact CI tests passed, and the full suite is 843 passed with the same 20 failures reproduced on current main.

@callumholt could you please review the durability and rollout updates?

[callumholt]

PR review

I reviewed the signed Slack action path, actor/date derivation, private feedback, durable outbox and lease lifecycle, coworking copy changes, backend client contract, workflow coverage, and companion backend PR at commit 53f920834594650e361fa45a1f1d082f8827a5d6.

Blocking finding

The Slack receipt dedupe can permanently discard a click before it reaches the new outbox.

verify_and_claim_slack_request records the signed request receipt before slack_actions runs. The handler then returns early for _is_duplicate_slack_request(request) before it parses or persists an Office Manager action. This leaves two loss paths:

1. Roo crashes after the generic receipt is committed but before record_action commits. Slack retries the same signed request, Roo sees it as a duplicate, returns 200, and never creates the outbox row.
2. record_action raises (for example, a temporary SQLite/disk error). The handler catches it and still returns 200, so Slack has no reason to retry and the click is lost.

That contradicts the stated “persist each verified click before returning 200” durability guarantee. The happy-path duplicate test does not cover either failure mode.

Handle this action before the generic duplicate early-return, while still relying on the outbox idempotency key to deduplicate processing. If persistence fails for a valid action, return a non-2xx response so Slack can retry. Please add a regression where the first persistence attempt fails and the exact duplicate delivery succeeds and schedules one durable action, plus a case where the signed receipt already exists but the feature outbox row does not.

Other review results

• The verified Slack payload remains the only source of member identity; button-provided identity is ignored.
• Public-surface, context allowlist, Melbourne-current-date, backend status whitelist, and ephemeral-to-DM privacy handling are sound.
• Backend claim identity is stable and idempotent, and lease owner fencing/restart recovery are otherwise correctly structured.
• Focused Office Manager, coworking-intent, backend-client, Start Here, and Slack-security suites: 78 passed.
• GitHub security-checks: passed (543 tests).
• Python compilation and git diff --check: passed.
• I inspected backend PR #649 and its migration/API contract read-only; I did not run or apply any database migration.

Dependencies and coordination

Backend PR #649 remains open and must be migrated/deployed first with the feature disabled. This branch also has documented overlap with open Roo PRs #209 and #225; whichever lands later must resolve those conflicts deliberately and retain both feature contracts.

Conclusion: Not safe or ready to merge yet. Fix the receipt-to-outbox loss window, then coordinate the overlapping Roo changes and only roll out after backend #649 is deployed and verified.

[Light-code11]

Addressed Callum's receipt-to-outbox blocker in 2522862 after rebasing onto current main.

Fix

• Office Manager actions are now identified before the generic signed-request duplicate early return.
• The durable Office Manager outbox atomically distinguishes a new/replayable click from an exact duplicate, so concurrent retries create one row and schedule one worker.
• A generic receipt with no Office Manager outbox row now creates the missing durable action.
• A valid click whose outbox write fails returns 503 instead of 200, allowing Slack to retry.
• Existing pending, processing, and completed outbox rows remain idempotent on exact delivery replay.

Fault-injection coverage

• first outbox write fails, exact signed retry succeeds and processes once
• generic signed receipt exists while the feature outbox is absent
• outbox commit succeeds but its result is lost; exact retry is acknowledged and restart recovery completes once
• concurrent exact retries create one outbox row and schedule one worker
• replay before and after completion does not duplicate processing

Validation

• final Office Manager suite: 36/36 (the preceding 35-test version was also repeated five times to exercise concurrency stability)
• focused Office Manager, Slack security, and surface security: 64/64
• exact current deploy.yml CI test list: 547/547
• GitHub security-checks, including Compose validation: passed
• compileall and git diff --check: passed
• companion backend #649 rechecked: mergeable, all four checks green, and action ID/date/result-code/refund/auth contracts still match

The local adversarial review skill was also strengthened with a mandatory multi-store handoff matrix and fault-injection gate; that personal skill change was not added to this repository.

@callumholt could you please re-review the receipt/outbox handoff and retry coverage?

[Light-code11]

Adversarial hardening update (ce25776):

• Backend transport and retryable HTTP failures now return the durable action to pending with capped exponential backoff instead of marking it complete.
• Private feedback must reach either Slack ephemeral delivery or DM before completion; failure of both paths is retried.
• Recovery workers lease one action immediately before processing, and completed outbox rows are retained for 90 days then pruned.
• Public Roo now has a default-off kill switch: OFFICE_MANAGER_ACTIONS_ENABLED=false.
• The persistence-failure regression now models a fresh human click; this flow does not depend on Slack automatically redelivering block_actions after a 503.
• Commit-uncertain replay, cancellation, restart, expired-lease, concurrent-worker, feedback-failure, retention, and kill-switch cases are covered.

Fresh adversarial state-machine review: no remaining blocker found in the reported paths. Source of truth remains the backend assignment; Roo completes only after observing a terminal backend result and delivering private feedback. Residual deployment constraint: local SQLite assumes a single shared Roo action-store volume/instance.

Validation: 210 required Roo workflow tests passed; Python compilation and diff checks passed.

Rollout order: deploy mlai-backend PR #649 first, deploy this PR with the switch off, then set OFFICE_MANAGER_ACTIONS_ENABLED=true on Public Roo only.

[callumholt]

PR re-review

I reviewed the changes since the prior review, from 53f920834594650e361fa45a1f1d082f8827a5d6 through the latest head ce25776e3ca51661fec880b64b624c5a68ed3e57, including the receipt/outbox handoff, commit-uncertain recovery, feedback delivery, leases, retention, and kill switch.

Prior blocker: resolved

Office Manager actions are now identified before the generic signed-receipt duplicate return. A missing feature outbox row is created even when the generic receipt already exists, persistence failure returns 503, concurrent exact deliveries create one durable row, and a commit-uncertain outbox write is recovered by the retry worker. This closes the previously reported loss window.

New blocking finding

Transient backend failures repeatedly notify the same volunteer on every retry, indefinitely.

For a retryable HTTP/transport failure, _claim_office_manager_from_action sends “still confirming … will retry automatically” and then raises so the outbox returns to pending. The retry schedule reaches a five-minute cap but has no terminal attempt limit. Every subsequent retry re-enters the same branch and sends the same ephemeral/DM feedback again. During a sustained outage this produces messages after roughly 5s, 10s, 20s, 40s, 80s, 160s, then every five minutes without bound.

Persist a notification state/timestamp with the outbox and send the uncertainty notice only on the first uncertain attempt (or at a deliberately bounded cadence), while retries after that remain silent until a terminal success/failure message can be delivered. Add a clock-driven regression proving repeated transient failures do not create repeated Slack feedback. The default-off OFFICE_MANAGER_ACTIONS_ENABLED switch limits current exposure but does not make the enabled workflow ready.

Other review results

• Backend outcomes and private feedback are now required before an action is marked complete.
• Retryable versus terminal backend results, cancellation release, one-at-a-time leasing, 90-day completed-row retention, and the default-off switch are otherwise sound.
• Latest Office Manager suite: 44 passed, 0 failed.
• Coworking, backend-client, Start Here, and Slack-security suites: 53 passed, 0 failed (97 focused tests total).
• GitHub security-checks: passed at the latest head.
• Python compilation and git diff --check: passed.
• Backend PR #649 remains open; its newest checks are still running. I did not run or apply any database migration.

Conclusion: Not ready to merge as a completed feature yet. The original receipt-to-outbox loss is fixed, but retry feedback must be deduplicated or bounded to prevent indefinite Slack notification spam. If the infrastructure is landed before that fix, OFFICE_MANAGER_ACTIONS_ENABLED must remain false; backend #649 must also be green, migrated with explicit approval, and deployed before enablement.

[Light-code11]

@callumholt Addressed the repeated transient-notification blocker in 95a9732.

Fix

• Added durable uncertainty_notice_attempted_at state to the Office Manager outbox, including an in-place SQLite schema upgrade for existing stores.
• A leased worker atomically claims that one notification attempt before calling Slack. Later transient retries remain silent, including after process restart and after the five-minute backoff cap is reached.
• Marking before Slack delivery closes the send-then-crash duplicate window. If the optional status notice itself fails, Roo does not spam retries; the backend claim continues retrying and the terminal success/rejection message is still required before completion.
• Notice claiming is lease-owner fenced, so a stale worker cannot consume the notification state after replacement.

Fault-injection coverage

• four clock-driven transient backend failures at 5/10/20/40-second backoffs produce exactly one notice
• reconstruction of the action store between attempts preserves the one-notice invariant
• a failed uncertainty-notice delivery is attempted once and not repeated
• cancellation during that delivery is not masked
• existing outbox databases gain the new column without losing pending work
• transient failure followed by already_claimed_by_you still delivers the final truthful result and completes

Validation

• Office Manager action suite: 47 passed
• exact required Roo workflow test list: 213 passed
• Python compilation and git diff --check: passed
• fresh adversarial state-machine review found no remaining blocker in this notification path
• companion backend #649 remains mergeable with all checks green

Could you please re-review the one-time notice state and clock-driven retry coverage?

[callumholt]

PR re-review

I reviewed the changes since my last review, from ce25776f through 95a9732f, focusing on the repeated transient-notification blocker.

Prior blocker: resolved

The uncertainty notice is now durably bounded to one attempt per action. A leased worker atomically records uncertainty_notice_attempted_at before contacting Slack, with both action status and lease-owner fencing. Subsequent transient backend retries therefore remain silent across backoff cycles and process restarts, while the authoritative terminal success or rejection feedback is still required before the action completes.

The failure semantics are deliberate and sound: recording the attempt before delivery closes the send-then-crash duplicate window; failure of this optional informational notice does not stop the durable backend retry; and cancellation during notice delivery is still propagated rather than swallowed.

Verification

• Office Manager action suite: 46 passed, 0 failed.
• The one legacy SQLite schema-upgrade test was deliberately not executed; I did not run or apply any database migration.
• Python compilation and git diff --check: passed.
• GitHub security-checks: passed at this head.
• Companion backend PR MLAI-AUS-Inc/mlai-backend#649 remains open with all current checks green.

Conclusion: The code changes are safe and the prior merge blocker is fixed; this PR is ready to merge. Keep OFFICE_MANAGER_ACTIONS_ENABLED false until backend #649 is merged, any migration is explicitly approved and applied, the backend is deployed, and the end-to-end smoke checks pass.

[Light-code11]

Rebased PR #210 onto current main (084cf7b) and force-pushed safely with an exact lease. New head: a367bd8.

The conflict came from the meeting-room PR merged on main; that PR was subsequently reverted by #235. The final rebase completed cleanly. git range-diff confirms all nine Office Manager commits are patch-identical before and after the rebase, including the durable receipt/outbox handoff and one-time uncertainty-notice state.

Validation on the rebased head:

• exact current deploy.yml test list: 563 passed
• Office Manager fault-injection coverage remains included
• Python compilation: passed
• git diff --check: passed
• companion backend #649: mergeable with all checks green

The PR branch is now based directly on latest main; fresh GitHub checks are running.

[Light-code11]

Rebased PR #210 onto latest main (c328f77) and resolved the conflicts introduced by restored Meeting Room booking. New head: 102d8d7.

Conflict resolution:

• roo/main.py: retained both durable signed-action paths. Exact Slack retries may reach the Meeting Room or Office Manager outbox; unrelated duplicate actions still stop at generic dedupe. Both retry workers and graceful shutdown paths remain active behind their independent kill switches.
• roo/skills/executor.py: retained the restored Meeting Room imports and the Office Manager coworking reminder.
• shared retained Slack task handling remains the current-main implementation.

Validation on the combined tree:

• conflict-sensitive Office Manager + Meeting Room tests: 160 passed
• exact current deploy.yml test list: 687 passed
• Python compilation and git diff --check: passed
• fresh adversarial state review found no conflict-induced regression in receipt/outbox handoff, lease recovery, one-time uncertainty notification, private terminal feedback, actor binding, or surface gating
• GitHub now reports the PR MERGEABLE; fresh security-checks are running

@callumholt the code you previously marked ready is preserved, now combined with restored Meeting Room booking. Could you please re-check the conflict resolution when the fresh CI run finishes?