Skip to content

Add worker and local dev nugget tools#15

Open
tj-cisco wants to merge 23 commits into
mainfrom
sdk_tools
Open

Add worker and local dev nugget tools#15
tj-cisco wants to merge 23 commits into
mainfrom
sdk_tools

Conversation

@tj-cisco

Copy link
Copy Markdown
Collaborator

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2af2694f1f

ℹ️ 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".

Comment thread src/rzb_dev.c Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 260d33549d

ℹ️ 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".

Comment thread src/rzb_dev.c Outdated
Comment thread src/rzb_worker.c
@tj-cisco

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 260d33549d

ℹ️ 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".

Comment thread src/rzb_worker.c

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98b5d92d8d

ℹ️ 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".

Comment thread src/command_and_control.c
Comment on lines +128 to +131
rzb_log(LOG_ERR, LOG_C_CNC,
"%s: Legacy dispatcher C&C runtime was removed for dispatcher-next; "
"use RzbNextRuntime instead", __func__);
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Allow rzb-worker to initialize normal nuggets

When rzb-worker reaches module.initNug(), a typical nugget creates its normal inspection/collection context without CONTEXT_FLAG_DEV_TOOL; that path calls CommandAndControl_Start(), and this new unconditional failure for every non-dev context makes Razorback_Init_*_Context() return NULL, causing initNug() and the worker startup to fail before any dispatcher-next runtime handoff exists. Either keep a working non-dev startup path here or have rzb-worker initialize through RzbNextRuntime before rejecting legacy C&C.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 291c9842d0

ℹ️ 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".

Comment thread src/rzb_dev.c
goto cleanup;
}

(void)Block_MetaData_Add_FileName(inputItem->pEvent->pBlock, filePath);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add filename metadata with a NUL-terminated helper

When rzb-dev prepares the local inspection block, this call adds filename metadata through Block_MetaData_Add_FileName(), whose current implementation stores only strlen(fileName) bytes even though it labels the value as NTLV_TYPE_STRING; unlike Metadata_Add_Filename(), the copied data is not NUL-terminated. Local inspections therefore receive malformed filename string metadata, and nuggets that read it as the existing string metadata convention permits can read past the buffer or see garbage. Use the metadata helper that includes the terminator, or fix the block helper before feeding this metadata to the nugget.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3408249f4

ℹ️ 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".

Comment thread src/fileserver.c
Comment on lines +843 to +847
block->data.fileName = fileName;
block->data.tempFile = tempFile;
block->data.pointer = mmap(NULL, block->pId->iLength, PROT_READ, MAP_PRIVATE,
fileno(block->data.file), 0);
if (block->data.pointer == MAP_FAILED) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear block file state when mmap fails

If mmap fails after block->data.fileName has been assigned, this returns false with the block still pointing at that filename even though the FILE has been closed. The local-dev caller now has a special cleanup path, but the public helper is also used by RzbNextFileserver_FetchBlock, which frees the filename on failure; callers that then retry, inspect, or call RzbNextFileserver_FreeBlockData on the same block can hit a dangling pointer/double free. Move these assignments until after the map succeeds or clear fileName/tempFile on this failure path.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a7ff0fbf8

ℹ️ 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".

Comment thread src/health.c
Comment on lines +725 to +727
bool live = Health_EvaluateInternal(RAZORBACK_HEALTH_LIVE);
bool ready = Health_EvaluateInternal(RAZORBACK_HEALTH_READY);
bool startup = Health_EvaluateInternal(RAZORBACK_HEALTH_STARTUP);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep probe endpoints from running unrelated checks

For a /livez, /readyz, or /startupz request this now evaluates all three probe kinds to render the JSON body, and Health_EvaluateInternal() runs the registered callbacks for each kind. A slow dependency/readiness callback can therefore block liveness probes (and the requested kind is evaluated twice), causing probe timeouts or side effects even though the endpoint status only depends on kind. Render these fields from cached/built-in state or avoid invoking unrelated custom checks on individual probe endpoints.

Useful? React with 👍 / 👎.

Comment thread api.vcxproj
<ClCompile Include="src\connected_entity.c" />
<ClCompile Include="src\crypto.c" />
<ClCompile Include="src\daemon.c" />
<ClCompile Include="src\dev_mode.c" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Compile dispatcher-next sources in the MSVC project

When building with api.vcxproj, this adds only dev_mode.c; the same commit exposes public dispatcher-next APIs but this project still omits their implementations (config_next.c, fileserver.c, message_body.c, messages_next.c, and runtime_next.c) that are included in the autotools build. MSVC builds will produce a library without those new symbols, so Windows consumers using the new headers get unresolved externals. Add the same new sources to the project and filters.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a98d6cc5a6

ℹ️ 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".

Comment thread src/fileserver.c
blockId->pHash->iType == HASH_TYPE_SHA256 &&
blockId->pHash->iSize == 32 &&
(blockId->pHash->iFlags & HASH_FLAG_FINAL) != 0 &&
blockId->iLength > 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow zero-length SHA-256 blocks

When a collector submits an empty file via BlockPool_AddData_FromFile, BlockPool_FinalizeItem can still produce a finalized SHA-256 block with iLength == 0, but all of the new fileserver store/fetch paths go through this helper and return RZB_NEXT_FILESERVER_LOCAL_ERROR before building the canonical ... .0 path or making the HTTP request. That makes dispatcher-next reject valid empty-file submissions that the previous file transfer path could store, so the identity check should not require a positive length (with zero-length mmap handling addressed separately).

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c200167ae

ℹ️ 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".

Comment thread src/dev_mode.c
return;

Mutex_Lock(sg_devCaptureLock);
(void)List_Remove(sg_devCaptures, context);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove dev captures using the capture object

When a dev-mode context is destroyed, this passes the RazorbackContext * as the list item to remove even though sg_devCaptures stores struct RazorbackDevContextCapture *; List_Remove() compares with the item comparator rather than DevCapture_KeyCmp, so DevCapture_Cmp interprets the context memory as a capture and normally never removes the real entry. After shutdownNug()/context teardown in rzb-dev or in tests that create dev contexts repeatedly, the stale capture retains old judgments/submissions and a freed context pointer, so a later context can leak capture state or reuse stale results if the address is recycled.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant