Skip to content

feat(core,plugin-history-sync): allow step actions for non-top activities #642

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
orionmiz wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat(core,plugin-history-sync): allow step actions for non-top activities #642

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
980c183
feat: allow step actions to target lower activities with targetActivi…
claude Oct 22, 2025
7696863
refactor: improve step action sync with history.back() for step pop
claude Oct 22, 2025
42cde0f
fix: handle multiple step operations correctly
claude Oct 22, 2025
398bcf8
Merge branch 'main' into claude/modify-step-action-scope-011CUNBq87uz…
orionmiz Oct 23, 2025
8f5a022
test: fix aggregate tests by adding hasZIndex field
claude Oct 23, 2025
ae7c98d
chore: add changeset for lower activity step actions
claude Oct 23, 2025
289cd62
fix: only sync lower activity steps, not active activity
claude Oct 23, 2025
a8167c8
test: add failing tests for lower activity step modifications
claude Oct 23, 2025
57b087f
fix: only sync lower activity steps, not active activity
claude Oct 23, 2025
d951a66
fix: prevent lower activity step actions from affecting current history
claude Oct 23, 2025
8807c12
WIP: attempt stepPush/stepReplace sync - has regression
claude Oct 24, 2025
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
42 changes: 23 additions & 19 deletions core/src/activity-utils/findTargetActivityIndices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,34 @@ export function findTargetActivityIndices(
}
case "StepPushed":
case "StepReplaced": {
const latestActivity = findLatestActiveActivity(activities);
let targetActivity: Activity | undefined;

if (latestActivity) {
if (
event.targetActivityId &&
event.targetActivityId !== latestActivity.id
) {
break;
}
targetActivities.push(activities.indexOf(latestActivity));
if (event.targetActivityId) {
targetActivity = activities.find(
(activity) => activity.id === event.targetActivityId,
);
} else {
targetActivity = findLatestActiveActivity(activities);
}

if (targetActivity) {
targetActivities.push(activities.indexOf(targetActivity));
}
break;
}
case "StepPopped": {
const latestActivity = findLatestActiveActivity(activities);

if (latestActivity && latestActivity.steps.length > 1) {
if (
event.targetActivityId &&
event.targetActivityId !== latestActivity.id
) {
break;
}
targetActivities.push(activities.indexOf(latestActivity));
let targetActivity: Activity | undefined;

if (event.targetActivityId) {
targetActivity = activities.find(
(activity) => activity.id === event.targetActivityId,
);
} else {
targetActivity = findLatestActiveActivity(activities);
}

if (targetActivity && targetActivity.steps.length > 1) {
targetActivities.push(activities.indexOf(targetActivity));
}

break;
Expand Down
205 changes: 202 additions & 3 deletions core/src/aggregate.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
PoppedEvent,
PushedEvent,
ReplacedEvent,
StepPoppedEvent,
StepPushedEvent,
StepReplacedEvent,
} from "./event-types";
Expand Down Expand Up @@ -3974,7 +3975,7 @@ test("aggregate - After Push > Push > Pop > Replace, first pushed activity shoul
});
});

test("aggregate - StepPushedEvent must be ignored when top activity is not target activity", () => {
test("aggregate - StepPushedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
Expand Down Expand Up @@ -4002,7 +4003,7 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe
})),
(stepPushedEvent = makeEvent("StepPushed", {
stepId: "s1",
stepParams: {},
stepParams: { tab: "profile" },
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
Expand All @@ -4016,14 +4017,20 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe
id: "A",
name: "sample",
transitionState: "enter-done",
params: {},
params: { tab: "profile" },
steps: [
{
id: "A",
params: {},
enteredBy: pushedEvent1,
zIndex: 0,
},
{
id: "s1",
params: { tab: "profile" },
enteredBy: stepPushedEvent,
zIndex: 0,
},
],
enteredBy: pushedEvent1,
isActive: false,
Expand Down Expand Up @@ -4333,3 +4340,195 @@ test("aggregate - StepPushedEvent에 hasZIndex 필드가 true이면, Step에 zIn
events,
});
});

