Skip to content

fix(mobilehog): fix photo uploads and task visibility - #108887

Merged
trunk-io[bot] merged 3 commits into
masterfrom
posthog/fix-mobilehog-attachment-upload
Sep 30, 2026
Merged

trunk-io[bot] merged 3 commits into
masterfrom
posthog/fix-mobilehog-attachment-upload

Conversation

@dmarticus

@dmarticus dmarticus commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Mobile users see other people's tasks in personal lists, and photo attachments fail before a task starts.

Changes

  • Recent tasks and search now request only tasks created by the signed-in user.
  • Task caches now include the user ID, which prevents results from another account.
  • Photo uploads now use Expo filesystem files, which its multipart encoder supports.
  • The app keeps the same UI and direct object-storage upload flow.

How did you test this code?

The MobileHog upload test confirms that Expo fetch receives the selected photo bytes. The MobileHog type check and lint passed. A device test was not available in this environment.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Desktop, GPT-5

Skills: posthog-desktop and writing-pr-descriptions. No matching open PR covers these failures.


Created with PostHog Desktop

Use expo-file-system File objects for multipart photo uploads so Expo fetch can read each form-data part.

Generated-By: PostHog Desktop
Task-Id: b9f2e11a-748d-45c9-8f24-2b6b33235ff4
@dmarticus dmarticus self-assigned this Sep 30, 2026
@trunk-io

trunk-io Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@dmarticus dmarticus added the skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com label Sep 30, 2026 — with PostHog
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit e083369.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane (fe:product:desktop)

This PR is assigned to the non-backend lane (fe:product:desktop). It does not run backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

Scope recent and searched task lists to the signed-in user and separate cached results by user ID.

Generated-By: PostHog Desktop
Task-Id: b9f2e11a-748d-45c9-8f24-2b6b33235ff4
@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Fixes photo uploads and filters task list by user.

The PR does not appear safe to merge while the previously flagged photo-upload failure remains.

Reviews (2) · Last reviewed commit: "fix(mobilehog): show only the user's tas..."

Comment thread products/desktop/apps/mobilehog/src/lib/attachments.ts
Comment thread products/desktop/apps/mobilehog/src/lib/attachments.ts
@dmarticus dmarticus changed the title fix(mobilehog): upload attachments with Expo files fix(mobilehog): fix photo uploads and task visibility Sep 30, 2026
@hosthog

hosthog Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

@dmarticus
dmarticus marked this pull request as ready for review September 30, 2026 02:52
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 30, 2026 02:53
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The mobile app now uses Expo’s File class as the multipart attachment value. The task query hooks include the session user ID in their cache keys and request tasks created by that user. The app adds Vitest configuration and a test for photo upload bytes, the POST request, and the returned artifact ID.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e0833

Task queries are creator-scoped, and Expo’s multipart path supports uploading the selected photo bytes. No production failure is established; the remaining change improves regression coverage, so the PR is mergeable with normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e0833

Task filtering improves account-specific visibility without expanding server permissions. Photo uploads preserve existing authorization, but interrupted uploads may leave photo data without confirmed cleanup. This limits confidence in a minimal-risk assessment.

Retained concerns

  • Low · security · inferred: Successfully posted photos can be left outside the staged retention path if another parallel POST fails, the operation is interrupted, or an account change prevents finalization. The client has no compensating deletion, and the one-day storage tag is applied only during finalization. This lifecycle predates the PR, but repaired transport may make its sensitive-data leftovers more reachable on affected devices. Production cleanup for unfinalized objects and the effective exposure increase from the base remain unverified; indefinite retention is not established.
Security review details

Security Blast Radius

  • inferred — The inspected staged-upload exposure is bounded by task-write scope, the authenticated caller's task-control permissions, and presigned capabilities for individual task/team storage keys. The retention concern involves selected photo data within those authorized uploads, not an established cross-tenant access path.

Security Findings and Attack Paths

  • inferred — A successful photo POST followed by failure or interruption before finalization can leave a storage object without the staged retention tag applied by this flow. Repeating preparation generates fresh artifact IDs rather than resuming prior objects. This supports a bounded retention/recovery concern, but does not establish unauthorized disclosure or indefinite retention.

Trust Boundaries and Controls

  • observed — Account identity includes generation, host, project, and user. The root initializes an account-lifecycle subscriber that clears query state and resets the API client on identity changes. Authenticated client operations check identity before and after network calls, while direct storage POSTs use the previously issued presigned form.

Resilience and Maintainability Implications

  • observed — Server finalization validates every submitted object before caching staged metadata. Attaching staged artifacts to a run checks for an existing manifest entry and removes the corresponding staged cache keys. These controls support normal completion, but do not clean up storage POSTs that never reach finalization.

Hardening Proposals

  • proposed — Establish a server-enforced expiration or cleanup bound for uploaded objects that never finalize, independent of mobile completion. If recovery is supported, retain stable upload identities so retries can reconcile existing objects rather than create additional abandoned copies.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and standalone. It explains the user impact, lists the task visibility and photo upload fixes, documents automated validation and the unavailable device test, records relea…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

Generated-By: PostHog Desktop
Task-Id: 3313e91d-d757-49f3-9553-dc14b9d26cc9

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
products/desktop/apps/mobilehog/src/lib/attachments.test.ts (1)

5-9: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Make MockFile URI-sensitive.

MockFile ignores its URI and always returns photoBytes. The byte assertion can therefore pass even when uploadStagedPhotos does not use the selected photo's photo.uri. Map the fixture bytes to the expected URI and reject unexpected URIs.

Suggested test fix
   const photoBytes = new Uint8Array([137, 80, 78, 71]);
+  const bytesByUri = new Map([["file:///photo.png", photoBytes]]);
   class MockFile extends Blob {
-    constructor(_uri: string) {
-      super([photoBytes], { type: "image/png" });
+    constructor(uri: string) {
+      const bytes = bytesByUri.get(uri);
+      if (!bytes) throw new Error(`Unexpected file URI: ${uri}`);
+      super([bytes], { type: "image/png" });
     }
   }

This is a URI/data coverage gap, not a filename or MIME requirement. The staged-upload contract does not require those multipart fields.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 7c651c93-a920-49dd-9860-1bd3a02e90d3

📥 Commits

Reviewing files that changed from the base of the PR and between 9d70d5c and e083369.

⛔ Files ignored due to path filters (1)
  • products/desktop/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • products/desktop/apps/mobilehog/package.json
  • products/desktop/apps/mobilehog/src/lib/attachments.test.ts
  • products/desktop/apps/mobilehog/src/lib/attachments.ts
  • products/desktop/apps/mobilehog/vitest.config.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@trunk-io
trunk-io Bot merged commit 2b973e0 into master Sep 30, 2026
252 checks passed
@trunk-io
trunk-io Bot deleted the posthog/fix-mobilehog-attachment-upload branch September 30, 2026 16:54
@deployment-status-posthog

deployment-status-posthog Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-09-30 17:30 UTC Run
prod-us ✅ Deployed 2026-09-30 17:41 UTC Run
prod-eu ✅ Deployed 2026-09-30 17:41 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants