packages/commonwell-cert-runner/README.md (1)

98-108: Clarify whether ROOT_OID expects raw OID (no urn:oid:)

The rest of the repo generally treats OIDs without the urn:oid: prefix (and adds it only where required). To prevent confusion, explicitly document the expected format here.

Apply this change to make the requirement explicit:

 # Member (Service Adopter) credentials
-ROOT_OID=1.2.3.4.5.678
+# Root OID (numeric only; do NOT include 'urn:oid:' prefix)
+ROOT_OID=1.2.3.4.5.678

packages/commonwell-cert-runner/src/flows/link-management.ts (2)

41-41: Removing waits can introduce flakiness; prefer bounded polling with backoff

Commenting out the waits will speed up the flow but may cause intermittent empty results due to eventual consistency on CW endpoints (you already call some endpoints twice to compensate). Consider replacing fixed sleeps with a small polling helper (bounded retries with delay) to keep runs fast but more reliable.

Example helper (place in ../util or similar) and usage:

// util/poll.ts
export async function poll<T>({
  fn,
  until,
  retries = 5,
  delayMs = 1500,
}: {
  fn: () => Promise<T>;
  until: (value: T) => boolean;
  retries?: number;
  delayMs?: number;
}): Promise<T> {
  let last: T;
  for (let i = 0; i < retries; i++) {
    last = await fn();
    if (until(last)) return last;
    await new Promise(r => setTimeout(r, delayMs));
  }
  return last!;
}

Usage example replacing a plain GET:

const resp_2_3_3 = await poll({
  fn: () => commonWell.getPatientLinksByPatientId(patientWithProbableLinksIdEnconded),
  until: r => Boolean(r.Patients && r.Patients.length > 0),
  retries: 4,
  delayMs: 2000,
});

Also applies to: 74-74, 137-137, 153-153, 174-174

---

81-85: Duplicate getPatientLinks call needs rationale or removal

This second immediate call to getPatientLinksByPatientId isn't accompanied by a comment (unlike the probable links call below). If it's intentionally mitigating eventual consistency, prefer the bounded polling approach, or add a short comment to explain why this duplication is needed.

packages/api/src/external/commonwell-v2/api.ts (1)

60-66: Normalize OIDs before checking to avoid 'urn:oid:' mismatches

The function docstring says orgOID is passed without urn:oid:, but in practice callers can slip a prefixed value. Normalizing once makes this robust and avoids false negatives in the disallowed check.

Apply:

-export function makeCommonWellAPI(orgName: string, orgOID: string, npi: string): CommonWellAPI {
+export function makeCommonWellAPI(orgName: string, orgOID: string, npi: string): CommonWellAPI {
   if (Config.isSandbox()) {
     return new CommonWellMock(orgName, orgOID);
   }
 
-  const isMemberAPI = [Config.getCWMemberOID(), Config.getSystemRootOID()].includes(orgOID);
-  if (isMemberAPI) {
+  // Normalize: accept both plain and 'urn:oid:'-prefixed inputs
+  const normalizedOID = orgOID.replace(/^urn:oid:/, "");
+  const isDisallowed = [Config.getCWMemberOID(), Config.getSystemRootOID()].includes(normalizedOID);
+  if (isDisallowed) {
     throw new MetriportError("Cannot use the member/root OID as an organization OID", undefined, {
-      orgOID,
+      orgOID,
     });
   }
 
   return new CommonWell({
     orgCert: Config.getCWOrgCertificate(),
     rsaPrivateKey: Config.getCWOrgPrivateKey(),
     orgName,
-    oid: orgOID,
-    homeCommunityId: orgOID,
+    oid: normalizedOID,
+    homeCommunityId: normalizedOID,
     npi,
     apiMode,
   });
 }

packages/commonwell-cert-runner/src/payloads.ts (2)

38-41: Double-check orgId normalization with ROOT_OID

makeOrgId now returns ${rootOID}.5.<org>. If ROOT_OID sometimes includes "urn:oid:" and sometimes not, orgId will vary in shape. Downstream code (e.g., DocumentReference meta.source) compensates for this, but other consumers might not.

Consider either:

• guaranteeing ROOT_OID format (documented expectation), or
• normalizing here (e.g., stripping trailing dots and/or optionally adding/removing "urn:oid:" once).

Given the surrounding code, a small comment noting the expected ROOT_OID format would also help future maintainers.

---

173-175: Minor: Update TODO to include actionable detail or tracking link

The ENG-541 TODO is fine; consider adding a brief “what/why” to ease triage later (e.g., exact networks change expected), or link to spec section if applicable.

packages/commonwell-cert-runner/src/flows/contribution/contribution-server.ts (1)

139-140: Duplicate express.json() middleware — remove the second registration

express.json() is already registered on Line 32. The second registration is redundant.

Apply this diff:

-app.use(express.json({ limit: "2mb" }));
+// app.use(express.json({ limit: "2mb" })); // already registered above

packages/commonwell-cert-runner/src/flows/document-consumption.ts (1)

51-51: Reconsider removing the wait — query may race with eventual consistency

Removing the delay can make the document query flaky if the system isn’t immediately consistent after patient creation. Prefer a short, bounded retry/poll instead of a fixed sleep.

Example approach (requires adding a small helper; replace the commented line):

-    // await waitSeconds(5);
+    // Retry querying docs up to 3 times with small backoff to avoid flakiness
+    const maxAttempts = 3;
+    for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+      const docs = await queryDocuments(commonWell, encodedPatientId);
+      if (docs.length > 0) {
+        // proceed with documents already fetched
+        console.log(`>>> Got ${docs.length} documents (attempt ${attempt})`);
+        break;
+      }
+      if (attempt < maxAttempts) {
+        await new Promise(r => setTimeout(r, attempt * 1000));
+      }
+      // On last attempt, fall through to the normal flow
+    }

If you prefer to keep current structure, at minimum, note this change in the test instructions since it can affect timing expectations when validating the flow.

packages/commonwell-cert-runner/src/flows/org-management-initiator-only.ts (3)

13-15: Remove unused exported type and drop unused import.

OrgManagementResponse isn’t used in this module, and CommonWell is imported only for that type. Clean it up to avoid dead code.

-import { APIMode, CommonWell, CommonWellMember } from "@metriport/commonwell-sdk";
+import { APIMode, CommonWellMember } from "@metriport/commonwell-sdk";
@@
-export type OrgManagementResponse = {
-  commonWell: CommonWell;
-};

Also applies to: 2-2

---

42-46: Add post-create orgId guard (mirrors org-management.ts).

Ensure createOrg returned an organizationId before proceeding.

-    const initiatorOnlyOrgId = respCreateInitiatorOnly.organizationId;
+    const initiatorOnlyOrgId = respCreateInitiatorOnly.organizationId;
+    if (!initiatorOnlyOrgId) {
+      throw new Error("No orgId on response from createOrg (initiator-only)");
+    }

---

72-82: Avoid in checks and delete; prefer truthy checks and explicit nulling for external payloads.

Coding guideline favors truthy checks over 'in' in obj. Also, since we're shaping a payload for an external API, setting fields to null (as you already do on create) is clearer and avoids potential type issues with delete.

-    if ("securityTokenKeyType" in initiatorOnlyOrg && initiatorOnlyOrg.securityTokenKeyType) {
+    if (initiatorOnlyOrg.securityTokenKeyType) {
       console.log(`HEADS UP: Security token key type is not null`);
-      delete initiatorOnlyOrg.securityTokenKeyType;
+      initiatorOnlyOrg.securityTokenKeyType = null;
     }
-    if (
-      "authorizationInformation" in initiatorOnlyOrg &&
-      initiatorOnlyOrg.authorizationInformation
-    ) {
+    if (initiatorOnlyOrg.authorizationInformation) {
       console.log(`HEADS UP: Authorization information is not null`);
-      delete initiatorOnlyOrg.authorizationInformation;
+      initiatorOnlyOrg.authorizationInformation = null;
     }

packages/commonwell-cert-runner/src/flows/document-contribution.ts (1)

223-241: Don’t mutate the shared patientShirleyDouglas object; clone before customizing.

Mutating the imported constant violates immutability/idempotency expectations and can leak state across runs. Clone then override fields.

-function makeNewDemographics() {
-  const demographics = patientShirleyDouglas;
+function makeNewDemographics() {
+  const demographics = { ...patientShirleyDouglas };
   demographics.name = [
     {
       use: NameUseCodes.official,
       given: [faker.person.firstName()],
       family: [faker.person.lastName()],
     },
   ];
   demographics.birthDate = faker.date.birthdate().toISOString().split("T")[0];
   demographics.gender = GenderCodes.M;
   demographics.address = [
     {
       use: AddressUseCodes.home,
       postalCode: faker.location.zipCode(),
       state: faker.location.state(),
     },
   ];
   return demographics;
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

Learnt from: RamilGaripov
PR: metriport/metriport#4176
File: packages/fhir-converter/src/lib/handlebars-converter/handlebars-helpers.js:296-320
Timestamp: 2025-07-17T21:24:37.077Z
Learning: RamilGaripov prefers to maintain consistency with existing patterns in a file rather than making isolated fixes to coding guideline violations like var vs const declarations, when the pattern is already established throughout the codebase.

Learnt from: leite08
PR: metriport/metriport#3953
File: packages/core/src/command/consolidated/search/fhir-resource/__tests__/search-consolidated-setup.ts:28-30
Timestamp: 2025-06-03T21:02:23.374Z
Learning: The `makePatient()` function returns `PatientWithId` type which requires the `id` field to be present, ensuring `patient.id` is never undefined.

Learnt from: leite08
PR: metriport/metriport#4095
File: packages/commonwell-cert-runner/src/flows/org-management.ts:79-85
Timestamp: 2025-07-16T19:02:26.395Z
Learning: In packages/commonwell-cert-runner/src/flows/org-management.ts, the silent error handling in the certificate retrieval catch block is intentional to allow the certification flow to continue processing even when certificate operations fail, as this is a testing tool rather than production code.

Learnt from: lucasdellabella
PR: metriport/metriport#3866
File: packages/api/src/routes/internal/medical/organization.ts:95-98
Timestamp: 2025-05-27T15:54:47.774Z
Learning: The conversion of the GET `/internal/organization` endpoint from returning a single organization to returning an array of organizations (batch retrieval) in `packages/api/src/routes/internal/medical/organization.ts` was an intentional breaking change planned by the team.

Learnt from: leite08
PR: metriport/metriport#3814
File: packages/api/src/routes/internal/medical/patient-consolidated.ts:141-174
Timestamp: 2025-05-20T21:26:26.804Z
Learning: The functionality introduced in packages/api/src/routes/internal/medical/patient-consolidated.ts is planned to be refactored in downstream PR #3857, including improvements to error handling and validation.

Learnt from: thomasyopes
PR: metriport/metriport#3427
File: packages/core/src/external/ehr/api/sync-patient.ts:16-55
Timestamp: 2025-03-11T20:42:46.516Z
Learning: In the patient synchronization architecture, the flow follows this pattern: (1) `ehr-sync-patient-cloud.ts` sends messages to an SQS queue, (2) the `ehr-sync-patient` Lambda consumes these messages, and (3) the Lambda uses the `syncPatient` function to make the API calls to process the patient data.