test("aggregate - StepReplacedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
let pushedEvent2: PushedEvent;
let stepReplacedEvent: StepReplacedEvent;

const events = [
initializedEvent({
transitionDuration: 300,
}),
registeredEvent({
activityName: "sample",
}),
(pushedEvent1 = makeEvent("Pushed", {
activityId: "A",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(pushedEvent2 = makeEvent("Pushed", {
activityId: "B",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepReplacedEvent = makeEvent("StepReplaced", {
stepId: "s1",
stepParams: { newParam: "value" },
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
];

const output = aggregate(events, t + 300);

expect(output).toStrictEqual({
activities: [
activity({
id: "A",
name: "sample",
transitionState: "enter-done",
params: { newParam: "value" },
steps: [
{
id: "s1",
params: { newParam: "value" },
enteredBy: stepReplacedEvent,
zIndex: 0,
},
],
enteredBy: pushedEvent1,
isActive: false,
isTop: false,
isRoot: true,
zIndex: 0,
}),
activity({
id: "B",
name: "sample",
transitionState: "enter-done",
params: {},
steps: [
{
id: "B",
params: {},
enteredBy: pushedEvent2,
zIndex: 1,
},
],
enteredBy: pushedEvent2,
isActive: true,
isTop: true,
isRoot: false,
zIndex: 1,
}),
],
registeredActivities: [
{
name: "sample",
},
],
transitionDuration: 300,
globalTransitionState: "idle",
events,
});
});

test("aggregate - StepPoppedEvent can target lower activity with targetActivityId", () => {
const t = nowTime();

let pushedEvent1: PushedEvent;
let pushedEvent2: PushedEvent;
let stepPushedEvent1: StepPushedEvent;
let stepPushedEvent2: StepPushedEvent;
let stepPoppedEvent: StepPoppedEvent;

const events = [
initializedEvent({
transitionDuration: 300,
}),
registeredEvent({
activityName: "sample",
}),
(pushedEvent1 = makeEvent("Pushed", {
activityId: "A",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepPushedEvent1 = makeEvent("StepPushed", {
stepId: "s1",
stepParams: { step: "1" },
eventDate: enoughPastTime(),
})),
(stepPushedEvent2 = makeEvent("StepPushed", {
stepId: "s2",
stepParams: { step: "2" },
eventDate: enoughPastTime(),
})),
(pushedEvent2 = makeEvent("Pushed", {
activityId: "B",
activityName: "sample",
activityParams: {},
eventDate: enoughPastTime(),
})),
(stepPoppedEvent = makeEvent("StepPopped", {
targetActivityId: pushedEvent1.activityId,
eventDate: enoughPastTime(),
})),
];

const output = aggregate(events, t + 300);

expect(output).toStrictEqual({
activities: [
activity({
id: "A",
name: "sample",
transitionState: "enter-done",
params: { step: "1" },
steps: [
{
id: "A",
params: {},
enteredBy: pushedEvent1,
zIndex: 0,
},
{
id: "s1",
params: { step: "1" },
enteredBy: stepPushedEvent1,
zIndex: 0,
},
],
enteredBy: pushedEvent1,
isActive: false,
isTop: false,
isRoot: true,
zIndex: 0,
}),
activity({
id: "B",
name: "sample",
transitionState: "enter-done",
params: {},
steps: [
{
id: "B",
params: {},
enteredBy: pushedEvent2,
zIndex: 1,
},
],
enteredBy: pushedEvent2,
isActive: true,
isTop: true,
isRoot: false,
zIndex: 1,
}),
],
registeredActivities: [
{
name: "sample",
},
],
transitionDuration: 300,
globalTransitionState: "idle",
events,
});
});
52 changes: 52 additions & 0 deletions extensions/plugin-history-sync/src/historySyncPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,58 @@ export function historySyncPlugin<
(step) => step.id === targetStep?.id,
);

// Synchronize history state with core state for lower activity step modifications
if (nextActivity) {
const coreSteps = nextActivity.steps;
const coreLastStep = last(coreSteps);
const historyStep = targetStep;
const historySteps = targetActivity.steps;

// Check if history step exists in core state
const historyStepExistsInCore = historyStep
? coreSteps.some((step) => step.id === historyStep.id)
: true;

if (!historyStepExistsInCore) {
// History step was removed or replaced
if (coreSteps.length < historySteps.length) {
// Step Pop: Step count decreased
// Use history.back() to skip the removed step entry
requestHistoryTick(() => {
silentFlag = true;
history.back();
});
return;
Copy link

@coderabbitai coderabbitai bot Oct 23, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Multi-step pop only calls back() once — can leave user on an invalid, still-removed step.

If core popped N>1 steps on a lower activity, a single history.back() won’t skip all removed entries. Use history.go(-delta) to jump exactly the difference.

Apply this diff:

-              if (coreSteps.length < historySteps.length) {
-                // Step Pop: Step count decreased
-                // Use history.back() to skip the removed step entry
-                requestHistoryTick(() => {
-                  silentFlag = true;
-                  history.back();
-                });
-                return;
-              } else {
+              if (coreSteps.length < historySteps.length) {
+                // Step Pop: jump back by the exact number of removed steps
+                const popDelta = historySteps.length - coreSteps.length;
+                if (popDelta > 0) {
+                  requestHistoryTick(() => {
+                    silentFlag = true;
+                    history.go(-popDelta);
+                  });
+                }
+                return;
+              } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (coreSteps.length < historySteps.length) {
// Step Pop: Step count decreased
// Use history.back() to skip the removed step entry
requestHistoryTick(() => {
silentFlag = true;
history.back();
});
return;
if (coreSteps.length < historySteps.length) {
// Step Pop: jump back by the exact number of removed steps
const popDelta = historySteps.length - coreSteps.length;
if (popDelta > 0) {
requestHistoryTick(() => {
silentFlag = true;
history.go(-popDelta);
});
}
return;
🤖 Prompt for AI Agents 
In extensions/plugin-history-sync/src/historySyncPlugin.tsx around lines 414 to
421, the code only calls history.back() when coreSteps.length <
historySteps.length which only moves back one entry; compute the exact delta =
historySteps.length - coreSteps.length and inside the requestHistoryTick
callback set silentFlag = true and call history.go(-delta) so the browser jumps
back by the full number of removed entries (then return as before).

} else {
// Step Replace or complex operations: Step count same or increased
// Use replaceState to show current core state
const match = activityRoutes.find(
(r) => r.activityName === nextActivity.name,
)!;
const template = makeTemplate(match, options.urlPatternOptions);

requestHistoryTick(() => {
silentFlag = true;
replaceState({
history,
pathname: template.fill(nextActivity.params),
state: {
activity: nextActivity,
step: coreLastStep,
},
useHash: options.useHash,
});
});
return;
}
}

// If history step exists in core, proceed with normal navigation logic
// This handles:
// - Step exists but is not the last (normal step backward)
// - Multiple step pushes then navigating to middle steps
}

const isBackward = () => currentActivity.id > targetActivityId;
const isForward = () => currentActivity.id < targetActivityId;
const isStep = () => currentActivity.id === targetActivityId;
Expand Down
Loading