[snyaggarwal]

Part of OpenConceptLab/ocl_online#418, following the design revision on the ticket.

• Header matches oclweb3: search (opens oclweb3 results), + menu, OCL mark opens oclweb3 /
• Tray uses the shared items and Create link
• / and /map-projects redirect to the oclweb3 profile tab when signed in
• Breadcrumb on the project page; page titles end with OCL
• Removes the spurious Leave site prompt (autosave guard covers unsaved work)
• TERMBROWSER_URL config replaces the unused OCL_ONLINE variable

Sibling PRs: OpenConceptLab/oclweb3#89, https://github.com/OpenConceptLab/ocl-community-site/pull/213

🤖 Generated with Claude Code

[snyaggarwal]

Updated review after Jon's revised unified-navigation design on OpenConceptLab/ocl_online#418.

Correction: I withdraw the earlier app-switcher finding and the suggestion to retain Mapper's local signed-in project list. The revised design intentionally uses one work-oriented tray and redirects the Mapper list to the TermBrowser profile tab.

Remaining findings — request changes:

1. getV3URL() (src/common/utils.js:1096-1105) still silently falls back to the production app.v3.openconceptlab.org deployment for an unconfigured custom host. Jon's revision says an integrated deployment is identified by configuration, with hostname fallback for recognized OCL hosts. A custom deployment missing TERMBROWSER_URL should fail clearly rather than redirecting users into a different deployment.
2. The tray does not highlight Mapping Projects while a Mapper project is open. LeftMenu.jsx:71-79 compares the Mapper pathname (for example /users/a/map-projects/123) with the TermBrowser profile-tab path (/users/a/map-projects) for exact equality, so selected is false. The design requires the current work area to be highlighted wherever it is served.
3. The shared tray requires a Mapping Projects count, but LeftMenu.jsx:96-100 renders only the repository count. Add the same project count as the TermBrowser tray and cover both the count and project-page selection with tests.

The corrected cross-repo review is on OpenConceptLab/ocl_online#418.

[snyaggarwal]

Addressed the review in 9cca2ab: the tray now highlights Mapping Projects on project pages, shows the Mapping Projects count (user plus organization projects), and tests cover TermBrowser URL resolution (configured, official host, localhost, custom host). An unconfigured custom host resolving to the production TermBrowser is intended, so that finding is not changed.

[snyaggarwal]

Final review — DO NOT MERGE (9cca2ab)

The follow-up commit resolves the earlier count and project-page-selection findings. CI is green.

One release blocker remains: resolveV3URL() sends an unconfigured custom Mapper host to https://app.v3.openconceptlab.org, and MapProjectsRedirect then redirects every signed-in visitor there. A missing TERMBROWSER_URL on a self-hosted deployment therefore crosses deployment/account boundaries instead of exposing a configuration error.

Jon's revised design makes TermBrowser configuration part of identifying an integrated deployment. Mapper may require TermBrowser, but a custom deployment must provide its own URL; it must not silently borrow the production deployment. Return an unavailable/misconfigured result (or fail clearly) for an unrecognized host without TERMBROWSER_URL, while retaining localhost and recognized openconceptlab.org fallbacks. Update the custom-host test accordingly. With that change and green checks, I recommend merge.

[snyaggarwal]

Addressed the final review in 538a4f6: an integrated TermBrowser now requires TERMBROWSER_URL, localhost, or a recognized openconceptlab.org Mapper host. Without it the Mapper keeps its own project list, and the TermBrowser-linked tray items, search, create items and logo link are hidden. Existing entity links keep their previous behavior. Tests added.

[snyaggarwal]

Final review — round 2

Recommendation: DO NOT MERGE yet.

Most of the prior fail-closed issue is fixed at head 538a4f6: the tray, search, logo, redirects, and add menu now behave correctly when Mapper has no configured Term Browser integration.

One blocking path remains in the new MapProjectBreadcrumb component. Both breadcrumb links call toV3URL() directly (src/components/map-projects/MapProjectBreadcrumb.jsx, lines 14 and 16). On an unconfigured custom/self-hosted Mapper, that legacy helper still falls back to production, so opening a project exposes links into an unrelated production Term Browser.

Please hide/replace those breadcrumb destinations when hasTermBrowser() is false (the Mapping Projects crumb can remain local), and add a standalone-deployment regression test. After that focused fix, I would recommend merge.

GitHub PR checks and Snyk are currently passing.

(Final review round 2)

[snyaggarwal]

Addressed round 2 in db31c5f: without a configured TermBrowser the breadcrumb shows the owner as text and the Mapping Projects crumb links to the local list; regression tests added.

[paynejd]

Codex review, pass 1 (codex-cli 0.166.0, commit 95852a0)

Verdict: findings: F1.1 P1, F1.2 P1, F1.3 P1.

