Skip to content

fix(mcp): server-derive artifact sender; default item(list) filter to identity (#75) - #207

Merged
getappz merged 2 commits into
masterfrom
fix/75-server-derive-identity
Jul 16, 2026
Merged

fix(mcp): server-derive artifact sender; default item(list) filter to identity (#75)#207
getappz merged 2 commits into
masterfrom
fix/75-server-derive-identity

Conversation

@getappz

@getappz getappz commented Jul 16, 2026

Copy link
Copy Markdown
Owner

What & why

Implements flare tracker item #75 (agent-identity hardening) — this is the agentflare item tracker, not GitHub issue #75 (an unrelated, already-closed ponytail item), so no GitHub close-keyword is used here. Stops trusting caller-supplied identity strings for authorship/scoping. The server already knows the calling agent non-spoofably (self.agent / claims::owner_id()); claim/heartbeat/release/done/comment/handoff already use it. This closes the two request fields that let a caller override it.

Full audit (item step 3)

Every caller-suppliable "which agent is acting" field on the MCP surface:

Field Struct Class Outcome
sender ArtifactRequest (publish) Authorship-sensitive — cross-agent handoff attribution the recipient trusts Removed — always self.agent
assignee_agent ItemRequest (list) Read-only filter Defaults to server identity when omitted; explicit still honored
assignee_agent ItemRequest (create/update) Assignment target, not authorship Unchanged (correct)
agent ReviewRequest (submit) Finder attribution; consensus counts distinct finders Kept caller-settable by design — see decision below
owner (claims/comment/handoff) Authorship Already server-derived, no change

Changes

  • artifact publish — dropped ArtifactRequest.sender; authorship is always the server-derived identity. Removing the field makes the spoof unrepresentable at the type level, which is stronger than a runtime "override is ignored" check (and makes such a check vacuous — so the old test cases that set sender: were removed).
  • item(list) — a bare list now defaults assignee_agent to the server identity, so it behaves like an inbox (mine + unassigned) instead of dumping every item, matching what the /handoff inbox prompt already tells callers to do manually. An explicit value is still honored — this is a read-only visibility filter (viewing a teammate's queue), not an authorization boundary. Falls back to no filter only when identity is undetected.
  • review submitagent (finder name) stays caller-settable by design, now with a documented SECURITY / step-3 rationale instead of an implicit fallback.

Tests

  • item_list_defaults_assignee_filter_to_server_identity — proves the new default and that an explicit assignee_agent still works.
  • artifact_publish_defaults_sender_to_agent_identity / artifact_list_filters_by_recipient_and_thread — updated for the removed field.

Verification

  • cargo test --workspace — 496 bin tests + all crates pass
  • cargo clippy --workspace --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic — clean
  • cargo fmt --check — clean

Design decision: ReviewRequest.agent stays caller-settable (resolved)

The original hand-off audit dismissed this field as "server fallback exists → leave as-is." That reasoning was wrong (the fallback is the hole), but the outcome is correct for a concrete reason:

  • The review findings DB is local, per-repo, single-user — no cross-principal trust boundary. A spoofed finder name is self-sabotage, not an attack on another party.
  • The /code-review orchestrator legitimately submits on behalf of many finder sub-agents, and consensus() counts distinct finder names — forcing one server identity would collapse them and break consensus, the feature's whole purpose.
  • Gating/removing buys no real security and breaks a real workflow.

So it's kept, with the classification documented in code. Unlike ArtifactRequest.sender (cross-agent handoff attribution another agent trusts), this is not an authorization boundary.

… identity (#75)

Stop trusting caller-supplied identity strings for authorship/scoping.

- artifact publish: remove ArtifactRequest.sender; always attribute to the
  server-derived identity (self.agent). Dropping the field makes sender
  spoofing unrepresentable at the type level rather than relying on a runtime
  override-ignored check.
- item(list): default assignee_agent to the server identity when omitted so a
  bare list behaves like an inbox (mine + unassigned) instead of dumping every
  item; an explicit value is still honored (read-only visibility filter).
- review submit: document why the finder `agent` stays caller-settable by
  design (local per-repo DB; /code-review orchestrator legitimately submits on
  behalf of many finder sub-agents; consensus counts distinct finder names).
- tests: add item(list) default + explicit-override regression; drop the now-
  vacuous artifact "explicit sender wins" cases.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

MCP artifact publishing now always assigns the server identity as sender. Item listing defaults omitted assignee filters to the server identity while preserving explicit filters and unassigned items.

Changes

MCP identity and filtering

Layer / File(s) Summary
Server-derived artifact authorship
src/mcp_server.rs
ArtifactRequest removes caller-provided sender input, publication uses self.agent, and related tests and review documentation are updated.
Default item assignee filtering
src/mcp_server/item.rs, src/mcp_server.rs
item(list) defaults omitted assignee filters to the server identity and tests both default and explicit filtering behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR changes MCP artifact and item filtering, not bare /ponytail reporting or reset behavior required by #75. Implement the /ponytail switcher changes: bare command reports current mode/scope/modes, and reset only via off/default.
Out of Scope Changes check ⚠️ Warning All shown code changes are unrelated to #75's /ponytail behavior and appear out of scope for the linked issue. Remove or split out the MCP server changes unless they are intended for a separate issue, and submit the /ponytail work separately.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly matches the main changes: server-derived artifact sender and default item list filtering.
Description check ✅ Passed The description covers the PR purpose, implementation details, tests, and review notes, though it doesn't follow the template headings exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/75-server-derive-identity

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

@getappz
getappz merged commit e9fdea9 into master Jul 16, 2026
14 checks passed
@getappz
getappz deleted the fix/75-server-derive-identity branch July 16, 2026 10:29
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