Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import type { ESIRepositoryItemLight } from '@root/middleware/shared/ports/esi-types'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if openplc-web#494 is merged and verify file existence

# Check if we can query the web repo (requires GitHub CLI with appropriate access)
echo "=== Checking openplc-web PR `#494` status ==="
gh pr view 494 --repo Autonomy-Logic/openplc-web --json state,title,merged,mergedAt 2>/dev/null || echo "Cannot access openplc-web repo or PR does not exist"

echo -e "\n=== Checking if test file exists in openplc-web ==="
gh api repos/Autonomy-Logic/openplc-web/contents/src/frontend/components/_features/workspace/editor/device/ethercat/components/__tests__/esi-upload.test.tsx 2>/dev/null && echo "File exists in web repo" || echo "File does NOT exist in web repo"

Repository: Autonomy-Logic/openplc-editor

Length of output: 381


🏁 Script executed:

# Find CI configuration files
find . -type f \( -name "*.yml" -o -name "*.yaml" \) | grep -E "(workflow|action|ci)" | head -20

Repository: Autonomy-Logic/openplc-editor

Length of output: 551


🏁 Script executed:

# Check for documentation about repository sync or relationships
fd -t f -E node_modules -E dist \( -name "README*" -o -name "CONTRIBUTING*" -o -name ".github" \)

Repository: Autonomy-Logic/openplc-editor

Length of output: 304


🏁 Script executed:

# Look for any sync-related scripts or configurations
rg "surface.*sync|sync.*surface" --type ts --type tsx --type js --type jsx --type yml --type yaml -l 2>/dev/null | head -20

Repository: Autonomy-Logic/openplc-editor

Length of output: 55


🏁 Script executed:

cat -n ./.github/workflows/ci-sync.yml

Repository: Autonomy-Logic/openplc-editor

Length of output: 19516


🏁 Script executed:

# Check if there's documentation about the sync strategy
rg -i "surface.*sync|byte.*identical|mirror" --type md --type txt -A 3 -B 1

Repository: Autonomy-Logic/openplc-editor

Length of output: 1819


🏁 Script executed:

# Look for any environment variables or configuration related to sync
rg "SYNC|surface" .github/workflows/ci-sync.yml

Repository: Autonomy-Logic/openplc-editor

Length of output: 989


🏁 Script executed:

cat -n scripts/compare-surfaces.py | head -100

Repository: Autonomy-Logic/openplc-editor

Length of output: 3414


🏁 Script executed:

# Find the SURFACES definition that's referenced in ci-sync.yml
find . -type f -name "*.py" -o -name "*.json" -o -name "*.ts" -o -name "*.tsx" | xargs grep -l "SURFACES" 2>/dev/null | grep -v node_modules | head -10

Repository: Autonomy-Logic/openplc-editor

Length of output: 144


🏁 Script executed:

# Look for configuration about what constitutes a "surface"
fd -t f "surface" --type f | grep -v node_modules | head -20

Repository: Autonomy-Logic/openplc-editor

Length of output: 140


🏁 Script executed:

cat -n scripts/compare-surfaces.py | tail -50

Repository: Autonomy-Logic/openplc-editor

Length of output: 2028


🏁 Script executed:

# Check if there are any filters or exclusions for test files in the comparison
grep -n "test\|spec\|__tests__\|skip\|exclude\|ignore" scripts/compare-surfaces.py

Repository: Autonomy-Logic/openplc-editor

Length of output: 55


🏁 Script executed:

# Verify the exact test file path structure in the editor repo
find src/frontend -path "*esi-upload.test.tsx" -o -path "*esi-upload*" 2>/dev/null

Repository: Autonomy-Logic/openplc-editor

Length of output: 278


Test file must be synced to openplc-web repository.

Test files are included in the byte-identical surface comparison (no exclusions exist in compare-surfaces.py). This file exists in the editor repo but is missing from the web repo, which will cause the CI surface-sync check to fail.

