fix(permissions): resolve 8 bugs preventing blueprint/action permissions from importing#70
Merged
EricFernandezPort merged 2 commits intoMay 28, 2026
Conversation
…ons from importing - Fix JSON loader ignoring permissions: loadJSON() now reads BlueprintPermissions and ActionPermissions keys (both PascalCase and snake_case for backward compat) - Fix JSON export key mismatch: WriteJSON() now emits snake_case keys consistent with tar format (blueprint_permissions, action_permissions) - Fix false success on permission errors: Execute() now sets Success=false when errors are present instead of unconditionally reporting success - Fix --include flag not enforced: permission diff now gated by shouldImport() for blueprint-permissions and action-permissions resource types - Fix unreliable permission comparison: replace reflect.DeepEqual with resourcesEqual() for normalized JSON comparison (prevents false positives from type mismatches after JSON unmarshaling) - Fix dry-run not reporting permissions: add BlueprintPermissionsUpdated and ActionPermissionsUpdated to Result struct, populate in dry-run and real import, display in CLI text and JSON output - Fix silent permission fetch failures: collector now records warnings when blueprint/action permission GETs fail instead of silently skipping - Fix migrate ignoring permissions: exportFromSource() now collects blueprint and action permissions; importToTarget() applies permission changes after resource import; dry-run includes permission counts - Fix CLI swallowing error details: import command now prints full results (counts, diff analysis, categorized errors) before returning error status Co-authored-by: Cursor <cursoragent@cursor.com>
EricFernandezPort
added a commit
to bgilleran-port/port-cli
that referenced
this pull request
May 28, 2026
- fix: importPermissions now returns actual success counts (not planned
diff size); previously BlueprintPermissionsUpdated/ActionPermissionsUpdated
reported the full diff even when some API calls failed
- fix: gate action-permissions collection in migrate.go behind
shouldCollect("action-permissions") to match blueprint-permissions
behavior; previously --include actions fetched action permissions
unconditionally
- fix: record warnings in migrate.go exportFromSource when blueprint or
action permission fetches fail, consistent with collector.go behavior
- test: add TestImportPermissions_CountsOnlySuccesses
- test: add TestLoader_LoadJSON_LegacyPascalCasePermissions and SnakeCase
- test: add TestExportFromSource_ActionPermissionsNotCollectedWhenExcluded
- test: add TestExportFromSource_ActionPermissionsFetchFailureRecordsWarning
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Entire-Checkpoint: efb168341b34
062cec9 to
794793e
Compare
EricFernandezPort
added a commit
to bgilleran-port/port-cli
that referenced
this pull request
May 28, 2026
- importPermissions returns actual success counts (not planned diff counts) - shouldCollect gate on action-permissions loops in migrate.go - Warnings recorded when permission fetches fail (blueprint and action) - 5 new tests covering count tracking, exclusion gating, and failure warnings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: 8141444f4dfe
EricFernandezPort
added a commit
to bgilleran-port/port-cli
that referenced
this pull request
May 28, 2026
- importPermissions returns actual success counts (not planned diff counts) - shouldCollect gate on action-permissions loops in migrate.go - Warnings recorded when permission fetches fail (blueprint and action) - 5 new tests covering count tracking, exclusion gating, and failure warnings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: 8141444f4dfe
33817b2 to
3c6462c
Compare
- importPermissions returns actual success counts (not planned diff counts) - shouldCollect gate on action-permissions loops in migrate.go - Warnings recorded when permission fetches fail (blueprint and action) - 5 new tests covering count tracking, exclusion gating, and failure warnings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: 8141444f4dfe
3c6462c to
9714f8c
Compare
3 tasks
bgilleran-port
pushed a commit
to bgilleran-port/port-cli
that referenced
this pull request
Jun 5, 2026
Add missing tests for the page permissions feature introduced in PRs port-experimental#70 and port-experimental#71, following Eric's established test patterns: - TestCollector_CollectsPagePermissions: positive collection verification - TestLoader_LoadJSON_PagePermissions: JSON round-trip loading - TestComparePermissions_DetectsExtraFieldsAsChange: diff detection - TestComparePermissions_NormalizesStringSliceOrder: slice normalization - Updated TestDiffResult_PermissionsFields to include PagePermissions Co-authored-by: Cursor <cursoragent@cursor.com>
EricFernandezPort
added a commit
that referenced
this pull request
Jun 5, 2026
…nt fields (#75) * fix(migrate): add topological sorting and retry for dependent blueprint fields The migrate module was applying aggregationProperties, mirrorProperties, and ownership concurrently without dependency ordering, while the import module already had these protections. This caused the Port API to silently nullify pathFilter in aggregation properties and drop inherited ownership when dependencies weren't ready yet. Changes: - Phase 2c (mirrorProperties): collect failures for retry after Phase 2d - Phase 2d (aggregationProperties): use TopologicalSortAggProps to ensure cross-blueprint agg prop dependencies are applied in order - Phase 2c retry: re-apply failed mirror props after agg props exist - Phase 2e (ownership): use TopologicalSortOwnership to ensure inherited ownership chains are applied in dependency order - Phase 3 retry: re-apply failed agg props after all other phases complete Fixes: aggregationProperties.pathFilter being silently nullified and inherited ownership being dropped during port migrate. Co-authored-by: Cursor <cursoragent@cursor.com> * test(permissions): add test coverage for page permissions Add missing tests for the page permissions feature introduced in PRs #70 and #71, following Eric's established test patterns: - TestCollector_CollectsPagePermissions: positive collection verification - TestLoader_LoadJSON_PagePermissions: JSON round-trip loading - TestComparePermissions_DetectsExtraFieldsAsChange: diff detection - TestComparePermissions_NormalizesStringSliceOrder: slice normalization - Updated TestDiffResult_PermissionsFields to include PagePermissions Co-authored-by: Cursor <cursoragent@cursor.com> * test(migrate): add tests for dependent field ordering and retry - TestImportToTarget_AggPropsAppliedInTopologicalOrder: verify cross- blueprint agg prop dependencies are applied in correct order - TestImportToTarget_FailedAggPropsRetried: verify failed agg prop updates are retried in a second pass - TestImportToTarget_OwnershipAppliedInTopologicalOrder: verify inherited ownership is applied after its dependency Co-authored-by: Cursor <cursoragent@cursor.com> * fix(review): apply pre-landing review fixes - Validate that page-permissions requires pages in --include (export, import, migrate, compare) to prevent silent empty export and idempotency-breaking force-overwrite - Fix invalidPermBodyPattern regex: add (?s) flag so multi-line JSON 422 bodies are matched - Fix migrate errors section mislabeled as "Warnings:" → "Errors:" - Add blueprint_permissions_updated and action_permissions_updated to migrate JSON output - Strengthen import_test retryBody nil guard: use t.Fatal instead of silent skip - Update ValidationWarning.Type comment to include "orphaned_permission_field" - Rewrite CHANGELOG 0.2.19 entry to describe features and fixes, not test names Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: a0896ec6ff15 * style: run make format Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: 55a24cc72286 --------- Co-authored-by: Bill Gilleran <bgilleran@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: EricFernandezPort <ericf@getport.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
3 participants
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.
Summary
loadJSON()never readBlueprintPermissionsorActionPermissionskeys, silently dropping all RBAC data on JSON-format imports. Now handles both PascalCase (legacy) and snake_case keys.WriteJSON()emitted PascalCase keys (BlueprintPermissions) while every other field and the tar format used snake_case. Aligned toblueprint_permissions/action_permissions.Execute()unconditionally setSuccess: trueafter permission import, masking failures. Now setsSuccess: falsewith error count when any errors occur.--includeflag not enforced for permissions: Permission diff ran unconditionally regardless of--includefilter. Now properly gated byshouldImport("blueprint-permissions")/shouldImport("action-permissions").reflect.DeepEqualcomparison: Permission diff used rawreflect.DeepEqualwhich produces false positives from JSON type mismatches. Replaced withresourcesEqual()for normalized JSON comparison.BlueprintPermissionsUpdatedandActionPermissionsUpdatedtoResultstruct, populated in both dry-run and real import, displayed in CLI text and JSON output.Data.Warningsfor visibility.port migrateignoring permissions entirely:exportFromSource()never collected permissions;importToTarget()never applied them. Added full blueprint/action permission collection and import pass with dry-run counts.!result.Successbefore printing any counts, diff analysis, or categorized errors. Now prints full results before returning error status.Files changed (8)
internal/modules/import_module/loader.goloadJSON()internal/modules/export/export.goWriteJSON()to snake_case keysinternal/modules/import_module/import.gointernal/modules/import_module/diff.go--includegating + normalized comparison for permissionsinternal/modules/export/collector.goWarningsfield, record permission fetch failuresinternal/modules/migrate/migrate.gointernal/commands/import.gointernal/modules/export/export_test.goTest plan
go vet ./...passesmake formatappliedmake test— all tests pass (including updatedTestWriteJSON_IncludesPermissions)blueprint_permissions.json(48 entries, 156KB) andaction_permissions.json(16 entries) present in tar.gzMade with Cursor