feat: add workspace governance resources (states, workflows, type governance) + workflow parity - #54
feat: add workspace governance resources (states, workflows, type governance) + workflow parity#54akhil-vamshi-konam wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe SDK adds workspace state and workflow resources, work-item type governance APIs, workflow hooks and activity operations, typed models, client wiring, exports, documentation, and integration tests for workspace-managed behavior. ChangesWorkspace workflow and governance APIs
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WorkspaceWorkflows
participant WorkflowStates
participant WorkflowTransitions
participant WorkflowHooks
Client->>WorkspaceWorkflows: create or retrieve workspace workflow
WorkspaceWorkflows->>WorkflowStates: configure workflow states
WorkspaceWorkflows->>WorkflowTransitions: configure state transitions
WorkspaceWorkflows->>WorkflowHooks: manage transition hooks
WorkspaceWorkflows-->>Client: return workflow activity and usage
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
tests/unit/workspace-workflows/workspace-workflow.test.ts (2)
86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for transition and hook writes.
The suite lists transitions but never calls
transitions.create,transitions.update, or anyhooksmethod. Those endpoints are new in this PR and stay untested. A create-then-delete transition step, plus one hook create, would cover the new paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/workspace-workflows/workspace-workflow.test.ts` around lines 86 - 88, Add coverage in the workspace workflow test around the existing transitions listing: create a transition, update it, then delete it, and add one hook creation using the workflow’s hooks API. Assert each operation succeeds and retain the existing transitions list assertion, using the returned transition and workflow identifiers for subsequent calls.
48-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport the ungoverned case as skipped, not passed.
The early return makes this test pass with zero assertions in an ungoverned workspace. A configuration mistake then looks like a green run. The repo already exposes conditional helpers in
tests/helpers/conditional-tests.governedis resolved inbeforeAll, so a runtime skip is needed rather than a declaration-time guard.♻️ Proposed skip
it("should create a workflow, configure a chain, and clean up", async () => { - if (!governed) return; + if (!governed) { + console.warn("Workspace is not governed; skipping governed-write assertions."); + return; + }If
tests/helpers/conditional-testsexports anitIfhelper, prefer it so Jest reports the test as skipped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/workspace-workflows/workspace-workflow.test.ts` around lines 48 - 49, Update the “should create a workflow, configure a chain, and clean up” test to report ungoverned workspaces as skipped rather than passing with zero assertions. Reuse the conditional helper from tests/helpers/conditional-tests, selecting an approach that evaluates the beforeAll-resolved governed value at runtime and preserves the existing test body for governed workspaces.src/models/WorkspaceWorkflow.ts (2)
54-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the Create/Update DTOs from
WorkspaceWorkflow.
CreateWorkspaceWorkflowandUpdateWorkspaceWorkflowredeclare fields that already exist onWorkspaceWorkflow. A rename on the entity will not propagate to these DTOs. The same pattern applies toCreateWorkspaceWorkflowTransitionandUpdateWorkspaceWorkflowTransitionat lines 141-157, where onlystate_idhas no entity counterpart.src/models/Workflow.tsalready uses the derived form.As per coding guidelines: "Use TypeScript interfaces for entity models with separate Create/Update DTOs using `Pick`, `Omit`, and `Partial`".♻️ Proposed derivation with `Pick` and `Partial`
-export type CreateWorkspaceWorkflow = { - name: string; - description?: string; -}; +export type CreateWorkspaceWorkflow = Pick<WorkspaceWorkflow, "name"> & + Partial<Pick<WorkspaceWorkflow, "description">>; /** * Request model for updating workspace workflow metadata */ -export type UpdateWorkspaceWorkflow = Partial<{ - name: string; - description: string; - is_active: boolean; -}>; +export type UpdateWorkspaceWorkflow = Partial<Pick<WorkspaceWorkflow, "name" | "description" | "is_active">>;Apply the same change to the transition DTOs:
export type CreateWorkspaceWorkflowTransition = { state_id: string } & Pick< WorkspaceWorkflowTransition, "transition_state_id" > & Partial<Pick<WorkspaceWorkflowTransition, "rejection_state_id" | "required_approvals" | "member_ids">>; export type UpdateWorkspaceWorkflowTransition = Partial< Pick<WorkspaceWorkflowTransition, "transition_state_id" | "rejection_state_id" | "required_approvals" | "member_ids"> >;Note that
Pick<WorkspaceWorkflowTransition, "transition_state_id">stays optional, so addRequired<...>if the API requires the field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/WorkspaceWorkflow.ts` around lines 54 - 70, Derive the workflow DTO fields from the entity models instead of redeclaring them. Update CreateWorkspaceWorkflow and UpdateWorkspaceWorkflow to use Pick and Partial<Pick> from WorkspaceWorkflow, preserving required creation fields and optional update fields; apply the same pattern to CreateWorkspaceWorkflowTransition and UpdateWorkspaceWorkflowTransition using WorkspaceWorkflowTransition, keeping state_id as the only non-entity field and making transition_state_id required only if the API contract requires it.Source: Coding guidelines
14-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
state_idor soften the doc comment.The doc comment states that the API never sends
state_idon these rows. Line 16 still declares the field. Consumers may branch on it and always readundefined. Either drop line 16 or document why the field remains declared.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/WorkspaceWorkflow.ts` around lines 14 - 22, Update WorkspaceWorkflowState to resolve the mismatch between the API contract and the type definition: either remove the state_id field from the interface or clearly document in the WorkspaceWorkflowState declaration why it remains present despite the API never populating it. Keep the existing id, type, allow_issue_creation, is_default, sequence, and transitions members unchanged.src/api/WorkspaceStates.ts (2)
75-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
deletetodel.
deleteis a standard resource method. The API resource rule requires the namedel. Update the workspace-state tests and public examples with the new method name.As per coding guidelines, “Standard resource methods should be named:
list,create,retrieve,update,del.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/WorkspaceStates.ts` around lines 75 - 76, Rename the WorkspaceStates resource method from delete to del, preserving its existing HTTP DELETE behavior and signature. Update all workspace-state tests and public examples to call del instead of delete, including any references to the renamed method.Source: Coding guidelines
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse kebab-case names for the new modules.
The new module paths use PascalCase filenames. Rename the files and update each import or export path.
src/api/WorkspaceStates.ts#L1-L4: RenameWorkspaceStates.tstoworkspace-states.ts.src/api/WorkspaceWorkflows/States.ts#L1-L8: RenameStates.tstostates.ts.src/models/index.ts#L41-L42: Update exports after renamingWorkspaceWorkflow.tsandWorkItemTypeGovernance.ts.src/client/plane-client.ts#L34-L36: Update theWorkspaceStatesimport after the file rename.src/index.ts#L46-L48: Update public resource exports after file renames.src/index.ts#L85-L92: Update sub-resource exports after file renames.As per coding guidelines, “Use kebab-case for file names.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/WorkspaceStates.ts` around lines 1 - 4, Rename the new modules to kebab-case: src/api/WorkspaceStates.ts to src/api/workspace-states.ts and src/api/WorkspaceWorkflows/States.ts to src/api/WorkspaceWorkflows/states.ts. Update all corresponding import and export paths in src/models/index.ts (including renamed WorkspaceWorkflow.ts and WorkItemTypeGovernance.ts), src/client/plane-client.ts, and both public and sub-resource export sections of src/index.ts.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/Workflows/Hooks.ts`:
- Line 1: Rename the Hooks.ts module to kebab-case as hooks.ts, and update the
import in the Workflows index entrypoint to point to the new filename. Keep the
existing exported symbols such as BaseResource unchanged; only adjust the file
name and the corresponding import reference so the workflow API continues to
resolve correctly.
In `@src/api/Workflows/index.ts`:
- Around line 75-76: Rename the Workflows resource method `delete` to `del`,
preserving its existing parameters, return type, and HTTP deletion behavior.
Update the corresponding workflow unit test invocation to use
`client.workflows.del(...)`.
In `@src/api/WorkItemTypeGovernance/Pins.ts`:
- Around line 1-3: Rename src/api/WorkItemTypeGovernance/Pins.ts to pins.ts and
update every import or export referencing it; rename
src/api/WorkItemTypeGovernance/ProjectWorkflows.ts to project-workflows.ts and
update every corresponding import or export, preserving the existing module
symbols and behavior.
- Around line 45-46: Rename the public deletion method in the Pins resource from
delete to del, preserving its parameters, return type, and existing httpDelete
request path so it conforms to the standard resource method contract.
In `@src/api/WorkspaceWorkflows/index.ts`:
- Around line 86-88: Rename the public WorkspaceWorkflows method `delete` to
`del` to match the standard resource API and sibling resources such as
`Transitions.del` and `Hooks.del`; update its call site in the workspace
workflow unit test to use `client.workspaceWorkflows.del(...)`.
In `@src/models/Workflow.ts`:
- Around line 101-106: Update the CreateWorkflowTransitionHook type so phase,
handler_name, and config are required by wrapping their Pick in Required, while
keeping is_enabled optional through the existing Partial branch; ensure
Hooks.create consumers reject incomplete bodies.
In `@tests/unit/work-item-types/types.test.ts`:
- Around line 47-50: Replace the direct console.warn in the
workspaceManagedReason handling with the repository-approved test logging
mechanism, preserving the existing skip message and early return. If no suitable
logger exists, add the narrowest approved localized suppression for this
specific warning.
---
Nitpick comments:
In `@src/api/WorkspaceStates.ts`:
- Around line 75-76: Rename the WorkspaceStates resource method from delete to
del, preserving its existing HTTP DELETE behavior and signature. Update all
workspace-state tests and public examples to call del instead of delete,
including any references to the renamed method.
- Around line 1-4: Rename the new modules to kebab-case:
src/api/WorkspaceStates.ts to src/api/workspace-states.ts and
src/api/WorkspaceWorkflows/States.ts to src/api/WorkspaceWorkflows/states.ts.
Update all corresponding import and export paths in src/models/index.ts
(including renamed WorkspaceWorkflow.ts and WorkItemTypeGovernance.ts),
src/client/plane-client.ts, and both public and sub-resource export sections of
src/index.ts.
In `@src/models/WorkspaceWorkflow.ts`:
- Around line 54-70: Derive the workflow DTO fields from the entity models
instead of redeclaring them. Update CreateWorkspaceWorkflow and
UpdateWorkspaceWorkflow to use Pick and Partial<Pick> from WorkspaceWorkflow,
preserving required creation fields and optional update fields; apply the same
pattern to CreateWorkspaceWorkflowTransition and
UpdateWorkspaceWorkflowTransition using WorkspaceWorkflowTransition, keeping
state_id as the only non-entity field and making transition_state_id required
only if the API contract requires it.
- Around line 14-22: Update WorkspaceWorkflowState to resolve the mismatch
between the API contract and the type definition: either remove the state_id
field from the interface or clearly document in the WorkspaceWorkflowState
declaration why it remains present despite the API never populating it. Keep the
existing id, type, allow_issue_creation, is_default, sequence, and transitions
members unchanged.
In `@tests/unit/workspace-workflows/workspace-workflow.test.ts`:
- Around line 86-88: Add coverage in the workspace workflow test around the
existing transitions listing: create a transition, update it, then delete it,
and add one hook creation using the workflow’s hooks API. Assert each operation
succeeds and retain the existing transitions list assertion, using the returned
transition and workflow identifiers for subsequent calls.
- Around line 48-49: Update the “should create a workflow, configure a chain,
and clean up” test to report ungoverned workspaces as skipped rather than
passing with zero assertions. Reuse the conditional helper from
tests/helpers/conditional-tests, selecting an approach that evaluates the
beforeAll-resolved governed value at runtime and preserves the existing test
body for governed workspaces.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06fc4d65-55e7-45a5-806d-00c7260c0c62
📒 Files selected for processing (31)
README.mdsrc/api/WorkItemTypeGovernance/Pins.tssrc/api/WorkItemTypeGovernance/ProjectWorkflows.tssrc/api/WorkItemTypeGovernance/index.tssrc/api/Workflows/Hooks.tssrc/api/Workflows/States.tssrc/api/Workflows/Transitions.tssrc/api/Workflows/index.tssrc/api/WorkspaceStates.tssrc/api/WorkspaceWorkflows/Hooks.tssrc/api/WorkspaceWorkflows/States.tssrc/api/WorkspaceWorkflows/Transitions.tssrc/api/WorkspaceWorkflows/index.tssrc/client/plane-client.tssrc/index.tssrc/models/State.tssrc/models/WorkItemTypeGovernance.tssrc/models/Workflow.tssrc/models/WorkspaceFeatures.tssrc/models/WorkspaceWorkflow.tssrc/models/index.tstests/helpers/governance.tstests/unit/project-templates.test.tstests/unit/state.test.tstests/unit/work-item-type-governance/work-item-type-governance.test.tstests/unit/work-item-types/project-properties.test.tstests/unit/work-item-types/properties-options.test.tstests/unit/work-item-types/types.test.tstests/unit/workflows/workflow.test.tstests/unit/workspace-states.test.tstests/unit/workspace-workflows/workspace-workflow.test.ts
| @@ -0,0 +1,102 @@ | |||
| import { BaseResource } from "../BaseResource"; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename Hooks.ts to hooks.ts.
This filename is not kebab-case. Update the import in src/api/Workflows/index.ts after the rename. As per coding guidelines, “Use kebab-case for file names.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/Workflows/Hooks.ts` at line 1, Rename the Hooks.ts module to
kebab-case as hooks.ts, and update the import in the Workflows index entrypoint
to point to the new filename. Keep the existing exported symbols such as
BaseResource unchanged; only adjust the file name and the corresponding import
reference so the workflow API continues to resolve correctly.
Source: Coding guidelines
| async delete(workspaceSlug: string, projectId: string, workflowId: string): Promise<void> { | ||
| return this.httpDelete(`/workspaces/${workspaceSlug}/projects/${projectId}/workflows/${workflowId}/`); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename delete to del.
This is a standard resource deletion method. Rename it to del. Update tests/unit/workflows/workflow.test.ts line 255 to call client.workflows.del(...). As per coding guidelines, “Standard resource methods should be named: list, create, retrieve, update, del.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/Workflows/index.ts` around lines 75 - 76, Rename the Workflows
resource method `delete` to `del`, preserving its existing parameters, return
type, and HTTP deletion behavior. Update the corresponding workflow unit test
invocation to use `client.workflows.del(...)`.
Source: Coding guidelines
| import { BaseResource } from "../BaseResource"; | ||
| import { Configuration } from "../../Configuration"; | ||
| import { CreateWorkItemTypeWorkflowPins, WorkItemTypeWorkflowPin } from "../../models/WorkItemTypeGovernance"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use kebab-case filenames for these API modules.
Rename the files and update their imports.
src/api/WorkItemTypeGovernance/Pins.ts#L1-L3: renamePins.tstopins.ts.src/api/WorkItemTypeGovernance/ProjectWorkflows.ts#L1-L9: renameProjectWorkflows.tstoproject-workflows.ts.
As per coding guidelines: src/**/*.ts: Use kebab-case for file names.
📍 Affects 2 files
src/api/WorkItemTypeGovernance/Pins.ts#L1-L3(this comment)src/api/WorkItemTypeGovernance/ProjectWorkflows.ts#L1-L9
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/WorkItemTypeGovernance/Pins.ts` around lines 1 - 3, Rename
src/api/WorkItemTypeGovernance/Pins.ts to pins.ts and update every import or
export referencing it; rename src/api/WorkItemTypeGovernance/ProjectWorkflows.ts
to project-workflows.ts and update every corresponding import or export,
preserving the existing module symbols and behavior.
Source: Coding guidelines
| async delete(workspaceSlug: string, typeId: string, pinId: string): Promise<void> { | ||
| return this.httpDelete(`/workspaces/${workspaceSlug}/work-item-types/${typeId}/governance/pins/${pinId}/`); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename delete to del.
Line 45 exposes a standard resource deletion method as delete. Use del to match the SDK resource contract.
Proposed fix
- async delete(workspaceSlug: string, typeId: string, pinId: string): Promise<void> {
+ async del(workspaceSlug: string, typeId: string, pinId: string): Promise<void> {As per coding guidelines: src/api/**/*.ts: Standard resource methods should be named: list, create, retrieve, update, del.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async delete(workspaceSlug: string, typeId: string, pinId: string): Promise<void> { | |
| return this.httpDelete(`/workspaces/${workspaceSlug}/work-item-types/${typeId}/governance/pins/${pinId}/`); | |
| async del(workspaceSlug: string, typeId: string, pinId: string): Promise<void> { | |
| return this.httpDelete(`/workspaces/${workspaceSlug}/work-item-types/${typeId}/governance/pins/${pinId}/`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/WorkItemTypeGovernance/Pins.ts` around lines 45 - 46, Rename the
public deletion method in the Pins resource from delete to del, preserving its
parameters, return type, and existing httpDelete request path so it conforms to
the standard resource method contract.
Source: Coding guidelines
| async delete(workspaceSlug: string, workflowId: string): Promise<void> { | ||
| return this.httpDelete(`/workspaces/${workspaceSlug}/workflows/${workflowId}/`); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename delete to del.
The sibling sub-resources in this directory use del (Transitions.del, Hooks.del). This class uses delete, so the public surface is inconsistent. Rename it now to avoid a breaking change later. Update the call site at tests/unit/workspace-workflows/workspace-workflow.test.ts line 27.
♻️ Proposed rename
- async delete(workspaceSlug: string, workflowId: string): Promise<void> {
+ async del(workspaceSlug: string, workflowId: string): Promise<void> {
return this.httpDelete(`/workspaces/${workspaceSlug}/workflows/${workflowId}/`);
}// tests/unit/workspace-workflows/workspace-workflow.test.ts
await client.workspaceWorkflows.del(workspaceSlug, workflow.id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async delete(workspaceSlug: string, workflowId: string): Promise<void> { | |
| return this.httpDelete(`/workspaces/${workspaceSlug}/workflows/${workflowId}/`); | |
| } | |
| async del(workspaceSlug: string, workflowId: string): Promise<void> { | |
| return this.httpDelete(`/workspaces/${workspaceSlug}/workflows/${workflowId}/`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/WorkspaceWorkflows/index.ts` around lines 86 - 88, Rename the public
WorkspaceWorkflows method `delete` to `del` to match the standard resource API
and sibling resources such as `Transitions.del` and `Hooks.del`; update its call
site in the workspace workflow unit test to use
`client.workspaceWorkflows.del(...)`.
Source: Coding guidelines
| /** | ||
| * Request model for creating a workflow transition hook. `phase` and | ||
| * `handler_name` are immutable post-create. | ||
| */ | ||
| export type CreateWorkflowTransitionHook = Pick<WorkflowTransitionHook, "phase" | "handler_name" | "config"> & | ||
| Partial<Pick<WorkflowTransitionHook, "is_enabled">>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does TypeScript Pick preserve optional property modifiers from the source type?
💡 Result:
Yes, the TypeScript Pick utility type preserves optional property modifiers, provided that the source type is a single object type [1][2]. Because Pick is implemented as a homomorphic (structure-preserving) mapped type, it automatically propagates property modifiers—including optional (?) and readonly modifiers—from the source type to the resulting type [1][2]. However, there is a known limitation regarding union types: when Pick is used on a union of object types, it may not preserve optional modifiers as expected, often causing properties to become required in the resulting type [3][4]. This occurs because the mapping behavior that enables modifier preservation is specifically optimized for mapping over the keys of a single type (e.g., keyof T), rather than the union of keys from multiple types [3].
Citations:
- 1: Improve propagation of modifiers in mapped types microsoft/TypeScript#12826
- 2: Preserve modifiers in homomorphic mapped types microsoft/TypeScript#12563
- 3: Pick doesn't preserve optional from unions microsoft/TypeScript#28483
- 4: Pick turns optional parameters into required ones when union types are used microsoft/TypeScript#20722
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate Workflow.ts and Hooks files =="
fd -a 'Workflow\.ts$|Hooks\.ts$' . | sed 's#^\./##' | sort
echo
echo "== WorkflowTransitionHook occurrences =="
rg -n "WorkflowTransitionHook|CreateWorkflowTransitionHook|interface|type .*Transition" src/models src/api -S || true
echo
echo "== Inspect relevant files =="
for f in $(fd 'Workflow\.ts$|Hooks\.ts$' src | sort); do
echo "--- $f ($(wc -l < "$f") lines) ---"
ast-grep outline "$f" --view compact || true
doneRepository: makeplane/plane-node-sdk
Length of output: 23476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/models/Workflow.ts lines 81-110 =="
sed -n '81,110p' src/models/Workflow.ts | nl -ba -v81
echo
echo "== Workflow API hooks create signatures and bodies =="
for f in src/api/Workflows/Hooks.ts src/api/WorkspaceWorkflows/Hooks.ts; do
echo "--- $f lines 45-58 ---"
sed -n '45,58p' "$f" | nl -ba -v45
done
echo
echo "== TypeScript Pick optional modifier probe =="
node - <<'JS'
const ts = require('typescript');
function identity(x) { return x; }
const fs = require('fs');
const temp = '/tmp/probe-pick-optional.ts';
fs.writeFileSync(temp, `
interface Source {
required: string;
optional?: string;
}
declare function post<T>(data: T): T;
type Create = Pick<Source, "required" | "optional">;
post<Create>({});
`);
const host = ts.createCompilerHost({ strict: true, noEmit: true });
host.getFileNames = () => [temp];
host.read = (name) => name === temp ? fs.readFileSync(temp, 'utf8') : host.readFile(name);
const compiler = ts.createProgram([temp], { strict: true, noEmit: true, target: ts.ScriptTarget.Latest }, host);
const diagnostics = ts.getPreEmitDiagnostics(compiler);
for (const d of diagnostics) {
const s = typeof d.file?.printLine === 'function'
? ts.stringifyDiagnosticMessageText(d.file.printLine(d.file.getLineAndCharacterOfPosition(d.start ?? 0), d.messageText))
: String(d.messageText);
console.log(`${d.file?.fileName ?? 'unknown'}:${d.start}:${d.length ?? 0} ${s}`);
}
fs.unlinkSync(temp);
JSRepository: makeplane/plane-node-sdk
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/models/Workflow.ts lines 81-110 =="
sed -n '81,110p' src/models/Workflow.ts
echo
echo "== Workflow API hooks create signatures and bodies =="
sed -n '45,58p' src/api/Workflows/Hooks.ts
echo "---"
sed -n '53,58p' src/api/WorkspaceWorkflows/Hooks.ts
echo
echo "== TypeScript Pick optional modifier probe =="
if command -v node >/dev/null 2>&1 && node -e "require.resolve('typescript')" >/dev/null 2>&1; then
node - <<'JS'
const ts = require('typescript');
const fs = require('fs');
const temp = '/tmp/probe-pick-optional.ts';
fs.writeFileSync(temp, `
interface Source {
required: string;
optional?: string;
}
declare function post<T>(data: T): T;
type Create = Pick<Source, "required" | "optional">;
post<Create>({});
`);
const host = ts.createCompilerHost({ strict: true, noEmit: true });
host.getFileNames = () => [temp];
host.readFile = (name) => name === temp ? fs.readFileSync(temp, 'utf8') : null;
host.useCaseSensitiveFileNames = () => true;
host.fileExists = (name) => name === temp;
const compiler = ts.createProgram([temp], { strict: true, noEmit: true, target: ts.ScriptTarget.Latest }, host);
let any = false;
const diagnostics = ts.getPreEmitDiagnostics(compiler);
for (const d of diagnostics) any = true;
console.log('node_modules/typescript present: true');
console.log(diagnostics.map(d => String(d.messageText)).join('\n'));
console.log('has diagnostics:', any);
fs.unlinkSync(temp);
JS
else
echo "node_modules/typescript not present; using TypeScript compiler semantics from compiler source"
node - <<'JS'
const ts = require('typescript');
const text = `type Source = { required: string; optional?: string}; type Picked = Pick<Source, "optional">;`;
const sourceFile = ts.createSourceFile('probe.ts', text, ts.ScriptTarget.Latest, true);
const program = ts.createProgram(['probe.ts'], {}, {
getSourceFile(name) { return name === 'probe.ts' ? sourceFile : undefined; },
fileExists() { return true; },
readFile() { return text; },
getCurrentDirectory() { return ''; },
getDirectories() { return []; },
getCanonicalFileName(f) { return f; },
useCaseSensitiveFileNames() { return false; },
getNewLine() { return '\n'; },
resolveModuleNames() { return []; },
});
const type = program.getTypeAtLocation(sourceFile);
console.log(ts.typeToString(type));
JS
fiRepository: makeplane/plane-node-sdk
Length of output: 2476
Make create fields required on CreateWorkflowTransitionHook.
phase, handler_name, and config stay optional because they are optional on WorkflowTransitionHook and Pick preserves that modifier. This makes Hooks.create in src/api/Workflows/Hooks.ts and src/api/WorkspaceWorkflows/Hooks.ts typecheck incomplete bodies like {} or { is_enabled: false }. Wrap the required fields with Required.
🐛 Proposed fix
-export type CreateWorkflowTransitionHook = Pick<WorkflowTransitionHook, "phase" | "handler_name" | "config"> &
- Partial<Pick<WorkflowTransitionHook, "is_enabled">>;
+export type CreateWorkflowTransitionHook = Required<
+ Pick<WorkflowTransitionHook, "phase" | "handler_name" | "config">
+> &
+ Partial<Pick<WorkflowTransitionHook, "is_enabled">>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Request model for creating a workflow transition hook. `phase` and | |
| * `handler_name` are immutable post-create. | |
| */ | |
| export type CreateWorkflowTransitionHook = Pick<WorkflowTransitionHook, "phase" | "handler_name" | "config"> & | |
| Partial<Pick<WorkflowTransitionHook, "is_enabled">>; | |
| /** | |
| * Request model for creating a workflow transition hook. `phase` and | |
| * `handler_name` are immutable post-create. | |
| */ | |
| export type CreateWorkflowTransitionHook = Required< | |
| Pick<WorkflowTransitionHook, "phase" | "handler_name" | "config"> | |
| > & | |
| Partial<Pick<WorkflowTransitionHook, "is_enabled">>; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/models/Workflow.ts` around lines 101 - 106, Update the
CreateWorkflowTransitionHook type so phase, handler_name, and config are
required by wrapping their Pick in Required, while keeping is_enabled optional
through the existing Partial branch; ensure Hooks.create consumers reject
incomplete bodies.
| const reason = workspaceManagedReason(error); | ||
| if (reason !== null) { | ||
| console.warn("Skipped: project-level work item types are managed at the workspace level —", reason); | ||
| return; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the lint violation.
Line 49 triggers eslint(no-console). Use the test logging mechanism accepted by the repository, or add an approved localized suppression.
🧰 Tools
🪛 GitHub Check: build-lint
[warning] 49-49: eslint(no-console)
Unexpected console statement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/work-item-types/types.test.ts` around lines 47 - 50, Replace the
direct console.warn in the workspaceManagedReason handling with the
repository-approved test logging mechanism, preserving the existing skip message
and early return. If no suitable logger exists, add the narrowest approved
localized suppression for this specific warning.
Source: Linters/SAST tools
1 participant
Summary
Adds workspace governance support.
New resources
client.workspaceStates— workspace-level (catalog) work-item states. Dual-mode reads (catalog under governance, cross-project aggregate otherwise); writes require the workspace to own states/workflows.client.workspaceWorkflows— the workspace workflow catalog, with.states(chain),.transitions, and.hookssub-resources; usage report and activity log.client.workItemTypeGovernance— governs which workflows a workspace-level work item type may use (any/constrained/required), with.pins(per-project overrides) and.projectWorkflows(project-side resolution, pick, fallback preview).Workflows housekeeping parity
client.workflows(project-scoped) was missing several methods present in the public API:retrieve,delete,activities,submitWorkItemApproval,states.list/update/transfer,transitions.retrieve, and a new.hookssub-resource.Model changes
WorkspaceFeaturesgainsstates_owned_by_workspace(read-only governance flag),work_item_types, andreleases; fields are now optional to match partial-PATCH semantics.Tests
workspace-states,workspace-workflows,work-item-type-governance) plus the Workflows housekeeping additions.workspaceManagedReasonhelper: when a workspace/project-level feature conflicts with its workspace-governed equivalent (project-scoped writes correctly rejected 400workspace_managed), the affected tests now log a clear skip warning and short-circuit instead of failing loud.Workspace Governance tests
Project-level tests: skipped with warning, not failed.
Summary by CodeRabbit