Repository navigation
Activate client telemetry for monitoring dashboards - #4
Merged
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Comment on lines
981
to
+1014
| model: selectedModel, | ||
| responseTime: Date.now() - startTime | ||
| }); | ||
|
|
||
|
|
||
| const usage = usagePromise ? await usagePromise : undefined; | ||
|
|
There was a problem hiding this comment.
[P1] Avoid referencing usagePromise outside its scope
The failure-handling block awaits usagePromise to report metrics, but that promise is declared inside the preceding try block (lines ~584‑585). In JavaScript/TypeScript, let declarations are block scoped, so usagePromise is undefined in this catch block. TypeScript will emit TS2304: Cannot find name 'usagePromise', breaking next build/type-checks. Even if the file transpiled, any error thrown before the variable is declared would produce a runtime ReferenceError and prevent the API from returning an error response. Hoist the declaration outside the try or guard against the variable being missing before using it in the catch.
Useful? React with 👍 / 👎.
19 of 20 tasks
This was referenced Apr 9, 2026
2witstudios
added a commit
that referenced
this pull request
Apr 17, 2026
Replaces the inert "No pages in this drive" block with role-aware Upload and Create CTAs (reusing CreatePageDialog) plus a drop zone that uploads each dropped file via POST /api/upload inside a useEditingStore session. Viewers with no drive role see a read-only message. Addresses Eric Elliott onboarding feedback #4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 of 5 tasks
2witstudios
added a commit
that referenced
this pull request
Apr 17, 2026
…1035) * fix(files): add discoverable empty-state CTAs with drag-drop upload Replaces the inert "No pages in this drive" block with role-aware Upload and Create CTAs (reusing CreatePageDialog) plus a drop zone that uploads each dropped file via POST /api/upload inside a useEditingStore session. Viewers with no drive role see a read-only message. Addresses Eric Elliott onboarding feedback #4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(files): restrict canWrite to OWNER/ADMIN to match server gate Boolean(drive?.role) was truthy for any role, including the MEMBER fallback that drive-service.ts assigns to users who only have page-level canView permission. Since POST /api/pages requires isDriveOwnerOrAdmin, those users would see Upload/Create CTAs and then hit 403. Align the client gate with the server rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(files): ignore dragleave when entering a child of the drop zone handleDragLeave was unconditionally clearing isDropActive, so dragging across the icon or CTA buttons inside the drop panel would flicker the highlight off/on. Gate the reset on event.relatedTarget leaving the container. Adds a test asserting the highlight stays active when the drag moves to a descendant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(files): mark empty-state CTA epic COMPLETED All three requirement blocks (CTA panel, drag-drop upload, test coverage) are implemented on this branch; review feedback addressed and merged into PR #1035. Flip the epic status and date so the tracker reflects reality. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
5 of 6 tasks
4 of 8 tasks
2witstudios
added a commit
that referenced
this pull request
May 29, 2026
…t privilege) Addresses Codex's fourth P1. The previous fix preserved custom-role agent memberships to avoid widening (#3), but that left them with stale custom-role access: getAgentAccessLevel reads driveAgentMembers.customRoleId directly, so an agent keeps reaching pages the downgraded granter can no longer grant. Since the safe least-privilege intersection ("view-only on a custom role's pages") can't be represented with the role/customRoleId fields, recap now, for each non-home membership the user granted (granter capped to MEMBER): - ADMIN role → reduce to the granter's ceiling (their custom role, or plain MEMBER view-only) — always strictly narrower than full access. - already at the ceiling → leave unchanged. - any other member-level grant whose customRoleId differs from the granter's → REVOKE. Removing the membership is the only representable non-widening reduction, and it eliminates the stale access. This satisfies both "never widen" (#3) and "no stale custom-role access" (#4). Updated the service tests: reduce ADMIN, revoke mismatched custom-role grants, preserve matching ones, no-op when nothing was granted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2witstudios
added a commit
that referenced
this pull request
May 30, 2026
…wngraded (#1458) * feat(agents): cascade agent revocation when a member is removed or downgraded An external agent's drive access is frozen into driveAgentMembers.role at grant time and read straight back at runtime (getAgentAccessLevel); it is never re-derived from the granting user's live permissions. So removing a user from a drive left the agents they had added still able to read it — for the revoked user and anyone who could run those agents — and downgrading a user left an agent they had elevated to ADMIN still at ADMIN. Make agent memberships follow the granting user's drive access, keyed on driveAgentMembers.addedBy and scoped to the drive (home-drive memberships are preserved): - revokeAgentMembershipsGrantedBy: deletes the user's non-home agent memberships; accepts a transaction so it runs atomically with the member removal in the DELETE handler. - recapAgentMembershipsGrantedBy: reuses resolveGranterAccess to recompute the user's current grant ceiling and downgrades any elevated agents they granted to plain MEMBER (no-op on upgrade), invoked from the PATCH handler on a role change. Both cascades emit authz audit events (reason: member_removal / member_downgrade). No realtime broadcast — consistent with the existing agent-membership endpoints, which audit only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): re-cap agents on custom-role change, not only standard-role change Addresses Codex P1: the PATCH re-cap cascade ran only inside the `role !== oldRole` block, so a member who stayed MEMBER but moved to a more restrictive custom role (or had it cleared) left their granted agents with a stale driveAgentMembers.customRoleId — access to pages the user could no longer grant. Compute the old custom role from the pre-update member details and trigger recapAgentMembershipsGrantedBy on `roleChanged || customRoleChanged`. The re-cap re-syncs each agent the member granted to the grant they could make today (resolveGranterAccess), which never escalates beyond the member's own access. Audit reason renamed member_downgrade -> member_recap to cover both triggers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): detect custom-role clears via updateMemberRole's returned old value Addresses Codex's second P1: the re-cap derived the previous custom role from memberData.customRole, but getDriveMemberDetails hardcodes `customRole: null` (and the MemberWithDetails type doesn't expose customRoleId). So `customRoleChanged` was always false for a custom-role CLEAR (customRoleId: null over a prior role with the standard role unchanged), skipping recapAgentMembershipsGrantedBy and leaving agents with stale access. Make updateMemberRole return the old customRoleId it already reads from the row (`{ oldRole, oldCustomRoleId }`) and compute `customRoleChanged` from that reliable value. Adds a regression test for the clear case and switches the "equals current" / "custom-role change" tests to drive the old value through updateMemberRole's return instead of the always-null getDriveMemberDetails.customRole. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): recap must not widen restrictive custom-role agents Addresses Codex's third P1: recapAgentMembershipsGrantedBy reset every elevated membership (including custom-role ones) to plain MEMBER when the granter dropped to plain MEMBER. But in the agent permission resolver a plain MEMBER can view EVERY page in the drive, whereas a custom role grants only its listed pages — so rewriting a restrictive custom-role agent to plain MEMBER broadened it from a subset to the whole drive. A downgrade-driven recap must only ever cap DOWN. Restrict recap to ADMIN agent memberships, which are unambiguously above any member-level ceiling (full access to all pages), so capping them to the granter's ceiling (their custom role, or plain MEMBER view-only) is always strictly narrower. Custom-role memberships are now preserved untouched — the role/customRoleId fields can't express the safe least-privilege intersection, and preserving them guarantees recap never widens an agent's page reach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): revoke stale custom-role agent grants on downgrade (least privilege) Addresses Codex's fourth P1. The previous fix preserved custom-role agent memberships to avoid widening (#3), but that left them with stale custom-role access: getAgentAccessLevel reads driveAgentMembers.customRoleId directly, so an agent keeps reaching pages the downgraded granter can no longer grant. Since the safe least-privilege intersection ("view-only on a custom role's pages") can't be represented with the role/customRoleId fields, recap now, for each non-home membership the user granted (granter capped to MEMBER): - ADMIN role → reduce to the granter's ceiling (their custom role, or plain MEMBER view-only) — always strictly narrower than full access. - already at the ceiling → leave unchanged. - any other member-level grant whose customRoleId differs from the granter's → REVOKE. Removing the membership is the only representable non-widening reduction, and it eliminates the stale access. This satisfies both "never widen" (#3) and "no stale custom-role access" (#4). Updated the service tests: reduce ADMIN, revoke mismatched custom-role grants, preserve matching ones, no-op when nothing was granted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): revoke ADMIN agents on downgrade to plain MEMBER (private-page leak) Addresses Codex's fifth P1. resolveRolePermissions gives a plain MEMBER membership canView:true on EVERY page (no isPrivate check), while a plain MEMBER *user* only sees non-private pages. So reducing an ADMIN agent to { role: MEMBER, customRoleId: null } let the agent keep reading private pages the downgraded (plain-member) granter can no longer access — the plain-MEMBER row cannot express "non-private only". recap now only *reduces* an ADMIN agent when the granter still holds a custom role (a representable, strictly-narrower per-page matrix). When the granter is a plain MEMBER, ADMIN agents are REVOKED instead. Pre-existing plain-MEMBER agent memberships (already exactly what a plain-member granter yields via addAgentToDrive) are left untouched. Updated/added service tests for: ADMIN + plain-member granter → revoke; ADMIN + custom-role granter → reduce to that role; plain-MEMBER agent preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(permissions): bottleneck agent access by granting user's access Root-cause fix for the divergence where a plain-MEMBER *agent* membership read every page in a drive — including private ones — while a plain-MEMBER *user* only sees non-private pages. An agent could out-read the member who granted it. - agent-permissions.ts: gate the plain-MEMBER path on isPrivate, mirroring the user-side membership rule (permissions.ts). getAgentAccessLevel denies a plain member on a private page; getAgentAccessiblePagesInDrive filters private pages out. ADMIN/OWNER and explicit custom-role grants keep their existing parity with the user-side paths. - drive-agent-service.ts: with the over-grant closed, recapAgentMembershipsGrantedBy simplifies to always reduce a downgraded granter's agents to the representable cap (role MEMBER + the granter's own custom role / none). The revoke-instead- of-reduce special case is gone — a plain-MEMBER row can no longer leak private pages, so reduction is always safe. - Tests: add private-page coverage to agent-permissions; update recap tests to assert reduce (not revoke) under a plain-member granter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): revoke (not rewrite) mismatched custom-role agents on recap CodeRabbit caught that unconditionally rewriting every mismatched membership to the granter's new cap can WIDEN an agent on downgrade: an agent scoped by a restrictive custom role rewritten to plain MEMBER/null would gain visibility to all non-private pages in the drive. A custom-role matrix is incomparable to the cap (it may cover fewer pages, or grant private/edit access the cap doesn't). recapAgentMembershipsGrantedBy now: - role ADMIN ⇒ reduce to the cap (ADMIN ⊇ any MEMBER cap ⇒ pure narrow). - role MEMBER == cap ⇒ leave. - role MEMBER != cap ⇒ REVOKE (not provably a subset ⇒ never rewrite/widen). The isPrivate fix keeps the common ADMIN→plain-MEMBER case a clean reduction; only the genuinely incomparable custom-role mismatch is revoked. Tests updated: mismatched custom role and plain-MEMBER-under-custom-cap both revoke. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): subset-check custom roles on recap (no false revoke on upgrade) Codex (P2) flagged that revoking every mismatched custom-role membership also fires on an *upgrade*: moving a member role_a → role_b (broader) would delete agents granted under role_a even though the user's access wasn't reduced. But an earlier finding requires recap to still cascade on a custom-role *tightening*. Reconcile both by comparing the role permission matrices in recapAgentMembershipsGrantedBy: - role ADMIN ⇒ reduce to the cap (pure narrowing). - role MEMBER == cap ⇒ leave. - role MEMBER != cap ⇒ keep iff the agent's role is provably a subset of the granter's new role (memberGrantWithinCap / customRoleSubset); else revoke. So a broadening change leaves the agent (still within the granter), while a tightening or otherwise-incomparable change still revokes. Plain-MEMBER (null) on either side is treated conservatively (fail-closed revoke), since it spans the whole drive and isn't a per-page matrix. Never rewrites a member row, so it can't widen (the CodeRabbit concern stays addressed). Trigger unchanged. Added subset leave/revoke unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 3, 2026
This was referenced Jul 13, 2026
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
2witstudios
added a commit
that referenced
this pull request
Sep 9, 2026
… revokePending — rows leave only on the machine's ack (Codex P1 #4, review round 1) 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 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
Testing
https://chatgpt.com/codex/tasks/task_e_68d2c8a5feec8320bcbfc9453450b65a