ci: Bump actions/checkout from 4 to 6#4
Merged
Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
added
dependencies
This was referenced May 13, 2026
ComBba
pushed a commit
that referenced
this pull request
May 14, 2026
External code-review pointed out 5 valid issues. All addressed in this commit on the same branch — PR #44 picks them up automatically. ## Gemini code-assist ### #1 — Manage menu: parallelize per-rule dry-run reads The previous serial `for` loop multiplied Redis latency by the draft count. With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs ~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed fetch doesn't stop the others. ### #2 — applyManageActions: enforce the 50-rule cap on apply pause / activate / unpause moves can push a bundle over the 50-rule cap even when no compose-rule call is made. Added an explicit check before the dual-write txn — the moderator gets a "Rule cap exceeded (active=N, draft=N, max=50). Delete a rule first" toast and the bundles stay untouched. New test in routes-manage.test.ts. ### #3 — Use the configured openaiModel as the fallback (not the literal "manage") The previous `'manage'` fallback for `bundle.llmModel` broke `estimateTokenCost()` because 'manage' isn't a key in `OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until a real compile lands. Now we read the configured `openaiModel` setting through `readOpenaiModel()` (already SELECTION-array-safe per PR #39). ### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong) The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])` was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause could leave a rule absent from active without showing up in draft. Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()` pattern (same shape executor.ts already uses for audit writes), and check `exec()`'s null return for the WATCH-aborted case so two moderators managing rules at the same time get an actionable "Another moderator changed the rules at the same time — re-open Manage rules" toast. ## CodeRabbit ### #5 — humanizeRule: cap value-stringification at 100 chars A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains) would otherwise render thousands of characters into the confirm form's `compiledSummary` field, blowing past Devvit's modal description budget and obscuring the actual rule structure. Truncate with `...` after 100 chars in the predicate-tree leaf renderer. ## Test infrastructure test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null` (matching real Devvit Redis MULTI/EXEC semantics) instead of `void`. Without this, the new WATCH-abort detection in applyManageActions would fire on every test. Updated `FakeTxn.exec` type accordingly. ## Verification - `npm run check` 4/4 gates green - 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts
ComBba
pushed a commit
that referenced
this pull request
May 14, 2026
5 review issues from Gemini code-assist on PR #45 (refactor/split-server-routes). ## HIGH severity ### #1 Non-ASCII regex made unambiguous helpers/openai.ts: the `replace(/[<U+0080>-<U+FFFF>]/g, ...)` literal contained the U+0080 control character invisibly, which made review tools render it as `/[-<box>]/` and flag it as "matches only `-`". Behaviour was always correct, but the explicit `/[�-ï¿¿]/g` form removes any doubt. ### #2 WATCH ordering in applyManageActions routes/manage.ts: the previous shape did GET → modify → WATCH → MULTI → EXEC, so the optimistic lock didn't actually cover the data we were about to mutate. Restructured into a CAS retry loop: 1. WATCH first 2. GET (under WATCH — uses redis.get because Devvit's `txn.get` queues into the transaction and returns a chainable, not the value) 3. modify in memory 4. MULTI + SET + EXEC (null exec → another mod's call landed first between WATCH and now → loop and retry up to MAX_CAS_ATTEMPTS = 3). After 3 contended attempts the moderator gets a clear "another mod changed the rules — re-open and try again" toast. ### #3 Sequential audit-entry hGetAll in undo handler routes/undo.ts: 100 sequential hGetAll calls could blow the menu handler's deadline. Switched to `Promise.allSettled([...])` and scan the resolved array in order so the "most recent first" break-on-found behaviour is preserved. ## MEDIUM severity ### #4 Sequential audit-entry hGetAll in dashboard routes/dashboard.ts: same parallelization for the 20-entry recent-actions fetch and the per-draft-rule dry-run fetch. ### #5 Outdated comment on /internal/trigger/on-app-install routes/triggers.ts: the comment claimed the submit triggers seed in-band, but they actually fail-safe by returning ok when no bundle exists. Updated to describe the real flow: starter rules are seeded by the deferred /internal/scheduler/seed-on-install task, registered in devvit.json's scheduler.tasks block. ## Verification - `npm run check` 4/4 gates green: typecheck + lint + Prettier + 211 tests (1 skipped) + 3 @devvit/test cases + acceptance G1-G4 - All PR #45 tests still pass with no test-side modifications
ComBba
pushed a commit
that referenced
this pull request
May 14, 2026
5 review issues from Gemini code-assist + CodeRabbit on PR #49. ## Major ### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC) Two concurrent submits with the same `pendingId` (back-button + re-submit, double-click on Save, multi-tab moderator) could both read the entry before either deleted it, doubling the bundle write + dry-run schedule. Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip. Whichever caller commits first wins; the loser sees `exec() == null` and gets an actionable "Another submission consumed this confirmation" toast. Updated FakeTxn to match (added `del()` method on the txn object — the production Devvit Redis client supports it, the fake didn't). ### CodeRabbit #5 — daily compile counter now bumps at compile time Quota was previously incremented inside `persistRuleAndStartDryRun()`, which only runs on Save. A moderator could repeatedly hit Compile + Cancel and burn through OpenAI tokens without the per-day counter ever moving. Moved the increment to immediately after a successful OpenAI return in compose-rule-submit. The token cost is real either way, so the quota now reflects it either way. ## Medium ### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again" Devvit's Cancel button does NOT trigger the form submit handler, so clicking the misnamed button could not actually persist `dismissOnboarding` — it just closed the modal. Reverted the label to "Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is the real opt-out path that submits the form values. ## Minor ### CodeRabbit #2 — verify script tolerates non-int SLOW_MO `int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g. `SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the default rather than ending the recording session before it starts. ### CodeRabbit #3 — atomic SET + TTL via the `expiration` option Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`, which could leak a TTL-less pending key if expire failed after set succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })` which is the same single round-trip the rollback writer in executor.ts already uses. ## Verification - `npm run check` 4/4 gates green (211 unit/integration tests) - All compose tests still pass with the new pending-id flow - Race condition smoke: a second compose-confirm-submit call against the same pendingId now returns the "another submission consumed" toast instead of double-persisting (previously: double bundle write).
ComBba
added a commit
that referenced
this pull request
May 15, 2026
…ctions/checkout-6 ci: Bump actions/checkout from 4 to 6
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
…Text + toast 1-line summary Audit-driven UX quick wins for the compose flow. Full audit + scenario rationale: - claudedocs/2026-05-14-compose-flow-audit.md (10 findings vs README promise matrix) - docs/demo-scenario.md (60s demo, README-aligned, Hypothesis 3) ## What changes (3 audit findings landed) ### Finding #1 (CRITICAL for demo) — Clarify modal renders LLM suggestedAnswers as a dropdown The LLM has *always* returned `{ needsClarification, question, suggestedAnswers: [...] }` (see system-prompt.ts:80-85), but the server discarded `suggestedAnswers` and showed only a free-text textarea. Moderators had to paraphrase the question. Now: when `suggestedAnswers` is present, render a `select` field with one click per option PLUS a `clarificationAnswerOther` paragraph as an override / escape hatch. When the LLM returns no suggestions, fall back to the original free-text path. `compose-rule-submit` defensively unwraps SELECTION-array values (PR #39 pattern) and prefers the "Other" override when non-empty. ### Finding #4 — ban/mute toggle gets explanatory helpText on both forms Mod mental model = "checkbox grants ban permission". Real behaviour = "checkbox lets compile succeed when the LLM emits ban/mute". Added a single shared `ALLOW_GUARDED_HELP` string used by the compose form and the clarify modal so both forms explain the toggle the same way. ### Finding #6 — Success toast carries a 1-line summary + an explicit menu pointer Previously: `Compiled rule "X". Dry-run started — check Dashboard in 30s.` The dashboard menu name is "vibe-mod: View rules + log" and Devvit toasts have no buttons, so "check Dashboard" is unreachable for a first-time mod. Now: `Compiled rule "X". → post: modqueue. Dry-run started — open the subreddit ⋯ menu → "vibe-mod: View rules + log" to see preview.` The 1-line `summarizeRule()` makes the deterministic JSON contract visible without leaving the toast (audit finding #2 alternative — the full preview form stays a post-publish item). ## Verification - `npm run check` 4/4 gates green (typecheck + lint + Prettier + 25 compose tests + 3 @devvit/test + acceptance G1–G4) - 6 new tests in routes-compose.test.ts cover: select rendering, free-text fallback, SELECTION-array unwrap, "Other" override precedence, helpText presence, summary in success toast. ## Why this PR (not bundled with module split) Demo video records this UI. UX changes must land before the README screenshot captures (Phase 3) and the Playwright video automation (Phase 4). Module split (Phase 2) is a separate, independent refactor. Refs: claudedocs/2026-05-14-compose-flow-audit.md (Phase 1.5/1.5b), docs/demo-scenario.md (Phase 2.5b)
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
External code-review pointed out 5 valid issues. All addressed in this commit on the same branch — PR #44 picks them up automatically. ## Gemini code-assist ### #1 — Manage menu: parallelize per-rule dry-run reads The previous serial `for` loop multiplied Redis latency by the draft count. With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs ~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed fetch doesn't stop the others. ### #2 — applyManageActions: enforce the 50-rule cap on apply pause / activate / unpause moves can push a bundle over the 50-rule cap even when no compose-rule call is made. Added an explicit check before the dual-write txn — the moderator gets a "Rule cap exceeded (active=N, draft=N, max=50). Delete a rule first" toast and the bundles stay untouched. New test in routes-manage.test.ts. ### #3 — Use the configured openaiModel as the fallback (not the literal "manage") The previous `'manage'` fallback for `bundle.llmModel` broke `estimateTokenCost()` because 'manage' isn't a key in `OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until a real compile lands. Now we read the configured `openaiModel` setting through `readOpenaiModel()` (already SELECTION-array-safe per PR #39). ### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong) The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])` was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause could leave a rule absent from active without showing up in draft. Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()` pattern (same shape executor.ts already uses for audit writes), and check `exec()`'s null return for the WATCH-aborted case so two moderators managing rules at the same time get an actionable "Another moderator changed the rules at the same time — re-open Manage rules" toast. ## CodeRabbit ### #5 — humanizeRule: cap value-stringification at 100 chars A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains) would otherwise render thousands of characters into the confirm form's `compiledSummary` field, blowing past Devvit's modal description budget and obscuring the actual rule structure. Truncate with `...` after 100 chars in the predicate-tree leaf renderer. ## Test infrastructure test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null` (matching real Devvit Redis MULTI/EXEC semantics) instead of `void`. Without this, the new WATCH-abort detection in applyManageActions would fire on every test. Updated `FakeTxn.exec` type accordingly. ## Verification - `npm run check` 4/4 gates green - 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
5 review issues from Gemini code-assist on PR #45 (refactor/split-server-routes). ## HIGH severity ### #1 Non-ASCII regex made unambiguous helpers/openai.ts: the `replace(/[<U+0080>-<U+FFFF>]/g, ...)` literal contained the U+0080 control character invisibly, which made review tools render it as `/[-<box>]/` and flag it as "matches only `-`". Behaviour was always correct, but the explicit `/[�-ï¿¿]/g` form removes any doubt. ### #2 WATCH ordering in applyManageActions routes/manage.ts: the previous shape did GET → modify → WATCH → MULTI → EXEC, so the optimistic lock didn't actually cover the data we were about to mutate. Restructured into a CAS retry loop: 1. WATCH first 2. GET (under WATCH — uses redis.get because Devvit's `txn.get` queues into the transaction and returns a chainable, not the value) 3. modify in memory 4. MULTI + SET + EXEC (null exec → another mod's call landed first between WATCH and now → loop and retry up to MAX_CAS_ATTEMPTS = 3). After 3 contended attempts the moderator gets a clear "another mod changed the rules — re-open and try again" toast. ### #3 Sequential audit-entry hGetAll in undo handler routes/undo.ts: 100 sequential hGetAll calls could blow the menu handler's deadline. Switched to `Promise.allSettled([...])` and scan the resolved array in order so the "most recent first" break-on-found behaviour is preserved. ## MEDIUM severity ### #4 Sequential audit-entry hGetAll in dashboard routes/dashboard.ts: same parallelization for the 20-entry recent-actions fetch and the per-draft-rule dry-run fetch. ### #5 Outdated comment on /internal/trigger/on-app-install routes/triggers.ts: the comment claimed the submit triggers seed in-band, but they actually fail-safe by returning ok when no bundle exists. Updated to describe the real flow: starter rules are seeded by the deferred /internal/scheduler/seed-on-install task, registered in devvit.json's scheduler.tasks block. ## Verification - `npm run check` 4/4 gates green: typecheck + lint + Prettier + 211 tests (1 skipped) + 3 @devvit/test cases + acceptance G1-G4 - All PR #45 tests still pass with no test-side modifications
ComBba
pushed a commit
that referenced
this pull request
May 15, 2026
5 review issues from Gemini code-assist + CodeRabbit on PR #49. ## Major ### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC) Two concurrent submits with the same `pendingId` (back-button + re-submit, double-click on Save, multi-tab moderator) could both read the entry before either deleted it, doubling the bundle write + dry-run schedule. Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip. Whichever caller commits first wins; the loser sees `exec() == null` and gets an actionable "Another submission consumed this confirmation" toast. Updated FakeTxn to match (added `del()` method on the txn object — the production Devvit Redis client supports it, the fake didn't). ### CodeRabbit #5 — daily compile counter now bumps at compile time Quota was previously incremented inside `persistRuleAndStartDryRun()`, which only runs on Save. A moderator could repeatedly hit Compile + Cancel and burn through OpenAI tokens without the per-day counter ever moving. Moved the increment to immediately after a successful OpenAI return in compose-rule-submit. The token cost is real either way, so the quota now reflects it either way. ## Medium ### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again" Devvit's Cancel button does NOT trigger the form submit handler, so clicking the misnamed button could not actually persist `dismissOnboarding` — it just closed the modal. Reverted the label to "Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is the real opt-out path that submits the form values. ## Minor ### CodeRabbit #2 — verify script tolerates non-int SLOW_MO `int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g. `SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the default rather than ending the recording session before it starts. ### CodeRabbit #3 — atomic SET + TTL via the `expiration` option Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`, which could leak a TTL-less pending key if expire failed after set succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })` which is the same single round-trip the rollback writer in executor.ts already uses. ## Verification - `npm run check` 4/4 gates green (211 unit/integration tests) - All compose tests still pass with the new pending-id flow - Race condition smoke: a second compose-confirm-submit call against the same pendingId now returns the "another submission consumed" toast instead of double-persisting (previously: double bundle write).
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
1 participant
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.
Bumps actions/checkout from 4 to 6.
Release notes
Sourced from actions/checkout's releases.
... (truncated)
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
de0fac2Fix tag handling: preserve annotations and explicit fetch-tags (#2356)064fe7fAdd orchestration_id to git user-agent when ACTIONS_ORCHESTRATION_ID is set (...8e8c483Clarify v6 README (#2328)033fa0dAdd worktree support for persist-credentials includeIf (#2327)c2d88d3Update all references from v5 and v4 to v6 (#2314)1af3b93update readme/changelog for v6 (#2311)71cf226v6-beta (#2298)069c695Persist creds to a separate file (#2286)ff7abcdUpdate README to include Node.js 24 support details and requirements (#2248)08c6903Prepare v5.0.0 release (#2238)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)