fix: bind docker-compose ports to loopback, enforce lint gate, split oversized UI files#245
Merged
Merged
Conversation
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
📝 WalkthroughWalkthroughDocker Compose now binds local service ports to loopback, and the web app adds shared headers, settings, and issue-detail components before rewiring the main pages to use them. ChangesLocal-only port binding
Shared web UI components and page rewiring
Lint warnings fail the web check
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #246 |
…s to avoid stale-closure races
nazarli-shabnam
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
apps/web/src/components/layout/page-header/ProjectsHeader.tsx (1)
86-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated query-param key union type.
The set of valid keys is spelled out identically in both
updateParam(Line 87-99) andupdateParams(Line 109-121). Extracting a shared type avoids the two falling out of sync as new params are added.♻️ Proposed refactor
+type ProjectsQueryParamKey = + | 'q' + | 'sort' + | 'sortField' + | 'sortDir' + | 'filter' + | 'access' + | 'lead' + | 'members' + | 'myProjects' + | 'createdDate' + | 'createdAfter' + | 'createdBefore'; + - const updateParam = ( - key: - | 'q' - | 'sort' - | 'sortField' - | 'sortDir' - | 'filter' - | 'access' - | 'lead' - | 'members' - | 'myProjects' - | 'createdDate' - | 'createdAfter' - | 'createdBefore', - value?: string, - ) => { + const updateParam = (key: ProjectsQueryParamKey, value?: string) => { ... }; const updateParams = ( - updates: Partial< - Record< - | 'q' - | 'sort' - | 'sortField' - | 'sortDir' - | 'filter' - | 'access' - | 'lead' - | 'members' - | 'myProjects' - | 'createdDate' - | 'createdAfter' - | 'createdBefore', - string | undefined - > - >, + updates: Partial<Record<ProjectsQueryParamKey, string | undefined>>, ) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/page-header/ProjectsHeader.tsx` around lines 86 - 132, The valid query-param key union is duplicated in ProjectsHeader’s updateParam and updateParams, which can drift out of sync. Extract a shared type alias for the allowed keys in ProjectsHeader and reuse it in both helpers so the key list is defined once and stays consistent as new params are added.apps/web/src/components/settings/modals/ExportModal.tsx (1)
128-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExport failures are silently swallowed.
On error, nothing is surfaced to the user beyond the button reverting to "Export" — they have no indication the export failed or why.
Want me to wire up an error state prop (similar to other modals in this PR) to surface this to the user?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/settings/modals/ExportModal.tsx` around lines 128 - 132, Export errors are currently swallowed in ExportModal, so the user never sees a failure state. Update the ExportModal.tsx error handling around the export flow to catch the thrown error, store it in an error state or prop like the other modals in this PR, and surface that state in the modal UI instead of only resetting setExporting(false). Use the ExportModal component and its export action handlers to wire the error through to the user-facing message.apps/web/src/components/work-item/IssueLinksPanel.tsx (2)
53-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd-link failures are silently swallowed.
Same pattern as
IssueAttachmentsPanel.tsx: thecatchblock is empty, so a failedcreateLinkcall leaves the form open with no indication to the user that the submission failed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/IssueLinksPanel.tsx` around lines 53 - 68, The add-link submit handler in IssueLinksPanel’s onSubmit is swallowing createLink failures, leaving users with no feedback when the request fails. Update the catch block to surface the error in the same way IssueAttachmentsPanel handles failed submissions, and keep the form state consistent by clearing the adding state while preserving the open form so the user can retry. Use the onSubmit handler and issueService.createLink flow as the reference points.
117-125: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winOptimistic delete without rollback on failure.
deleteLinkerrors are swallowed (.catch(() => {})) yetonLinksChangestill removes the link from local state unconditionally, so a failed server-side delete leaves the UI showing the link as removed while it still exists server-side (it will reappear on next full refresh, causing user confusion).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/IssueLinksPanel.tsx` around lines 117 - 125, The delete handler in IssueLinksPanel is doing an optimistic removal without any failure recovery, so a failed issueService.deleteLink call still updates local state via onLinksChange and desyncs the UI from the server. In the button onClick for the link item, remove the swallowed catch or handle the error explicitly, and only call onLinksChange after deleteLink succeeds; if you keep optimistic UI, add rollback logic using the current link id from l.id so the link is restored when the request fails.apps/web/src/components/work-item/IssueAttachmentsPanel.tsx (2)
41-81: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUpload failures are silently swallowed with no user feedback.
The
catchblock only has a comment (ignore — 503 if MinIO not configured); the uploading indicator simply disappears with no attachment added and no error shown, leaving the user unsure whether the upload succeeded or failed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/IssueAttachmentsPanel.tsx` around lines 41 - 81, The attachment upload flow in IssueAttachmentsPanel silently ignores failures, leaving users with no indication of what went wrong. Update the async onChange handler to catch and surface errors from issueService.initiateAttachmentUpload, the fetch to resp.upload_data.url, and issueService.confirmAttachmentUpload, and show a user-facing error state or message instead of swallowing the exception. Keep the existing uploading reset and file input clear behavior, but ensure failures in the upload path are handled explicitly so the user can tell the upload did not complete.
92-100: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAttachment URL rendered without sanitization, unlike the sibling Links panel.
IssueLinksPanel.tsxappliessafeUrl()before using a link's URL as anhref(Line 109 of that file), but hereatt.asset_urlis used directly.asset_urlis server-generated (presigned upload URL), so risk is low today, but applying the samesafeUrl()guard would keep the two right-rail panels consistent and defend against any future change in howasset_urlis populated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/IssueAttachmentsPanel.tsx` around lines 92 - 100, The attachment link in IssueAttachmentsPanel is using att.asset_url directly, unlike the sanitized approach in IssueLinksPanel. Update the anchor in IssueAttachmentsPanel to run the URL through safeUrl() before assigning it to href, keeping the attachment and links panels consistent and guarding against unsafe future values; use the existing att.asset_url and safeUrl helper to locate the change.apps/web/src/components/layout/page-header/AnalyticsHeader.tsx (1)
1-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating repeated project-dropdown logic.
The project-fetch effect (13-26),
q()search helper (32), and filtered-list pattern here are duplicated nearly verbatim acrossPageDetailHeader.tsx,ProjectSavedViewDetailHeader.tsx, andProjectSectionHeader.tsx. Extracting a shareduseProjectDropdown(workspaceSlug)hook would reduce copy/paste drift risk across these header components.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/page-header/AnalyticsHeader.tsx` around lines 1 - 100, The project dropdown logic is duplicated across AnalyticsHeader and the other header components, so extract the repeated fetch/search/filter state into a shared useProjectDropdown(workspaceSlug) hook. Move the projectService.list effect, the q search helper, and the filteredProjects/selection state handling into that hook, then update AnalyticsHeader to consume the hook’s returned projects, search state, and selection setters instead of owning the copy-pasted logic.apps/web/src/components/layout/page-header/ProjectSectionHeader.tsx (1)
530-567: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated collapsible-search and date-preset filter markup across sections.
The expand/collapse search-input pattern is duplicated near-verbatim for cycles (Lines 530-567) and modules (Lines 789-831), and the start/due date preset checklist (Lines 660-707 vs 709-768) is duplicated with only label/state differences. Extracting shared
CollapsibleSearchInputandDatePresetFilterSectioncomponents would cut a large amount of duplicated JSX and reduce drift risk between the copies.Also applies to: 611-646, 660-768, 789-831
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/page-header/ProjectSectionHeader.tsx` around lines 530 - 567, The cycles and modules expand/collapse search UI, along with the start/due date preset checklist blocks, are duplicated across ProjectSectionHeader instead of being shared. Extract reusable components such as CollapsibleSearchInput and DatePresetFilterSection, then replace the repeated JSX in the cycles and modules sections with those components while keeping the existing state/label differences passed as props.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/layout/page-header/InboxHeader.tsx`:
- Around line 13-40: The action buttons in InboxHeader are currently inert, so
users can click Mark as read, Archive, Filters, and More options without any
behavior. Update the button elements in InboxHeader to either pass the
appropriate click handlers for each action or render them disabled/hidden until
the functionality exists, and use the existing icon/button labels to keep the
implementation easy to locate.
In `@apps/web/src/components/layout/page-header/ProjectDetailHeader.tsx`:
- Around line 5-38: The Search control in ProjectDetailHeader is a stub with
unused workspaceSlug and projectId props, so either wire the button to the
intended search/command-palette action using those props and add an onClick
handler, or remove the button and unused props if the feature is not ready.
Update ProjectDetailHeader to make the Search button functional or eliminate the
non-working UI so it no longer appears clickable without behavior.
In `@apps/web/src/components/layout/page-header/ProjectSavedViewDetailHeader.tsx`:
- Around line 63-114: The view-fetching and filter-parsing logic is duplicated
between `refreshView.current` and the mount `useEffect`, so consolidate it into
a single shared path. Move the `viewService.get`, title fallback, and
`parseWorkspaceViewFiltersFromSearchParams` handling into one helper or into
`refreshView.current`, then have the `useEffect` call that shared logic and keep
only the `cancelled` guard there. Use the existing `refreshView`,
`viewService.get`, and `parseWorkspaceViewFiltersFromSearchParams` symbols to
locate the duplicated code and ensure both refresh and mount paths stay in sync.
In `@apps/web/src/components/layout/page-header/ProjectSectionHeader.tsx`:
- Around line 89-140: Reset the project-scoped state in ProjectSectionHeader
when the active project changes, since PageHeader reuses this component without
a key and state like cyclesSearch, viewsSearchQuery, issuesFilters, and related
dropdown/filter state can leak across projects. Add a projectId-based reset
effect in ProjectSectionHeader or key the component by projectId in the parent
so all project-specific hooks and defaults are reinitialized on switch.
In `@apps/web/src/components/layout/page-header/WorkspaceViewsHeader.tsx`:
- Around line 38-47: The custom-views load in WorkspaceViewsHeader lacks a
cancellation guard, so an older viewService.list result can overwrite newer
state after workspaceSlug changes. Update the useEffect to follow the same
cancelled-flag pattern used in the sibling header fetch effects: track whether
the effect has been cleaned up, and only call setCustomViews from the promise
handlers when the current request is still active. Keep the existing
workspaceSlug-based reset path, but ensure both the success and error handling
in useEffect respect the guard.
In `@apps/web/src/components/settings/modals/ProjectLabelModal.tsx`:
- Around line 44-73: Add a pending-state guard to the save action in
ProjectLabelModal so the Button cannot trigger duplicate label create/update
requests while the async handler is in flight, and replace the empty catch with
error handling that surfaces the failure to the user instead of silently
swallowing it. Update the onClick handler around labelService.create/update and
labelService.list to use a loading state, disable the action during save, and
only close the modal after a successful response.
In `@apps/web/src/components/settings/modals/ProjectStateModal.tsx`:
- Around line 49-77: The ProjectStateModal action handler has the same
duplicate-submit and silent-error problem as ProjectLabelModal: the Create/Save
Button in ProjectStateModal can be clicked repeatedly while
stateService.create/update is in flight, and the empty catch hides failures. Add
a pending/loading guard around the onClick flow so the button is disabled during
submission, and replace the empty catch with error handling that surfaces the
failure to the user while preserving the existing state refresh and modal-close
logic in the projectStateEdit branch and stateService.create/update calls.
In `@apps/web/src/components/settings/SettingsNav.tsx`:
- Around line 67-99: The section nav button markup is duplicated across the
SettingsNav branches, so extract the repeated icon+label+active-state button
into a reusable helper such as SectionNavButton and use it for
ACCOUNT_SECTIONS_PROFILE, ACCOUNT_SECTIONS_DEVELOPER, and the other section
maps. Keep the helper responsible for the shared className logic and onClick
handling, and pass in id, label, icon, active, and callback props so all
branches render consistently and future style changes happen in one place.
---
Nitpick comments:
In `@apps/web/src/components/layout/page-header/AnalyticsHeader.tsx`:
- Around line 1-100: The project dropdown logic is duplicated across
AnalyticsHeader and the other header components, so extract the repeated
fetch/search/filter state into a shared useProjectDropdown(workspaceSlug) hook.
Move the projectService.list effect, the q search helper, and the
filteredProjects/selection state handling into that hook, then update
AnalyticsHeader to consume the hook’s returned projects, search state, and
selection setters instead of owning the copy-pasted logic.
In `@apps/web/src/components/layout/page-header/ProjectSectionHeader.tsx`:
- Around line 530-567: The cycles and modules expand/collapse search UI, along
with the start/due date preset checklist blocks, are duplicated across
ProjectSectionHeader instead of being shared. Extract reusable components such
as CollapsibleSearchInput and DatePresetFilterSection, then replace the repeated
JSX in the cycles and modules sections with those components while keeping the
existing state/label differences passed as props.
In `@apps/web/src/components/layout/page-header/ProjectsHeader.tsx`:
- Around line 86-132: The valid query-param key union is duplicated in
ProjectsHeader’s updateParam and updateParams, which can drift out of sync.
Extract a shared type alias for the allowed keys in ProjectsHeader and reuse it
in both helpers so the key list is defined once and stays consistent as new
params are added.
In `@apps/web/src/components/settings/modals/ExportModal.tsx`:
- Around line 128-132: Export errors are currently swallowed in ExportModal, so
the user never sees a failure state. Update the ExportModal.tsx error handling
around the export flow to catch the thrown error, store it in an error state or
prop like the other modals in this PR, and surface that state in the modal UI
instead of only resetting setExporting(false). Use the ExportModal component and
its export action handlers to wire the error through to the user-facing message.
In `@apps/web/src/components/work-item/IssueAttachmentsPanel.tsx`:
- Around line 41-81: The attachment upload flow in IssueAttachmentsPanel
silently ignores failures, leaving users with no indication of what went wrong.
Update the async onChange handler to catch and surface errors from
issueService.initiateAttachmentUpload, the fetch to resp.upload_data.url, and
issueService.confirmAttachmentUpload, and show a user-facing error state or
message instead of swallowing the exception. Keep the existing uploading reset
and file input clear behavior, but ensure failures in the upload path are
handled explicitly so the user can tell the upload did not complete.
- Around line 92-100: The attachment link in IssueAttachmentsPanel is using
att.asset_url directly, unlike the sanitized approach in IssueLinksPanel. Update
the anchor in IssueAttachmentsPanel to run the URL through safeUrl() before
assigning it to href, keeping the attachment and links panels consistent and
guarding against unsafe future values; use the existing att.asset_url and
safeUrl helper to locate the change.
In `@apps/web/src/components/work-item/IssueLinksPanel.tsx`:
- Around line 53-68: The add-link submit handler in IssueLinksPanel’s onSubmit
is swallowing createLink failures, leaving users with no feedback when the
request fails. Update the catch block to surface the error in the same way
IssueAttachmentsPanel handles failed submissions, and keep the form state
consistent by clearing the adding state while preserving the open form so the
user can retry. Use the onSubmit handler and issueService.createLink flow as the
reference points.
- Around line 117-125: The delete handler in IssueLinksPanel is doing an
optimistic removal without any failure recovery, so a failed
issueService.deleteLink call still updates local state via onLinksChange and
desyncs the UI from the server. In the button onClick for the link item, remove
the swallowed catch or handle the error explicitly, and only call onLinksChange
after deleteLink succeeds; if you keep optimistic UI, add rollback logic using
the current link id from l.id so the link is restored when the request fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c98f19b-817b-4367-995f-4f6a20676c08
📒 Files selected for processing (31)
apps/web/package.jsonapps/web/src/components/layout/PageHeader.tsxapps/web/src/components/layout/page-header/AnalyticsHeader.tsxapps/web/src/components/layout/page-header/DraftsHeader.tsxapps/web/src/components/layout/page-header/HomeHeader.tsxapps/web/src/components/layout/page-header/InboxHeader.tsxapps/web/src/components/layout/page-header/PageDetailHeader.tsxapps/web/src/components/layout/page-header/ProjectDetailHeader.tsxapps/web/src/components/layout/page-header/ProjectSavedViewDetailHeader.tsxapps/web/src/components/layout/page-header/ProjectSectionDropdown.tsxapps/web/src/components/layout/page-header/ProjectSectionHeader.tsxapps/web/src/components/layout/page-header/ProjectsHeader.tsxapps/web/src/components/layout/page-header/SettingsHeader.tsxapps/web/src/components/layout/page-header/WorkspaceViewsHeader.tsxapps/web/src/components/layout/page-header/YourWorkHeader.tsxapps/web/src/components/layout/page-header/icons.tsxapps/web/src/components/settings/SettingsNav.tsxapps/web/src/components/settings/icons.tsxapps/web/src/components/settings/modals/ExportModal.tsxapps/web/src/components/settings/modals/InviteModal.tsxapps/web/src/components/settings/modals/ProjectLabelModal.tsxapps/web/src/components/settings/modals/ProjectStateModal.tsxapps/web/src/components/settings/sections-config.tsxapps/web/src/components/work-item/IssueAttachmentsPanel.tsxapps/web/src/components/work-item/IssueLinksPanel.tsxapps/web/src/components/work-item/IssueRelationsPanel.tsxapps/web/src/components/work-item/MoveWorkItemModal.tsxapps/web/src/components/work-item/issue-detail-icons.tsxapps/web/src/lib/settingsHelpers.tsapps/web/src/pages/IssueDetailPage.tsxapps/web/src/pages/SettingsPage.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/components/work-item/IssueRelationsPanel.tsx
Member
Author
|
Reviewed all 8 CodeRabbit findings + the code-quality bot's type-comparison note. I diffed each flagged location against pre-refactor `main` and confirmed every single one is pre-existing behavior, byte-for-byte identical to the original monolithic file — the mechanical extraction correctly carried them over verbatim rather than introducing them:
Since this PR's scope was explicitly pure code-motion with zero behavior change (no automated UI test suite exists to safety-net behavioral fixes), I'm leaving these as-is here rather than expanding scope. They're all legitimate, worth fixing — I'd suggest filing them as separate follow-up issues (several are good "quick win" candidates per CodeRabbit's own labels). Happy to pick those up next if wanted. Strix: no security issues found. |
nazarli-shabnam
commented
Jul 3, 2026
…rfaced by the refactor)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/settings/modals/ProjectStateModal.tsx (1)
40-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStale error persists across modal re-opens.
error/savingare local state, and the modal appears to stay mounted (toggled viaopenprop perSettingsPage.tsxusage). If a save fails and the user closes the modal, the error banner will reappear on next open until another submit attempt runs.🛠️ Proposed fix
const [saving, setSaving] = useState(false); const [error, setError] = useState<string | null>(null); + useEffect(() => { + if (!open) { + setError(null); + setSaving(false); + } + }, [open]); +Also applies to: 89-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/settings/modals/ProjectStateModal.tsx` around lines 40 - 41, The ProjectStateModal local state is not being reset between open/close cycles, so stale error and saving values persist when the modal reopens. Update ProjectStateModal to clear error and reset saving whenever the modal is opened or closed, using the open prop from SettingsPage and the existing setError/setSaving state in the component. Make sure the reset happens on each modal reopen so the error banner does not reappear from a previous failed submit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/components/settings/modals/ProjectStateModal.tsx`:
- Around line 40-41: The ProjectStateModal local state is not being reset
between open/close cycles, so stale error and saving values persist when the
modal reopens. Update ProjectStateModal to clear error and reset saving whenever
the modal is opened or closed, using the open prop from SettingsPage and the
existing setError/setSaving state in the component. Make sure the reset happens
on each modal reopen so the error banner does not reappear from a previous
failed submit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74bf4e6e-fbb3-4c26-a14c-68d4d624f850
📒 Files selected for processing (11)
apps/web/src/components/layout/PageHeader.tsxapps/web/src/components/layout/Sidebar.tsxapps/web/src/components/layout/page-header/InboxHeader.tsxapps/web/src/components/layout/page-header/ProjectDetailHeader.tsxapps/web/src/components/layout/page-header/ProjectSavedViewDetailHeader.tsxapps/web/src/components/layout/page-header/WorkspaceViewsHeader.tsxapps/web/src/components/settings/SettingsNav.tsxapps/web/src/components/settings/modals/ExportModal.tsxapps/web/src/components/settings/modals/ProjectLabelModal.tsxapps/web/src/components/settings/modals/ProjectStateModal.tsxapps/web/src/lib/commandPaletteEvents.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/components/layout/page-header/InboxHeader.tsx
- apps/web/src/components/layout/page-header/ProjectDetailHeader.tsx
- apps/web/src/components/layout/page-header/WorkspaceViewsHeader.tsx
- apps/web/src/components/settings/SettingsNav.tsx
- apps/web/src/components/settings/modals/ProjectLabelModal.tsx
- apps/web/src/components/layout/page-header/ProjectSavedViewDetailHeader.tsx
- apps/web/src/components/settings/modals/ExportModal.tsx
nazarli-shabnam
commented
Jul 4, 2026
nazarli-shabnam
left a comment
nazarli-shabnam
left a comment
Member
Author
There was a problem hiding this comment.
Addressed in commit 05fe17c
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
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
this PR bundles three issues (#150, #151, #133) instead of one PR per issue.
#150 — docker-compose ports bound to loopback
docker-compose.ymlpublished every local-dev infra port using Docker's short syntax, which binds0.0.0.0(all host interfaces). Combined with the default credentials already in the file, anyone reachable on the host's network could connect to Postgres/Redis/RabbitMQ/MinIO. Now bound to127.0.0.1explicitly, plus a comment noting the file is local-dev-only.#151 — lint script now matches lint-staged's strictness
lint-stagedran ESLint with--max-warnings=0, but thelintscript used by CI/validatewas plaineslint .(passes with warnings). Added--max-warnings=0toapps/web/package.json'slintscript so pre-commit/CI/validate are in lock-step. Verified zero pre-existing warnings before tightening this, so nothing breaks.#133 — split oversized UI files (partial)
Three files were large enough to be hard to review:
SettingsPage.tsx(4,250 lines),PageHeader.tsx(3,948 lines),IssueDetailPage.tsx(2,068 lines). This is a risk-scoped, partial pass — pure code motion only, no state-management redesign, since there's no automated UI test suite to lean on:PageHeader.tsx: fully split (3,948 → 278-line dispatcher). It was already internally decomposed into 13 independent functions with no shared mutable state, so this was a clean, low-risk, mechanical move intocomponents/layout/page-header/.ProjectSectionHeader.tsx(1,589 lines) was deliberately left unsplit — it branches internally onsectionsimilarly to the top-level dispatcher, and splitting it further is a reasonable follow-up but wasn't done here to bound scope.IssueDetailPage.tsx: partially split (2,068 → 1,434 lines). Extracted the self-contained sidebar panels (IssueLinksPanel,IssueRelationsPanel,IssueAttachmentsPanel,MoveWorkItemModal) and icon/constant helpers. Left in place: the big parallel-fetch effect, the Properties card, and the comments/activity section — all share state too broadly to extract safely in this pass.SettingsPage.tsx: partially split (4,250 → 3,098 lines). Extracted icons, static section config, the two pure helpers, the sidebar nav, and the four trailing standalone modals (Export/Invite/ProjectState/ProjectLabel). Not done: splitting the actual tab-section bodies (Account/Project/Workspace sub-sections) — they share ~90useStatehooks in one scope; a clean split needs a shared hook/context, which is a bigger, separate effort. This means [CHORE] Split oversized UI pages and layout components #133's "SettingsPage split into section-level components" acceptance criterion is only partially met.Testing
npm run typecheck && npm run lint && npm run buildall pass cleanly after every commit.docker compose config --quiet,docker compose up -d(confirmed127.0.0.1:<port>binding viadocker compose ps), and a fullgo run ./cmd/apiboot confirming real Postgres/Redis/RabbitMQ/MinIO connectivity over loopback./code-review(high effort, 8 finder angles + verification) run against the entire branch diff. It found and I fixed one real regression: three extracted issue-detail panels (IssueLinksPanel,IssueRelationsPanel,IssueAttachmentsPanel) computed delete/remove updates from a plain prop array instead of the functional-updater form the original code used, reintroducing a stale-closure race on rapid successive deletes (e.g. delete two links quickly → the first one could silently reappear). Fixed by switching all three toDispatch<SetStateAction<T>>+ functional updates. Other review findings were either pre-existing (icon/helper duplication that predates this refactor, a couple of pre-existing dead props/fetches inPageHeader.tsxuntouched by this PR) or deliberate scope tradeoffs already noted above (settings modals still take individual props rather than a hook;ProjectSectionHeaderleft unsplit).curlconfirmed both the dev server and API responded normally). Live UI verification could not be completed this session. Verification instead relied on: exact diffing of every extracted function against its original source (confirmed byte-for-byte identical moves apart from intentional prop-threading), a full 8-angle code review catching the one real regression above, and clean typecheck/lint/build. Recommend a manual click-through of Settings (all tabs), a project header (all sub-sections), and an issue detail page before merging, given this touches high-traffic UI surfaces.AI assistance
Implemented with the help of Claude Code (Claude Sonnet 5) — investigation, the docker-compose/lint fixes, the mechanical file-splitting (via parallel sub-agent tasks per file, each independently verified), the full code-review pass, and the regression fix it surfaced.
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
127.0.0.1to reduce accidental exposure, and local-only credentials/ports are clearly documented.New Features