• F1.1 (P1) src/components/map-projects/leaveGuard.js:14: Failed autosaves no longer leave an unload warning protecting unsaved decisions. Response: Fixed in 14d0134: a failed save keeps the unload warning until a save succeeds.
• F1.2 (P1) src/components/map-projects/MapProjectsRedirect.jsx:7: The redirect immediately removes the existing blocked-project dialog. Response: Fixed in 14d0134: the redirect keeps the refused-new-project explanation for that visit.
• F1.3 (P1) src/common/appUrls.js:9: Unconfigured standalone installs incorrectly enable Term Browser redirects. Response: Fixed in 14d0134: localhost counts as connected to a TermBrowser only when TERMBROWSER_URL is set; docker-compose defaults it to empty.

findings.json

{
 "reviewer": "codex-cli 0.166.0",
 "perspective": "defects",
 "mode": "full",
 "commit": "95852a062fba7867d87d53df4fd97f41bf15f7cc",
 "verdict": "findings",
 "prior": [],
 "findings": [
  {
   "id": "F1.1",
   "severity": "P1",
   "tag": null,
   "file": "src/components/map-projects/leaveGuard.js",
   "line": 14,
   "claim": "Failed autosaves no longer leave an unload warning protecting unsaved decisions.",
   "scenario": "Change a decision in a saved project and let its autosave fail with a network error or HTTP 500. The scheduler has cleared its pending timer and onSave clears inFlightSaveRef, so this predicate returns false. Closing the tab or following a cross-app link silently discards the change. The removed decisions-based beforeunload handler previously warned in this case.",
   "action": "Track unsaved changes until a save succeeds and retain the unload warning after failure. Add a regression test covering a failed autosave followed by navigation.",
   "lines_delta": null,
   "confidence": "high"
  },
  {
   "id": "F1.2",
   "severity": "P1",
   "tag": null,
   "file": "src/components/map-projects/MapProjectsRedirect.jsx",
   "line": 7,
   "claim": "The redirect immediately removes the existing blocked-project dialog.",
   "scenario": "With a Term Browser configured, a denied new-project attempt redirects to '/' with state.newProjectBlock. MapProjects mounts, copies that reason into component state, and clears the history state. This wrapper then recomputes redirect as true, unmounts the dialog, and sends the user to TBv3 without explaining why creation failed. Reproduced with the actual wrapper and MapProjects lifecycle.",
   "action": "Preserve the local fallback while the block message is being displayed, independently of the consumed history state. Test the complete route transition.",
   "lines_delta": null,
   "confidence": "high"
  },
  {
   "id": "F1.3",
   "severity": "P1",
   "tag": null,
   "file": "src/common/appUrls.js",
   "line": 9,
   "claim": "Unconfigured standalone installs incorrectly enable Term Browser redirects.",
   "scenario": "Run only Mapper at localhost:4004 with TERMBROWSER_URL unset. This resolver reports localhost:4002 as an integrated Term Browser, so signed-in visits to '/' or '/map-projects' redirect to a nonexistent service instead of displaying the local project list. docker-compose.yml also injects that default on custom hosts, contrary to the approved configuration rule.",
   "action": "Remove implicit localhost integration and default the Compose setting to empty. Enable cross-app redirects only for an explicitly configured URL or recognized official hostname.",
   "lines_delta": null,
   "confidence": "high"
  }
 ]
}

Codex review, pass 2 (codex-cli 0.166.0, commit 14d0134)

Verdict: findings: F2.1 P1.

• F2.1 (P1) src/components/map-projects/MapProject.jsx:430: Removing the decisions-based unload warning leaves saved-project edits that do not schedule autosave unprotected. Response: Fixed in 42acd47 at the cause: editing a review note, and mapping or unmapping a candidate by hand (same gap), now schedule autosave.

findings.json

{
 "reviewer": "codex-cli 0.166.0",
 "perspective": "defects",
 "mode": "verify",
 "commit": "14d0134510d4fc440a5e7acbdb636650b81a5fd9",
 "verdict": "findings",
 "prior": [
  {
   "id": "F1.1",
   "status": "resolved"
  },
  {
   "id": "F1.2",
   "status": "resolved"
  },
  {
   "id": "F1.3",
   "status": "resolved"
  }
 ],
 "findings": [
  {
   "id": "F2.1",
   "severity": "P1",
   "tag": null,
   "file": "src/components/map-projects/MapProject.jsx",
   "line": 430,
   "claim": "Removing the decisions-based unload warning leaves saved-project edits that do not schedule autosave unprotected.",
   "scenario": "Open a saved project with an existing mapping decision, wait for saves to finish, edit its Review note, then click the OCL logo or close the tab. The note handler only calls setNotes; it schedules no autosave. Every condition in the new unload guard is therefore false, and the note is silently discarded. The removed decisions-based listener previously warned in this case.",
   "action": "Track unsaved persistent edits independently of pending requests, or ensure those edits schedule autosave, and include them in the unload guard. Add a regression test covering a note edit followed by navigation.",
   "lines_delta": null,
   "confidence": "high"
  }
 ]
}

