fix: item metadata double-encoding + stale daemon dispatch binary - #403
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR prevents double-encoding of pre-stringified metadata during item creation and updates. It also validates the executable path from ChangesMetadata Encoding
Executable Dispatch
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server/item.rs`:
- Around line 62-73: Update metadata_to_json_string to validate Value::String
contents as JSON before storing them: preserve valid JSON strings, but serialize
invalid or empty strings as JSON string values (or reject the request
consistently with existing behavior). Keep non-string values serialized once,
and add regression tests covering empty and malformed string metadata so
parsed_metadata remains valid.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1834f0b9-6561-4bd0-a18a-5f324cf4365b
📒 Files selected for processing (2)
src/mcp_server/item.rssrc/supervisor.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server/item.rs`:
- Around line 71-76: The metadata_to_json_string helper currently parses
JSON-looking plain strings and loses their string type. Narrow or remove this
legacy unwrapping so values such as "true", "123", "null", "[]", and "{}" remain
JSON strings through parsed_metadata, while preserving intended pre-encoded
metadata behavior; add round-trip coverage for both item(create) and
item(update), which use this helper.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17f934d4-b572-4030-8dfd-ce51c4c733c5
📒 Files selected for processing (1)
src/mcp_server/item.rs
getappz
force-pushed
the
fix/dogfood-mcp-metadata-and-daemon-dispatch
branch
from
August 8, 2026 08:20
621ddb3 to
72d0462
Compare
added 3 commits
August 8, 2026 13:58
…ngified req.metadata: Option<serde_json::Value> can deserialize as Value::String when a caller sends already-JSON-encoded text instead of a native object. item_create/item_update blindly called .to_string() on whatever Value variant they got, which is correct for Value::Object but wraps a Value::String in another layer of escaped quotes -- exactly the double-encoding parsed_metadata() was already defensively unwrapping on the read side for size/kind, but with no fix at the write side, so any other reader (e.g. agentflare_backend::goal::parse_goal_metadata) still saw corrupted data. metadata_to_json_string() uses a Value::String as-is instead of re-stringifying it, fixing the write path at its source. Read-side unwrap in parsed_metadata() stays as a defense for rows written before this fix.
std::env::current_exe() can return Ok with a path that no longer exists: once the running binary's file is replaced (cargo install, package upgrade, agentflare update), it resolves via /proc/self/exe to the old, now-deleted inode -- Ok(path), but that path won't exec. A long-running daemon resolved this once per dispatch and got the same dead path every time, failing every job dispatch until someone noticed and restarted it. dispatch_item now checks the resolved path still exists on disk and falls back to a bare "agentflare" (resolved via PATH at spawn time, same fallback already used when current_exe() errors outright) rather than trust a stale exe path.
… JSON metadata_to_json_string() previously used ANY Value::String verbatim, but a caller can also legitimately send a genuine plain-string metadata value (metadata: "hello"), which is not JSON-encoded text and must not be stored raw -- that leaves the metadata column holding invalid JSON. Narrows the check to match parsed_metadata's own criterion: only treat the string as pre-encoded if its content successfully reparses as JSON.
getappz
force-pushed
the
fix/dogfood-mcp-metadata-and-daemon-dispatch
branch
from
August 8, 2026 08:29
72d0462 to
a34e319
Compare
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
1 participant
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
Two bugs found while dogfooding the supervisor/jobs dispatch pipeline and the item/goal model:
item_create/item_updatecalled.to_string()onreq.metadata: Option<serde_json::Value>unconditionally. When a caller sendsmetadataas already-JSON-encoded text rather than a native object, it deserializes asValue::String(json_text), and.to_string()on that wraps it in another layer of escaped quotes. There was already a documented defensive read-side unwrap for this (parsed_metadata, used byparsed_size/parsed_kind) but no write-side fix, so any other reader — e.g.agentflare_backend::goal::parse_goal_metadata— still saw corrupted data and silently treated a real goal as "not a goal". Fixed at the source withmetadata_to_json_string().dispatch_itemresolves its own binary viastd::env::current_exe()to build the job command. On Linux, once the running binary's file is replaced (cargo install, package upgrade,agentflare update),current_exe()keeps returningOkwith the old, now-deleted-inode path — which doesn't exec. This silently breaks every dispatch for a long-running daemon until someone notices and restarts it. Now checks the resolved path still exists and falls back to a bare"agentflare"(PATH lookup at spawn time) otherwise.Test plan
metadata_to_json_string(plain object vs. pre-stringified value)agentflare+agentflare-backendsuite (1240+ tests) passessupervisor::tests (9) pass unchanged after thecurrent_exe()fixSummary by CodeRabbit
PATHwhen the detected executable is unavailable.