Skip to content

feat(NEX-25): add flow context details and logs for execution inspection#30

Merged
sonisoft-cnanda merged 2 commits intomasterfrom
feat/NEX-25-flow-context-details
Mar 10, 2026
Merged

feat(NEX-25): add flow context details and logs for execution inspection#30
sonisoft-cnanda merged 2 commits intomasterfrom
feat/NEX-25-flow-context-details

Conversation

@sonisoft-cnanda
Copy link
Owner

Summary

  • Adds getFlowContextDetails() method that calls the ProcessFlow operations API (GET /api/now/processflow/operations/flow/context/{id}) to return rich execution data including per-action timing, state, inputs, outputs, and human-readable step labels
  • Adds getFlowLogs() method that queries the sys_flow_log table via Table API for execution log entries
  • Automatically cross-references action report IDs with the flow definition to resolve stepLabel (e.g. "Update Record (Approve Change)"), actionTypeName, and stepComment — critical for an agent to map failing steps back to flow code

These methods support the full agent iteration workflow: Copy → Pull → Edit → Deploy → Test → Inspect → Iterate

Workflow Step SDK Method What the Agent Gets
Test the flow testFlow() Returns contextId
Get execution overview getFlowContextDetails(contextId) State, runtime, who ran it, test vs prod
See which step failed flowReport.actionOperationsReports Per-step: stepLabel, state, error, timing
See step inputs/outputs operationsInput / operationsOutput What values each step received and produced
Get execution logs getFlowLogs(contextId) Log entries with messages, levels, timestamps
Fallback (no ops view) getFlowContextStatus() + getFlowError() Basic state, error message, outputs

Test plan

  • 24 new unit tests covering both methods (550 total for FlowManager)
  • 7 new integration tests verified against dev224436
  • Step label resolution verified against real instance data
  • All 550 FlowManager unit tests pass
  • TypeScript compiles cleanly

Closes #25

🤖 Generated with Claude Code

…ution inspection

Adds two new FlowManager methods to support the agent iteration workflow
(test → inspect → iterate) by providing detailed flow execution data:

- getFlowContextDetails(): calls the ProcessFlow operations API to return
  per-action execution reports with timing, state, inputs, outputs, and
  human-readable step labels resolved from the flow definition
- getFlowLogs(): queries sys_flow_log via Table API for execution log entries

Closes #25

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a clean, well-documented addition with excellent test coverage (24 unit tests + 7 integration tests). The step-label resolution via _buildActionInstanceLookup is a nice touch for agent usability. A few issues worth addressing.

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a clean, well-documented addition with excellent test coverage (24 unit tests + 7 integration tests). The step-label resolution via _buildActionInstanceLookup is a nice touch for agent usability. A few issues worth addressing:

Bugs / Correctness:

  1. sysparm_query injection via unvalidated contextId - In getFlowLogs, contextId is interpolated directly into the query string (sysparm_query: context=contextId^orderBy). _validateContextId only rejects empty/whitespace and does NOT enforce a hex format. A caller passing abc^ORDERBYDESCsys_created_on would silently inject extra query conditions. Since sys_ids are always 32 hex chars, a simple format guard would close this. Or at minimum, document that callers are responsible for providing a valid sys_id.

  2. No validation on the limit option in getFlowLogs - limit: 0 or a negative number will be forwarded as-is (sysparm_limit: 0), which may return unexpected results. A guard makes failure explicit and matches the validation style used elsewhere.

Type Safety:

  1. Generic type parameter immediately cast away with as any - Both new methods immediately discard the generic type from pfr.get() with (response as any)?.data?.result ?? (response as any)?.bodyObject?.result. This is a pre-existing pattern, but the new code adds two more instances. Consider a typed helper (similar to _extractResult) to keep the as any surface minimal.

  2. Misleading as string casts in _buildActionInstanceLookup - The pattern actionType?.displayName as string ?? actionType?.name as string ?? undefined has as string casts that give false type safety and will mask null vs undefined differences across SN API versions. Dropping the casts and relying on TypeScript inference is safer.

Code Style:

  1. FlowLogRecord interface defined inside a try block - Defining an interface inside a function body (inside a try block) is unusual TypeScript. It cannot be referenced from tests or other files and makes the code harder to navigate. Moving it to module scope (even unexported) is cleaner and consistent with how FlowContextOperationsResponse is handled.

  2. Verbose query construction in getFlowContextDetails - The current pattern initialises an empty object, conditionally fills it, then checks Object.keys(query).length > 0. This can be simplified to: const query = scope ? { sysparm_transaction_scope: scope } : undefined;

Integration Tests:

  1. Hardcoded instance-specific sys_ids and assertions - KNOWN_CONTEXT_ID = 811e52d5702372105d88c5714b9b559b and the assertion that the flow name equals Copy of Change - Standard will fail against any instance other than dev224436. Worth adding a comment so contributors are not confused by failures in their own environments.

  2. console.log calls in integration tests - There are many console.log statements throughout the new integration tests. Useful during development but they pollute CI output. Consider removing or gating them once the feature stabilises.

Pre-existing (worth a note): FlowContextState is defined as a union including string at the end. Including string in a union with string literals makes it effectively just string - TypeScript widens all comparisons and autocomplete loses its value. Consider removing the trailing string catch-all.

The implementation logic is correct, the JSDoc is thorough, and edge-case test coverage (missing flowContext, missing actionInstances, HTTP errors, dual response shapes) is solid. Items 1 and 2 (input validation) and 5 (interface scope) are the highest-priority fixes before merge.

- Validate contextId as 32-char hex to prevent query injection (#1)
- Validate limit option as positive integer in getFlowLogs (#2)
- Drop misleading `as string` casts in _buildActionInstanceLookup (#4)
- Move FlowLogRecord interface to module scope (#5)
- Simplify query construction in getFlowContextDetails (#6)
- Add instance-specific comment to integration test IDs (#7)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: feat(NEX-25): add flow context details and logs for execution inspection

Overall this is a well-structured addition — good type coverage in FlowModels.ts, solid input validation, and thorough test coverage (24 unit + 7 integration tests). A few issues worth addressing before merging.


Bug: _validateContextId validates trimmed value but passes original to API

In FlowManager.ts (_validateContextId, ~line 1132):

private _validateContextId(contextId: string): void {
    if (!contextId || contextId.trim().length === 0) {
        throw new Error('Context ID is required');
    }
    if (!/^[0-9a-f]{32}$/i.test(contextId.trim())) { // ← tests trimmed
        throw new Error(...);
    }
}

The regex test runs on contextId.trim(), so a value like " abc123...def " (valid 32 hex chars with surrounding whitespace) passes validation — but the raw untrimmed string is then used in the API URL and embedded in sysparm_query. The method should either reject whitespace-padded input outright (test against the untrimmed value) or normalise before validation:

// Option A – reject whitespace
if (!/^[0-9a-f]{32}$/i.test(contextId)) { ... }

// Option B – normalise at call site before passing to _validateContextId
const id = contextId.trim();
this._validateContextId(id);
// ...use id in API call

_extractProcessFlowResult helper is not reused

There is already a private helper at ~line 942:

private _extractProcessFlowResult(response: any): ProcessFlowApiResponse['result'] {
    const result = response?.data?.result ?? response?.bodyObject?.result;
    if (!result) {
        throw new Error('Unexpected response structure from processflow API: no result field');
    }
    return result;
}

getFlowContextDetails duplicates this pattern inline with an explicit as any cast instead:

const result = (response as any)?.data?.result ?? (response as any)?.bodyObject?.result;

The new code needs to return { success: false } rather than throw, which is why the helper wasn't used as-is. A small refactor would avoid the duplication — e.g. extract a non-throwing _tryExtractResult(response) helper and use it in both places.


scope parameter in getFlowContextDetails is unvalidated

const query = scope ? { sysparm_transaction_scope: scope } : undefined;

Unlike contextId, the scope value passes through to the query string without validation. This is low-risk (it's a URL query param, not embedded in a sysparm_query expression), but unexpected characters could still produce confusing API errors. Even a simple typeof scope === 'string' && scope.trim().length > 0 guard (or at least an @throws note in the JSDoc) would help callers.


FlowLogRecord interface placement

The FlowLogRecord interface (~line 44) is defined inside FlowManager.ts even though every other public/private shape lives in FlowModels.ts. Since it's an implementation detail (only used in getFlowLogs), keeping it in the manager file is defensible — but it breaks the consistent pattern. Consider either moving it to FlowModels.ts as @internal or adding a brief comment explaining why it stays local.


iterationOperationsReports is untyped and unmapped

subflowOperationsReports and actionOperationsReports are both mapped through mapActionReports, but iterationOperationsReports is passed through verbatim as Record<string, unknown>. This is understandable if the shape isn't known yet, but it deserves a comment explaining the gap so future contributors know it's intentional:

/** @todo Map once loop iteration report shape is confirmed */
iterationOperationsReports: Record<string, unknown>;

Minor: mapActionReports inner function vs private method

mapActionReports is defined as a closure inside _mapFlowExecutionReport and called twice. It captures actionLookup from the outer scope. Extracting it as a private method (_mapActionReports(reports, lookup)) would make it independently testable and slightly easier to read.


Integration tests are hardcoded to a specific instance

The KNOWN_CONTEXT_ID constant in FlowManager_IT.test.ts is tied to dev224436 — the inline comment explains this, which is good. Consider extracting it to test_config.ts alongside SN_INSTANCE_ALIAS so that swapping the test instance only requires changing one file.


What's done well

  • The contextId regex validation (/^[0-9a-f]{32}$/i) is a meaningful security improvement over the old empty-string check — it prevents query injection via malformed IDs.
  • Error handling in both new public methods returns structured { success: false } results rather than throwing, consistent with the rest of FlowManager.
  • _buildActionInstanceLookup + stepLabel resolution from the flow definition is well thought-out and directly addresses the agent use-case described in the PR.
  • Type definitions in FlowModels.ts are thorough and well-documented.

@sonisoft-cnanda sonisoft-cnanda merged commit 45ee498 into master Mar 10, 2026
3 checks passed
sonisoft-cnanda pushed a commit that referenced this pull request Mar 10, 2026
# [3.8.0](v3.7.0...v3.8.0) (2026-03-10)

### Features

* **NEX-25:** add flow context details and logs for execution inspection ([#30](#30)) ([45ee498](45ee498)), closes [#25](#25) [#1](#1) [#2](#2) [#4](#4) [#5](#5) [#6](#6) [#7](#7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flow Log and Context Details

1 participant