Skip to content

feat(people): restore device people and thread ownership on v2 - #180

Merged
lukemaj merged 11 commits into
fork/v2from
feat/170-thread-people-v2
Oct 8, 2026
Merged

lukemaj merged 11 commits into
fork/v2from
feat/170-thread-people-v2

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What: Restore device-person selection, thread ownership and sharing on v2, including frozen v1 ownership carryover.
Why: Upstream v2 omits the fork's person fields and its importer does not carry the frozen owner columns.
So what: Exact-head independent review and current CI pass; this one PR stays draft pending planner UI evidence and user acceptance, with no merge authorized.

Closes #170

Devices can choose a person in Connections settings; the sidebar filters thread and draft views, and the chat header offers sharing and leaving. Server-side commands stamp the device person, agent creations inherit the caller's owner, forks belong to the acting person and children inherit their parent owner while starting unshared. Ownership remains a view, without access control. Sharing uses the existing metadata event and adds no timeline row.

A feature-owned backfill runs after #167 imports v2 shells, using only frozen v1 fields. Existing v2 fields win. Missing fields carry over exactly, including null and empty arrays. Malformed needed legacy values become null/[] with retained thread-id/reason diagnostics; structurally invalid payloads consumed by the migration block readiness through a typed error. This is migration validation, not a general v2 integrity scan. Atomic per-thread markers make partial failures and reruns safe.

