Skip to content

Parse quoted payment config keys#741

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/sh1pt-config-quoted-payment-keys
Jun 14, 2026
Merged

Parse quoted payment config keys#741
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/sh1pt-config-quoted-payment-keys

Conversation

@rissrice2105-agent

Copy link
Copy Markdown
Contributor

Closes #740.

Summary

  • Teach the payments config parser to match quoted object keys such as "payments", "providers", and "defaultProvider".
  • Move the payments parser into a dependency-light module so parser tests do not need the full CLI runtime imports.
  • Add regression coverage for JSON-style quoted config keys.

Verification

  • corepack pnpm vitest run packages/cli/src/commands/config.test.ts
  • corepack pnpm --filter @profullstack/sh1pt typecheck (fails on existing workspace/module-resolution errors unrelated to this change: missing @profullstack/sh1pt-core, @profullstack/sh1pt-openapi/*, etc.)

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR teaches the payments config parser to match JSON-style quoted object keys ("payments", "providers", "defaultProvider", etc.) by introducing a propertyKeyPattern helper that wraps each key with optional quote characters. It also extracts the parser into a dependency-light config-payments.ts module so tests can run without pulling in the full CLI runtime.

  • config-payments.ts — new module containing parsePaymentsSummary and all private parsing helpers; the fix to readObjectBody is correct and well-anchored, but the same ['\"']? suffix applied in readStringProperty, readNumberProperty, and readBooleanProperty can false-match quoted keys that end with the searched key name.
  • config.ts — removes the inline helpers, imports from the new module, and re-exports parsePaymentsSummary to preserve backward compatibility.
  • config.test.ts — imports from the new module path and adds a regression test covering fully-quoted JSON-style config keys.

Confidence Score: 3/5

The refactor and readObjectBody fix are clean, but the trailing ['"']? in the shared propertyKeyPattern is applied to the value-reading helpers without the preceding-character anchor that makes it safe in readObjectBody, leaving those helpers open to false-positive matches on provider body keys that happen to end with a searched key name.

The regression in readStringProperty, readNumberProperty, and readBooleanProperty is real — the trailing optional quote in propertyKeyPattern can consume a closing quote from a longer key, causing a wrong property value to be returned silently. The bug only fires with unusual key naming in a provider body, but fixing it before merge avoids a silent data-integrity issue in an otherwise well-tested parser path.

packages/cli/src/commands/config-payments.ts — specifically the propertyKeyPattern function and its use in the three value-reading helpers.

Important Files Changed

Filename Overview
packages/cli/src/commands/config-payments.ts New module extracting payment config parsing helpers; fixes readObjectBody to handle quoted keys via propertyKeyPattern, but the same pattern introduces a false-positive regression in readStringProperty, readNumberProperty, and readBooleanProperty.
packages/cli/src/commands/config.ts Cleanly delegates to the new config-payments.ts module and re-exports parsePaymentsSummary for backward compatibility.
packages/cli/src/commands/config.test.ts Updates import to the new module and adds a correct regression test for fully-quoted JSON-style config keys.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[parsePaymentsSummary] --> B[readObjectBody - payments]
    B -->|no match| RU[return undefined]
    B -->|found| C[readObjectBody - providers]
    B --> D[readStringProperty - defaultProvider]
    B --> E[readNumberProperty - platformFeeBps]
    C --> F[readTopLevelObjectEntries]
    F --> G[readStringProperty - use]
    F --> H[readBooleanProperty - enabled]
    G & H --> I[map to PaymentProviderSummary]
    D & E & I --> J[return PaymentsSummary]
    PK[propertyKeyPattern - optional quotes around key]
    B -.->|anchor guards false positives| PK
    D -.->|trailing quote may false-match| PK
    E -.->|trailing quote may false-match| PK
    G -.->|trailing quote may false-match| PK
    H -.->|trailing quote may false-match| PK
Loading

Reviews (1): Last reviewed commit: "Parse quoted payment config keys" | Re-trigger Greptile

Comment on lines +85 to +97
function readStringProperty(source: string, key: string): string | undefined {
const match = new RegExp(`${propertyKeyPattern(key)}\\s*:\\s*['"]([^'"]+)['"]`).exec(source);
return match?.[1];
}

function readNumberProperty(source: string, key: string): number | undefined {
const match = new RegExp(`${propertyKeyPattern(key)}\\s*:\\s*(\\d+)`).exec(source);
return match?.[1] ? Number(match[1]) : undefined;
}

function readBooleanProperty(source: string, key: string): boolean | undefined {
const match = new RegExp(`${propertyKeyPattern(key)}\\s*:\\s*(true|false)`).exec(source);
return match?.[1] === undefined ? undefined : match[1] === 'true';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Trailing ['"']? in propertyKeyPattern causes false-positive matches in value-reading helpers

propertyKeyPattern(key) returns ['"']?key['"']?. When used inside readStringProperty, readNumberProperty, and readBooleanProperty, the optional trailing quote can consume the closing quote of a longer key that ends with the searched key. For example, searching for use in a provider body containing "disuse": "something" — the old pattern (use\s*:) would fail because the " closing disuse appears before :. The new pattern (['"']?use['"']?\s*:) matches because ['"']? absorbs that ", and \s*: then matches :, returning something as the value of use. readObjectBody is safe because its (?:^|[,{\s]) anchor limits false starts, but the three value-reading helpers have no such anchor.

Comment on lines +100 to +102
function propertyKeyPattern(key: string): string {
return `['"]?${escapeRegExp(key)}['"]?`;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 propertyKeyPattern permits mismatched opening/closing quotes

['"']?key['"']? treats the opening and closing quote as independent optional characters, so 'key" or "key' are accepted. While valid TS/JSON configs won't emit mismatched quotes, capturing the opening quote and back-referencing it for the close would make the intent unambiguous.

@rissrice2105-agent

Copy link
Copy Markdown
Contributor Author

CI is green for PR #741.

Verification:

  • corepack pnpm vitest run packages/cli/src/commands/config.test.ts
  • corepack pnpm --filter @profullstack/sh1pt typecheck was attempted and still fails on existing workspace/module-resolution errors unrelated to this PR.

uGig invoice evidence has been sent for this PR.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

3 similar comments
@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio ralyodio merged commit a2541df into profullstack:master Jun 14, 2026
5 checks passed
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.

config payments parser misses quoted object keys

2 participants