GV-10: Gate public projections by current visibility - #695
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 393ef657e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
393ef65 to
c1c51b5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1c51b5c12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review Stabilization Ledger - Cycle 2PR: #695 Review Timeline
Missing Invariant FamilyPublic fanout needs a single structural source-scope invariant:
Fresh Findings To Prove
Stabilization Proof ObligationsBefore requesting another review, the fix must prove the source-scope invariant across the touched fanout paths, not just the three comments:
Stabilization DecisionContinue #649 with a stabilization fix worker. This does not require a new product decision or a new durable/public contract concept; it is enforcement of the already-approved #649 visibility and reveal-publication contract. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5208291f3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| return | ||
| promotionSetContext | ||
| |> Option.map (fun (promotionSet, _) -> allowsPublicProjection promotionSet.Visibility promotionSet.Ownership) |
There was a problem hiding this comment.
When a public/repository-owned PromotionSet is logically deleted, PromotionSetActor.Exists still returns true and Get returns the DTO with DeletedAt = Some, but this current-source check ignores DeletedAt. Any stale redelivery for that PromotionSet (apply, queue, validation, work-item, etc.) therefore still passes the new visibility gate and can publish automation/webhook data for a resource that is no longer observable; include the deleted state in this decision, with any intentional deletion notification handled explicitly.
Useful? React with 👍 / 👎.
| match reviewEvent.Event with | ||
| | ReviewEventType.NotesUpserted notes -> notes.PromotionSetId | ||
| | ReviewEventType.CheckpointAdded checkpoint -> checkpoint.PromotionSetId | ||
| | ReviewEventType.FindingResolved _ -> tryGetPromotionSetIdFromMetadata reviewEvent.Metadata |
There was a problem hiding this comment.
For ReviewEventType.FindingResolved, the event payload has no PromotionSetId, and Review.Server.processCommand only copies inherited visibility/ownership into metadata, not the PromotionSet id. This branch commonly returns None, so currentSourceAllowsPublicProjection treats redelivered finding-resolution events as unscoped and still emits ReviewNotesUpdated after the associated PromotionSet has been logically deleted/hidden; carry the Review actor's PromotionSet id in the event metadata or derive it before allowing fanout.
Useful? React with 👍 / 👎.
| | ValidationResultEvent validationResultEvent -> | ||
| match validationResultEvent.Event with | ||
| | ValidationResultEventType.Recorded result -> result.PromotionSetId | ||
| | ArtifactEvent artifactEvent -> tryGetPromotionSetIdFromMetadata artifactEvent.Metadata |
There was a problem hiding this comment.
When PromotionSetActor creates conflict-report artifacts it calls ArtifactCommand.Create with actor metadata, which sets ActorId to the PromotionSet id but does not add PromotionSetId. This branch only looks for PromotionSetId, so those ArtifactCreated notifications are treated as unscoped; if the public PromotionSet is logically deleted before the artifact event is redelivered, the current-source recheck is skipped and the artifact id can still be published. Fall back to ActorId for PromotionSet-sourced artifact metadata.
Useful? React with 👍 / 👎.
Third-Cycle Stabilization StopCoding is stopped for #649 / PR #695 after the third substantive Codex Code Review Bot cycle on the high-risk public fanout surface. Review Timeline
Suspected Missing Invariant FamilyThe existing ledger was directionally right, but still too narrow. The broader invariant is:
Maintainer Decisions NeededA local unblock page has been generated at:
It asks for explicit approval on:
No further coding or fix worker will be assigned until the maintainer approves the revised issue/ledger or explicitly directs continued patching. |
1 participant
Related to #649.
Summary
Surface Preflight Outcomes
WebhookDispatch.dispatchCommittedEventAsyncWithCurrentVisibilitybefore delivery creation.tryCreateEnvelopeWithCurrentVisibilitybefore public automation envelopes are created.Validation
dotnet tool run fantomas src/Grace.Server/Eventing.Server.fs src/Grace.Server/WebhookDispatch.Server.fs src/Grace.Server/Notification.Server.fs src/Grace.Server.Unit.Tests/Eventing.Server.Tests.fs src/Grace.Server.Unit.Tests/WebhookDispatch.Server.Tests.fspassed.dotnet tool run fantomas src/Grace.Server/Notification.Server.fs src/Grace.Server.Unit.Tests/Notification.Server.Tests.fspassed.dotnet build --configuration Release src/Grace.Server.Unit.Tests/Grace.Server.Unit.Tests.fsprojpassed.dotnet test --no-build --configuration Release src/Grace.Server.Unit.Tests/Grace.Server.Unit.Tests.fsproj --filter "FullyQualifiedName~AutomationEventingTests|FullyQualifiedName~WebhookDispatchUnitTests|FullyQualifiedName~NotificationServerTests"passed: 53 passed.pwsh ./scripts/validate.ps1 -Fastpassed after the final diff.git diff --checkpassed.Review Status
5208291f.29014521720, job86106064372.5208291fwith three fresh source-scope findings.C:\Users\scott\AppData\Local\Temp\grace-638-orchestration\grace-649-unblock-current.html.