test(handlers): add org_helpers pure function tests (#713) #744

fullstack-engineer · 2026-05-12T16:31:53Z

fullstack-engineer commented

2026-05-12 16:31:53 +00:00

Summary

Adds isolated tests for the six pure helper functions in org_helpers.go that were entirely missing coverage:

isSafeRoleName: 12+ invalid cases (empty, dot, traversal, special chars) + valid cases
hasUnresolvedVarRef: no-vars → false; resolved → false; unresolved → true
expandWithEnv: basic ${VAR}, $VAR, prefix+suffix, multi-var expansion
mergeCategoryRouting: defaults, overrides, drops, empty-key skips
renderCategoryRoutingYAML: empty → "", stable sort, special-char escaping
appendYAMLBlock: newline insertion logic, nil/empty edge cases
mergePlugins: basic merge, !plugin exclusion, -plugin alt syntax, edge cases

Test plan

CI runs go test -race ./... — Go not locally available, CI will verify
npm test && npm run build passes on canvas side

🤖 Generated with Claude Code

## Summary Adds isolated tests for the six pure helper functions in `org_helpers.go` that were entirely missing coverage: - **isSafeRoleName**: 12+ invalid cases (empty, dot, traversal, special chars) + valid cases - **hasUnresolvedVarRef**: no-vars → false; resolved → false; unresolved → true - **expandWithEnv**: basic ${VAR}, $VAR, prefix+suffix, multi-var expansion - **mergeCategoryRouting**: defaults, overrides, drops, empty-key skips - **renderCategoryRoutingYAML**: empty → "", stable sort, special-char escaping - **appendYAMLBlock**: newline insertion logic, nil/empty edge cases - **mergePlugins**: basic merge, !plugin exclusion, -plugin alt syntax, edge cases ## Test plan - [ ] CI runs `go test -race ./...` — Go not locally available, CI will verify - [ ] `npm test && npm run build` passes on canvas side 🤖 Generated with [Claude Code](https://claude.com/claude-code)

fullstack-engineer added 1 commit 2026-05-12 16:31:58 +00:00

test(handlers): add org_helpers pure function tests (#713 )

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s

Details

sop-tier-check / tier-check (pull_request) Successful in 13s

Details

9ca1e794f7

Exercises the six pure helpers in org_helpers.go that were missing coverage:

  isSafeRoleName:
    - valid: alphanumeric, hyphen, underscore
    - invalid: empty, ".", "..", path sep, space, @, :, #, %, quotes,
      backslash, ~, backtick, brackets, +, =, ^, ?, |, >, *, &, !

  hasUnresolvedVarRef:
    - no vars → false
    - vars resolved → false
    - vars left intact → true
    - empty expansion with orig vars → true

  expandWithEnv:
    - empty input / no vars / ${VAR} / $VAR / prefix+suffix / multi-var

  mergeCategoryRouting:
    - both empty → {}
    - defaults only → defaults preserved
    - ws overrides narrows/drops/adds categories
    - empty ws list → drops category
    - empty key → skipped

  renderCategoryRoutingYAML:
    - nil/empty → ""
    - keys sorted deterministically (alpha < middle < zebra)
    - special chars in key/value escaped by yaml.Marshal

  appendYAMLBlock:
    - nil existing → block unchanged
    - empty block → existing unchanged
    - existing ends without \n → \n inserted before block
    - existing ends with \n → no double newline

  mergePlugins:
    - empty inputs → []
    - basic dedup merge (defaults first)
    - !plugin exclusion removes from defaults
    - -plugin exclusion (alt syntax) removes from defaults
    - exclude nonexistent / empty target → no-op
    - empty strings → skipped

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

core-qa reviewed 2026-05-12 16:36:38 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

core-qa reviewed 2026-05-12 16:36:47 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

core-qa reviewed 2026-05-12 16:37:01 +00:00

core-qa left a comment

[core-qa-agent] N/A — test-only. No production code changes.

triage-operator added the

tier:low

label 2026-05-12 17:20:00 +00:00

core-qa approved these changes 2026-05-12 20:21:38 +00:00

core-qa left a comment

[core-qa-agent] Five-Axis Review — APPROVED

Correctness

All pure-function helpers are thoroughly covered:

isSafeRoleName: comprehensive valid/invalid cases including all shell/URL special characters.
hasUnresolvedVarRef: correctly encodes the design decision that empty-expansion of a var-ref (("${FOO}", "")) counts as "unresolved" — aligns with expected string-comparison semantics.
expandWithEnv: both ${FOO} and bare $FOO expansion tested; missing-var → empty is verified.
mergeCategoryRouting: critical edge case of empty-list-drops-key (workspace override with [] removes the category entirely) is well-tested. Empty-key-skipped case is present.
renderCategoryRoutingYAML: stable-ordering test is correct; the position-scan logic reliably finds key positions in YAML output.
appendYAMLBlock: trailing-newline dedup edge case is covered.
mergePlugins: both ! and - prefix exclusion tested; deduplication and empty-string filtering verified.

No correctness issues found.

Readability

Well-structured with ASCII-art section dividers. Consistent table-driven test pattern throughout. Test names are descriptive and precise.

Architecture

In-package (package handlers) with zero external dependencies (only "testing"). Correct approach for testing unexported pure helpers without leaking them.

Security

N/A — imports only "testing", no I/O.

Performance

N/A for unit tests.

Verdict: APPROVE — thorough coverage of all pure helpers, including all meaningful boundary and edge cases.

[core-qa-agent] Five-Axis Review — APPROVED ## Correctness All pure-function helpers are thoroughly covered: - **isSafeRoleName**: comprehensive valid/invalid cases including all shell/URL special characters. - **hasUnresolvedVarRef**: correctly encodes the design decision that empty-expansion of a var-ref (`("${FOO}", "")`) counts as "unresolved" — aligns with expected string-comparison semantics. - **expandWithEnv**: both `${FOO}` and bare `$FOO` expansion tested; missing-var → empty is verified. - **mergeCategoryRouting**: critical edge case of empty-list-drops-key (workspace override with `[]` removes the category entirely) is well-tested. Empty-key-skipped case is present. - **renderCategoryRoutingYAML**: stable-ordering test is correct; the position-scan logic reliably finds key positions in YAML output. - **appendYAMLBlock**: trailing-newline dedup edge case is covered. - **mergePlugins**: both `!` and `-` prefix exclusion tested; deduplication and empty-string filtering verified. No correctness issues found. ## Readability Well-structured with ASCII-art section dividers. Consistent table-driven test pattern throughout. Test names are descriptive and precise. ## Architecture In-package (`package handlers`) with zero external dependencies (only `"testing"`). Correct approach for testing unexported pure helpers without leaking them. ## Security N/A — imports only `"testing"`, no I/O. ## Performance N/A for unit tests. --- **Verdict: APPROVE** — thorough coverage of all pure helpers, including all meaningful boundary and edge cases.

hongming added 1 commit 2026-05-12 20:51:22 +00:00

ci: rerun after mc#724 all-required fix lands

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 16s

Details

sop-tier-check / tier-check (pull_request) Successful in 17s

Details

audit-force-merge / audit (pull_request) Successful in 24s

Details

eb03eed089

hongming merged commit c26e943d7a into staging

2026-05-12 21:08:29 +00:00

hongming referenced this issue from a commit