FE-614: Support disabling SDCPN extensions#8763
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryMedium Risk Overview Core: Resolves effective UI: React context exposes Includes tests, Reviewed by Cursor Bugbot for commit 7194bc7. Bugbot is set up for automated code reviews on this repo. Configure here. |
github-actions
Bot
added
area/infra
🤖 Augment PR SummarySummary: Adds handle-level capability metadata so Petrinaut can run against “bare” Petri nets (e.g. CatCollab embeds) without exposing HASH SDCPN extensions. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| disabled={isReadOnly} | ||
| tooltip={isReadOnly ? UI_MESSAGES.READ_ONLY_MODE : "Add token type"} | ||
| disabled={isDisabled} | ||
| tooltip={isDisabled ? UI_MESSAGES.READ_ONLY_MODE : "Add token type"} |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/ui/views/Editor/panels/LeftSideBar/subviews/types-list.tsx:84 — When the button is disabled due to !extensions.colors, the tooltip uses UI_MESSAGES.READ_ONLY_MODE ("Editing is disabled"), which can mislead since the editor may still be writable. Consider using a capability-specific message for the extension-unavailable state (also applies to .../differential-equations-list.tsx:42).
Severity: low
Other Locations
libs/@hashintel/petrinaut/src/ui/views/Editor/panels/LeftSideBar/subviews/differential-equations-list.tsx:42
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| extensions: { | ||
| colors: !disabled.has("colors"), | ||
| stochasticity: !disabled.has("stochasticity"), | ||
| dynamics: !disabled.has("dynamics"), |
There was a problem hiding this comment.
libs/@hashintel/petrinaut-core/src/extensions.ts:54 — resolvePetrinautHandleCapabilities() can return { colors: false, dynamics: true }, but elsewhere dynamics is treated as effectively gated by extensions.colors && extensions.dynamics. That mismatch could confuse consumers treating extensions as the effective availability surface; consider normalizing extensions.dynamics (or exposing a derived canUseDynamics) to reflect the effective behavior.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| change(fn) { | ||
| if (capabilities?.readonly) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Handle change skips extension sanitization
Medium Severity
The change method in createJsonDocHandle only respects readonly capabilities. It doesn't account for disabledExtensions, allowing direct edits to introduce or retain extension-specific data (e.g., types, equations) that should otherwise be unavailable or stripped.
Reviewed by Cursor Bugbot for commit 0ac1966. Configure here.
| mutate, | ||
| () => definition.get(), | ||
| capabilities.extensions, | ||
| ); |
There was a problem hiding this comment.
No sanitize on instance create
High Severity
The createPetrinaut function's definition store exposes the document without sanitizing data for disabled extensions. This allows initial or loaded SDCPNs to retain types, stochastic transitions, or dynamics, causing inconsistencies between UI capabilities and the underlying data until a mutation eventually applies stripDisabledExtensionData.
Reviewed by Cursor Bugbot for commit 0ac1966. Configure here.
| sanitizeTransitionForExtensions(transition, extensions), | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Strip ignores scenario initial state
Medium Severity
stripDisabledExtensionData clears types, differential equations, and place or transition extension fields, but leaves scenarios (and their initialState payloads) untouched. After the first guarded mutation with colors disabled, the net can have no types while scenarios still describe colored token matrices for places, producing an inconsistent SDCPN for simulation and scenario UI.
Reviewed by Cursor Bugbot for commit 0ac1966. Configure here.
| fn(sdcpn); | ||
| stripDisabledExtensionData(sdcpn, extensions); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Undo restores stripped extensions
Medium Severity
stripDisabledExtensionData runs in the same handle.change transaction as the user’s edit. Undo applies inverse patches for the whole transaction, so extension data removed during sanitization can reappear in the stored document after undo even though capabilities still disable those extensions.
Reviewed by Cursor Bugbot for commit 0ac1966. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 7 total unresolved issues (including 4 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7194bc7. Configure here.
| try { | ||
| sim = await createSimulation({ | ||
| sdcpn, | ||
| sdcpn: simulationSdcpn, |
There was a problem hiding this comment.
Simulation ignores disabled extensions
High Severity
When SDCPN extensions are disabled, the SimulationProvider only sanitizes global parameters before passing the definition to the simulation engine. Other disabled extensions, such as colors, stochasticity, dynamics, and token types, are not removed. This can cause the simulation to use data from disabled features, leading to behavior that doesn't align with the UI.
Reviewed by Cursor Bugbot for commit 7194bc7. Configure here.
| stripDisabledExtensionData(sdcpn, extensions); | ||
| newItemIds = result.newItemIds.filter((item) => | ||
| isSelectionTypeAvailableForExtensions(item.type, extensions), | ||
| ); |
There was a problem hiding this comment.
Auto layout skips extension strip
Medium Severity
The applyAutoLayout command uses a direct mutate call and doesn't invoke stripDisabledExtensionData. This can leave data from disabled extensions in the document, unlike other mutation paths (like clipboard paste) that now correctly remove it.
Reviewed by Cursor Bugbot for commit 7194bc7. Configure here.
|
|
||
| return { | ||
| id, | ||
| capabilities, |
There was a problem hiding this comment.
Undo restores disabled extension data
Medium Severity
History undo and redo replay Immer patches without re-applying extension sanitization. Stepping back to an earlier checkpoint can restore token types, stochastic transitions, parameters, and other fields that guarded mutations had stripped for the current capability set.
Reviewed by Cursor Bugbot for commit 7194bc7. Configure here.
1 participant
🌟 What is the purpose of this PR?
Allow Petrinaut document handles to declare which SDCPN extensions and global parameters are unavailable, so hosts such as CatCollab can embed Petrinaut around a bare Petri net without exposing HASH-specific extensions.
This introduces handle-level capabilities for read-only mode and extension availability, then adapts core mutation paths, runtime data paths, and the editor UI to respect those capabilities.
🔗 Related links
🔍 What does this change?
PetrinautHandleCapabilitieswithreadonlyanddisabledExtensionsmetadata.colors,stochasticity,dynamics, andparameters.dynamicsis resolved as disabled whenevercolorsis disabled.Petrinaut/ExtensionsDisabledStorybook coverage for a bare Petri net with SDCPN extensions and global parameters disabled.ANALYSIS.mdin@hashintel/petrinautdescribing touchpoints, tradeoffs, and follow-ups.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
CatCollabPetriNetHandle; it only adds the capability surface and UI/core behavior it can use later.🐾 Next steps
CatCollabPetriNetHandleonce CatCollab's source model and edit semantics are finalized.🛡 What tests cover this?
yarn lint:eslintinlibs/@hashintel/petrinaut-coreyarn lint:tscinlibs/@hashintel/petrinaut-coreyarn test:unit --runinlibs/@hashintel/petrinaut-coreturbo run lint:tsc --filter='@hashintel/petrinaut'turbo run lint:eslint --filter='@hashintel/petrinaut'turbo run test:unit --filter='@hashintel/petrinaut' -- --runyarn lint:markdownlint libs/@hashintel/petrinaut/ANALYSIS.md .changeset/petrinaut-sdcpn-capabilities.mdgit diff --check❓ How to test this?
Petrinaut/ExtensionsDisabled.capabilities.disabledExtensionsset to['colors']and confirminstance.extensions.dynamicsresolves tofalse.capabilities.disabledExtensionsset to['parameters']and confirm global parameter UI and simulation defaults are unavailable.capabilities.readonlyand confirm edit mutations are ignored and the editor reports read-only state.