Proof and dependencies

  • Actual base: 2647478da553b442c6e5000b76c3b42042b01e69 (merged feat(fork): carry Chromeria foundation onto upstream V2 #178 foundation and feat(fork): port GitHub Issues onto v2 #179/Port the GitHub Issues features #171 Issues). Only Port thread people and ownership #170 feature commits rebased; no foundation ancestors replayed.
  • Current head: e3f487b846b8134304630a32c431f95bef9d4474. Supported Node24.13.1, host-neutral environment: full auth/session/boundary plus affected server/client RPC permissions 79/79 pass. Focused coverage covers all157 unique tests across17 files: initial serial run155 passed/two snapshot timeouts; root directed low-load serial full-file proof then passed foundation2/2 and people5/5 without test/timeout/source changes. Duplicate fixture assertions are not counted twice. After review fixes and rebase, the backend-proof head61815505341d1112c8a86f00102d53c7b9c60607 people full file passes5/5 at initial1-minute load4.11, real test16.920s; foundation2/2 at initial load3.36 is reused because its backfill source/registry/fixtures/environment/input are unchanged. Failed logs remain retained.
  • Real VACUUM INTO snapshot:739 imported,0 missing,0 mismatched,0 unresolved,0 hydrated transcripts,0 rerun events. Synthetic shared/other-person/malformed fixtures and injected transaction-failure rollback are covered.
  • Four affected package typechecks after Port the GitHub Issues features #171 (contracts/client-runtime/server/web), targeted changed-file lint and formatting, forkcheck and feature-map validation pass. Unchanged mobile/desktop type proof from the earlier candidate is retained; no repository-wide local checks.
  • Port child threads onto upstream lineage and delegation #168 seam coordinated read-only. SubagentProjection primary ownership is temporarily Port thread people and ownership #170 and transfers when Port child threads onto upstream lineage and delegation #168 lands; existing shared-file primary owners are preserved.
  • Original exact-head review found three P2 issues, preserved at feat(people): restore device people and thread ownership on v2 #180 (comment) . All are fixed: read-only person and sharing controls honor existing grants; sharing checks the existing target-environment operate scope both in rendering and its handler, and retains the common guarded RPC helper; malformed consumed JSON reaches the feature-owned typed error. The malformed fixture failed before and passed after. The intermediate618 review still found the unregistered dispatch scope; its failure finding remains preserved. Final UI-only correction uses the explicit existing scope hook and fresh handler guard without widening the RPC registry; scoped web typecheck/lint/format pass, unchanged backend/snapshot proof reused. Independent review passed on exact head e3f487b846b8134304630a32c431f95bef9d4474; findings and resolution were published before review/independent success, verified by creator lukemaj. Current-head CI is all green, including independent review (gh pr checks exit 0, live verified). Planner integrated UI before/after evidence remains pending and unwaived; no browser verification claimed. PR remains draft for this acceptance item.
  • Cost accounting: Agent Observer comment published; estimated cost unknown and 0 attributed responses across 41 sessions (5.0h wall time) is an accounting gap, never zero cost. Parent refreshed the existing comment; attribution remains unresolved.
  • Canonical decisions, exact commands, retained failures and handoff: Port thread people and ownership #170 (comment)

Known intentional property

Preliminary reviewer observed that a non-owner can send thread.share directly despite the UI hiding the action. Root adjudicated preservation of v1 semantics under D17, view filtering without access control; reviewer withdrew it as a defect. Frozen source v1 decider has the same checks. No owner-only access policy is introduced.

Elon record

Requirements and who asked: User requested #170, device person/auth, v2 ownership, frozen owner/co-owner backfill, one PR and focused proof.
Deleted: Sharing timeline row, v1 projection machinery, component PRs and unrelated ports.
Bottleneck: Planner integrated UI evidence and user acceptance; source review, current CI and required local proof have passed.
Checked myself: Live landed base, feature-only rebase, source and draft/shell consumers, actual test output, sanitized snapshot equality, inventory ownership and root decisions.

Implemented by Claude Opus5.5 through T3/Prism; integration by GPT-6.1-Sol through Codex/T3. Independent reviewer routed by Prism through Codex (GPT-6-Luna).

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL labels Oct 8, 2026
@lukemaj

lukemaj commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost unknown · 0 responses · 41 sessions · 5.0 h wall time

Model Responses Tokens Estimated cost

Flags: 3 human corrections · 64 large tool outputs · 44 repeated commands · 8 repeated failures · 65 repeated reads · 5 repeated skill loads · 40 sessions with usage bound to no task · 1 session without usage records

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/chromeria#170: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: Port thread people and ownership #170
  • Snapshot 0f9c3e8b3250452513c48ebf841a52d32d056754d43744fd7a65a5b9253ea3d6, records up to 2026-10-08 18:41 UTC.
  • 41 sessions on claude, codex; AgentsMD 14.6.0.
  • Not counted: 3,528 responses (at least $56.28) in sessions shared with other PRs that worked in no single PR's checkout.
  • 2,966 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: no.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-09-28, schedule 696aae45933d0a684a015d3f08cfb6f1aa2fef02e7d272b4689e3a692ea04fff. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
  • Native session usage or worker ownership is unavailable.
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Codex Live turn messages — 1 — 8 ✅
Claude Total thread wire — 5.0 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.8 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 21.2 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: e3f487b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent source review of PR #180 at e89100f86185f7ffe62bae992d7a2499f8b9d2ae found three actionable issues.
Why: Two write controls are exposed to read-only grants, and malformed imported JSON bypasses the feature's corruption error.
So what: Keep this candidate unapproved; address these findings and request review on the new head. Planner UI before/after evidence remains pending.

Findings

  1. [P2] Gate device-person edits on access:write
    apps/web/src/components/settings/ConnectionsSettings.tsx:1103; endpoint scope check at apps/server/src/auth/http.ts:501.
    Trigger: A session with access:read but without access:write can view Authorized clients (ConnectionsSettings.tsx:3817-3818)). Each row still mounts an enabled ClientPersonSelect; choosing Luke or Pauli calls the person endpoint and receives the server's forbidden response, which the component shows as “Could not change the person.” **Expected:** Gate or disable this selector with the existing canWriteAccess` state. Server authorization is effective; this is a read-only UI affordance that always fails.

  2. [P2] Gate sharing actions on orchestration:operate
    apps/web/src/components/people/ThreadSharingControl.tsx:42,47-75; dispatchCommand requires the operate scope in apps/server/src/auth/RpcAuthorization.ts:45.
    Trigger: A session with orchestration:read but without orchestration:operate can load the thread and device person. The control derives an owner/co-owner action without checking the destination environment's operate grant, then dispatches through useAtomCommand; the server rejects the command.
    Expected: Use the scope-aware orchestration command path and withhold/disable the action when the environment lacks operate permission. This concerns the UI's command grant only; it does not change the accepted D17 view-only sharing semantics or add an owner-equals-actor rule.

  3. [P2] Keep malformed consumed payloads on the typed corruption path
    apps/server/src/persistence/forkThreadPeopleBackfill.ts:86-89; mapping at apps/server/src/persistence/forkV1Backfills.ts:44-47.
    Trigger: For an imported row with malformed payload_json and missing people fields, SQLite's json_type predicate fails while selecting rows, before decodeStoredThread can raise ThreadPeopleBackfillPayloadError with the thread id. The registry maps only that feature error to ForkV1BackfillStepError; this SQL failure is reduced to generic ForkV1BackfillError({ backfillId: "thread-people" }). The test explicitly expects the feature error to be absent for malformed JSON (forkThreadPeopleBackfill.test.ts:233-238).
    Expected: Ensure every consumed corrupt payload reaches the feature-owned typed error with its thread id, while still stopping startup. This is the accepted typed-corruption contract.

Elon record

Requirements and who asked: Issue #170 and root-approved #166 decisions; user requested an independent exact-head review.
Deleted: No requirements; retained D17's view-only sharing semantics and no owner-equals-actor constraint.
Bottleneck: Missing UI scope gates and malformed-payload error routing block this review.
Checked myself: Verified live PR/base/head, the full 49-path diff and consumers, issue/handoff/acceptance references, and current command-helper rules; reviewer ran no tests, typechecks, lint, or browser checks.

lukemaj and others added 10 commits October 8, 2026 20:21
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…elds

