fix(middleware): resolve /workspace correctly under launcher mode#181
Closed
Lempkey wants to merge 1 commit into
Closed
fix(middleware): resolve /workspace correctly under launcher mode#181Lempkey wants to merge 1 commit into
Lempkey wants to merge 1 commit into
Conversation
Treat workspace_path="/workspace" as a valid engagement-scoped root rather than collapsing it to None. The Docker bind-mount determines whether /workspace is shared or engagement-scoped; the middleware must trust that contract. Invalid paths still fail closed and are not silently coerced into /workspace. Closes PurpleAILAB#152 Closes PurpleAILAB#173 Addresses the filesystem failure reported in PurpleAILAB#175
3 tasks
Member
|
This PR's fix has been incorporated into #182 (
When #182 merged, #152, #173, and #175 (filesystem half) were resolved alongside it. Closing this PR. Thank you for the diagnosis and contract design — that insight was the starting point for the broader refactor. |
3 tasks
PurpleCHOIms
added a commit
that referenced
this pull request
May 9, 2026
…able (#182) * refactor(engagement): launcher-authoritative slug via config.configurable Move engagement_name and workspace_path from input state to runnable config.configurable. The launcher (and web terminal-server env) is the single source of truth; the LLM no longer decides the engagement slug. Backend: - complete_engagement_planning: drop engagement_name arg; pure boolean handoff signal. The emitted engagement_ready event carries no slug. - EngagementContextMiddleware: add before_agent hydration that copies engagement_name + workspace_path from configurable into state on the first agent step. Idempotent — state wins when present. Downstream middleware (OPPLAN, filesystem) read state as before. - filesystem._normalize_engagement_workspace: strict 4-case contract (empty -> None, /workspace accepted as engagement root for launcher mode, /workspace/<slug> normalized, traversal/invalid -> None, no silent coercion). Frontend: - useAgent.ts: send engagement_name/workspace_path via config.configurable on every run; pendingHandoffRef downgraded to boolean. - types.ts: drop engagement? field from SubagentCustomEvent. - soundwave.md: prompt no longer instructs the LLM to pass a slug. Tests: - test_engagement.py: +6 tests covering hydration paths and workspace resolution precedence. - test_filesystem.py: +5 tests for the new contract; flips the old test_root_workspace_fails_closed to test_root_workspace_accepted_as _engagement_root. Closes #152, #159, #173. Addresses the filesystem failure in #175. Supersedes PR #180 (slug queuing) and PR #181 (filesystem normalize). * style(scripts): ruff format render_benchmark_charts.py Pre-existing format drift on main; CI's repo-wide ruff format --check was failing on this file unrelated to the refactor. Whitespace-only. * fix(subagent_streaming): guard None tool_call_id from corrupting active dict LangChain's ToolCall.id is `str | None` per the spec. The previous code keyed `active_tool_calls[tc["id"]] = tc` directly, so multiple id-less tool calls in the same iteration would collide under the same `None` key — the second overwrite wins and the subsequent ToolMessage lookup returns the wrong tool's name and args. Tool-result events were not dropped (they were emitted regardless), but they surfaced under the wrong tool identity. Changes: - Skip the active_tool_calls write when tc.get("id") is None and emit a warning so we can spot subagent/provider configurations that drop the id. Subsequent ToolMessage lookups for those calls fall through to the "unknown" branch instead of mis-attributing. - Tighten the active_tool_calls type from dict[str, dict] to dict[str, ToolCall] (the actual TypedDict) — both signature and call-site annotations. - Add three regression tests: None-id storage skipped + warning emitted, ToolMessage with tc-less id falls through to "unknown", and a valid id keeps its mapping when a concurrent None-id call is present. Supersedes the corresponding fix in PR #178 (which closed without landing) — the diagnosis "results dropped from the streaming pipeline" was inaccurate; the practical effect is wrong-name attribution. The type annotation also lands as `dict[str, ToolCall]` rather than the laxer `dict[str, Any]` from that PR.
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.
Summary
_normalize_engagement_workspace()was collapsingworkspace_path="/workspace"toNone, causing every filesystem tool (ls,read,write,edit,grep,glob,download_files) to return"No engagement workspace is set..."even when an engagement is active./workspace" with "no engagement configured", which is only true in rawmake devmode. In launcher mode the Docker bind-mount already scopes/workspaceto the engagement, so/workspaceIS the correct engagement root./workspaceexplicitly, accept/workspace/<safe-slug>(unchanged), and fail closed forNone/empty/traversal/invalid paths. Invalid paths are not silently coerced into/workspace.Prerequisite: #180 (slug propagation) must be merged first — this PR fixes the second half of the chain.
What changed
decepticon/middleware/filesystem.py: Rewrote_normalize_engagement_workspacewith a strict 4-case contract: empty→None,/workspace→/workspace,/workspace/<slug>→normalized (only if_normalize_workspace_pathdid not silently coerce), anything else (including traversal like/workspace/../etc)→None. Addedos.path.normpathguard to catch..-based traversal that the slug regex would otherwise accept. Addedimport os.tests/unit/middleware/test_filesystem.py: Flippedtest_root_workspace_fails_closed→test_root_workspace_accepted_as_engagement_root. Added: no-prefix-doubling under/workspaceroot, glob scoping, trailing-slash acceptance, explicit negative regression that invalid paths fail closed and do NOT coerce to/workspace.Test plan
pytest tests/unit/middleware/test_filesystem.py -v— 11/11 passmake test-local— 726 passed, 1 skipped (pre-existing skip), 0 failuresruff check+ruff format --checkon changed files — cleanbasedpyright decepticon/middleware/filesystem.py— 0 errorsls /workspacefrom an active engagement) requires the LangGraph + Docker stack and is out of scope for CINotes
DECEPTICON_ENGAGEMENT_WORKSPACEenv fallback was considered and intentionally excluded: the variable is used by compose for host-side bind interpolation and is not currently propagated into the LangGraph container environment. A follow-up PR adding compose/launcher env propagation would be needed to make that fallback reliable.Closes #152
Closes #173
Addresses the filesystem failure reported in #175