Repository navigation
feat(isn): add a published report's link to its ISN order - #361
Merged
important-new merged 3 commits intoSep 24, 2026
Merged
Conversation
The report half of InspectorHub#321. When a report is published and the inspection's Reference Number holds an ISN order number, the report link (the primary client's portal-token URL) is added to that ISN order as a public report, then read back from /orderfiles before the sync counts as done. A failed sync never undoes the publish; the outcome is returned as `isnSync` on the publish response, with ISN's own message. Two ISN behaviours its published document does not state, both verified against a live company and pinned in tests/contract/isn/isn-api.live.spec.ts (which writes nothing): - errors arrive as HTTP 200 with `status: "error"`, so success is the body's status, never the HTTP code; - PUT /orders/addreporturl ignores the documented form body and reads the query string. Credentials (ISN_DOMAIN, ISN_COMPANY_KEY, ISN_ACCESS_KEY, ISN_SECRET_KEY) are catalogued secrets set in Settings -> Advanced -> ISN, and tenant-owned: a platform key must not put every company's reports on one ISN account. No default domain, since ISN is white-labelled. The sync touches none of OpenInspection's agreement, payment or report-release gates: the link is an ordinary report link, so those gates apply to it as to any other. Whether ISN should hold or release them is left for a later change. Laid out per docs/develop/integration-adapters.md: api-base / pure payload / sync, a vendored excerpt of ISN's schema with provenance and a contract spec, and docs/integrations/isn.md. The ISN module is imported lazily by the publish route so deployments without ISN pay nothing at startup. Chrome: /settings/advanced — ISN panel in dark theme at 682px wide: four credential fields (address, company key, access key, secret key) then one Save, no hold switch or help text (0 checkboxes in the panel), ISN tab in the section nav, panel sits after the integration keys and before Data management Chrome: /settings/advanced — light theme (root attribute and dark class flipped as useTheme does): panel and inputs white, SET badges and "Not configured" placeholders legible Chrome: /settings/advanced — "inspectionsupport.com" refused under the ISN address field with the ISN_DOMAIN message; https://inspectionsupport.com saved (action returned success) and address and company key both show SET after a reload Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
@NCHILLC Are you still working on this PR? Just checking in since the CI is still failing. |
Contributor
Author
|
I am. I'm low tier subscription with Claude and codex. I just burnt through
a lot getting ai-memory and a homelab server set-up, so I could work more
on the repo. I'm working on my fork of it, today. I don't ever want to
commit anything to your repo until it's an honest attempt for a PR.
[image:
https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d]
<https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d>
…On Thu, Sep 24, 2026, 12:00 PM important-new ***@***.***> wrote:
*important-new* left a comment (InspectorHub/OpenInspection#361)
<#361 (comment)>
@NCHILLC <https://github.com/NCHILLC> Are you still working on this PR?
Just checking in since the CI is still failing.
—
Reply to this email directly, view it on GitHub
<#361?email_source=notifications&email_token=CEGDFF4R6GCQ67TWOUTM6A35QVALRA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBRG43DCMRRGM22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5817612135>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CEGDFFZ5XORVXKYMA776G3L5QVALRAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMBXGU3TQNRWHE5US43TOVSTWNJVGEYDGOBRGMZDLILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Contributor
Author
|
And the type-checks are killing me.
[image:
https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d]
<https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d>
On Thu, Sep 24, 2026, 12:07 PM Aaron Scott - New Charlotte <
***@***.***> wrote:
… I am. I'm low tier subscription with Claude and codex. I just burnt
through a lot getting ai-memory and a homelab server set-up, so I could
work more on the repo. I'm working on my fork of it, today. I don't ever
want to commit anything to your repo until it's an honest attempt for a PR.
[image:
https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d]
<https://inspectionsupport.com/newcharlottehome/online-scheduler?t=aGtkWFQ2QXhiLzJ0Yw==&office=10aa5407-2ebc-11f1-bdcb-0a63506e1b9d>
On Thu, Sep 24, 2026, 12:00 PM important-new ***@***.***>
wrote:
> *important-new* left a comment (InspectorHub/OpenInspection#361)
> <#361 (comment)>
>
> @NCHILLC <https://github.com/NCHILLC> Are you still working on this PR?
> Just checking in since the CI is still failing.
>
> —
> Reply to this email directly, view it on GitHub
> <#361?email_source=notifications&email_token=CEGDFF4R6GCQ67TWOUTM6A35QVALRA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBRG43DCMRRGM22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5817612135>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/CEGDFFZ5XORVXKYMA776G3L5QVALRAVCNFSNUABGKJSXA33TNF2G64TZHMYTEMBXGU3TQNRWHE5US43TOVSTWNJVGEYDGOBRGMZDLILWAI>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
Contributor
|
I know the type-checks are painful right now. The current type-check has fairly high memory requirements, and I’m waiting for a more stable version of tsgo before making changes around that.
|
Two CI gates failed on this branch; both are fixed with the code, not the baseline. lint:tenant-scope — server/services/isn/report-link-sync.ts:71 selected `reports.title` by `reports.id` alone, with no `tenantId` predicate in the same `.where()`. `reports` is a tenant-scoped table (tenant_id column), so this was a cross-tenant read: given another tenant's report id it would happily title the ISN report link with that tenant's report title. Fixed by adding `eq(reports.tenantId, tenantId)` alongside the id predicate, using the tenantId `syncPublishedReportToIsn` already has in scope — the same pattern the inspection lookup two lines above it already uses. `targetId` itself comes from `resolvePublishTargetReport`, which re-resolves within the tenant, so this is defense in depth on the read that actually names the row. lint:middleware-budget — the new app/lib/isn-settings.server.ts gave itself its own `api.secrets.secrets.$put(...)` call, and check-middleware-budget.mjs ratchets in-process API fan-out per file under app/: a brand-new file with a nonzero call count fails outright, because the gate counts call SITES, not runtime invocations, and has no way to know only one intent ever runs per form submit. settings-advanced.tsx's own `save-advanced-secrets` branch (Integration Keys panel) already made the identical call, so the fix shares one call site between both panels instead of each owning its own: a `putSecrets` closure defined once inside `action` and used by both the `save-isn` and `save-advanced-secrets` branches. Same key lists, same intent strings, same fallback error messages, same response shape for both — this is a call-site merge, not a behavior change, and Integration Keys' save is covered by the same (pre-existing, already-unwritten) level of test trust it had before this commit. The helper stays module-private (not exported from the route file): this repo's react-router.config.ts sets `splitRouteModules: "enforce"`, and an exported async function that calls the API is not one of the route module's recognized server-only exports, so exporting it risks either a failed split or the call leaking into the client chunk. Nesting it as a nested function inside `action` keeps every line of it part of `action`'s own server-only closure, exactly as the two branches it replaces already were. That reshuffle then tripped two more static ratchets that only look at a file's post-change shape: check-file-size.mjs (settings-advanced.tsx crossed 400 lines) and check-loader-awaits.mjs (a module-scope helper's body sits at the same 2-space indent the gate reads as "top level of a route module", so its one `await` — not sequential with anything, it's the only await in its own function — was counted as a new one). Nesting the helper inside `action` (4-space indent) instead of at module scope, plus trimming its comment, fixed both without touching either baseline: settings-advanced.tsx is back to 374 lines and 4 top-level awaits, matching what it was before this branch. app/lib/isn-settings.server.ts keeps only `loadIsnSettings` (the read side, which owns no API call and was untested before this commit — now covered). Its test file moves with it: the three `saveIsnSettings`-shaped cases move to being covered by the shared `putSecrets` closure at the same trust level `save-advanced-secrets` already had (no dedicated test), and a new `loadIsnSettings` test fills the gap that existed before. No baseline touched: scripts/middleware-budget-baseline.json, scripts/file-size-baseline.json, scripts/loader-awaits-baseline.json, and scripts/tenant-scoping-baseline.json are all unchanged. `node scripts/run-gates.mjs` (81/81), tests/unit/integrations/isn-report-link.spec.ts (7/7), and app/lib/isn-settings.server.test.ts (2/2) all pass. chrome-allow: no rendered output changed — this moves an existing `api.secrets.secrets.$put` call and its existing error-message key between files; no JSX, className, or message text was added or changed, for either the ISN panel or the Integration Keys panel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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
When a report is published and the inspection's Reference Number holds an ISN order number, the report link (the primary client's portal-token URL) is added to that ISN order as a public report, then read back from
/orderfilesbefore the sync counts as done. Scope follows the discussion on #321: outbound link + read-back only. This app's agreement, payment and report-release gates are unchanged, and ISN does not control them.ISN_DOMAIN,ISN_COMPANY_KEY,ISN_ACCESS_KEY,ISN_SECRET_KEY). They are catalogued secrets and tenant-owned, so a platform key cannot put every company's reports on one ISN account. There is no default domain, because ISN is white-labelled.isnSyncon the publish response, with ISN's own message; it isnullwhen ISN is not configured or the inspection has no Reference Number.tests/contract/isn/: errors arrive as HTTP 200 withstatus: "error", so success is the body's status and never the HTTP code; andPUT /orders/addreporturlreads the query string, not the documented form body. Deleting a report row in ISN also hides it (show: falsein/orderfiles) rather than removing it, so the read-back requires the row to be shown.docs/develop/integration-adapters.md:api-base/ pure payload / sync, a vendored excerpt of ISN's schema with provenance and a contract spec, a live spec that writes nothing (needs an ISN company; not run in CI), anddocs/integrations/isn.md. The ISN module is imported lazily by the publish route, so deployments without ISN pay nothing at startup.Not in this PR: importing ISN orders (footprints/polling), field mapping, per-inspector opt-in, persisted sync-failure records with retry, completing the ISN order from here, and whether ISN should eventually control agreement, payment or report release.
CI gate fixes
Two gates failed on this branch. Both are fixed in code; no baseline file changed.
lint:tenant-scope.server/services/isn/report-link-sync.tsreadreports.titleby id alone. It now also matchesreports.tenantId, the same pattern the inspection lookup above it uses.lint:middleware-budget.app/lib/isn-settings.server.tsadded its ownPUT /api/secretscall site. The ISN save now shares one call site with the existing Integration Keys save: aputSecretsclosure insidesettings-advanced.tsx'saction. Keys, intents, error messages and response shape are unchanged for both panels. It isn't exported, becausesplitRouteModulesis"enforce".Note on the budget. This save is an action-only
PUT. It runs once per submit and never from a loader, so it adds no render fan-out. Re-baselining withcheck-middleware-budget.mjs --updatewould have been the smaller change, and I held off on purpose. If you'd rather keep the ISN save in its own module and take the one-call re-baseline, say so and I'll switch it. In that case, the threesaveIsnSettingsunit tests this commit replaced withloadIsnSettingstests come back too.Type of change
Related issues / discussions
Refs #321 (the report half only; the issue stays open for the inbound half).
Screenshots / video
No images; observations from
npm run dev:hmragainst a local D1, signed in as an owner, at/settings/advanced:https://is refused under its field with theISN_DOMAINmessage;https://inspectionsupport.comsaves, and address and company key both show as set after a reload. The access-key and secret-key fields were not exercised.Test plan
npm run type-check:appandnpm run type-check:apiare green (the e2e and tests programs were not run locally; CI runs them)npm run lintleft to CI. The pre-commit gate rung passed 27 of 27.tests/unit/integrations+tests/unit/inspections(118 files, 755 tests) green;tests/contract/isn(3 tests) green; web:app/lib+app/components/settings(138 files, 1236 tests) green. The fulltest:unit,test:workersandtest:e2esuites were not run locally.npm run dev:hmr(see above)Checklist
syncPublishedReportToIsnhas no automated test of its own, and its ISN calls are covered throughaddReportLinkToIsnOrder)docs/where relevant (docs/integrations/isn.mdplus the index links)CHANGELOG.md(release-please generates it from thefeat(isn):commit)Co-Authored-By:lines for any AI assistants used🤖 Generated with Claude Code