Extract shared utilities for duplicated error, file, version, and stack logic - #20
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
Extract shared utilities for duplicated error, file, version, and stack logic#20devin-ai-integration[bot] wants to merge 1 commit into
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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
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
Duplicated logic found with
jscpd(plus manual review) is now extracted into shared modules. Clone count acrosssrc/,apps/,packages/,scripts/drops from 11 to 2 (0.78% → 0.17% duplicated lines), with 443 lines deleted against 199 added. No behavior is intended to change except one guard noted below.New shared modules:
src/platform/errors.js—errorCode,hasErrorCode,isMissingFile,isMissingPathSegment,isAlreadyExists,ignoreMissingFile. Replaces ~12 hand-rolled copies ofin
server/server.js,cli/stack-selection.js,platform/ownership.js,supervision/{command,launchd,manager,purge,systemd}.js,scripts/verify-release.js.src/platform/files.js—atomicWrite,readOptionalFile,fileExists(moved out ofsrc/supervision/files.js, which keeps only the executable promote/restore helpers) plusworkspacePathExists, which replaces the two copies inadministration/service.jsandstacks/administration-service.js. The desktop update adapter's privatewriteCachedStateis nowatomicWrite(path, json)instead of its own mkdir/tmp/rename dance.src/domain/semantic-version.js— onecompareSemanticVersions, replacing the copy insrc/supervision/version.js(deleted; its test moved totest/domain/semantic-version.test.js) and the copy inapps/desktop/main/update.js. Validation is now the sharedSEMANTIC_VERSION_PATTERNrather thanSemanticVersionSchema+ a looser regex; both patterns were already identical, so accepted input is unchanged.delay(ms)insrc/domain/time.js, replacing four inlinenew Promise((resolve) => setTimeout(resolve, ms)).Within-module dedupe:
src/docker/evidence.jsgainspublishedContainerPorts(component)(3 inline copies instacks/coordination-service.js), and that service gains#resolveDockerBindingandverifyDockerBindingso the confirm path and the reconcile path share the "resolve stack/component/endpoint definition, build expected labels, verify container" sequence while keeping their different failure shapes (throwRegistryErrorvs. return anunknownprovider status).storage/registry.js:#releaseActivationRuns(activationId, timestamp, failureReason)replaces the 29-line run/endpoint release block duplicated bymarkStackActivationLostandendStackActivation;#leaseIsConfirmable(activationId, leaseId, bindingKind?)replaces the two near-identical leased-endpoint lookups inconfirmLeaseandconfirmDockerLease.protocol/schemas.jsexportsCLAIM_RECORD_FIELDS, sodomain/schemas.js'sClaimRecordSchemaisz.object(CLAIM_RECORD_FIELDS).refine(...)instead of restating all ten fields.supervision/manager.jsgainsassertSupervisable(status, manualServerMessage)for the identical install/manual-server guards instart()andrestart()(each keeps its own manual-server message);supervision/purge.js'sinspectTreenow delegates toinspectTreeChildren;platform/ephemeral-ports.jsparsesstart endonce inparsePortRange.One deliberate behavior change: the shared
workspacePathExiststreats onlyENOENTas absence and reportstruefor any other error.administration/service.jsalready behaved that way;stacks/administration-service.jsused to rethrow, so an unreadable stack root now skips pruning instead of failing the prune operation.Not deduped:
apps/desktop/renderer/state.jskeeps its owncompareVersions, andapps/desktop/preload/index.cjsits own channel list — the renderer is served through theapp://protocol rooted atapps/desktop/renderer, and the preload script is sandboxed CJS, so neither can import shared ESM fromsrc/.Verification
bun run check(toolchain, typecheck, eslint, prettier, tests): the same 13 tests fail before and after this branch on this machine, alllsof-dependent integration tests plusrelease metadata > renders one checksum-pinned Homebrew formula for all targets.Link to Devin session: https://app.devin.ai/sessions/41eb2e735ff343c08b1406a042b909ca
Requested by: @TrentBrown