chore: repo setup for Claude workflows and doc cleanup - #913
Conversation
Ticket-based workflows need the tracker and project key documented in-repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
The convention existed only in branch history; agents and new contributors could not reproduce it from docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
docs/outdated was archived in April with no inbound references; docs/ports was a design snapshot superseded by src/middleware/shared/ ports; CONTRIBUTING.md was an empty stub; engines requires Node >=22. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
EtherCAT and debugger docs still referenced the pre-reorg src/main and src/renderer trees and a deleted IEC system-task helper; the pipeline diagram claimed iec2c where STruC++ compiles; README gains the real test commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
WalkthroughThis PR updates documentation for the current architecture, tooling, EtherCAT behavior, compilation pipeline, and contributor workflow. It adds test and review guidance, refreshes debugger and STruC++ references, and removes a deprecated theme port reference. ChangesDocumentation and architecture reference updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ethercat-architecture.md (1)
336-360: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the state tree in sync with
taskPriority.The
Project Persistenceblock above now includestaskPriorityinEthercatConfig.masterConfig, but this diagram still shows onlynetworkInterface,cycleTimeUs, andwatchdogTimeoutCycles. That leaves the documented schema inconsistent.Suggested fix
- └── masterConfig: { networkInterface, cycleTimeUs, watchdogTimeoutCycles } + └── masterConfig: { networkInterface, cycleTimeUs, watchdogTimeoutCycles, taskPriority }🤖 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 `@docs/ethercat-architecture.md` around lines 336 - 360, Update the EtherCAT state tree diagram in the architecture doc so `EthercatConfig.masterConfig` matches the persisted schema by including `taskPriority` alongside `networkInterface`, `cycleTimeUs`, and `watchdogTimeoutCycles`. Keep the change localized to the “Zustand Store Slices” section and the `Project Slice` / `updateEthercatConfig()` description so the documented shape stays consistent with the `Project Persistence` block.
🤖 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 `@docs/strucpp-migration/00-overview.md`:
- Around line 3-10: Clarify the migration status wording in the overview so it
distinguishes the editor-side removal of the local iec2c binary from the
remaining Runtime v3 MatIEC recompilation path. Update the status text near
compiler-module.ts, scripts/download-binaries.ts, and
src/backend/shared/compile/pipeline.ts so it explicitly says the editor no
longer ships or invokes iec2c locally, while Runtime v3 still uploads program.st
for on-device MatIEC recompilation.
---
Outside diff comments:
In `@docs/ethercat-architecture.md`:
- Around line 336-360: Update the EtherCAT state tree diagram in the
architecture doc so `EthercatConfig.masterConfig` matches the persisted schema
by including `taskPriority` alongside `networkInterface`, `cycleTimeUs`, and
`watchdogTimeoutCycles`. Keep the change localized to the “Zustand Store Slices”
section and the `Project Slice` / `updateEthercatConfig()` description so the
documented shape stays consistent with the `Project Persistence` block.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 937c95a2-425d-4632-afe3-bd9afdd61f39
📒 Files selected for processing (33)
CLAUDE.mdCONTRIBUTING.mdREADME.mddocs/debugger-scalability-analysis.mddocs/ethercat-architecture.mddocs/outdated/ARDUINO_UNO_Q_BINARY_SIZE_FIX.mddocs/outdated/HEADLESS_SETUP.mddocs/outdated/dead-code-inventory.mddocs/outdated/debugger-opcua-shared-utilities.mddocs/outdated/external-binaries-strategy.mddocs/outdated/name-type-linking-design.mddocs/outdated/opcua-server-configuration/01-design-overview.mddocs/outdated/opcua-server-configuration/02-ui-screen-specifications.mddocs/outdated/opcua-server-configuration/03-json-configuration-mapping.mddocs/outdated/opcua-server-configuration/04-implementation-phases.mddocs/outdated/opcua-server-configuration/README.mddocs/outdated/s7comm-server-implementation.mddocs/outdated/unified-frontend-serialization.mddocs/outdated/variable-id-audit.mddocs/ports/accelerator-port.tsdocs/ports/compiler-port.tsdocs/ports/debugger-port.tsdocs/ports/device-port.tsdocs/ports/index.tsdocs/ports/platform-capabilities.tsdocs/ports/project-port.tsdocs/ports/runtime-port.tsdocs/ports/simulator-port.tsdocs/ports/system-port.tsdocs/ports/theme-port.tsdocs/ports/types.tsdocs/ports/window-port.tsdocs/strucpp-migration/00-overview.md
💤 Files with no reviewable changes (27)
- docs/outdated/opcua-server-configuration/README.md
- docs/ports/system-port.ts
- docs/outdated/opcua-server-configuration/01-design-overview.md
- docs/outdated/opcua-server-configuration/02-ui-screen-specifications.md
- docs/outdated/unified-frontend-serialization.md
- docs/ports/accelerator-port.ts
- docs/outdated/variable-id-audit.md
- docs/outdated/debugger-opcua-shared-utilities.md
- docs/ports/window-port.ts
- docs/outdated/dead-code-inventory.md
- docs/outdated/opcua-server-configuration/03-json-configuration-mapping.md
- docs/outdated/external-binaries-strategy.md
- docs/ports/device-port.ts
- docs/ports/compiler-port.ts
- docs/outdated/ARDUINO_UNO_Q_BINARY_SIZE_FIX.md
- docs/ports/simulator-port.ts
- docs/outdated/s7comm-server-implementation.md
- docs/outdated/opcua-server-configuration/04-implementation-phases.md
- docs/outdated/name-type-linking-design.md
- docs/ports/platform-capabilities.ts
- docs/ports/debugger-port.ts
- docs/outdated/HEADLESS_SETUP.md
- docs/ports/project-port.ts
- docs/ports/types.ts
- docs/ports/runtime-port.ts
- docs/ports/index.ts
- docs/ports/theme-port.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.claude/settings.json:
- Around line 3-8: The Bash auto-approval rules in the settings are too broad
and currently permit arbitrary npm/npx/git/gh commands. Tighten the
permissions.allow entries by replacing the wildcard patterns with only the
specific safe subcommands actually needed, and keep the scope limited within the
settings configuration so the policy remains least-privilege.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ed748572-67f5-4642-bb57-1039c2ecb5d7
📒 Files selected for processing (3)
.claude/review-guidelines.md.claude/settings.jsonCONTRIBUTING.md
✅ Files skipped from review due to trivial changes (1)
- .claude/review-guidelines.md
Contribution rules, the PR checklist, and the tracker pointer existed only in scattered or personal files; this surfaces them where GitHub and the review tooling actually read them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
3e970ad to
7caea57
Compare
Conventions covered style but not the practices that prevent real bugs: type assertions and floating promises in TS, unchecked buffers and scan-cycle blocking in C, broad excepts and event-loop blocking in Python. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
| "permissions": { | ||
| "allow": [ | ||
| "Bash(npm:*)", | ||
| "Bash(npx:*)", |
There was a problem hiding this comment.
This committed project-level allowlist auto-approves Bash(npx:*), Bash(git:*) and Bash(gh:*) for every collaborator's Claude session. npx:* is arbitrary remote package execution (any prompt-injected session can fetch and run code without a prompt), git:* includes push --force, and gh:* includes gh pr merge, gh release, gh repo delete — outward-facing/destructive operations. Consider narrowing to the specific subcommands the workflows need (e.g. Bash(git status:*), Bash(git diff:*), Bash(git log:*), Bash(gh pr view:*), Bash(npx playwright test:*), Bash(npx tsx:*)) and leaving broader grants to each developer's untracked .claude/settings.local.json.
| ## Workflow | ||
|
|
||
| 1. Internal work is tracked in Jira, project DOPE (internal tracker). External contributors: open a GitHub issue using the provided templates. | ||
| 2. Branch from `development`, named `<type>/DOPE-<n>-<kebab-slug>` (`<type>`: feature, bugfix, task, improvement). Maintenance without a ticket uses `chore/`, `ci/`, `docs/`. |
There was a problem hiding this comment.
The branch-naming scheme <type>/DOPE-<n>-<kebab-slug> requires a Jira ticket number that external contributors (addressed in step 1) cannot obtain — the only ticket-less escape hatch offered is chore//ci//docs/, which doesn't cover an external bugfix or feature. Spell out what external contributors should do (e.g. reference the GitHub issue number in the branch name, or note that a maintainer will file the DOPE ticket).
| - Layer dependencies respect ports and adapters: frontend, backend, and middleware only communicate through the ports in `src/middleware/shared/ports/`. `npm run validate:arch` must pass. | ||
| - DTOs cross layer boundaries, never domain entities. | ||
| - 100%-coverage directories stay at 100%; new behavior comes with tests (Jest for logic, Playwright for user flows). | ||
| - TypeScript strict: no `any`, named exports over default exports. |
There was a problem hiding this comment.
This bullet overlaps with the later "TypeScript Best Practices in CLAUDE.md apply to every diff" bullet, which already forbids any (CLAUDE.md: "never any"). Also, "named exports over default exports" is an export-style preference, not a strictness rule, and it does not appear in CLAUDE.md's TypeScript Best Practices. Consider dropping the duplicated any clause, making "named exports over default exports" its own bullet, and (if CLAUDE.md is meant to be the canonical style source) adding the export rule there too.
|
|
||
| ## Git Workflow | ||
|
|
||
| - Base branch: `development` — feature branches start from it and PRs target it. |
There was a problem hiding this comment.
The Git Workflow section duplicates CONTRIBUTING.md's Workflow section (base branch, branch naming, Conventional Commits) with slightly different wording — a drift risk for two docs landing in the same PR. Consider making one the source of truth and having the other reference it (e.g. CLAUDE.md: "Follow CONTRIBUTING.md; <type> maps from the Jira issue type: Story → feature, ...").
- Narrow .claude/settings.json Bash allowlist to specific subcommands - CONTRIBUTING.md: branch-naming escape hatch for external contributors - CLAUDE.md: Git Workflow defers to CONTRIBUTING.md; add named-exports rule - review-guidelines: dedupe any-clause, split named-exports bullet - strucpp overview: separate local iec2c removal from Runtime v3 MatIEC path - ethercat doc: add taskPriority to state-tree masterConfig Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRdcPv4CCxEKR3N8nYKYp6
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.claude/settings.json:
- Line 4: Update the Bash permission entry in the settings configuration to
replace the wildcard npm script pattern with explicit allowlisted validation
commands required by the workflow. Exclude mutating or external-effect scripts
such as build, package, postinstall, prepare, and rebuild, and avoid permitting
arbitrary current or future npm scripts.
In `@CONTRIBUTING.md`:
- Line 5: Update the setup reference in CONTRIBUTING.md to say “full
step-by-step guide” instead of “full step by step,” preserving the existing
README.md link and surrounding wording.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 47c2cf54-e2bc-40e7-9243-342a60c7863e
📒 Files selected for processing (6)
.claude/review-guidelines.md.claude/settings.jsonCLAUDE.mdCONTRIBUTING.mddocs/ethercat-architecture.mddocs/strucpp-migration/00-overview.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude/review-guidelines.md
- CLAUDE.md
- docs/strucpp-migration/00-overview.md
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(npm run:*)", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import json
with open("package.json", encoding="utf-8") as f:
scripts = json.load(f).get("scripts", {})
print("\n".join(sorted(scripts)))
PYRepository: Autonomy-Logic/openplc-editor
Length of output: 386
🏁 Script executed:
#!/bin/bash
sed -n '1,120p' .claude/settings.json
printf '\n--- package.json scripts ---\n'
python3 - <<'PY'
import json
with open("package.json", encoding="utf-8") as f:
scripts = json.load(f).get("scripts", {})
for name in sorted(scripts):
print(f"{name}: {scripts[name]}")
PYRepository: Autonomy-Logic/openplc-editor
Length of output: 2697
Narrow Bash(npm run:*) to explicit scripts.
package.json includes mutating and external-effect scripts like build, package, postinstall, prepare, and rebuild, so this rule allows arbitrary repo scripts now and in the future. Allow only the specific validation commands the workflow needs.
🤖 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 @.claude/settings.json at line 4, Update the Bash permission entry in the
settings configuration to replace the wildcard npm script pattern with explicit
allowlisted validation commands required by the workflow. Exclude mutating or
external-effect scripts such as build, package, postinstall, prepare, and
rebuild, and avoid permitting arbitrary current or future npm scripts.
|
|
||
| ## Setup | ||
|
|
||
| Requires Node.js >= 22 < 24. See README.md for the full step by step. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use “step-by-step” wording.
Change “full step by step” to “full step-by-step guide” for grammatical correctness.
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...22 < 24. See README.md for the full step by step. ```bash npm install npm run dev ...
(QB_NEW_EN_HYPHEN)
🤖 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 `@CONTRIBUTING.md` at line 5, Update the setup reference in CONTRIBUTING.md to
say “full step-by-step guide” instead of “full step by step,” preserving the
existing README.md link and surrounding wording.
Source: Linters/SAST tools
2 participants
Summary
DOPE; GitHub Issues stays for external reports) and Git Workflow sectiondocs/outdated/(archived, no inbound refs),docs/ports/(superseded snapshot ofsrc/middleware/shared/ports/) and the empty CONTRIBUTING.md stub🤖 Generated with Claude Code
https://claude.ai/code/session_019thCMmxxVfACL7dBodtZB9
Summary by CodeRabbit
Documentation
>=22 <24.Chores