Skip to content

fix: compare hiddenAppIds as a set for editor dirty state - #226

Merged
arpandhakal merged 1 commit into
feature/studio-apps-actionfrom
arpandhakal/fix-hidden-app-ids-set-comparison
Jul 29, 2026
Merged

fix: compare hiddenAppIds as a set for editor dirty state#226
arpandhakal merged 1 commit into
feature/studio-apps-actionfrom
arpandhakal/fix-hidden-app-ids-set-comparison

Conversation

@arpandhakal

@arpandhakal arpandhakal commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Changes

Addresses the unresolved Greptile P1 on #220 (useAppControls.tsx): Deny list order leaks.

  • hiddenAppIds is an unordered deny-list, but the editor's dirty check compared it with JSON.stringify, which is order-sensitive. The toggle path in useActions.tsx rebuilds the array by filter-then-append, so hiding an app, unhiding it, and hiding it again yields the same members in a different position — and the editor reported a change that wasn't there.
  • Adds areArraysEqualAsSets to src/utils/array.ts and uses it for hiddenAppIds. order is a genuine sequence, so it keeps its serialized comparison.

Testing Criteria

  • pnpm typecheck and pnpm lint clean.

  • Manual repro, in a workspace with two or more Studio apps carrying registered action labels:

    1. Toggle all Studio apps on, Save. Baseline is [], CTAs hidden.
    2. Toggle app A off, then app B off, Save. Baseline is ["A","B"], CTAs hidden.
    3. Toggle app A back on — CTAs appear. Correct, this is a real change.
    4. Toggle app A off again — array is now ["B","A"]. Before this fix: Save/Cancel stay visible even though the hidden set matches step 2. With this fix: CTAs correctly disappear.

    Re-toggle app A specifically — the first of the two hidden. Re-toggling the last element rebuilds the same order, so the old bug would not show.

  • Regression: reordering rows via drag still marks the form dirty (order comparison untouched), and toggling a Studio app genuinely on/off still does too.

  • Loom

Notes

Impact & Surface Area of Change

  • useAppControls drives the editor's Save Changes / Cancel CTAs. The only behavioural change is that a no-op reordering of hiddenAppIds no longer marks the form dirty.
  • src/utils/array.ts gains a new exported helper; no existing exports were modified.
  • No API, schema, or persistence changes — the stored hiddenAppIds shape is untouched.

@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
client-home-v3 Ready Ready Preview, Comment Jul 29, 2026 8:50am

Request Review

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown

Greptile Summary

The PR corrects editor dirty-state detection for the unordered hidden-app deny-list.

  • Adds a generic array membership-comparison helper that treats ordering and duplicates as insignificant and missing arrays as empty.
  • Uses the helper for hiddenAppIds while preserving order-sensitive comparison for the action sequence.
  • Adds unit coverage for reordered, changed, missing, duplicate, and toggle-cycle inputs.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable correctness or security issues identified.

The new comparison matches the deny-list’s membership-based semantics, while the order-sensitive action sequence remains unchanged and the relevant edge cases are covered.

Important Files Changed

Filename Overview
src/features/editor/hooks/useAppControls.tsx Replaces order-sensitive serialization of hiddenAppIds with membership equality while retaining sequence comparison for order.
src/utils/array.ts Adds a correctly scoped generic helper for comparing arrays as mathematical sets.
tests/unit/array.test.ts Covers the helper’s relevant equality cases and the reported hide/unhide/hide regression.

Reviews (1): Last reviewed commit: "fix: compare hiddenAppIds as a set for e..." | Re-trigger Greptile

`hiddenAppIds` is an unordered deny-list, but the toggle path rebuilds it by
filter-then-append, so a hide/unhide/hide cycle returns the same members in a
different position. The serialized comparison read that as a change and left
Save/Cancel visible on a clean form.

Adds `areArraysEqualAsSets` and uses it for `hiddenAppIds` only — `order` is a
genuine sequence and keeps its serialized comparison.

Reported by Greptile on #220.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@arpandhakal
arpandhakal force-pushed the arpandhakal/fix-hidden-app-ids-set-comparison branch from 70a2dd7 to 21e00e7 Compare July 29, 2026 08:49
@arpandhakal
arpandhakal merged commit 7e74900 into feature/studio-apps-action Jul 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant