fix(ui): refresh deployments logs stream on status change

[myftija]

The logs stream is created after the deployment moves from PENDING status to INSTALLING.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5b65b98

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

This pull request modifies a single deployment details route component. A new local boolean variable isPending is introduced, derived from the deployment status field. This variable is added to the dependency array of a logs streaming effect hook, causing the effect to re-run whenever the deployment transitions between pending and non-pending states. The variable itself is not referenced elsewhere in the code.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Single file modification with localized, focused changes
• Logic is straightforward: boolean derivation and dependency array addition
• No exported or public entity changes

Areas requiring attention:

• The isPending constant is derived but remains unused outside the dependency array—verify this is intentional (for readability/clarity) versus incomplete implementation
• Confirm that adding this dependency correctly addresses the intended re-run behavior for the logs streaming effect and doesn't introduce unnecessary re-renders

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is incomplete. It lacks most required sections from the template including issue reference, testing details, comprehensive changelog, and checklist items.	Add issue reference (Closes #), complete the checklist, describe testing steps taken, and expand the changelog section with more detail about the fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(ui): refresh deployments logs stream on status change' is specific and directly reflects the main change—ensuring logs stream refreshes when deployment status transitions occur.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refresh-logs-on-status-change

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (1)

89-161: The fix is incomplete: adding isPending to dependencies won't update it since deployment data never revalidates in the child route.

The parent route uses useAutoRevalidate() to poll for deployment list updates, but in Remix, parent revalidation doesn't cascade to child routes. The deployment detail loader only runs once on navigation. This means isPending is calculated once from the initial deployment.status and never changes, even if the status updates on the backend.

To fix this properly, the child route needs its own revalidation mechanism—either polling the loader with useAutoRevalidate() (matching the parent pattern), using event streaming like the runs route, or manually triggering revalidation when appropriate.

🧹 Nitpick comments (1)

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (1)

87-87: Consider using deployment.status directly in the dependency array.

The isPending variable is only used as a dependency trigger and is never referenced elsewhere. Using deployment.status directly in the dependency array (line 161) would be more idiomatic and would cause the logs stream to refresh on any status change, not just transitions to/from PENDING. This provides better coverage if logs need to be reloaded during other status transitions.

Apply this diff to simplify:

-  const isPending = deployment.status === "PENDING";

And update line 161:

-  }, [s2Logs?.basin, s2Logs?.stream, s2Logs?.accessToken, isPending]);
+  }, [s2Logs?.basin, s2Logs?.stream, s2Logs?.accessToken, deployment.status]);

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb99af5 and 5b65b98.

📒 Files selected for processing (1)

• apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (1)

apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (1)

• deployment (182-200)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)

• GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
• GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
• GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
• GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
• GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
• GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
• GitHub Check: typecheck / typecheck
• GitHub Check: Analyze (javascript-typescript)