Codex review, pass 3 (codex-cli 0.166.0, commit 42acd47)

Verdict: findings: F3.1 P1, F3.2 P1, F3.3 P2.

• F3.1 (P1) src/components/map-projects/MapProject.jsx:431: Unsaved configuration edits no longer trigger the unload warning. Response: Fixed in ee77521: leaving with Configure open and changed warns, and an in-app leave saves those changes.
• F3.2 (P1) src/components/map-projects/MapProject.jsx:6803: The review-note fix schedules a save that can omit the note entirely. Response: Accepted, not caused by this PR: the save payload has never included notes for unmapped rows (main and production too). Filed as a follow-up.
• F3.3 (P2) src/components/map-projects/MapProjectsRedirect.jsx:17: Cancelling the automatic redirect leaves an empty project-list page. Response: Accepted (P2): needs a refused new project, then an edit and Back inside the autosave window, then Stay.

findings.json

{
 "reviewer": "codex-cli 0.166.0",
 "perspective": "defects",
 "mode": "verify",
 "commit": "42acd47832ee0e70cb8d81adbe0a9583ecb5299d",
 "verdict": "findings",
 "prior": [
  {
   "id": "F1.1",
   "status": "resolved"
  },
  {
   "id": "F1.2",
   "status": "resolved"
  },
  {
   "id": "F1.3",
   "status": "resolved"
  },
  {
   "id": "F2.1",
   "status": "unresolved"
  }
 ],
 "findings": [
  {
   "id": "F3.1",
   "severity": "P1",
   "tag": null,
   "file": "src/components/map-projects/MapProject.jsx",
   "line": 431,
   "claim": "Unsaved configuration edits no longer trigger the unload warning.",
   "scenario": "Open Configure on a saved project, change its name or description, then follow the header logo without closing Configure. These setters schedule no autosave until closeConfiguration runs, so every unload-guard condition is false and the edits disappear without warning. The removed decisions-based guard previously protected this navigation.",
   "action": "Track outstanding configuration edits independently of the autosave timer and include them in leave protection.",
   "lines_delta": null,
   "confidence": "high"
  },
  {
   "id": "F3.2",
   "severity": "P1",
   "tag": null,
   "file": "src/components/map-projects/MapProject.jsx",
   "line": 6803,
   "claim": "The review-note fix schedules a save that can omit the note entirely.",
   "scenario": "In a saved project, enter a review note on a row that has never been mapped. onSave builds matches only from mapSelected, which excludes that row, and the CSV export also omits notes. After autosave succeeds, the unload warning clears although the note was never persisted; navigating away loses it. This leaves F2.1 unresolved for unmapped rows.",
   "action": "Persist and restore notes for every row, including rows absent from mapSelected, and verify an edit\u2013autosave\u2013reopen round trip.",
   "lines_delta": null,
   "confidence": "high"
  },
  {
   "id": "F3.3",
   "severity": "P2",
   "tag": null,
   "file": "src/components/map-projects/MapProjectsRedirect.jsx",
   "line": 17,
   "claim": "Cancelling the automatic redirect leaves an empty project-list page.",
   "scenario": "With TBv3 configured, open a saved project from the local list retained after a refused new project, edit it, then press Back within the autosave window. Unmount starts a save, so the automatic location.replace triggers the global unload warning. Choosing Stay leaves this component rendering null permanently, even after the save completes.",
   "action": "Wait for outstanding saves before redirecting and retain a usable fallback when navigation is cancelled.",
   "lines_delta": null,
   "confidence": "high"
  }
 ]
}

Simplification review (Claude subagent, commit 14d0134)

5 suggestions. Applied in 1b1a1fd: S1 (an unused search placeholder string removed from four locales) and S2 (Stop and leave reuses leaveTo). Declined for today's deploy: S3 (a duplicate leave-guard test; cheap, kept), S4 (getAppTitle alias; touches DocumentTitle), S5 (shared tray sx constants; restyles the main tray render, follow-up together with oclweb3).

Codex check of commits 42acd47..ee77521 (codex-cli 0.166.0)

Verdict: clean.

findings.json

{
 "reviewer": "codex-cli 0.166.0",
 "perspective": "defects",
 "mode": "final",
 "commit": "ee7752144a5aeb484a21f4bc6e6744802cd9cf4e",
 "verdict": "clean",
 "prior": [
  {
   "id": "F3.1",
   "status": "resolved"
  },
  {
   "id": "F3.2",
   "status": "unresolved"
  },
  {
   "id": "F3.3",
   "status": "unresolved"
  }
 ],
 "findings": []
}

[paynejd]

Approved after the takeover review: Codex 3 passes (all P1s fixed) + clean final check, simplification review applied, CI green. OpenConceptLab/ocl_online#418