Skip to content

fix(actions): prevent state mutation and retain newest actions in ActionLogger #34053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tysoncung wants to merge 1 commit into
base: next
Choose a base branch
from
+7 −6
Open

fix(actions): prevent state mutation and retain newest actions in ActionLogger #34053

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions code/core/src/actions/containers/ActionLogger/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,16 @@ export default function ActionLogger({ active, api }: ActionLoggerProps) {

const addAction = useCallback((action: ActionDisplay) => {
setActions((prevActions) => {
const newActions = [...prevActions];
const previous = newActions.length && newActions[newActions.length - 1];
const previous = prevActions.length ? prevActions[prevActions.length - 1] : null;

if (previous && safeDeepEqual(previous.data, action.data)) {
previous.count++;
const updated = [...prevActions];
updated[updated.length - 1] = { ...previous, count: previous.count + 1 };
return updated.slice(-action.options.limit);
} else {
action.count = 1;
newActions.push(action);
const newAction = { ...action, count: 1 };
return [...prevActions, newAction].slice(-action.options.limit);
Comment on lines +44 to +47
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find ActionsParameters type definition and limit property
rg -n "ActionsParameters|limit" code/core/src/actions --type=ts --type=tsx -A 5 -B 2 | head -100

# Also search more broadly for the type definition
rg -n "interface.*ActionsParameters|type.*ActionsParameters" . --type=ts --type=tsx

Repository: storybookjs/storybook

Length of output: 92

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for explicit zero-limit examples/configuration
rg -n "limit\s*:\s*0|limit.*=.*0" code/core/src --type=ts --type=tsx

# Look for limit validation or guards
rg -n "limit.*[<>].*0|if.*limit" code/core/src/actions --type=ts --type=tsx -A 2 -B 1

Repository: storybookjs/storybook

Length of output: 92

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the actual file in question for context
cat -n code/core/src/actions/containers/ActionLogger/index.tsx | head -60

Repository: storybookjs/storybook

Length of output: 2466

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the types file - look for it in the actions directory
find code/core/src/actions -name "types*" -o -name "index.ts" | head -20

Repository: storybookjs/storybook

Length of output: 203

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for ActionsParameters definition across the codebase
rg -n "ActionsParameters" code/core/src --max-count=20 -A 3 -B 1

Repository: storybookjs/storybook

Length of output: 1427

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for limit property within types or interfaces
rg -n "limit.*:" code/core/src/actions -A 1 -B 1 | head -80

Repository: storybookjs/storybook

Length of output: 480

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the types.ts file to see the full ActionsParameters definition
cat -n code/core/src/actions/types.ts

Repository: storybookjs/storybook

Length of output: 2315

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the ActionOptions model for limit type and any constraints
cat -n code/core/src/actions/models/ActionOptions.ts

Repository: storybookjs/storybook

Length of output: 440

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check configureActions for more context on limit and validation
cat -n code/core/src/actions/runtime/configureActions.ts

Repository: storybookjs/storybook

Length of output: 397

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any validation or guards on limit in the codebase
rg -n "limit.*[>0<]|if.*limit|limit.*\|\|" code/core/src/actions -B 2 -A 2

Repository: storybookjs/storybook

Length of output: 830

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if limit can be undefined or if action.options.limit is always defined
# Look at ActionDisplay type definition
rg -n "ActionDisplay" code/core/src/actions -A 5 -B 1 | head -80

Repository: storybookjs/storybook

Length of output: 6227

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if there are any tests that set limit to 0 or negative values
rg -n "limit.*0|limit.*-" code/core --max-count=30

Repository: storybookjs/storybook

Length of output: 1646

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check ActionDisplay interface to see how options are structured
cat -n code/core/src/actions/models/ActionDisplay.ts

Repository: storybookjs/storybook

Length of output: 338

🏁 Script executed:

#!/bin/bash
set -eulo pipefail

# Check if there's any validation or type narrowing that ensures limit is positive
rg -n "ActionDisplay.*count|options.*limit" code/core/src/actions -B 3 -A 3 | head -100

Repository: storybookjs/storybook

Length of output: 1198

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the runtime/action.ts to see how actionOptions is constructed and if limit is always set
cat -n code/core/src/actions/runtime/action.ts | sed -n '80,105p'

Repository: storybookjs/storybook

Length of output: 1124

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if config.limit is always applied or if there's fallback handling
rg -n "config\." code/core/src/actions/runtime/action.ts -B 2 -A 2

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Let me trace how actionOptions are passed to the action function
rg -n "export.*action\s*=" code/core/src/actions/runtime/action.ts -A 3 -B 3

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if limit can be undefined - look at how ActionOptions is actually used
rg -n "actionOptions\|ActionOptions" code/core/src/actions/runtime/action.ts -B 2 -A 2

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for where the limit is actually resolved or if there's a fallback
rg -n "\.limit|limit\s*\?" code/core/src/actions -B 1 -A 1 | head -60

Repository: storybookjs/storybook

Length of output: 700

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Let me check more precisely how action.options.limit is set
# by looking at the full action function signature
cat -n code/core/src/actions/runtime/action.ts | sed -n '40,100p'

Repository: storybookjs/storybook

Length of output: 2418

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if limit value is ever validated or constrained elsewhere
rg -n "if.*limit\|limit.*?.*:|limit.*>|limit.*<" code/core/src/actions

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Double-check by looking at the complete ActionOptions type definition
cat -n code/core/src/actions/models/ActionOptions.ts

Repository: storybookjs/storybook

Length of output: 440

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check what happens when slice is called with -0
node <<'EOF'
const arr = [1, 2, 3, 4, 5];
console.log("slice(-50):", arr.slice(-50));      // Should give [1,2,3,4,5]
console.log("slice(-0):", arr.slice(-0));        // Equivalent to slice(0)
console.log("slice(0):", arr.slice(0));          // Gives full array
console.log("[] with slice(-0):", [].slice(-0)); // Empty array
EOF

Repository: storybookjs/storybook

Length of output: 173

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if there are any integration tests or stories that verify the limit=0 behavior
rg -n "limit.*0|limit:.*0" code/core/src/actions --type=js -B 2 -A 2

Repository: storybookjs/storybook

Length of output: 509

Guard non-positive or undefined limit before using slice(-limit).

Lines 44 and 47 use slice(-action.options.limit) without validation. Since ActionOptions is Partial<Options>, the limit property can be undefined. Additionally, limit has no type constraint enforcing positive integers. When limit is 0 or undefined, slice(-0) and slice(-undefined) both return the full array, silently disabling log truncation instead of keeping zero or all entries as expected. Add a guard to handle these edge cases explicitly.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/actions/containers/ActionLogger/index.tsx` around lines 44 -
47, Guard against non-positive or undefined limits before calling
slice(-action.options.limit): validate and normalize action.options.limit to a
non-negative integer (e.g., parse to Number, floor, and clamp to >=0) and then
branch on that value so you only call slice(-limit) when limit > 0; when limit
=== 0 return an empty array (or otherwise enforce the intended zero-behavior)
instead of using slice(-0) or slice(-undefined). Apply this for both uses in the
block that references updated.slice(-action.options.limit) and [...prevActions,
newAction].slice(-action.options.limit), using the normalized limit variable and
the newAction/prevActions/updated symbols to locate the code.

}
return newActions.slice(0, action.options.limit);
});
}, []);

Expand Down