Skip to content

init ExecutionId in DefaultOptions func#6598

Merged
Mzack9999 merged 6 commits intodevfrom
6594_init_exec_id
Jan 8, 2026
Merged

init ExecutionId in DefaultOptions func#6598
Mzack9999 merged 6 commits intodevfrom
6594_init_exec_id

Conversation

@dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Nov 10, 2025

Proposed changes

closes #6594

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Refactor
    • Changed when unique execution identifiers are set: default options are now pre-populated with an ExecutionId, and engine initialization no longer auto-assigns one.
    • No changes to public interfaces or externally visible behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@dogancanbakir dogancanbakir self-assigned this Nov 10, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Walkthrough

DefaultOptions now generates and embeds an ExecutionId; SDK constructors (lib.NewNucleiEngineCtx, lib/multi.go) removed their own xid-based ExecutionId assignment and xid imports, relying on provided options instead.

Changes

Cohort / File(s) Change Summary
SDK removal of ExecutionId assignment
lib/sdk.go, lib/multi.go
Removed import/use of github.com/rs/xid and deleted code that assigned a new ExecutionId during engine initialization paths.
DefaultOptions now populates ExecutionId
pkg/types/types.go
Added import of github.com/rs/xid and initialize DefaultOptions().ExecutionId with xid.New().String() so default options include a generated ExecutionId.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant Types as pkg/types.DefaultOptions
    participant SDK as lib.NewNucleiEngineCtx

    Dev->>Types: call DefaultOptions()
    activate Types
    Types-->>Dev: returns Options { ExecutionId = xid.New().String(), ... }
    deactivate Types

    Dev->>SDK: call NewNucleiEngineCtx(ctx, WithOptions(Options?))
    activate SDK
    SDK-->>SDK: does not set ExecutionId (uses provided Options)
    SDK-->>Dev: returns NucleiEngine (ExecutionId present if provided by Options)
    deactivate SDK
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I found a tiny ID in the loam,
Hopped it gently into DefaultOptions' home.
SDK stopped planting seeds by chance,
Now options wear the ID at first glance.
A quiet hop, a tidy dance.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective: initializing ExecutionId in the DefaultOptions function to prevent override issues.
Linked Issues check ✅ Passed The PR implements the fix for issue #6594 by moving ExecutionId initialization to DefaultOptions, ensuring it won't be overridden by WithOptions calls.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the ExecutionId initialization issue described in issue #6594; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 683bc04 and 942f9f0.

📒 Files selected for processing (3)
  • lib/multi.go
  • lib/sdk.go
  • pkg/types/types.go
💤 Files with no reviewable changes (2)
  • lib/sdk.go
  • lib/multi.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/types/types.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Lint

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc0f1e9 and 6a1451f.

📒 Files selected for processing (2)
  • lib/sdk.go (0 hunks)
  • pkg/types/types.go (2 hunks)
💤 Files with no reviewable changes (1)
  • lib/sdk.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

  • pkg/types/types.go
🧬 Code graph analysis (1)
pkg/types/types.go (1)
pkg/reporting/reporting.go (1)
  • New (79-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Lint
🔇 Additional comments (1)
pkg/types/types.go (1)

21-21: LGTM: Import addition is appropriate.

The xid import supports the ExecutionId auto-generation in DefaultOptions() and represents the shift of responsibility from the SDK initialization to the types package default constructor.

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

This looks like a good approach, but we need to ensure that the executionId is generated and persisted properly for the execution duration. For example NewThreadSafeNucleiEngineCtx requires a new ExecutionId as well, which is overwritten after DefaultOptions() is called, making the instruction apparently redundant at

defaultOptions.ExecutionId = xid.New().String()

@Mzack9999 Mzack9999 added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Jan 8, 2026
@Mzack9999 Mzack9999 merged commit a215d71 into dev Jan 8, 2026
18 checks passed
@Mzack9999 Mzack9999 deleted the 6594_init_exec_id branch January 8, 2026 11:53
@dwisiswant0 dwisiswant0 added this to the v3.7.0 milestone Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] fatal error when using WithOptions

3 participants