feat: support server-owned session IDs with body/header config#167
Merged
jithin23-kv merged 2 commits intoJul 6, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughIntroduces structured session configuration for HTTP-based agent targets, including session planning, request injection, and response capture. The change is wired through core execution, autonomous targets, SDK, CLI, MCP setup flows, tests, and updated documentation. ChangesSession Handling Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant AgentAttackDriver
participant agentTarget
participant httpClient
participant RemoteTarget
AgentAttackDriver->>httpClient: resolveSessionPlan(targetConfig)
httpClient-->>AgentAttackDriver: SessionPlan
AgentAttackDriver->>agentTarget: send(prompt, captureSession)
agentTarget->>httpClient: applySessionToRequest(plan, sessionId)
agentTarget->>RemoteTarget: fetch(request)
RemoteTarget-->>agentTarget: response
agentTarget->>httpClient: captureSessionFromResponse(response, plan)
httpClient-->>agentTarget: sessionId
agentTarget->>AgentAttackDriver: captureSession(sessionId)
AgentAttackDriver->>agentTarget: next send with reused sessionId
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 10
🧹 Nitpick comments (6)
core/tests/agentTargetSession.test.ts (1)
124-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCookie test only checks capture, not the turn-2 echo.
Unlike the body/header server-owned tests, this test doesn't verify that the captured cookie pair is actually echoed back via the
Cookieheader on turn 2. Adding that assertion would close a coverage gap for a format-sensitive path (raw"name=value"cookie serialization).✅ Suggested addition
let captured: string | undefined; await target.send("turn1", { sessionId: undefined, captureSession: (id) => (captured = id) }); assert.equal(captured, "sid=cookie-1"); + + await target.send("turn2", { sessionId: captured }); + assert.equal(received[1].sessionHeader, undefined); });🤖 Prompt for 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. In `@core/tests/agentTargetSession.test.ts` around lines 124 - 139, The server-owned cookie test in agentTargetSession.test.ts only verifies that `captureSession` receives the cookie pair from the first turn; update the same test around createAgentTarget and target.send so it also performs a second send using the captured session and asserts the outgoing Cookie header contains the echoed raw `sid=cookie-1` value. Reuse the existing test name and the send/receive session config to validate both capture and turn-2 echo behavior for the Cookie path.core/src/execute/agentAttackDriver.ts (1)
160-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the capture-miss warning actionable.
The message reports what happened but doesn't tell the operator what to check (e.g., verify the target actually returns a session id at the configured
receivelocation, or confirm the target is configured for server-owned sessions at all).As per coding guidelines: "Write actionable error messages that tell the user what to fix, not just what went wrong."
🤖 Prompt for 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. In `@core/src/execute/agentAttackDriver.ts` around lines 160 - 167, The capture-miss warning in agentAttackDriver’s session handling is too generic; update the log in the branch that checks this.sessionPlan.mode, capturedSessionId, and warnedCaptureMiss to tell the operator what to verify. Make the message actionable by pointing them to confirm the target returns a session id at the configured receive location and that the target is actually set up for server-owned sessions, while keeping the existing fallback behavior unchanged.Source: Coding guidelines
runners/sdk/src/hunt.ts (1)
109-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant legacy-to-
sessionconversion; core already does this.
resolveSessionPlanin core already derivessendfrom a rawsessionField/sessionIdFieldwhensessionis absent (send = source.session?.send ?? (legacyName ? {...} : undefined)). Since this code also passes the rawsessionFieldthrough assessionField:, manually rebuildingsession.sendhere duplicates that fallback logic in two places, risking drift if core's legacy-mapping rule changes.♻️ Simplify by relying on core's fallback
- session: - options.target.session ?? - (options.target.sessionField - ? { send: { in: "body", name: options.target.sessionField } } - : undefined), + session: options.target.session, sessionField: options.target.session ? undefined : options.target.sessionField,🤖 Prompt for 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. In `@runners/sdk/src/hunt.ts` around lines 109 - 114, The session mapping in hunt.ts is duplicating core’s legacy fallback logic by rebuilding session.send from sessionField before passing it onward. Simplify the options object around the session/sessionField handling so it only forwards the raw sessionField when session is absent and lets resolveSessionPlan in core derive send itself, keeping the behavior centralized and avoiding drift.runners/sdk/src/run.ts (1)
99-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame redundant legacy-to-
sessionderivation ashunt.ts.
resolveSessionPlanalready falls back fromsessionIdField/sessionFieldto a{in:"body",name}send object whensessionis unset (see context snippet fromcore/src/targets/httpClient.ts). Manually rebuilding it here duplicates that logic.♻️ Simplify by relying on core's fallback
- // Prefer the structured `session`; fall back to legacy `sessionField` (body). - session: - httpTarget.session ?? - (httpTarget.sessionField - ? { send: { in: "body", name: httpTarget.sessionField } } - : undefined), + session: httpTarget.session, sessionIdField: httpTarget.session ? undefined : httpTarget.sessionField,🤖 Prompt for 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. In `@runners/sdk/src/run.ts` around lines 99 - 105, The session mapping in run.ts duplicates the legacy-to-session fallback already handled by resolveSessionPlan in the HTTP client core. Update the code around the session/sessionIdField construction in the run() flow to pass through httpTarget.session and httpTarget.sessionField directly, and let core/src/targets/httpClient.ts perform the fallback instead of rebuilding the { in: "body", name } object here. Use the existing identifiers resolveSessionPlan, session, and sessionIdField to locate and simplify this logic.runners/mcp/src/index.ts (1)
428-441: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
receivecan be built withname: undefinedfor body/header, silently disabling capture.
receive = receiveIn ? { in: receiveIn, name: receiveName } : undefinedallowsreceiveIn: "body"or"header"with noreceiveName. PercaptureSessionFromResponse(core/src/targets/httpClient.ts), a body/header receive without a name always returnsundefined, so the LLM-driven setup would silently produce a server-owned session that never actually captures an id. Only"set-cookie"is valid without a name.♻️ Proposed fix
const send = sendIn && sendName ? { in: sendIn, name: sendName } : undefined; - const receive = receiveIn ? { in: receiveIn, name: receiveName } : undefined; + const receive = + receiveIn === "set-cookie" + ? { in: receiveIn, name: receiveName } + : receiveIn && receiveName + ? { in: receiveIn, name: receiveName } + : undefined; if (!send && !receive) return undefined; return { send, receive };🤖 Prompt for 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. In `@runners/mcp/src/index.ts` around lines 428 - 441, Update buildSessionConfig so receive is only created when the inputs are valid for captureSessionFromResponse: body and header must require a non-empty receiveName, while set-cookie may omit it. Right now buildSessionConfig in runners/mcp/src/index.ts can return a receive object with name: undefined, which later gets ignored and silently disables session capture; validate receiveIn/receiveName before constructing SessionConfig and return undefined or omit receive when the combination is invalid. Use the buildSessionConfig function and the SessionConfig receive shape as the main points to adjust.runners/cli/ui/src/components/SetupPage.tsx (1)
205-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider guiding users toward the correct Send config when Set-Cookie is selected.
When
sessionReceiveIn === "set-cookie", the target's session id must be echoed via aCookieheader (seerunners/cli/src/commands/setup.ts'scollectSessionConfig, which auto-selects this). Here the Send location/name fields remain freely editable with no hint, so a user can easily leave them at body/session_idand end up with a broken server-owned session. Consider disabling/pre-filling the Send fields (or showing a note) when Set-Cookie is chosen, consistent with the mapping fix suggested inrunners/cli/src/ui/server.ts.Also applies to: 230-255
🤖 Prompt for 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. In `@runners/cli/ui/src/components/SetupPage.tsx` around lines 205 - 229, When sessionReceiveIn is set to set-cookie in SetupPage, the Send location/name controls should guide users to the required Cookie header mapping instead of оставing them editable with a misleading default. Update the SetupPage form logic for the sessionSendIn/sessionSendName fields to either prefill and lock them to the correct values or display a clear note tied to the set-cookie case, matching the behavior in collectSessionConfig and the UI pattern used in server.ts.
🤖 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 `@core/src/autonomous/target/http.ts`:
- Around line 39-59: Add a one-time warning in createTargetClient when the
server-owned session path fails to capture a session id. In the send method,
after httpSend returns, detect the case where plan.mode is "server" but
result.sessionId is missing and emit a single warning using a persistent guard
so it only logs once per client. Reference createTargetClient, send, and the
serverSessions flow so the warning clearly points to the
session.receive/response-path mismatch that leaves later turns with undefined
sessionId.
In `@core/src/config/schema.ts`:
- Around line 104-111: The session schema currently allows receive.in values for
body/header without requiring a usable name, which lets parse succeed even
though captureSessionFromResponse() cannot capture or echo the session id.
Update the session.receive validation in schema.ts to use location-specific
branches so body/header require a non-empty trimmed name while set-cookie
remains optional, and keep the existing legacy send alias behavior unchanged.
In `@core/src/execute/agentAttackDriver.ts`:
- Around line 49-55: Persist the server-owned session state across
AgentAttackDriver resumes by carrying the captured session id and send counter
through AgentAttackContext or the constructor. The reset happens in
AgentAttackDriver when it is rebuilt from initialHistory, so update the
initialization path for capturedSessionId and sendCount so resumed turns in
sessionPlan.mode === "server" reuse the existing conversation instead of sending
undefined and minting a new one.
In `@core/src/execute/types.ts`:
- Around line 10-18: Tighten SessionConfig so it only allows valid session
flows: a server-returned session id should only be representable when paired
with a matching send path, and receive should require name for body/header while
allowing name to be omitted only for set-cookie. Update the SessionConfig type
in types.ts by making the send/receive relationship explicit, and use the
existing SessionConfig symbol to locate and refine the union/type shape so
unusable combinations cannot be expressed.
In `@core/src/targets/httpClient.ts`:
- Around line 236-238: The prompt field selection in httpClient.ts currently
falls back to "prompt" only when promptPath is nullish, so empty or
whitespace-only values still produce a blank key. Update the logic around
setByPath in the request body construction to trim config.promptPath first, then
use "prompt" when the trimmed value is empty. Keep the fix localized to the
prompt path handling in the request-building flow so body assignment and
applySessionToRequest continue to work as-is.
In `@docs/cli.md`:
- Around line 241-242: Update the `target.session` overview in docs/cli.md to
match the actual session contract in the config reference. In the paragraph near
the `target.sessionIdField` note, revise the description in `Target session
handling`/`target.session` so it explicitly includes cookie-based receive
sessions (for example, server-owned sessions captured from `set-cookie`)
alongside body/header handling. Keep the wording aligned with the
`target.session` and `target.sessionIdField` references so the overview and
detailed reference agree.
In `@runners/cli/src/commands/hunt.ts`:
- Around line 333-360: `--session-field` is being ignored whenever
`baseTarget.session` is already present because `target` keeps the structured
session config from the spread and `resolveSessionPlan` prefers it over the
legacy field. Update the `hunt.ts` target assembly so `opts.sessionField` can
override or clear any existing `baseTarget.session`/`sessionField`
configuration, or otherwise ensure the CLI override is converted into the
structured session shape before `resolveSessionPlan` runs. Use the `target`
object construction and `resolveSessionPlan` behavior as the key places to align
the override precedence.
In `@runners/cli/src/commands/setup.ts`:
- Around line 266-286: The receiveName prompt in setup.ts allows blank input for
the body and header session receive modes, which can later cause
captureSessionFromResponse to never match anything. Add validation to the
receiveName input in the session setup flow so that when receiveIn is "body" or
"header" the value is required and cannot be empty, while still allowing an
empty value only for "set-cookie". Keep the logic in collectSessionSend and
SessionConfig["receive"] aligned so the generated receive config always contains
a valid name for body/header modes.
In `@runners/cli/src/ui/server.ts`:
- Around line 17-38: buildSessionFromSetup is not matching the wizard defaults
for server sessions. Update the sessionMode handling in buildSessionFromSetup so
that sessionReceiveIn set to "set-cookie" forces send to use header with the
Cookie name, and for "body" or "header" receive modes do not collapse a blank
sessionReceiveName to undefined; instead use the mode default or validate/reject
the empty value. Keep the change localized to buildSessionFromSetup in server.ts
and preserve the existing client/server shape returned as SessionConfig.
In `@runners/sdk/src/types.ts`:
- Around line 25-28: The JSDoc for the session config is missing cookie support
in the receive direction, so update the comments on both sessionField and
session in the target config types to match the PR behavior. Adjust the wording
in the HttpTargetConfig.session and HuntTargetConfig.session documentation to
say receive supports body, header, or cookie, while keeping the existing
send/client- vs server-owned wording intact.
---
Nitpick comments:
In `@core/src/execute/agentAttackDriver.ts`:
- Around line 160-167: The capture-miss warning in agentAttackDriver’s session
handling is too generic; update the log in the branch that checks
this.sessionPlan.mode, capturedSessionId, and warnedCaptureMiss to tell the
operator what to verify. Make the message actionable by pointing them to confirm
the target returns a session id at the configured receive location and that the
target is actually set up for server-owned sessions, while keeping the existing
fallback behavior unchanged.
In `@core/tests/agentTargetSession.test.ts`:
- Around line 124-139: The server-owned cookie test in
agentTargetSession.test.ts only verifies that `captureSession` receives the
cookie pair from the first turn; update the same test around createAgentTarget
and target.send so it also performs a second send using the captured session and
asserts the outgoing Cookie header contains the echoed raw `sid=cookie-1` value.
Reuse the existing test name and the send/receive session config to validate
both capture and turn-2 echo behavior for the Cookie path.
In `@runners/cli/ui/src/components/SetupPage.tsx`:
- Around line 205-229: When sessionReceiveIn is set to set-cookie in SetupPage,
the Send location/name controls should guide users to the required Cookie header
mapping instead of оставing them editable with a misleading default. Update the
SetupPage form logic for the sessionSendIn/sessionSendName fields to either
prefill and lock them to the correct values or display a clear note tied to the
set-cookie case, matching the behavior in collectSessionConfig and the UI
pattern used in server.ts.
In `@runners/mcp/src/index.ts`:
- Around line 428-441: Update buildSessionConfig so receive is only created when
the inputs are valid for captureSessionFromResponse: body and header must
require a non-empty receiveName, while set-cookie may omit it. Right now
buildSessionConfig in runners/mcp/src/index.ts can return a receive object with
name: undefined, which later gets ignored and silently disables session capture;
validate receiveIn/receiveName before constructing SessionConfig and return
undefined or omit receive when the combination is invalid. Use the
buildSessionConfig function and the SessionConfig receive shape as the main
points to adjust.
In `@runners/sdk/src/hunt.ts`:
- Around line 109-114: The session mapping in hunt.ts is duplicating core’s
legacy fallback logic by rebuilding session.send from sessionField before
passing it onward. Simplify the options object around the session/sessionField
handling so it only forwards the raw sessionField when session is absent and
lets resolveSessionPlan in core derive send itself, keeping the behavior
centralized and avoiding drift.
In `@runners/sdk/src/run.ts`:
- Around line 99-105: The session mapping in run.ts duplicates the
legacy-to-session fallback already handled by resolveSessionPlan in the HTTP
client core. Update the code around the session/sessionIdField construction in
the run() flow to pass through httpTarget.session and httpTarget.sessionField
directly, and let core/src/targets/httpClient.ts perform the fallback instead of
rebuilding the { in: "body", name } object here. Use the existing identifiers
resolveSessionPlan, session, and sessionIdField to locate and simplify this
logic.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 120966d4-622b-4d40-8393-e19a99a081c0
📒 Files selected for processing (26)
AGENTS.mdREADME.mdcore/src/autonomous/lib/types.tscore/src/autonomous/target/http.tscore/src/config/schema.tscore/src/execute/agentAttackDriver.tscore/src/execute/types.tscore/src/targets/agentTarget.tscore/src/targets/httpClient.tscore/tests/agentTargetSession.test.tscore/tests/autonomousTargetSession.test.tscore/tests/sessionPlan.test.tsdocs/cli.mddocs/hunt.mddocs/mcp.mddocs/sdk.mddocs/sessions.mdrunners/cli/src/commands/hunt.tsrunners/cli/src/commands/setup.tsrunners/cli/src/ui/server.tsrunners/cli/ui/src/components/SetupPage.tsxrunners/mcp/src/index.tsrunners/sdk/src/hunt.tsrunners/sdk/src/index.tsrunners/sdk/src/run.tsrunners/sdk/src/types.ts
Tighten SessionConfig (send required, receive.name required except set-cookie) so misconfigured sessions fail validation instead of silently never capturing/echoing an id. Fix set-cookie handling in the hunt setup UI and MCP session builder, session-id precedence in hunt's --target-config flag override, a promptPath trim inconsistency in the hunt HTTP client, and add a capture-miss warning + resume-session plumbing to match the run path. Also drops redundant legacy-session-fallback duplication in the SDK and tightens docs/test coverage flagged in review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
Addressed the 6 nitpicks from the review summary as well:
All in 485e3fd. |
achuvyas-kv
approved these changes
Jul 6, 2026
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
2 participants
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.
Problem
Opfor only supported "client-owned" sessions — it mints a session id and sends it in a body
field every turn. Many real agents work the opposite way ("server-owned"): the target itself
mints a session id and returns it in the response (body, header, or cookie), and the client is
expected to echo it back. Against these targets, opfor was sending an id nobody used and
throwing away the id it should have been using — every turn looked like a fresh conversation.
There was also no way to send a session id via a header, and
opfor hunthad almost no way toconfigure sessions at all.
Solution
Added a
sessionconfig that supports both directions:send— where opfor puts the session id in a request (body or header)receive— where opfor reads a session id back from a response (body, header, or cookie)If
receiveis set, opfor sends no id on turn 1, captures whatever the target returns, andechoes it on later turns. If
receiveis not set, it behaves exactly like before (client-owned).This works the same way across
opfor run,opfor hunt, the SDK, and the MCP server. Oldconfigs using
sessionIdFieldkeep working unchanged.Changes
responses (body/header/cookie); session config validation.
opfor hunt --target-config <file>lets you configure rich sessions for hunt; hunt's setup UIgot the same options.
sessionconfig exposed in both.docs/sessions.mdexplaining the whole model, linked from the CLI/SDK/MCP/huntdocs.
paths.
Issue
N/A
How to test
Summary by CodeRabbit
Summary
New Features
opfor huntadds--target-config.Bug Fixes
Documentation
Tests