fix: abort stale unban skill restore batches

[vyctorbrzezowski]

Summary

When a user is unbanned, ClawHub restores their hidden skills in background steps. This PR makes each step check whether the user was banned again before restoring anything. The latest ban always wins.

What changed

• Unban skill-restore pages abort when the user has already been banned/deactivated again.
• Later ban markers are monotonic; older delayed ban pages cannot roll softDeletedAt back from a newer ban timestamp.
• Removed rows are not retimestamped or restored.

Public behavior

Unban restore is fail-closed. If a restore page is stale because the user was banned or deactivated again, the page aborts instead of restoring old state.

Ban-hidden rows keep the newer ban marker. Removed rows remain removed.

Behavior proof

Live Convex runtime proof from a stale unban restore page after a later ban:

$ bunx convex run --push --typecheck=disable --codegen disable proof2284:run
- Preparing Convex functions...

✔ Convex functions ready!
{
  "restoreResult": {
    "aborted": true,
    "ok": true,
    "restoredCount": 0,
    "scheduled": false
  },
  "after": {
    "moderationReason": "user.banned",
    "moderationStatus": "hidden",
    "softDeletedAt": "<later-ban-timestamp>"
  },
  "fixture": {
    "bannedAt": "<later-ban-timestamp>",
    "ownerUserId": "<redacted>",
    "skillId": "<redacted>"
  }
}

This proof creates a stale restore page, applies a later ban before the page runs, then confirms the restore aborts and the skill remains hidden with the later ban marker.

Focused regression suite:

$ bun run test convex/skills.banBatches.test.ts
Test Files  1 passed (1)
Tests       6 passed (6)

Validation

$ bun run ci:unit
Test Files  205 passed (205)
Tests       1991 passed (1991)
Statements  85.98%
Branches    75.17%
Functions   86.37%
Lines        89.48%

$ bun run ci:types-build
tsc, package typechecks, clawhub-mod typecheck, Vite build, and Nitro build passed.

Current GitHub CI for this head also has packages, types-build, e2e-http, playwright-smoke, and playwright-local-auth passing. The static job currently stops at bun audit on the existing transitive ws advisory GHSA-58qx-3vcg-4xpx.

[vercel]

@vyctorbrzezowski is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

[clawsweeper]

Codex review: needs changes before merge.

Latest ClawSweeper review: 2026-05-23 22:44 UTC / May 23, 2026, 6:44 PM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The branch adds fail-closed owner-state checks to paginated unban skill restore batches, monotonic ban-marker retimestamping, removed-row skips, regression tests, a moderation spec note, and a changelog entry.

Reproducibility: yes. Source inspection shows current main restores unban pages without re-reading owner state, and PR head's remaining gap is reproducible from source with a legacy moderationReason: "user.banned" row whose moderationStatus is undefined.

PR rating
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Summary: Runtime proof is strong, but the legacy marker correctness gap keeps the patch from being merge-ready.

Rank-up moves:

• Fix the legacy no-status retimestamp predicate and add the missing re-ban/final-unban regression.
• Rerun bun run test convex/skills.banBatches.test.ts after the repair.

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Sufficient (live_output): The PR body includes redacted live Convex output showing a stale unban restore page aborting and leaving the skill hidden with the later ban marker.

Risk before merge

• As written, legacy moderationReason: "user.banned" skills with undefined moderationStatus can keep an old softDeletedAt through a later ban; the final unban uses the newer marker and skips them, leaving the skills hidden.
• The PR intentionally changes delayed restore pages to fail closed after later owner-state changes; that is the safer moderation boundary, but maintainers should explicitly accept that behavior while landing the legacy repair.

Maintainer options:

1. Fix legacy marker retimestamping (recommended)
   Retimestamp older user.banned rows when moderationStatus is hidden or undefined, keep removed rows skipped, and add a regression for final unban restoration after a later ban.
2. Confirm fail-closed restore semantics
   After the legacy marker fix, maintainers can explicitly accept that stale restore pages abort instead of attempting best-effort restore after later owner-state changes.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update `applyBanToOwnedSkillsBatchInternal` so older soft-deleted `moderationReason: "user.banned"` rows with `moderationStatus` either `"hidden"` or undefined are retimestamped when `softDeletedAt < bannedAt`, while `moderationStatus: "removed"` rows remain skipped; add a regression covering a legacy no-status re-ban followed by final unban.

Next step before merge
A focused automated repair can change the retimestamp predicate and add the missing legacy regression without a broader design decision.

Security
Cleared: The diff tightens internal Convex moderation checks and adds tests/spec coverage without dependency, workflow, secret, package-resolution, or script changes.

Review findings

• [P2] Retimestamp legacy ban-hidden rows on re-ban — convex/skills.ts:6716-6718

Review details

Best possible solution:

