Bug fixes - #40
Merged
Merged
Conversation
The `secrets` and `secrets validate` commands each carried a verbatim ~40-line copy of the same discovery routine: build the provider set, scan monorepo-level config (env/vars/tasks/defaults/flavors), scan every component's config (tasks/resources), then aggregate by unique provider+path+key. Any change to what gets scanned had to be made twice and would silently drift. Extract it into collectAllSecrets(monorepo, secretProviders) alongside discoverSecrets/aggregateSecrets and call it from both commands. Each command keeps its own downstream mapping (SecretInfo table vs. per-secret validation); only the shared discovery+aggregation moves. Addresses REPORT.md finding #9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `images` and `images delete` commands each filtered an image's RepoTags with the same expression — `delete.ts` even carried a "De-duplicate this (also in images/index.ts)" TODO. The match was also fragile: `tag.indexOf(name) === 0` is an unanchored prefix test, so a project named `foo` would also capture a different project's `foobar/...` tags. Extract projectImageTags(repoTags, projectName) into src/docker/images/ and call it from both commands. It anchors on the `<project>/` boundary (startsWith(name + '/')) instead of a bare prefix, fixing the cross-project match while removing the duplication. Addresses REPORT.md finding #17. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`validateUserConfig` and `validateEmbfile` guarded the read with
`if (await stat(path))`. stat() resolves to a (truthy) Stats object or
rejects — it is never falsy — so the `else { throw 'Could not find
file' }` branch was dead code, and a missing file surfaced a raw
`ENOENT: no such file or directory, stat '...'` instead.
Wrap stat() in try/catch so the intended friendly message actually
fires when the file is absent.
Addresses REPORT.md finding #23.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The constructor runs top-level tasks through toIdentifedHash, which stamps each with synthesized `id`/`name` keys for runtime resolution. toJSON() returned those tasks verbatim, so the keys leaked into the serialized config. Since `emb config print` emits `toJSON()` output and TaskConfig has `additionalProperties: false`, the printed config failed its own re-validation with `unknown property 'id'`. Strip `id`/`name`/`component` from tasks at the serialization boundary, leaving `this.tasks` untouched for runtime consumers. `with()` still works — it re-runs toIdentifedHash on the cleaned tasks. Addresses REPORT.md finding #26. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
validateUserConfig and validateEmbfile carried copy-pasted load/parse
logic (stat check, readFile, redundant double .toString(), yaml.parse),
and the duplication had already let behavior drift: validateEmbfile
guarded against an empty document (`if (!component) return {}`) while
validateUserConfig did not, so an empty or comments-only .emb.yml failed
every command with the opaque Ajv message "/: must be object".
Extract a shared loadYamlDocument() helper (missing-file friendly error
+ read + parse) consumed by both validators, and have validateUserConfig
report an empty document as "Configuration file is empty: <file>".
Addresses REPORT.md finding #28.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
withFlavor() builds a fresh Monorepo, whose new TaskManagerFactory
defaults to the standard renderer. BaseCommand.init() calls
setTaskRenderer('verbose') on the base repo when --verbose/EMB_VERBOSE
is set, but FlavouredCommand.init then swaps in the withFlavor() result,
so `emb up --verbose --flavor production` silently rendered with the
default renderer instead of the verbose one.
Add TaskManagerFactory.getRenderer() and have withFlavor() copy the
base repo's renderer onto the flavored repo before returning.
Addresses REPORT.md finding #31.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mkdirp() normalized paths (normalize(join('/', path))) to keep writes
inside the store, but join() — used by writeFile/readFile/stat and the
read/write streams to build the actual target — passed the raw path to
node:path.join, which resolves '..'. So a '..' path escaped the
per-flavor sandbox: store.writeFile('../shared.txt', ...) landed in
<store>/ instead of <store>/<flavor>/, and deeper '../../' paths ENOENT
because mkdirp created the parent in the sanitized location while the
write targeted the unsanitized one.
Move the confinement into join() (the single choke point) and have
mkdirp() rely on it, so directory creation and the file operation always
agree and nothing can leave the flavor directory.
Addresses REPORT.md finding #32.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
No description provided.