fix: enforce Zod describe ordering and extend coding standards checks#387
Closed
cursor[bot] wants to merge 4 commits into
Closed
fix: enforce Zod describe ordering and extend coding standards checks#387cursor[bot] wants to merge 4 commits into
cursor[bot] wants to merge 4 commits into
Conversation
…ctFn path Add regression tests for recent production fixes where coverage was thin: - GitOps supportedScopes: lock account/org/project declaration on the four scopeOptional resources fixed in #342 and verify account-scope list dispatch omits org/project params; contrast with gitops_application fallback behavior - hasRequiredDiscoveryScope: direct unit tests for account/org/project gating used by execution-summary and pipeline-yaml resources (#325) - HarnessClient: include config-service in x-tenant-id gRPC-proxy coverage (#361) - compactItems: test custom compactFn delegation and openInHarness merge Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
… pipeline-yaml edges Add regression tests for recently merged production paths with thin coverage: - HarnessClient: append API description field on /chaos/ and /loadTest/ errors for both request() and requestStream(); verify non-matching paths stay unchanged; extend requestStream x-tenant-id coverage to schema-service and config-service - templateV1BasePathFromScope: explicit account/org/project paths, legacy inference, validation errors, and URL encoding - pipeline-yaml resource: scopeOptional bypass and pipeline_v1 discovery when HARNESS_PIPELINE_VERSION is v1 Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
- Add docs/coding-standards.md as the canonical architecture guide - Add scripts/check-standards.js and tests/coding-standards/architecture.test.ts - Wire pnpm standards:check into CI alongside build/test/typecheck - Fix ToolsetName union missing knowledge-graph and semantic-layer - Remove HarnessClient imports from sto/ccm toolsets (use narrow interfaces) - Replace idp console.error with structured createLogger Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Fix Zod 4 schema chaining in six harness tool handlers so .describe() comes after .optional()/.default(), preserving LLM-visible parameter descriptions on the MCP tool surface. Extend pnpm standards:check and architecture tests to guard Zod describe ordering and write-handler confirmation wiring (confirm param + confirmViaElicitation). Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
|
7 tasks
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds on the automated coding standards enforcement from #377 by fixing real Zod 4 schema violations and adding guardrails for two more checklist items.
Changes
Zod describe ordering fixes (6 handlers)
In Zod 4,
.optional()/.default()return new schema wrappers that do not inherit.descriptionfrom an earlier.describe()call. Fixed chaining order in:harness_list,harness_get,harness_search,harness_status,harness_diagnose,harness_describeAll optional/default params now use
.optional().describe(...)/.default(...).optional().describe(...)so MCP tool metadata exposes descriptions to LLMs.Extended automated enforcement
scripts/check-standards.js: flags.describe()before.optional()/.default(); verifies write handlers exposeconfirm+confirmViaElicitation()tests/coding-standards/architecture.test.ts: matching Vitest coverage (11 tests total)Verification
Standards coverage status
standards:check+ architecture testsstandards:check+ architecture testsstandards:check+ architecture testsstandards:check+ architecture testsstandards:checkstandards:check.describe()orderingstandards:check+ architecture testsstandards:check+ architecture teststests/registry/structural-validation.test.ts(full test suite)