The sync check will report an error and block the merge unless:

  1. A corresponding web PR (such as #494) with this file is merged first, OR
  2. The file is added to the web repo on the same target branch.

Coordinate with the web repository to ensure this test file is synced before merging.

🧰 Tools
🪛 GitHub Actions: CI / 3_sync _ Shared Surface Sync.txt

[error] 1-1: compare-surfaces.py reported a mismatch: file exists only in editor repo (not in web/src).

🪛 GitHub Actions: CI / sync _ Shared Surface Sync

[error] 1-1: [frontend] File exists only in editor repo

🤖 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/frontend/components/_features/`[workspace]/editor/device/ethercat/components/__tests__/esi-upload.test.tsx
at line 1, The test file
src/frontend/components/_features/[workspace]/editor/device/ethercat/components/__tests__/esi-upload.test.tsx
is present in this repo but missing from the openplc-web repository, causing the
surface-sync CI check to fail; either add the identical file to the web repo on
the same target branch or coordinate with the web repo maintainers to merge the
corresponding PR (e.g., the one that should contain esi-upload.test.tsx) before
merging this PR so the byte-identical test is present in both repos and the
compare-surfaces.py check passes.

import { fireEvent, render, waitFor } from '@testing-library/react'

// Mocked EsiPort surface — the two methods the upload flow touches. The
// `mock`-prefixed name is referenced lazily inside the factory (only when
// `useEsi()` is called at render time), so this works under both Vitest and
// the editor's Jest+vi shim — without `vi.hoisted`, which the shim lacks.
const mockEsi = {
parseAndSaveFile: vi.fn(),
loadRepositoryLight: vi.fn(),
}

vi.mock('@root/middleware/shared/providers/platform-context', () => ({
useEsi: () => mockEsi,
}))

import { ESIUpload } from '../esi-upload'

const SAMPLE_ITEM: ESIRepositoryItemLight = {
id: 'item-1',
filename: 'Beckhoff.xml',
vendor: { id: '0x0002', name: 'Beckhoff' },
devices: [],
loadedAt: '2026-01-01T00:00:00.000Z',
}

/** Build an XML File whose `.text()` resolves regardless of jsdom version. */
function xmlFile(name = 'Beckhoff.xml', content = '<xml />'): File {
const file = new File([content], name, { type: 'text/xml' })
Object.defineProperty(file, 'text', { value: () => Promise.resolve(content) })
return file
}

/** Render the component and drive a single-file upload through the input. */
function uploadFile(repository: ESIRepositoryItemLight[] = []) {
const onFilesLoaded = vi.fn()
const { container } = render(<ESIUpload onFilesLoaded={onFilesLoaded} repository={repository} />)
const input = container.querySelector('input[type="file"]') as HTMLInputElement
fireEvent.change(input, { target: { files: [xmlFile()] } })
return { onFilesLoaded }
}

describe('ESIUpload — dedupAfterRetry handling', () => {
beforeEach(() => {
mockEsi.parseAndSaveFile.mockReset()
mockEsi.loadRepositoryLight.mockReset()
})

it('appends a newly added item without re-listing the repository', async () => {
mockEsi.parseAndSaveFile.mockResolvedValueOnce({ success: true, item: SAMPLE_ITEM })

const { onFilesLoaded } = uploadFile()

await waitFor(() => expect(onFilesLoaded).toHaveBeenCalled())
expect(onFilesLoaded).toHaveBeenCalledWith([SAMPLE_ITEM], undefined)
expect(mockEsi.loadRepositoryLight).not.toHaveBeenCalled()
})

it('re-lists the repository when a dedupAfterRetry lands without an item', async () => {
mockEsi.parseAndSaveFile.mockResolvedValueOnce({ success: true, dedupAfterRetry: true })
mockEsi.loadRepositoryLight.mockResolvedValueOnce({ success: true, items: [SAMPLE_ITEM] })

const { onFilesLoaded } = uploadFile()

await waitFor(() => expect(onFilesLoaded).toHaveBeenCalled())
expect(mockEsi.loadRepositoryLight).toHaveBeenCalledTimes(1)
// The refreshed list is authoritative — the recovered row appears here.
expect(onFilesLoaded).toHaveBeenCalledWith([SAMPLE_ITEM], undefined)
})

it('falls back to the local list when the refresh itself fails', async () => {
const existing: ESIRepositoryItemLight[] = [{ ...SAMPLE_ITEM, id: 'old', filename: 'Old.xml' }]
mockEsi.parseAndSaveFile.mockResolvedValueOnce({ success: true, dedupAfterRetry: true })
mockEsi.loadRepositoryLight.mockResolvedValueOnce({ success: false, error: 'list failed' })

const onFilesLoaded = vi.fn()
const { container } = render(<ESIUpload onFilesLoaded={onFilesLoaded} repository={existing} />)
const input = container.querySelector('input[type="file"]') as HTMLInputElement
fireEvent.change(input, { target: { files: [xmlFile()] } })

await waitFor(() => expect(onFilesLoaded).toHaveBeenCalled())
expect(mockEsi.loadRepositoryLight).toHaveBeenCalledTimes(1)
// No new item was returned, so the fallback keeps the existing repository.
expect(onFilesLoaded).toHaveBeenCalledWith(existing, undefined)
})

it('skips a real duplicate silently without re-listing', async () => {
mockEsi.parseAndSaveFile.mockResolvedValueOnce({ success: true, duplicate: true })

const { onFilesLoaded } = uploadFile()

await waitFor(() => expect(onFilesLoaded).toHaveBeenCalled())
expect(onFilesLoaded).toHaveBeenCalledWith([], undefined)
expect(mockEsi.loadRepositoryLight).not.toHaveBeenCalled()
})
})
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { cn } from '@root/frontend/utils/cn'
import type { ESIRepositoryItemLight } from '@root/middleware/shared/ports/esi-types'
import { useEsi } from '@root/middleware/shared/providers/platform-context'
Expand Down Expand Up @@ -55,6 +55,12 @@

const newItems: ESIRepositoryItemLight[] = []
const errors: Array<{ filename: string; error: string }> = []
// A dedup-after-retry result means the file was persisted on the backend
// but its row was missing from the upload response (and the adapter's own
// recovery lookup also failed). Honor the EsiPort contract by re-listing
// the repository after the batch so the file appears instead of silently
// vanishing — see EsiPort.parseAndSaveFile (`dedupAfterRetry`).
let needsRepositoryRefresh = false

const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB

Expand Down Expand Up @@ -84,8 +90,13 @@

if (result.success && result.item) {
newItems.push(result.item)
} else if (result.success && result.dedupAfterRetry) {
// Uploaded but absent from the response: a transient-failure retry
// hit the backend dedup and the adapter couldn't recover the row.
// Flag a repository refresh so the file surfaces after the batch.
needsRepositoryRefresh = true
} else if (result.success) {
// Duplicate content already in the repository — skip silently.
// Real duplicate — content already in the repository. Skip silently.
// See EsiPort.parseAndSaveFile for the duplicate-handling contract.
} else {
errors.push({ filename: file.name, error: result.error ?? 'Parse failed' })
Expand All @@ -102,6 +113,17 @@
percentage: 100,
})

// A recovered-but-unlisted upload (dedupAfterRetry) is on the backend yet
// missing from `newItems`. Re-list the repository so it shows up; fall
// back to the locally accumulated list if the refresh itself fails.
if (needsRepositoryRefresh) {
const refreshed = await esi!.loadRepositoryLight()
if (refreshed.success && refreshed.items) {
onFilesLoaded(refreshed.items, errors.length > 0 ? errors : undefined)
return
}
}

onFilesLoaded([...repository, ...newItems], errors.length > 0 ? errors : undefined)
Comment on lines +116 to 127

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check IPC bridge type for esiParseAndSaveFile

# Search for the bridge method signature
rg -n -A5 'esiParseAndSaveFile.*Promise' --type=ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 55


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find all occurrences of dedupAfterRetry
rg -n --hidden --no-ignore-vcs "dedupAfterRetry" . || true

# 2) Find ESI port / loadRepositoryLight in the repo
rg -n --hidden --no-ignore-vcs "loadRepositoryLight" src || true
rg -n --hidden --no-ignore-vcs "EsiPort" src || true

# 3) Inspect main/renderer IPC bridge files for any ESI-related methods
rg -n --hidden --no-ignore-vcs "(esi|Esi).*Parse|esiParse|loadRepository" src/main/modules/ipc || true
rg -n --hidden --no-ignore-vcs "window\\.bridge\\." src/middleware/adapters/editor || true

# 4) Directly search for IPC method declarations mentioning ESI in renderer bridge
rg -n --hidden --no-ignore-vcs "ipc.*esi|esi.*ipc|loadRepositoryLight|RepositoryLight" src/main/modules/ipc/renderer.ts || true

Repository: Autonomy-Logic/openplc-editor

Length of output: 50385


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# If dedupAfterRetry exists, print surrounding type declarations in main IPC renderer bridge
# (Use a small window to avoid huge output)
for pat in "dedupAfterRetry" "loadRepositoryLight" "RepositoryLight" "needsRepositoryRefresh" ; do
  echo "---- pattern: $pat ----"
  rg -n -A12 -B4 "$pat" src/main/modules/ipc/renderer.ts src/main/modules/ipc/main.ts || true
done

Repository: Autonomy-Logic/openplc-editor

Length of output: 6062


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Print EsiPort contract for parseAndSaveFile (includes dedupAfterRetry)
sed -n '1,120p' src/middleware/shared/ports/esi-port.ts

# Print exact IPC renderer bridge signature for esiParseAndSaveFile
# (small window around the line indicated in the earlier grep)
sed -n '540,640p' src/main/modules/ipc/renderer.ts

# Print editor ESI adapter mapping for parseAndSaveFile
sed -n '1,140p' src/middleware/adapters/editor/esi-adapter.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 11447


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/frontend/components/_features/[workspace]/editor/device/ethercat/components/esi-upload.tsx

Repository: Autonomy-Logic/openplc-editor

Length of output: 7707


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the main-process handler implementation
rg -n "handleESIParseAndSaveFile" src/main/modules/ipc/main.ts

# Print the handler body
rg -n -A60 "handleESIParseAndSaveFile" src/main/modules/ipc/main.ts

# Locate backend ESI service parse method
rg -n "parseAndSaveFile" src/backend -S

# Print likely method signature / return type around dedupAfterRetry
rg -n -A40 -B5 "dedupAfterRetry" src/backend/editor src/backend/shared src/backend -S || true

Repository: Autonomy-Logic/openplc-editor

Length of output: 6076


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Print the parseAndSaveFile implementation area
rg -n -A140 -B20 "async parseAndSaveFile" src/backend/editor/ethercat/esi-service.ts

# Also look for dedupAfterRetry usage in backend
rg -n "dedupAfterRetry" src/backend/editor/ethercat/esi-service.ts src/backend/shared -S || true

Repository: Autonomy-Logic/openplc-editor

Length of output: 5383


Propagate dedupAfterRetry consistently through backend → IPC bridge → EsiPort adapter (or remove the UI dead branch)

  • EsiPort.parseAndSaveFile already models dedupAfterRetry?: boolean in src/middleware/shared/ports/esi-port.ts.
  • src/backend/editor/ethercat/esi-service.ts parseAndSaveFile currently never returns dedupAfterRetry (only item?, duplicate?, error?).
  • src/main/modules/ipc/renderer.ts esiParseAndSaveFile return type also omits dedupAfterRetry.
  • src/middleware/adapters/editor/esi-adapter.ts drops any such field (it maps only item and duplicate).

This makes the ESIUpload path that sets needsRepositoryRefresh on result.dedupAfterRetry effectively unreachable in the real flow; it only works when mocked in tests. Fix by either implementing dedupAfterRetry end-to-end (service → IPC bridge → adapter) or removing dedupAfterRetry from the port/UI until the backend supports it.

🤖 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/frontend/components/_features/`[workspace]/editor/device/ethercat/components/esi-upload.tsx
around lines 116 - 127, The UI branch relying on result.dedupAfterRetry is dead
because dedupAfterRetry isn't propagated end-to-end; either implement
propagation or remove the UI branch. To implement propagation: update backend
function parseAndSaveFile in esi-service.ts to set and return
dedupAfterRetry:boolean when appropriate, update the IPC handler
esiParseAndSaveFile in src/main/modules/ipc/renderer.ts to include
dedupAfterRetry in its response, and update the middleware adapter
editor/esi-adapter.ts mapping to pass dedupAfterRetry through when calling
EsiPort.parseAndSaveFile. Alternatively, remove the
needsRepositoryRefresh/dedupAfterRetry handling from the UI component
(esi-upload.tsx) to match current backend behavior.

},
[onFilesLoaded, repository, esi],
Expand Down
8 changes: 7 additions & 1 deletion src/middleware/shared/ports/esi-port.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/**
* EsiPort — Abstracts ESI (EtherCAT Slave Information) repository management.
*
Expand Down Expand Up @@ -39,11 +39,17 @@
* Dedup is filename-based: reimporting the same bytes under a different name
* will add a new entry, and replacing a file's contents without renaming it
* is reported as a duplicate.
*
* `dedupAfterRetry: true` indicates the duplicate response landed after a
* transient-failure retry — the first attempt likely persisted the file and
* its response was lost. Callers should treat this as "uploaded but absent
* from the response" rather than silently skipping, since the user expects
* the file to appear in the repository.
*/
parseAndSaveFile(
filename: string,
content: string,
): Promise<Result<{ item?: ESIRepositoryItemLight; duplicate?: boolean }>>
): Promise<Result<{ item?: ESIRepositoryItemLight; duplicate?: boolean; dedupAfterRetry?: boolean }>>

/**
* Delete a single repository item and its associated XML file.
Expand Down
Loading