fix(compile): unbreak Runtime v3 build; warn on unsupported blocks - #866
Merged
Conversation
Contributor
WalkthroughMoves the Runtime v3 upload branch earlier in runCompilePipeline to upload an existing programSt via port.uploadRuntimeV3 (with compileOnly and missing-device checks) and adds focused Jest tests for v3 behavior. Separately, the board-switch warning modal now labels unsupported Python and/or C/C++ function blocks. ChangesRuntime v3 pipeline and Board warning
Sequence Diagram(s)sequenceDiagram
participant runCompilePipeline
participant Strucpp
participant port_uploadRuntimeV3
participant ArduinoFlow
runCompilePipeline->>Strucpp: strucpp.checkErrors(program)
runCompilePipeline->>port_uploadRuntimeV3: uploadRuntimeV3(programSt)
port_uploadRuntimeV3-->>runCompilePipeline: upload result (success/failure)
runCompilePipeline--xArduinoFlow: Arduino/core/lib/compile is skipped for v3 branch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts (1)
18-48: ⚡ Quick winUse
@root/*imports in this new test file.These imports are all reaching into
src/via deep relative paths, which goes against the repo alias rule and makes later file moves noisier.Suggested fix
-import type { DevicePin } from '../../types/PLC/devices' -import type { PLCProjectData } from '../../types/PLC/open-plc' +import type { DevicePin } from '`@root/backend/shared/types/PLC/devices`' +import type { PLCProjectData } from '`@root/backend/shared/types/PLC/open-plc`' import type { CompilerPlatformPort, PlatformDeviceContext, -} from '../../../../middleware/shared/ports/compiler-platform-port' +} from '`@root/middleware/shared/ports/compiler-platform-port`' @@ -import { runProgramBuildPipeline } from '../../library/program-build-pipeline' +import { runProgramBuildPipeline } from '`@root/backend/shared/library/program-build-pipeline`' import { embedFilesIntoProgramSt, type PipelineProgressEvent, runCompilePipeline, type RunCompilePipelineArgs, -} from '../pipeline' +} from '`@root/backend/shared/compile/pipeline`'As per coding guidelines, "Use path alias
@root/*to reference./src/*."🤖 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/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts` around lines 18 - 48, Replace all deep relative imports at the top of the test (the type imports DevicePin, PLCProjectData, CompilerPlatformPort, PlatformDeviceContext and the mocked module imports such as ../../utils/PLC/xml-generator (XmlGenerator), ../../library/program-build-pipeline (runProgramBuildPipeline), ../../library/program-build-helpers, ../../firmware/*, ../steps/generate-confs, and the subsequent imports of runProgramBuildPipeline, embedFilesIntoProgramSt, PipelineProgressEvent, runCompilePipeline, RunCompilePipelineArgs) with the repository path-alias `@root/`* equivalents that map to src/*. Update each import to use `@root/`<path-from-src> so the test follows the repo alias rule and avoids deep relative paths.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/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts`:
- Around line 116-205: The tests never exercise the C/C++-POU v3 path because
runCompilePipeline is never called with originalCppPous set; add a new test that
calls runCompilePipeline(makeArgs({ originalCppPous: [...] }), port, emit)
supplying a non-empty originalCppPous array so the pipeline generates c_blocks.h
and c_blocks_code.cpp, then assert that embedFilesIntoProgramSt behavior is
triggered by checking port.uploadRuntimeV3 was called once and that the upload
argument's programSt contains (*FILE:c_blocks.h ...) and
(*FILE:c_blocks_code.cpp ...); reuse helpers from the file (makePort,
captureEvents, makeArgs) and assert result.uploaded is true and mockedStrucpp
still runs.
In `@src/backend/shared/compile/pipeline.ts`:
- Around line 299-305: The helper embedFilesIntoProgramSt emits dangling markers
for empty files because after content.replace(/\n+$/, '') an empty file becomes
['']; update embedFilesIntoProgramSt to skip embedding when a file's trimmed
body is empty: after computing lines (or after trimming), check if lines.length
=== 0 or (lines.length === 1 && lines[0] === '') and continue to next file;
otherwise build embedded from lines and append to out. Reference the function
name embedFilesIntoProgramSt and the variables path, content, and lines when
making the change.
---
Nitpick comments:
In `@src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts`:
- Around line 18-48: Replace all deep relative imports at the top of the test
(the type imports DevicePin, PLCProjectData, CompilerPlatformPort,
PlatformDeviceContext and the mocked module imports such as
../../utils/PLC/xml-generator (XmlGenerator),
../../library/program-build-pipeline (runProgramBuildPipeline),
../../library/program-build-helpers, ../../firmware/*, ../steps/generate-confs,
and the subsequent imports of runProgramBuildPipeline, embedFilesIntoProgramSt,
PipelineProgressEvent, runCompilePipeline, RunCompilePipelineArgs) with the
repository path-alias `@root/`* equivalents that map to src/*. Update each import
to use `@root/`<path-from-src> so the test follows the repo alias rule and avoids
deep relative paths.
🪄 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: cc09a6d4-c84b-4150-85e9-2fbbe38c5651
📒 Files selected for processing (2)
src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.tssrc/backend/shared/compile/pipeline.ts
Comment on lines
+116
to
+205
| describe('runCompilePipeline — Runtime v3 branch', () => { | ||
| it('uploads program.st via uploadRuntimeV3 WITHOUT touching any arduino-cli step', async () => { | ||
| const port = makePort() | ||
| const { events, emit } = captureEvents() | ||
|
|
||
| const result = await runCompilePipeline(makeArgs(), port, emit) | ||
|
|
||
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: true }) | ||
| // The regression guard: v3 must never reach the Arduino path. | ||
| expect(port.installArduinoCore).not.toHaveBeenCalled() | ||
| expect(port.installArduinoLib).not.toHaveBeenCalled() | ||
| expect(port.compileArduino).not.toHaveBeenCalled() | ||
| expect(port.uploadRuntimeV4).not.toHaveBeenCalled() | ||
| // Strucpp still runs as an error-check before upload. | ||
| expect(mockedStrucpp).toHaveBeenCalledTimes(1) | ||
| expect(port.uploadRuntimeV3).toHaveBeenCalledTimes(1) | ||
| expect(events.map((e) => e.stage)).toContain('done') | ||
| }) | ||
|
|
||
| it('passes the plain program.st (no FILE markers) when there are no C/C++ POUs', async () => { | ||
| const port = makePort() | ||
| const { emit } = captureEvents() | ||
|
|
||
| await runCompilePipeline(makeArgs(), port, emit) | ||
|
|
||
| const [uploadArg] = port.uploadRuntimeV3.mock.calls[0] | ||
| expect(uploadArg.programSt).toBe('PROGRAM main\nEND_PROGRAM') | ||
| expect(uploadArg.programSt).not.toContain('(*FILE:') | ||
| }) | ||
|
|
||
| it('skips upload in compileOnly mode', async () => { | ||
| const port = makePort() | ||
| const { emit } = captureEvents() | ||
|
|
||
| const result = await runCompilePipeline(makeArgs({ compileOnly: true }), port, emit) | ||
|
|
||
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: false }) | ||
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | ||
| expect(port.installArduinoCore).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('warns and skips upload when no device context is configured', async () => { | ||
| const port = makePort() | ||
| const { events, emit } = captureEvents() | ||
|
|
||
| const result = await runCompilePipeline(makeArgs({ deviceContext: undefined }), port, emit) | ||
|
|
||
| expect(result.uploaded).toBe(false) | ||
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | ||
| expect(events.some((e) => e.level === 'warning' && /v3 not configured/i.test(e.message))).toBe(true) | ||
| }) | ||
|
|
||
| it('bails when uploadRuntimeV3 reports failure', async () => { | ||
| const port = makePort({ uploadRuntimeV3: jest.fn().mockResolvedValue({ ok: false }) }) | ||
| const { emit } = captureEvents() | ||
|
|
||
| const result = await runCompilePipeline(makeArgs(), port, emit) | ||
|
|
||
| expect(result.success).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe('embedFilesIntoProgramSt', () => { | ||
| it('appends one (*FILE:<path> <line> *) comment per source line, matching the v3 parser', () => { | ||
| const out = embedFilesIntoProgramSt('PROGRAM main\nEND_PROGRAM', [ | ||
| { path: 'c_blocks.h', content: '#include <x>\nvoid f();' }, | ||
| ]) | ||
| const lines = out.split('\n') | ||
| expect(lines).toContain('(*FILE:c_blocks.h #include <x> *)') | ||
| expect(lines).toContain('(*FILE:c_blocks.h void f(); *)') | ||
| // Original program text is preserved ahead of the markers. | ||
| expect(out.startsWith('PROGRAM main\nEND_PROGRAM')).toBe(true) | ||
| }) | ||
|
|
||
| it('drops trailing blank lines so no dangling empty marker is emitted', () => { | ||
| const out = embedFilesIntoProgramSt('X', [{ path: 'c_blocks_code.cpp', content: 'a\n\n' }]) | ||
| expect(out).toBe('X\n(*FILE:c_blocks_code.cpp a *)') | ||
| }) | ||
|
|
||
| it('emits multiple files in order', () => { | ||
| const out = embedFilesIntoProgramSt('P', [ | ||
| { path: 'c_blocks.h', content: 'h' }, | ||
| { path: 'c_blocks_code.cpp', content: 'c' }, | ||
| ]) | ||
| expect(out).toBe('P\n(*FILE:c_blocks.h h *)\n(*FILE:c_blocks_code.cpp c *)') | ||
| }) | ||
|
|
||
| it('is a no-op for an empty file list', () => { | ||
| expect(embedFilesIntoProgramSt('P', [])).toBe('P') | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add one pipeline test that exercises the C/C++-POU v3 path.
The suite covers the no-C++ branch and the helper in isolation, but it never runs runCompilePipeline with originalCppPous, so the new glue that builds c_blocks.h / c_blocks_code.cpp and uploads the embedded program.st is still untested.
Suggested test shape
describe('runCompilePipeline — Runtime v3 branch', () => {
+ it('embeds generated C-block files before upload when C/C++ POUs are present', async () => {
+ const port = makePort()
+ const { emit } = captureEvents()
+
+ await runCompilePipeline(
+ makeArgs({
+ projectData: {
+ pous: [],
+ dataTypes: [],
+ configuration: { resource: { tasks: [], instances: [], globalVariables: [] } },
+ servers: [],
+ remoteDevices: [],
+ originalCppPous: [{ name: 'Foo', code: 'void Foo() {}', variables: [] }],
+ } as PLCProjectData,
+ }),
+ port,
+ emit,
+ )
+
+ const [uploadArg] = port.uploadRuntimeV3.mock.calls[0]
+ expect(uploadArg.programSt).toContain('(*FILE:c_blocks.h ')
+ expect(uploadArg.programSt).toContain('(*FILE:c_blocks_code.cpp ')
+ })As per coding guidelines, "Maintain 100% code coverage for: src/backend/shared/."
📝 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.
Suggested change
| describe('runCompilePipeline — Runtime v3 branch', () => { | |
| it('uploads program.st via uploadRuntimeV3 WITHOUT touching any arduino-cli step', async () => { | |
| const port = makePort() | |
| const { events, emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs(), port, emit) | |
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: true }) | |
| // The regression guard: v3 must never reach the Arduino path. | |
| expect(port.installArduinoCore).not.toHaveBeenCalled() | |
| expect(port.installArduinoLib).not.toHaveBeenCalled() | |
| expect(port.compileArduino).not.toHaveBeenCalled() | |
| expect(port.uploadRuntimeV4).not.toHaveBeenCalled() | |
| // Strucpp still runs as an error-check before upload. | |
| expect(mockedStrucpp).toHaveBeenCalledTimes(1) | |
| expect(port.uploadRuntimeV3).toHaveBeenCalledTimes(1) | |
| expect(events.map((e) => e.stage)).toContain('done') | |
| }) | |
| it('passes the plain program.st (no FILE markers) when there are no C/C++ POUs', async () => { | |
| const port = makePort() | |
| const { emit } = captureEvents() | |
| await runCompilePipeline(makeArgs(), port, emit) | |
| const [uploadArg] = port.uploadRuntimeV3.mock.calls[0] | |
| expect(uploadArg.programSt).toBe('PROGRAM main\nEND_PROGRAM') | |
| expect(uploadArg.programSt).not.toContain('(*FILE:') | |
| }) | |
| it('skips upload in compileOnly mode', async () => { | |
| const port = makePort() | |
| const { emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs({ compileOnly: true }), port, emit) | |
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: false }) | |
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | |
| expect(port.installArduinoCore).not.toHaveBeenCalled() | |
| }) | |
| it('warns and skips upload when no device context is configured', async () => { | |
| const port = makePort() | |
| const { events, emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs({ deviceContext: undefined }), port, emit) | |
| expect(result.uploaded).toBe(false) | |
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | |
| expect(events.some((e) => e.level === 'warning' && /v3 not configured/i.test(e.message))).toBe(true) | |
| }) | |
| it('bails when uploadRuntimeV3 reports failure', async () => { | |
| const port = makePort({ uploadRuntimeV3: jest.fn().mockResolvedValue({ ok: false }) }) | |
| const { emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs(), port, emit) | |
| expect(result.success).toBe(false) | |
| }) | |
| }) | |
| describe('embedFilesIntoProgramSt', () => { | |
| it('appends one (*FILE:<path> <line> *) comment per source line, matching the v3 parser', () => { | |
| const out = embedFilesIntoProgramSt('PROGRAM main\nEND_PROGRAM', [ | |
| { path: 'c_blocks.h', content: '#include <x>\nvoid f();' }, | |
| ]) | |
| const lines = out.split('\n') | |
| expect(lines).toContain('(*FILE:c_blocks.h #include <x> *)') | |
| expect(lines).toContain('(*FILE:c_blocks.h void f(); *)') | |
| // Original program text is preserved ahead of the markers. | |
| expect(out.startsWith('PROGRAM main\nEND_PROGRAM')).toBe(true) | |
| }) | |
| it('drops trailing blank lines so no dangling empty marker is emitted', () => { | |
| const out = embedFilesIntoProgramSt('X', [{ path: 'c_blocks_code.cpp', content: 'a\n\n' }]) | |
| expect(out).toBe('X\n(*FILE:c_blocks_code.cpp a *)') | |
| }) | |
| it('emits multiple files in order', () => { | |
| const out = embedFilesIntoProgramSt('P', [ | |
| { path: 'c_blocks.h', content: 'h' }, | |
| { path: 'c_blocks_code.cpp', content: 'c' }, | |
| ]) | |
| expect(out).toBe('P\n(*FILE:c_blocks.h h *)\n(*FILE:c_blocks_code.cpp c *)') | |
| }) | |
| it('is a no-op for an empty file list', () => { | |
| expect(embedFilesIntoProgramSt('P', [])).toBe('P') | |
| }) | |
| describe('runCompilePipeline — Runtime v3 branch', () => { | |
| it('embeds generated C-block files before upload when C/C++ POUs are present', async () => { | |
| const port = makePort() | |
| const { emit } = captureEvents() | |
| await runCompilePipeline( | |
| makeArgs({ | |
| projectData: { | |
| pous: [], | |
| dataTypes: [], | |
| configuration: { resource: { tasks: [], instances: [], globalVariables: [] } }, | |
| servers: [], | |
| remoteDevices: [], | |
| originalCppPous: [{ name: 'Foo', code: 'void Foo() {}', variables: [] }], | |
| } as PLCProjectData, | |
| }), | |
| port, | |
| emit, | |
| ) | |
| const [uploadArg] = port.uploadRuntimeV3.mock.calls[0] | |
| expect(uploadArg.programSt).toContain('(*FILE:c_blocks.h ') | |
| expect(uploadArg.programSt).toContain('(*FILE:c_blocks_code.cpp ') | |
| }) | |
| it('uploads program.st via uploadRuntimeV3 WITHOUT touching any arduino-cli step', async () => { | |
| const port = makePort() | |
| const { events, emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs(), port, emit) | |
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: true }) | |
| // The regression guard: v3 must never reach the Arduino path. | |
| expect(port.installArduinoCore).not.toHaveBeenCalled() | |
| expect(port.installArduinoLib).not.toHaveBeenCalled() | |
| expect(port.compileArduino).not.toHaveBeenCalled() | |
| expect(port.uploadRuntimeV4).not.toHaveBeenCalled() | |
| // Strucpp still runs as an error-check before upload. | |
| expect(mockedStrucpp).toHaveBeenCalledTimes(1) | |
| expect(port.uploadRuntimeV3).toHaveBeenCalledTimes(1) | |
| expect(events.map((e) => e.stage)).toContain('done') | |
| }) | |
| it('passes the plain program.st (no FILE markers) when there are no C/C++ POUs', async () => { | |
| const port = makePort() | |
| const { emit } = captureEvents() | |
| await runCompilePipeline(makeArgs(), port, emit) | |
| const [uploadArg] = port.uploadRuntimeV3.mock.calls[0] | |
| expect(uploadArg.programSt).toBe('PROGRAM main\nEND_PROGRAM') | |
| expect(uploadArg.programSt).not.toContain('(*FILE:') | |
| }) | |
| it('skips upload in compileOnly mode', async () => { | |
| const port = makePort() | |
| const { emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs({ compileOnly: true }), port, emit) | |
| expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: false }) | |
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | |
| expect(port.installArduinoCore).not.toHaveBeenCalled() | |
| }) | |
| it('warns and skips upload when no device context is configured', async () => { | |
| const port = makePort() | |
| const { events, emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs({ deviceContext: undefined }), port, emit) | |
| expect(result.uploaded).toBe(false) | |
| expect(port.uploadRuntimeV3).not.toHaveBeenCalled() | |
| expect(events.some((e) => e.level === 'warning' && /v3 not configured/i.test(e.message))).toBe(true) | |
| }) | |
| it('bails when uploadRuntimeV3 reports failure', async () => { | |
| const port = makePort({ uploadRuntimeV3: jest.fn().mockResolvedValue({ ok: false }) }) | |
| const { emit } = captureEvents() | |
| const result = await runCompilePipeline(makeArgs(), port, emit) | |
| expect(result.success).toBe(false) | |
| }) | |
| }) | |
| describe('embedFilesIntoProgramSt', () => { | |
| it('appends one (*FILE:<path> <line> *) comment per source line, matching the v3 parser', () => { | |
| const out = embedFilesIntoProgramSt('PROGRAM main\nEND_PROGRAM', [ | |
| { path: 'c_blocks.h', content: '`#include` <x>\nvoid f();' }, | |
| ]) | |
| const lines = out.split('\n') | |
| expect(lines).toContain('(*FILE:c_blocks.h `#include` <x> *)') | |
| expect(lines).toContain('(*FILE:c_blocks.h void f(); *)') | |
| // Original program text is preserved ahead of the markers. | |
| expect(out.startsWith('PROGRAM main\nEND_PROGRAM')).toBe(true) | |
| }) | |
| it('drops trailing blank lines so no dangling empty marker is emitted', () => { | |
| const out = embedFilesIntoProgramSt('X', [{ path: 'c_blocks_code.cpp', content: 'a\n\n' }]) | |
| expect(out).toBe('X\n(*FILE:c_blocks_code.cpp a *)') | |
| }) | |
| it('emits multiple files in order', () => { | |
| const out = embedFilesIntoProgramSt('P', [ | |
| { path: 'c_blocks.h', content: 'h' }, | |
| { path: 'c_blocks_code.cpp', content: 'c' }, | |
| ]) | |
| expect(out).toBe('P\n(*FILE:c_blocks.h h *)\n(*FILE:c_blocks_code.cpp c *)') | |
| }) | |
| it('is a no-op for an empty file list', () => { | |
| expect(embedFilesIntoProgramSt('P', [])).toBe('P') | |
| }) | |
| }) |
🧰 Tools
🪛 ast-grep (0.43.0)
[warning] 116-132: Avoid weak hash algorithm from CryptoJS
Context: it('uploads program.st via uploadRuntimeV3 WITHOUT touching any arduino-cli step', async () => {
const port = makePort()
const { events, emit } = captureEvents()
const result = await runCompilePipeline(makeArgs(), port, emit)
expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: true })
// The regression guard: v3 must never reach the Arduino path.
expect(port.installArduinoCore).not.toHaveBeenCalled()
expect(port.installArduinoLib).not.toHaveBeenCalled()
expect(port.compileArduino).not.toHaveBeenCalled()
expect(port.uploadRuntimeV4).not.toHaveBeenCalled()
// Strucpp still runs as an error-check before upload.
expect(mockedStrucpp).toHaveBeenCalledTimes(1)
expect(port.uploadRuntimeV3).toHaveBeenCalledTimes(1)
expect(events.map((e) => e.stage)).toContain('done')
})
Note: Security best practice.
(crypto-avoid-weak-hash-typescript)
[warning] 122-122: Avoid weak hash algorithm from CryptoJS
Context: expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: true })
Note: Security best practice.
(crypto-avoid-weak-hash-typescript)
[warning] 145-154: Avoid weak hash algorithm from CryptoJS
Context: it('skips upload in compileOnly mode', async () => {
const port = makePort()
const { emit } = captureEvents()
const result = await runCompilePipeline(makeArgs({ compileOnly: true }), port, emit)
expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: false })
expect(port.uploadRuntimeV3).not.toHaveBeenCalled()
expect(port.installArduinoCore).not.toHaveBeenCalled()
})
Note: Security best practice.
(crypto-avoid-weak-hash-typescript)
[warning] 151-151: Avoid weak hash algorithm from CryptoJS
Context: expect(result).toEqual({ success: true, md5: 'a'.repeat(32), uploaded: false })
Note: Security best practice.
(crypto-avoid-weak-hash-typescript)
🤖 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/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts` around
lines 116 - 205, The tests never exercise the C/C++-POU v3 path because
runCompilePipeline is never called with originalCppPous set; add a new test that
calls runCompilePipeline(makeArgs({ originalCppPous: [...] }), port, emit)
supplying a non-empty originalCppPous array so the pipeline generates c_blocks.h
and c_blocks_code.cpp, then assert that embedFilesIntoProgramSt behavior is
triggered by checking port.uploadRuntimeV3 was called once and that the upload
argument's programSt contains (*FILE:c_blocks.h ...) and
(*FILE:c_blocks_code.cpp ...); reuse helpers from the file (makePort,
captureEvents, makeArgs) and assert result.uploaded is true and mockedStrucpp
still runs.
Source: Coding guidelines
Comment on lines
+299
to
+305
| export function embedFilesIntoProgramSt(programSt: string, files: Array<{ path: string; content: string }>): string { | ||
| let out = programSt | ||
| for (const { path, content } of files) { | ||
| const lines = content.replace(/\n+$/, '').split('\n') | ||
| const embedded = lines.map((line) => `(*FILE:${path} ${line} *)`).join('\n') | ||
| out += '\n' + embedded | ||
| } |
Contributor
There was a problem hiding this comment.
Skip empty file bodies before emitting (*FILE:...) markers.
After replace(/\n+$/, ''), an empty file still becomes [''], so this helper emits (*FILE:path *) — the dangling marker the doc says it avoids. A small guard keeps the helper consistent for empty generated side files.
Suggested fix
export function embedFilesIntoProgramSt(programSt: string, files: Array<{ path: string; content: string }>): string {
let out = programSt
for (const { path, content } of files) {
- const lines = content.replace(/\n+$/, '').split('\n')
+ const trimmed = content.replace(/\n+$/, '')
+ if (trimmed === '') continue
+ const lines = trimmed.split('\n')
const embedded = lines.map((line) => `(*FILE:${path} ${line} *)`).join('\n')
out += '\n' + embedded
}
return out
}📝 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.
Suggested change
| export function embedFilesIntoProgramSt(programSt: string, files: Array<{ path: string; content: string }>): string { | |
| let out = programSt | |
| for (const { path, content } of files) { | |
| const lines = content.replace(/\n+$/, '').split('\n') | |
| const embedded = lines.map((line) => `(*FILE:${path} ${line} *)`).join('\n') | |
| out += '\n' + embedded | |
| } | |
| export function embedFilesIntoProgramSt(programSt: string, files: Array<{ path: string; content: string }>): string { | |
| let out = programSt | |
| for (const { path, content } of files) { | |
| const trimmed = content.replace(/\n+$/, '') | |
| if (trimmed === '') continue | |
| const lines = trimmed.split('\n') | |
| const embedded = lines.map((line) => `(*FILE:${path} ${line} *)`).join('\n') | |
| out += '\n' + embedded | |
| } | |
| return out | |
| } |
🤖 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/backend/shared/compile/pipeline.ts` around lines 299 - 305, The helper
embedFilesIntoProgramSt emits dangling markers for empty files because after
content.replace(/\n+$/, '') an empty file becomes ['']; update
embedFilesIntoProgramSt to skip embedding when a file's trimmed body is empty:
after computing lines (or after trimming), check if lines.length === 0 or
(lines.length === 1 && lines[0] === '') and continue to next file; otherwise
build embedded from lines and append to out. Reference the function name
embedFilesIntoProgramSt and the variables path, content, and lines when making
the change.
The v3 upload branch sat AFTER installArduinoCore in the shared pipeline,
so every v3 build ran strucpp (fine), then tried to install an Arduino
core with an empty FQBN ("invalid empty core argument") and aborted
before the program.st upload was reached. v3 has nothing to do with
arduino-cli — its on-device MatIEC recompiles the uploaded program.st.
- Move the isRuntimeV3 branch above installArduinoCore (right after the
strucpp compile / v4 branch). Strucpp still runs first purely as a
user-code error check; a strucpp error bails before upload.
- C/C++ and Python function blocks are NOT compatible with v3: both lower
to strucpp `{external ...}` inline-C that v3's MatIEC toolchain can't
parse (it bails with a wall of ST syntax errors). Rather than a hard
block, warn on board switch — the same soft prompt Arduino already
shows for Python — and let the user proceed (the on-device compile
fails if they do). board.tsx now warns for Python and/or C/C++ blocks
on v3 via a dynamic label reusing the existing modal.
- New pipeline-runtime-v3.test.ts (the existing pipeline.test.ts is stale
from the xml2st->JSON-transpiler migration and references the removed
transpileXmlToSt port method).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thiagoralves
force-pushed
the
fix/runtime-v3-build
branch
from
June 10, 2026 00:22
99fb129 to
fb01406
Compare
thiagoralves
changed the title
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts (1)
18-24: ⚡ Quick winUse
@root/*aliases for these new test imports.The new file uses relative
../../..imports; switch them to@root/*paths to match repo import policy.As per coding guidelines, "Use path alias
@root/*to reference./src/*".Also applies to: 42-43
🤖 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/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts` around lines 18 - 24, Replace the relative imports in the test file by using the repository path alias: change imports of DevicePin and PLCProjectData from '../../types/PLC/devices' and '../../types/PLC/open-plc' to '`@root/backend/shared/types/PLC/devices`' and '`@root/backend/shared/types/PLC/open-plc`' (or the correct `@root` path that maps to those modules), and change the CompilerPlatformPort and PlatformDeviceContext import from '../../../../middleware/shared/ports/compiler-platform-port' to the corresponding '`@root/middleware/shared/ports/compiler-platform-port`' alias; also update the similar imports referenced at the other two locations mentioned (lines 42–43) so all test imports use `@root/`* aliases.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.
Nitpick comments:
In `@src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.ts`:
- Around line 18-24: Replace the relative imports in the test file by using the
repository path alias: change imports of DevicePin and PLCProjectData from
'../../types/PLC/devices' and '../../types/PLC/open-plc' to
'`@root/backend/shared/types/PLC/devices`' and
'`@root/backend/shared/types/PLC/open-plc`' (or the correct `@root` path that maps
to those modules), and change the CompilerPlatformPort and PlatformDeviceContext
import from '../../../../middleware/shared/ports/compiler-platform-port' to the
corresponding '`@root/middleware/shared/ports/compiler-platform-port`' alias; also
update the similar imports referenced at the other two locations mentioned
(lines 42–43) so all test imports use `@root/`* aliases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: be45c789-f015-4e2e-83bd-e1d745d89ecc
📒 Files selected for processing (3)
src/backend/shared/compile/__tests__/pipeline-runtime-v3.test.tssrc/backend/shared/compile/pipeline.tssrc/frontend/components/_features/[workspace]/editor/device/configuration/board.tsx
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
1 participant
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
Runtime v3 builds died at "Installing Arduino core… invalid empty core argument": the
isRuntimeV3upload branch in the shared pipeline sat afterinstallArduinoCore, so a v3 build ran strucpp (fine), then tried to install an Arduino core with an empty FQBN and aborted before theprogram.stupload. v3 has nothing to do with arduino-cli — its on-device MatIEC recompiles the uploadedprogram.st.Changes
isRuntimeV3branch aboveinstallArduinoCore(right after strucpp / the v4 branch). Strucpp still runs first as a user-code error check; a strucpp error bails before upload. Plain ST / LD / IL / FBD / SFC v3 projects now compile + upload correctly.{external …}inline-C that v3's MatIEC can't parse (verified: a multi-language project produces a wall of MatIEC syntax errors on the device). Rather than a hard block,board.tsxnow warns on board switch — the same soft prompt Arduino already shows for Python — covering Python and/or C/C++ blocks via a dynamic label. The user may proceed; the on-device compile then fails, as expected.pipeline-runtime-v3.test.ts(5 tests). The existingpipeline.test.tsis stale from the xml2st→JSON-transpiler migration (references the removedtranspileXmlToSt) and is out of scope.Paired with openplc-web (byte-identical shared
pipeline.ts+board.tsx; web never targets v3, so inert there, but the sync gate requires the mirror).Test plan
pipeline-runtime-v3.test.ts— 5 pass (v3 never calls installArduinoCore/lib/compileArduino; uploadsprogram.st; compileOnly / no-context / failure cases)tsc, eslint, prettier clean on changed files🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Refactor
New Features