Land the owner re-read and fail-closed restore behavior after extending monotonic retimestamping to legacy no-status user.banned rows while still skipping removed rows.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main restores unban pages without re-reading owner state, and PR head's remaining gap is reproducible from source with a legacy moderationReason: "user.banned" row whose moderationStatus is undefined.

Is this the best way to solve the issue?

Not yet. The owner re-read and monotonic marker direction is the right narrow fix, but the retimestamp guard should include legacy no-status user.banned rows before merge.

Label justifications:

• P1: A stale unban restore page can cross an account-moderation boundary by reactivating skills after the owner is banned again or deactivated.
• merge-risk: 🚨 compatibility: The current diff can strand legacy ban-hidden rows during upgrade because the retimestamp path skips undefined moderationStatus rows.
• merge-risk: 🚨 security-boundary: The PR changes which delayed account-moderation batch is allowed to restore user-owned skills after owner-state changes.
• rating: 🦐 gold shrimp: Current PR rating is 🦐 gold shrimp because proof is 🦞 diamond lobster, patch quality is 🦐 gold shrimp, and Runtime proof is strong, but the legacy marker correctness gap keeps the patch from being merge-ready.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes redacted live Convex output showing a stale unban restore page aborting and leaving the skill hidden with the later ban marker.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live Convex output showing a stale unban restore page aborting and leaving the skill hidden with the later ban marker.

Full review comments:

• [P2] Retimestamp legacy ban-hidden rows on re-ban — convex/skills.ts:6716-6718
  The new retimestamp path only runs when moderationStatus === "hidden", but older ban-hidden rows can have moderationReason: "user.banned" with no status. If a later ban lands while an earlier unban restore is still paginating, those rows keep the old softDeletedAt; the final unban uses the newer marker and skips them, leaving the skill hidden indefinitely. Include undefined-status legacy rows while still skipping removed rows.
  Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.88

Acceptance criteria:

• bun run test convex/skills.banBatches.test.ts
• bun run ci:unit

What I checked:

• Current main lacks stale restore owner recheck: On current main, restoreOwnedSkillsForUnbanBatchInternal proceeds from Date.now() into the skills query without re-reading the owner, so a delayed restore page does not verify whether the owner was banned again or deactivated. (convex/skills.ts:6867, c538848a3d1e)
• Unban restore uses the exact ban marker: unbanUserWithActor captures bannedAt = target.deletedAt and passes that marker to restoreOwnedSkillsForUnbanBatchInternal; rows left with an older marker are skipped by a later final unban. (convex/users.ts:1695, c538848a3d1e)
• PR adds the intended fail-closed owner guard: At PR head, restore pages call ctx.db.get(args.ownerUserId) and abort without querying or patching skills when the owner is missing, banned again, or deactivated. (convex/skills.ts:6846, cb1143e133a6)
• PR retimestamp predicate still misses legacy rows: The new retimestamp branch requires skill.moderationStatus === "hidden", so legacy moderationReason: "user.banned" rows with undefined status keep the old softDeletedAt through a later ban. (convex/skills.ts:6716, cb1143e133a6)
• Schema allows the legacy no-status shape: moderationStatusValidator is optional on skills, so undefined moderationStatus is a supported row shape that migration-sensitive batch code must handle deliberately. (convex/schema.ts:241, c538848a3d1e)
• Tests cover adjacent cases but not the failing combination: The PR tests cover hidden-row retimestamping and direct restoration of a legacy no-status row, but they do not cover a legacy no-status row being re-banned and then restored by the final unban marker. (convex/skills.banBatches.test.ts:73, cb1143e133a6)

Likely related people:

• steipete: PR 315 moved the ban/unban skill restore batch functions into convex/skills.ts and rewired convex/users.ts to call them. (role: introduced and consolidated behavior; confidence: high; commits: f94e20d4c35f; files: convex/skills.ts, convex/users.ts)
• autogame-17: PR 298 introduced the ban flow change from hard-delete toward soft-delete plus unban recovery for ban-hidden skills. (role: introduced ban recovery behavior; confidence: medium; commits: e2592684ed96; files: convex/skills.ts, convex/users.ts)
• Patrick-Erichsen: Recent merged autoban remediation and release work touched the same moderation batch area and current-main blame for these files is through the v0.17.0 release graft. (role: recent area contributor; confidence: medium; commits: 6814af95dff0, b753b1f7ab0e; files: convex/skills.ts, convex/users.ts, specs/security-moderation.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against c538848a3d1e.

[vyctorbrzezowski]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26042381418
• Updated: 2026-05-18T15:17:53.156Z

[vyctorbrzezowski]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26058457221
• Updated: 2026-05-18T20:30:51.309Z

[clawsweeper]

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

• Merged PRs are hatchable.
• Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
• Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

What is this egg doing here?

• Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
• The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
• Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
• The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
• Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.