The fixture fails on purpose until the backfill decision lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Allow the thread people backfill to read the frozen V1 owner columns, drop
the never-consumed payload fixture, and fix two test type errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the feat/170-thread-people-v2 branch from 95e5f2c to 6181550 Compare October 8, 2026 18:27
@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What: Review of PR #180 at 61815505341d1112c8a86f00102d53c7b9c60607 resolves two prior findings; one sharing-scope finding remains.
Why: The new permissionAtom does not enforce the server's orchestration:operate requirement for dispatchCommand.
So what: Keep this head unapproved until the sharing action checks the existing operate grant. Planner UI evidence remains pending and unverified.

Resolved from the e89100f review

  • The Connections selector now reads AuthAccessWriteScope, disables without it, and rechecks the grant before saving.
  • The backfill selects malformed, unmarked imported payloads safely with json_valid; decoding raises ThreadPeopleBackfillPayloadError with the synthetic thread id, and registration maps that feature error as accepted.

The original e89100f findings and failure status remain unchanged.

Remaining finding

[P2] Gate sharing actions on the server's existing operate scope

At apps/web/src/components/people/ThreadSharingControl.tsx:47-52, action visibility depends on threadSharingCommand.permissionAtom(environmentId). The command tags ORCHESTRATION_V2_WS_METHODS.dispatchCommand at apps/web/src/components/people/threadSharing.ts:16-20. On the landed base, clientRpcRequiredScopes returns scopes only for methods present in CLIENT_GUARDED_RPC_SCOPES (packages/contracts/src/clientRpcPermissions.ts:51-62)); dispatchCommandis absent from that map. Consequently,createCommandPermissionsgives this permission atom an empty required-scope list and returns true, andrequestGuardedskips its client authorization check. The server still requiresAuthOrchestrationOperateScopefordispatchCommand (apps/server/src/auth/RpcAuthorization.ts:46-49`).

Trigger: A session can read threads and has a person label matching the thread owner (or co-owner), but lacks orchestration:operate. The sharing menu is still rendered; clicking it sends a command the server rejects.

Expected: Gate the action using the existing AuthOrchestrationOperateScope for the destination environment, or add the existing server-required scope to the client guard for dispatchCommand. Keep server authorization and D17's view-only sharing semantics unchanged; this does not call for an owner-equals-actor policy.

Elon record

Requirements and who asked: Issue #170 and root-approved #166 decisions; user requested exact-head independent re-review.
Deleted: No requirements or policy; D17's intentional v1 sharing semantics remain unchanged.
Bottleneck: The client permission map omits the existing operate grant for dispatchCommand.
Checked myself: Verified live PR/base/head, the complete 49-path diff, both resolved paths, landed Issue RPC scopes/stream types/WS handlers, the client permission helper and server scope map; ran no tests, typechecks, lint, or browser checks.

@lukemaj

lukemaj commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of PR #180 at e3f487b against base 2647478 found no remaining actionable source findings.
Why: The last sharing UI gap is closed by checking the target environment's existing orchestration:operate grant when showing and invoking the action.
So what: Record success for this exact-head independent review; the PR stays draft while planner UI evidence remains pending and unwaived.

Resolved finding

Gate thread sharing actions on the existing operate scope

In apps/web/src/components/people/ThreadSharingControl.tsx:54, action visibility now requires useEnvironmentScope(environmentId, AuthOrchestrationOperateScope) alongside the existing command permission atom. At line 87, the click handler re-reads the same target environment's current scope before dispatch. The server already requires that scope for dispatchCommand. The client permission helper and global RPC scope registry remain unchanged, so this adds the requested feature-local UI gate without widening authorization behavior.

The prior device-person selector and malformed consumed-payload findings remain resolved as recorded in the 618 review. The root-adjudicated D17 v1 sharing semantics remain intentional: this UI check does not add an owner-equals-actor rule or change server access policy.

Review coverage and limits

The cumulative 49-path candidate was reviewed by carrying forward the completed 618 review and inspecting this sole changed path, its target-environment session scope helpers, command permission helper, and server dispatch scope. No new actionable finding remains. The reviewer ran no tests, typechecks, lint, or browser verification. Planner integrated UI before/after evidence remains pending and unwaived; this is not a claim that the draft is ready for merge.

Elon record

Requirements and who asked: Issue #170, root-approved #166 decisions, and the user's request for an independent exact-head review; use existing authorization scopes without changing sharing policy.
Deleted: No global RPC registry expansion, no owner-equals-actor requirement, and no unrelated source changes.
Bottleneck: The final exact-head review after correcting the sharing UI gate; planner UI evidence remains an open PR acceptance item.
Checked myself: Verified live PR base/head, the 49-path cumulative diff identity, the one-file delta and relevant scope consumers; reused the completed 618 source review; ran no checks or browser verification.

@lukemaj
lukemaj marked this pull request as ready for review October 8, 2026 18:50
@lukemaj
lukemaj merged commit a97591e into fork/v2 Oct 8, 2026
30 checks passed
@lukemaj
lukemaj deleted the feat/170-thread-people-v2 branch October 8, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant