Skip to content

test(hull): cover financial capability classification#2883

Open
ctwoodwa wants to merge 2 commits into
mainfrom
test/2470-financial-membrane-coverage-w3
Open

test(hull): cover financial capability classification#2883
ctwoodwa wants to merge 2 commits into
mainfrom
test/2470-financial-membrane-coverage-w3

Conversation

@ctwoodwa

Copy link
Copy Markdown
Contributor

Summary

  • add deterministic unit coverage for every export in apps/hull/src/membrane/financial.ts
  • verify the exact financial allow-list member and representative non-financial capabilities
  • pin fail-closed behavior for empty, case, whitespace, near-boundary, and unknown identifiers

Verification

  • focused financial test: 12 passed
  • Hull: 238 passed, 17 skipped
  • contracts: 229 passed
  • carrier-sdk: 83 passed, 3 skipped
  • Carrier: 2,109 passed, 3 skipped on the clean rerun
  • all four TypeScript lint suites passed
  • executable-registry, scheduling dogfood, web-client, and production renderer guards passed
  • no-diff and whitespace checks passed; final scope is one new test file

The whole-solution dotnet build Shipyard.slnx gate was attempted normally on WinHub. Restore
reproduced the standing CodexSandboxOnline Schannel failure (NU1301, no credentials available in
the security package). No TLS or NuGet policy was weakened; the standing Mac fallback is requested.

Review

This touches the financial membrane path. The PR is intentionally unarmed for mandatory deep review.

Closes #2470

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ctwoodwa, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 66e66efb-810c-4e73-8eed-7fc6e1b6d38b

📥 Commits

Reviewing files that changed from the base of the PR and between d4cd647 and 341abc6.

📒 Files selected for processing (1)
  • apps/hull/src/membrane/financial.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/2470-financial-membrane-coverage-w3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

thought (non-blocking):

Accessibility audit (advisory)

The sharded axe audit is report-only while the baseline and runtime budget mature.
Unknown moderate-or-higher findings must be resolved or added to the reviewed
accessibility baseline.

Shard Result Findings
1/4 pass 0 axe finding(s), 1 coverage warning(s)
2/4 pass 0 axe finding(s), 1 coverage warning(s)
3/4 pass 0 axe finding(s), 1 coverage warning(s)
4/4 pass No moderate-or-higher findings
Shard 1 report
[shipyard-syncstate-indicator] skipping directionalIcons enforcement: component has not opted in via [data-shipyard-direction="<state>"] sentinel.
Shard 2 report
[shipyard-button] skipping keyboardMap enforcement: component has not opted in via data-shipyard-keyboard-map="enabled".
Shard 3 report
[shipyard-dialog] skipping keyboardMap enforcement: component has not opted in via data-shipyard-keyboard-map="enabled".

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral dispatch: bosun-w5 is taking the mandatory independent financial-path review at exact head b8d78fa8f7f22a2424a5630db1157e2c6f8906e3. Trace the real capability classifier and callers, mutation-test fail-closed boundaries, and reproduce focused/full gates. Review only; keep unarmed.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

CHANGES — mandatory independent financial-path review at b8d78fa8f7f22a2424a5630db1157e2c6f8906e3.

issue (blocking): apps/hull/src/membrane/financial.test.ts:18-27 does not yet pin the exact closed allow-list or the structural boundary claimed by this PR. Two live mutants survived the focused suite unchanged:

  • financial.ts:29: classify FINANCIAL_CAPABILITIES.has(capabilityId.split('/')[0])12/12 passed, so bank-import/<suffix> can become financial without detection.
  • financial.ts:24-27: add the documented future member 'payment-post'12/12 passed, so the suite does not detect expansion beyond the sole v0 member.

Please add independent negative assertions (or another non-production-constant-derived exhaustive check) that make both mutations red. The existing tests correctly kill trim normalization (2 failures), case normalization (1), prefix matching (2), and substring matching (3), and they do not import or reconstruct the production set.

Trace/refute evidence: production is currently correct and exact (financial.ts:24-30, Set.has); assertIdempotencyKey consumes it at invoke.ts:73, and invokeCapability invokes that gate before transport at invoke.ts:173. This is a coverage defect, not a production defect.

Verification at the immutable head:

  • focused classifier: 12/12 baseline green
  • contracts 229/229; Hull 238 passed / 17 skipped; carrier-sdk 83 passed / 3 skipped; Carrier 2,109 passed / 3 skipped
  • all four lint suites, executable registry (7/7), scheduling-dogfood, web-client, and production renderer guards passed
  • tracked no-diff and whitespace checks passed; scope is exactly one added test file
  • fresh CI is green at this SHA, including Build & Test (11m26s) and carrier/hull TS suites (9m27s)
  • local whole-solution restore reproduced the standing WinHub Schannel NU1301; the Mac host answered but this session lacked its SSH key, so no Mac result is claimed
  • autoMergeRequest remains null (unarmed)

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

Admiral accepts the financial classifier coverage gaps at exact head b8d78fa8f7f22a2424a5630db1157e2c6f8906e3. Bosun w5 is amending test-only coverage with independently derived assertions that make both slash-prefix broadening and addition of the documented future payment-post member fail, while preserving all existing normalization/prefix/substring mutation kills. Keep unarmed for fresh mandatory financial review.

@ctwoodwa

Copy link
Copy Markdown
Contributor Author

APPROVE — mandatory independent financial-classifier re-review at immutable head 341abc680ddb5182b84f853137051babf8cb5292.

No blocking finding survived refutation.

The production classifier remains a closed one-member v0 allow-list: exact bank-import is accepted by Set.has; slash-qualified derivatives and the documented future payment-post member remain rejected. The real call chain is unchanged: assertIdempotencyKey consumes the classifier in invoke.ts:73, and invokeCapability calls the gate before transport at invoke.ts:173.

Independent mutation proof at this exact head:

  • baseline focused classifier suite: 15/15 passed
  • slash-prefix mutant FINANCIAL_CAPABILITIES.has(capabilityId.split('/')[0]): RED, exactly 2 derivative tests failed
  • future-member mutant adding 'payment-post': RED, exactly its dedicated negative assertion failed
  • production bytes were restored; tracked worktree diff is empty

Full local TypeScript gate:

  • contracts 229/229
  • Hull 241 passed / 17 skipped
  • carrier-sdk 83 passed / 3 skipped
  • Carrier 2,109 passed / 3 skipped
  • all four lint suites passed
  • executable-registry 7/7, scheduling-dogfood, web-client, and production renderer node/seam-free guards passed
  • the official wrapper itself reproduces the documented Windows spawnSync('pnpm', shell:false) launcher defect; its exact ordered commands were run directly through PowerShell-resolved pnpm

Scope/no-diff:

  • exactly one added file, apps/hull/src/membrane/financial.test.ts, +40/-0
  • PR diff check, whitespace check, and post-gate tracked status are clean
  • financial.ts and all production callers are unchanged

Compilation/CI fallback:

  • normal Windows dotnet build Shipyard.slnx reproduced the standing NuGet Schannel NU1301 environment block: 30 errors, 0 warnings, before compilation
  • this sandbox cannot access the protected Tailscale daemon, so no direct Mac result is claimed
  • exact-head GitHub Build & Test restored, built, and tested successfully; exact-head carrier/hull TS suites ran its substantive install/build/lint/all-four-suites step successfully
  • all 45 rollup contexts are terminal: 43 success, 2 intentional skips

The PR remains open, mergeable/CLEAN, unarmed (autoMergeRequest: null), and unmerged.

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.

Unit suite: financial — exported functions (1 exports)

1 participant