Repository navigation
Upload files, agents, dm's, more - #1
Merged
Merged
Conversation
2witstudios
added a commit
that referenced
this pull request
Jul 4, 2026
…ope, task trigger echo (#1837) (#1846) * fix(w7): live-test triage — consult conversation listing, calendar scope, task trigger echo (#1837) W7 of the SDK/CLI/OAuth epic post-review remediation plan. Fixes the hard failure and two epic-surface findings from the 70-tool live MCP battery in #1837; the remaining 7 findings are traced with evidence and deferred (see PR description). - #1 (hard fail): ask_agent-created conversations were invisible to list_conversations. Neither the consult route nor the internal ask_agent tool ever wrote a `conversations` row — only chat_messages — so the listing query's ownership join (`conv.userId = userId OR conv.isShared`) never matched and the conversation silently dropped out of every listing. Fixed both call sites to eagerly createConversation, mirroring the existing pattern in apps/web/src/app/api/ai/chat/route.ts. - #2: create_calendar_event with no driveId (a personal event) 403'd with "Scoped tokens cannot create new drives". checkMCPCreateScope treats a null targetDriveId as "creating a brand-new drive" (its real caller is POST /api/drives) — the events route reused it for "no drive" too. Now only enforced when a driveId is actually supplied. - #10 (partial): update_task/create_task with an inline agentTrigger created the trigger but the response was indistinguishable from a no-op. createTaskTriggerWorkflow now returns the trigger identity, surfaced as `agentTrigger` in both routes' responses and in the SDK's tasks output schema. Every fix ships with a RED→GREEN test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYvh7hg8kwwTeXLJMBjE4x * fix(security): don't let a supplied conversationId claim ownership of another user's conversation Codex P2 finding on #1846: the finding #1 fix eagerly created a `conversations` row for any conversationId the caller supplied, including one that already had real chat_messages authored by a DIFFERENT user (the exact "legacy conversation, no conversations row yet" backfill case the fix targets). Any caller who learned that ID could claim `conversations.userId` for themselves, and ownership-gated actions elsewhere (e.g. the conversation DELETE handler's `isOwner` check) trust that field — letting them delete a thread they never participated in. Fixed in both the consult route and the internal ask_agent tool: before backfilling ownership, check whether any existing message in that conversation was authored by a different user. Only create/claim the row when there's no conflicting owner (empty history, or all prior messages already belong to the caller). RED→GREEN test added for both call sites, verified against the pre-fix code to reproduce the hijack, then against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYvh7hg8kwwTeXLJMBjE4x * refactor: centralize conversation-ownership guard, dedupe agentTrigger response shaping Proactive review pass (8 finder angles) surfaced that the Codex P2 fix (previous commit) duplicated its ownership-conflict check identically across consult/route.ts and agent-communication-tools.ts, and — more importantly — that the exact same hijack pattern (unconditional createConversation with a caller-supplied conversationId) still exists, unfixed, in apps/web/src/app/api/ai/chat/route.ts and apps/web/src/app/api/v1/chat/completions/route.ts, which this PR's own code comments cite as the pattern being mirrored. Root-caused by moving the guard into conversationRepository.createConversation itself: a cheap indexed lookup short-circuits once the conversations row exists (the common case), and only a brand-new/legacy row triggers the ownership-conflict check against existing chat_messages. Every caller — including the two pre-existing routes above, which this commit does not touch — now gets the guarantee automatically instead of requiring each call site to remember a bespoke check. consult/route.ts and agent-communication-tools.ts go back to calling createConversation unconditionally. Also dedupes the agentTrigger response-shaping in the task create/update routes: TaskTriggerWorkflowResult already matches the response shape field-for-field, so both routes now just do `agentTrigger: result` instead of reconstructing the object, and import the exported type instead of re-deriving it via Awaited<ReturnType<...>>. Ownership-conflict tests moved from the two call-site test files (which now mock the repository and have nothing conflict-specific to assert) to a new suite directly against conversationRepository.createConversation — verified RED against the pre-fix repository (both the short-circuit and the hijack guard failed), GREEN against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYvh7hg8kwwTeXLJMBjE4x * fix(security): let a scoped token read/edit/delete the personal event it created Codex P2 (2nd round) on #1846: allowing a scoped MCP/OAuth token to create a personal (driveless) calendar event (previous commit's finding #2 fix) is a dead end without this — canAccessEvent/canEditEvent both explicitly denied ANY scoped token ANY access to a driveless event, and the GET listing route excluded personal events entirely for scoped auth. A scoped token could get a 201 creating a personal event, then never list, read, update, or delete it again. Fixed by reordering the creator-identity check ahead of the "no identity power over driveless events" denial in canAccessEvent and canEditEvent (apps/web/src/app/api/calendar/events/[eventId]/route.ts) — a scoped token still acts on behalf of its owning user, so it retains full access to a driveless event it created itself, but still has zero power over a different user's personal event (that denial still applies to non-creators). The listing route (apps/web/src/app/api/calendar/events/route.ts) needed two matching changes: the personal-events inclusion condition is already scoped to `createdById = userId` so the blanket `!isScopedMCPAuth` exclusion was unnecessary and is removed; the final scoped-token cap filter now keeps a driveless event through when the caller created it, while still capping out driveless events reachable only via the (identity-derived, not drive-scoped) attendee branch. RED→GREEN tests added for all three surfaces (GET single event, PATCH/DELETE via canEditEvent, GET listing) — each verified failing against the pre-fix ordering/filter, then passing against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GYvh7hg8kwwTeXLJMBjE4x --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 5, 2026
This was referenced Jul 12, 2026
This was referenced Jul 21, 2026
Merged
2witstudios
added a commit
that referenced
this pull request
Jul 29, 2026
…ty, error UX, reconcile resilience Six findings from the full-branch review. (#1 SSRF, #2 storage metering and #7 concurrency quota were already fixed in the two prior commits; #3's cron suite now collects and passes 5 tests, so it was fixed by a later phase than the review sampled.) #4/#15 — prompt regression on default config. `buildAgentAwarenessPrompt` unconditionally told the model to delegate with `spawn_session`, but session tools only exist when CODE_EXECUTION_ENABLED is on (default off), so the assistant confidently called a tool it did not have and delegation silently broke. The delegation sentence is now gated on a `canDelegate` flag that both call sites derive from `isCodeExecutionEnabled()`; without it the section still lists the agents, it just stops naming a tool that isn't there. #13/#14 — git argv safety. `git_add` spliced paths straight into argv with no `--`, unlike its siblings `git status`/`git diff`: a path named `-p` was read as a flag (interactive add). It now separates, and only when there are paths, so no bare trailing `--`. `git_reset.ref` and `git_remote_add.name` gained the `validateFlagSafe` guard that structurally identical sibling fields already apply. #10/#11 — infinite spinner on a failed agent load. `AgentView` guarded on `agentLoading || !agent`, so once SWR gave up retrying, `isLoading` went false, `agent` stayed null, and the user watched a spinner that would never resolve with no error text and no escape but a reload. Loading and failure are now distinct states: the failure surfaces the server's own message and a Try again button, backed by a new `retry` from `useResolvedAgent`. #6 — one failing candidate query no longer parks the other pass. The orphan reconcile listed the reclaim outbox and the teardown-intent rows under `Promise.all`, so either failing dropped both. Now `allSettled` with per-source error logs: a degraded query costs its own candidates, not every reclaim, and those are billing VMs nobody is using. #8 — the leak signal is no longer silent. When a confirmed-unreferenced Sprite fails BOTH its kill and its reclaim-outbox insert, nothing in the system knows that VM exists — no row points at it, so no trigger and no cross-check will find it. That path swallowed its error, making the one path built to catch a permanently leaked VM the one path with no signal. It now logs loudly with the sandbox id and both failure reasons. #5 — the destructive teardown binding gets tests. 10 cases over `killSprite` (confirmed kill, replaced-name-as-success, genuine failure, unpinned instance), `markSessionTornDown` (CAS win/loss), and `listOrphanCandidates` (both sources, each single-source failure, cap + backlog). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Merged
7 tasks done
2witstudios
added a commit
that referenced
this pull request
Aug 9, 2026
Fix the two HIGH defects (#1, #2) and the MEDIUM-HIGH (#3) from `.pu-reports/pu-rev-phase1.md`. All are latent — nothing writes these tables yet — and each would be a hand-written DELETE against production later. Finding #1 — `applyNodeWrite` prescribed drop-then-put. The composite self-FK is ON DELETE cascade, so dropping a container took its whole subtree with it. The collapse path drops a split whose children are being reparented, not deleted, and those children are only in `put`. Drop-first cascades them away and `put` cannot resurrect them. Fixed: put before drop, with the cascade named in the docblock so nobody tidies it back. Finding #2 — `create` accepted an empty `nodeId` and a blank `targetId`. Postgres stores both (text NOT NULL is satisfied by ''), and the read then rejects the whole set rather than filtering, so the workspace becomes permanently unreadable. Fixed: `create` refuses both with typed codes; `validateTree` refuses them too (the gate every write path runs, especially `put(nodes[])`); the false comment in `bind` is corrected; the untested compensating guard in `open` is removed, with a new test for the split path it covered. Finding #3 — `validateTree` skipped the finiteness sweep for parked panes, so a NaN/Infinity share on a detached pane passed. Fixed: the sweep is hoisted out of the group loop entirely, ahead of the per-container fraction rules, with a comment that parking does not make a share a number. Also fixed: finding #6 (the byte-identical round-trip claim is false; corrected to structural identity, with the change-test hazard named), #10 (the FK's truncated live name is recorded), #11 (`put`'s ordering guarantee is stated), and #12 (`validateTree`'s cast is removed, using the idiom `descendantsOf` already follows). Report with mutation table and the deferrable-FK argument (no, tested against PostgreSQL 17.5): `.pu-reports/pu-fix-review.md` Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4EZg67rRbxgutMEFi2UZJ
2witstudios
added a commit
that referenced
this pull request
Sep 9, 2026
…ants refused typed revoke_pending, socket closed 1008 so the daemon replays again (Codex P1 #1, review round 1) The hold now releases into open or blocked: replay returns {owed, acknowledged}; owed > acknowledged (or an unreadable ledger) blocks the env, sendGrant refuses typed revoke_pending with a refusal audit row, the socket closes 1008 revoke_pending, and only a later hello whose replay is fully acked opens it. Mutants: release on failure, and skip the blocked check — both red. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6iGLmPYL3w565nDECjM5T
This was referenced Sep 16, 2026
This was referenced Oct 5, 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
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.
No description provided.