Skip to content

Revert "[9.5](backport #7794) refactor: clone ParsedPolicy in process… - #7804

Merged
ebeahan merged 1 commit into
elastic:9.5from
ebeahan:revert/9.5/pr-7794
Sep 11, 2026
Merged

Revert "[9.5](backport #7794) refactor: clone ParsedPolicy in process…#7804
ebeahan merged 1 commit into
elastic:9.5from
ebeahan:revert/9.5/pr-7794

Conversation

@ebeahan

@ebeahan ebeahan commented Sep 11, 2026

Copy link
Copy Markdown
Member

…Policy to prevent shared-state races (#7801)"

This reverts commit 14ae085.

…processPolicy to prevent shared-state races (elastic#7801)"

This reverts commit 14ae085.
@ebeahan ebeahan self-assigned this Sep 11, 2026
@ebeahan
ebeahan requested a review from a team as a code owner September 11, 2026 13:16
@ebeahan
ebeahan removed the request for review from a team September 11, 2026 13:16
@ebeahan
ebeahan requested review from blakerouse and ycombinator and a lite review from Copilot September 11, 2026 13:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical nested-map aliasing issue and a moderate monitor-locking issue remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Reverts the ParsedPolicy cloning refactor while retaining local policy-data copies during check-in processing.

Changes:

  • Removes ParsedPolicy.Clone() and its isolation test.
  • Restores shared policy dispatch and broader monitor locking.
  • Simplifies policy-data cloning while preserving per-request processing copies.
File summaries
File Summary
internal/pkg/policy/parsed_policy.go Removes parsed-policy cloning.
internal/pkg/policy/parsed_policy_test.go Removes clone isolation coverage.
internal/pkg/policy/monitor.go Restores shared dispatch; moderate (3 votes): global mutex is held during rate limiting and queue delivery.
internal/pkg/model/ext.go Reverts deep cloning; critical (3 votes): nested exporter maps remain shared and can race.
internal/pkg/api/handleCheckin.go Uses local policy-data and secret-key copies; nit (1 vote): comment wording is unclear.
internal/pkg/api/handleCheckin_test.go Updates concurrent processing coverage.
Review details

Suppressed comments (1)

internal/pkg/api/handleCheckin.go:1161

  • The new comment is grammatically incorrect and unclear; it should describe replacing the inputs, not “add replace” them.
	// Add replace inputs with agent prepared version.
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/pkg/model/ext.go
Comment thread internal/pkg/policy/monitor.go
@ebeahan
ebeahan enabled auto-merge (squash) September 11, 2026 13:52
@ebeahan
ebeahan merged commit 55cd7b7 into elastic:9.5 Sep 11, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants