Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions workspace-configuration/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,42 @@ components:
schemas:
WorkspaceConfiguration:
type: object
additionalProperties: false
properties:
mounts:
$ref: '#/components/schemas/Mounts'
environment:
type: array
items:
$ref: '#/components/schemas/EnvironmentVariable'

Mounts:
type: object
additionalProperties: false
properties:
dependencies:
type: array
items:
type: string
description: List of additional source directories to mount, with paths relative to the sources directory of the project (useful when working with git worktrees)
configs:
type: array
items:
type: string
description: List of home directories to mount, with paths relative to the HOME directory of the user

EnvironmentVariable:
type: object
additionalProperties: false
required:
- name
properties:
name:
type: string
description: Name of the environment variable
value:
type: string
description: Value of the environment variable (mutually exclusive with secret)
secret:
type: string
description: Name of the secret containing the value (mutually exclusive with value)
Comment on lines +44 to +58
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Mutual exclusivity between value and secret is not enforced.

The descriptions state that value and secret are mutually exclusive, but the current schema:

  1. Allows both to be provided simultaneously
  2. Allows neither to be provided (only name is required)

Per the PR objectives, each environment variable should have either a value or a secret reference.

🛠️ Proposed fix using `oneOf` to enforce mutual exclusivity
     EnvironmentVariable:
       type: object
       additionalProperties: false
       required:
         - name
+      oneOf:
+        - required: [value]
+          not:
+            required: [secret]
+        - required: [secret]
+          not:
+            required: [value]
       properties:
         name:
           type: string
           description: Name of the environment variable
         value:
           type: string
           description: Value of the environment variable (mutually exclusive with secret)
         secret:
           type: string
           description: Name of the secret containing the value (mutually exclusive with value)

Alternatively, a simpler approach if tooling supports it:

     EnvironmentVariable:
       type: object
       additionalProperties: false
       required:
         - name
+      oneOf:
+        - required: [value]
+        - required: [secret]
       properties:
         name:
           type: string
           description: Name of the environment variable
         value:
           type: string
           description: Value of the environment variable (mutually exclusive with secret)
         secret:
           type: string
           description: Name of the secret containing the value (mutually exclusive with value)

Note: The simpler oneOf approach ensures at least one is present but technically allows both (the oneOf would fail if both are provided since both branches would match). If strict exclusivity is needed, use the first approach.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workspace-configuration/openapi.yaml` around lines 44 - 58, Update the
EnvironmentVariable schema to enforce that exactly one of "value" or "secret" is
provided: keep "name" required and additionalProperties: false, then replace the
loose "value" and "secret" properties with a oneOf that defines two alternatives
(one schema requiring "value", the other requiring "secret") so a payload must
match exactly one branch (preventing both or neither). Refer to the
EnvironmentVariable schema and the "value" and "secret" properties when making
this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the oneof implementation for generated code is not very convenient. We will let the users enforce this rule

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@feloy, understood — that's a reasonable trade-off. Codegen tooling often struggles with oneOf compositions, and keeping the schema simple while delegating enforcement to the application layer is a pragmatic choice. I'll keep this in mind for future reviews.


✏️ Learnings added
Learnt from: feloy
Repo: kortex-hub/kortex-cli-api PR: 15
File: workspace-configuration/openapi.yaml:44-58
Timestamp: 2026-03-18T15:44:51.144Z
Learning: In the kortex-cli-api repository, the `EnvironmentVariable` schema in `workspace-configuration/openapi.yaml` intentionally does NOT use `oneOf` to enforce mutual exclusivity between `value` and `secret` fields. The team prefers to keep the schema simple for generated code ergonomics and lets users/application logic enforce the mutual exclusivity rule instead. Do not flag this as an issue in future reviews.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: feloy
Repo: kortex-hub/kortex-cli-api PR: 10
File: cli/openapi.yaml:0-0
Timestamp: 2026-03-11T08:49:39.162Z
Learning: In the kortex-cli-api repository, the OpenAPI spec (cli/openapi.yaml) intentionally documents only CLI command **output** (response schemas and examples). Input parameters (e.g., workspace id passed to `kortex-cli workspace remove <id>`) are deliberately **not** described in the spec. Review comments about missing input/query/path parameters in this spec are not applicable.