Big 0.8.0 update - #15
Open
marcepartment wants to merge 40 commits into
Open
Conversation
Milestones 1 and 2 of the 0.8.0 plan (findings H1, H3, L9, M9). They share
utils/core.sh, so they land together.
Fix all 10 ShellCheck errors: quote the DOCKER_PEERED_SERVICES expansions,
exit 1 instead of the invalid exit -1, the SC2145 string/array mixes in
restore/restore-full/magepack, and the misplaced shebang in tableplus.help.
fixowns/fixperms only ever handled a single path: the emptiness test is now a
count test and each argument is passed through as its own array element.
${var^} is bash 4 and dies with "bad substitution" under the macOS system
bash, so the three registry sites now go through a capitalize helper.
Add .shellcheckrc documenting each accepted warning/note class, and widen the
CI glob and paths triggers to commands/**/*.cmd, commands/**/*.help and
.shellcheckrc, which the gate did not cover before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Finding H2, milestone 6. MariaDB 11 images dropped the mysql*/mysqldump compatibility symlinks, so roll db dump and roll db connect failed outright on any project pinned to mariadb:11.x. Probe the db container once per invocation and use whichever pair is present (mariadb/mariadb-dump preferred, mysql/mysqldump as fallback), rather than adding a symlink to the images repository, which this release does not touch. Both command substitutions on the failure path carry || true: bin/roll runs under set -e, so an unguarded assignment would kill the CLI before the fatal could name the missing binaries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Finding M12, milestone 9. WSL sets ROLL_ENV_SUBT=wsl, so appendEnvPartialIfExists never matched the .linux.yml fragments even though WSL2 is Linux and needs them. The path list becomes a directory x suffix loop, which reproduces the previous eight-path order exactly for linux and darwin. On wsl the suffix set is base, linux, wsl, with wsl last so a future wsl-specific fragment can override. No .wsl.yml exists today. ROLL_ENV_SUBT keeps wsl as a distinct value; config.sh still needs it to derive XDEBUG_CONNECT_BACK_HOST. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS L1, milestone 20. Without it, redirects in roll env exec php-fpm echo hi > /tmp/probe apply on the host, which people hit constantly. The command must arrive as one quoted argument. Extra words are refused rather than silently dropped, since running only the first of them is the same confusion this subcommand exists to remove. Dash-prefixed extras need their own check: env is in ROLL_CMD_ANYARGS, so roll's parse loop stops at the first dash and leaves it in "$@" instead of ROLL_PARAMS, from where it would have reached sh -c as $0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS M3, milestone 19. MySQL 8 dumps carry the ENCRYPTION table option and utf8mb4_0900_* collations, both of which MariaDB rejects; myloader reports this as an unhelpful "Trace/breakpoint trap (core dumped)". The opt-in flag rewrites only the *-schema*.sql files in the dump directory, through sed_inplace so BSD and GNU sed behave the same. Data files are never matched. Without the flag, a failed run's output is now inspected for the two signatures and names the cause plus the flag. --directory is a container path while normalization runs on the host, so the /var/www/html mount prefix is translated and a path outside it is refused rather than silently concatenated into a directory that does not exist. nullglob is saved and restored around the glob, verified in both directions under bash 3.2.57. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Neither is in 0.8.0's scope, and both would otherwise be lost: the magepack one is now actively hidden, because the .shellcheckrc added in milestone 1 disables SC2317 repo-wide, which is the check that was reporting it. L1 is the same credential-in-ps exposure as RECLU H2, which milestone 24 fixes for TablePlus. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deletes H1, H2, H3, M9, M12 and L9 from Known issues, along with their Troubleshooting rows and the cross-references naming them from the platform table, the M10 finding and Audited and clean. Deleted rather than marked done, per the repo's own convention; git history is the record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Finding M11, milestone 8. isOnline used ping -t, where -t is a timeout on BSD and the TTL on Linux, so the probe misbehaved on Linux and roll svc up never refreshed images there. ICMP is also commonly blocked. Probe https://ghcr.io/v2/ instead, which is the registry svc pull actually needs. Deliberately no -f: that endpoint answers 401 unauthenticated, and any HTTP response proves reachability. Verified true on a working network and false in 3s against an unroutable address. The Linux leg is unverified from here, but the flags used behave identically on both platforms, which is the point of the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Finding M10, milestone 3. CI was Ubuntu-only, so every macOS-specific defect
was invisible by construction: the bash-4 ${var^} bug fixed earlier in this
release linted clean and ran fine on Ubuntu while hard-failing under the macOS
system bash that most users have.
Extend the existing workflow into a ubuntu-latest/macos-latest matrix rather
than adding a second workflow, which would have run the same lint twice per
push. The macOS leg invokes /bin/bash bin/roll so bash 3.2 is exercised.
The smoke script needs the docker CLI even though it never talks to a daemon:
bin/roll fatals for every command, version included, when docker is absent or
docker compose version does not parse. macOS runners also lack envsubst, which
env-init needs. Both are installed on that leg.
The workflow itself cannot be verified until pushed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings M1 and M2, milestone 4, decision D4a.
setConfigDefault ran before postProcessConfig and skips any key already in the
cache, so every schema key with a literal default was set before the env-type
derivation ran. That made each ${VAR:-x} in postProcessConfig dead, which is
why a magento2 project resolved ROLL_VARNISH=0 despite the code reading
${ROLL_VARNISH:-1}. Verified against a bare magento2 project before the change.
The order is now: env-type defaults, then schema literals, then version-pin
handling, then derivations. applyEnvTypeDefaults never overwrites a key that
came from a config file, so an explicit value still wins.
commands/env.cmd recomputed all of this a second time, which is what made real
projects work and what kept the loader's own bug invisible. Those 82 lines are
gone; config.sh is the sole owner. It now also exports NGINX_TEMPLATE,
NGINX_PUBLIC and CHOWN_DIR_LIST unconditionally, as env.cmd used to.
Service versions become string:optional and an unpinned enabled service warns,
falling back to the version the project was already running. Per D4a this is a
warning in 0.8.0 and an error in 0.9.0: measured against the machine's 48 local
projects, 34 had no NGINX_VERSION, so a hard error would have broken most of
them and the nightly build server on upgrade day. The fallback is deliberately
the old schema literal and not the env type's init.env value, which would have
jumped existing projects from Elasticsearch 7.17 to 8.11.
roll config fix-pins writes the missing pins after taking a backup;
roll config check-pins is the read-only form and exits 1 when any are missing.
Neither takes a --dry-run flag, because config is not on ROLL_CMD_ANYARGS and
adding it would make roll config --help recurse.
Two further defects found while doing this: the mongodb fragment interpolated
MONGODB_VERSION, which the schema never defined, so that image tag always
resolved to the fragment's own literal - the fragment now reads MONGO_VERSION
and the legacy fallback is 7, matching what actually ran. laravel/init.env
pinned the same misspelled key. Every init.env gained the NGINX_VERSION pin no
type had ever set, and local and vuejs gained init.env files so env-init
produces valid projects for them.
Verified: rendered compose config is byte-identical to 0.7.2 for the live
magento2 project and for seeded fixtures of all ten renderable environment
types, both before and after fix-pins. The local type fails to render, before
and after alike - pre-existing, an appended fragment depends on php-fpm, which
local never gets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings M3, L2, L3, L4, L6, L7, L8 and L10, milestone 5. M3: the network-existence filter was single-quoted, so the command substitution inside it never ran and the filter never matched, making env up do a redundant compose pass every time. Now anchored and double-quoted. L2: six eval "$(cat ~/.roll/.env ...)" sites replaced with the schema-validated loadConfigFromFile. In shell.cmd and rootshell.cmd the pre-load is deleted outright rather than swapped: it ran immediately before loadEnvConfig, which reads that same file through the same path, so it was duplicate work. L3: the Portainer and Startpage defaults are read through getConfig instead of literals that disagreed with the schema (0 against the schema's 1). L4: the three box functions collapse into one parameterised box; the old names stay as wrappers, since RECLU calls them too. The interior colours were 7/2/1, not the 3/2/1 the finding claimed. L6: the Traefik docker.sock mount is read-only. L7: the ssh-agent fixup is guarded on SSH_AUTH_SOCK and narrowed from chmod 777 to chown www-data plus chmod 600. L8: dead _static/_redirects paths dropped from the docs config. L10: an unsupported Linux distro now warns with the CA path instead of silently skipping the trust-store step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings M7 and M8, milestone 7. M7: ROLL_ENV_NAME becomes the Compose project name, so an invalid one was accepted at creation and only failed later, confusingly. env-init now re-prompts on a TTY and fatals otherwise, and validateConfigValue rejects it with the naming rule quoted and the volume-orphan consequence of a rename spelled out. M8: describe.cmd and vnc.cmd built container names by string concatenation, which breaks whenever Compose names differently. Both now resolve through Compose. describe asks docker once for every container in the project, keyed by service label, rather than resolving each service separately: a per-service lookup is a full roll re-entry, and measured 9.8s against 1.3s for the whole command before. The single query brings it to 1.0s. Verified statuses match the previous implementation, and that a stopped container still reads stopped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings L1, L5 and LB2, milestone 10, decision D7. usage.help was missing eight commands; every rolldev-owned command except usage itself is now listed, including this release's roll env sh and roll config check-pins/fix-pins. L5 was already resolved by milestone 4, which added the vuejs and local init.env files. Per D7 the local type stays and is documented rather than removed. The documentation is honest about it not being inert: env.cmd only skips the php-fpm partial for that type, so nginx/db/redis still come up while their toggles default to 1, and a bare local project therefore fails to render at all. That failure predates this release, so it is recorded in FEATURE-REQUESTS.md rather than fixed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deletes M3, M7, M8, L1, L2, L3, L4, L5, L6, L7, L8, L10 and LB2, along with their Troubleshooting rows and the cross-references naming them from the platform table, the coupling tables and Audited and clean. Keeps the Troubleshooting row about renaming an environment orphaning its volumes: that behaviour is unchanged and still worth documenting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 11, decisions D3, D8 and D9. Every prompt added from here follows one rule: a value from a flag or environment variable wins, gum runs only when there is a terminal, and anything else is a hard error naming the flag that would have supplied the value. That rule exists because gum has no non-interactive mode. gum choose and gum input open /dev/tty directly and exit 1 with no terminal, and piping into them supplies options rather than an answer, so a prompt must never be the only route to a value. This is what makes roll drivable from a script or CI. assertGum is called lazily by the wrappers rather than at startup, so a non-interactive run of a command that happens not to prompt still works with no gum installed. It enforces gum 0.14.0, the first release with the uniform 0/1/130 exit-code contract, and prints a per-platform install hint. The plain helpers in core.sh stay gum-free: they must work in pipes and CI. Boxes render through gum style only when interactive and fall back to the existing ASCII renderer otherwise. test-interact.sh asserts the non-interactive half of the contract - 17 assertions covering preset short-circuiting with gum removed from PATH entirely, each prompt failing with its flag named, the box fallback and set -e safety - and runs as part of the smoke suite on both CI platforms. The interactive half cannot be tested without a TTY; every gum flag used here was instead checked against the installed gum's own help. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 12, decision D3, rolldev side. roll install now reports gum's state through checkGum. It warns rather than failing: installing roll on a machine that only ever drives it with flags must not be blocked, and the prompts check for gum themselves when reached. The per-platform install hint moves into gumInstallHint so assertGum and checkGum share one copy, and gumVersion is factored out alongside it. README and docs/installing.md list gum as a prerequisite and are explicit that it is needed only for prompts, since every prompt is also reachable by flag, environment variable or positional argument. Still outstanding for this milestone: `depends_on "gum"` in Formula/roll.rb.template, which lives in the separate epartment/homebrew-roll repository and is not part of this checkout. Per the plan it merges with the 0.8.0 release, so it is tracked against milestone 32. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 13. env-init's overwrite, name and type prompts become promptConfirm, promptInput and promptChoose over fetchValidEnvTypes; backup, restore, restore-full and duplicate each had their own local promptPassword shadowing the shared one, and now use it. Positional arguments stay the non-interactive path for env-init, milestone 7's name validation is intact, and backup's existing --encrypt= flag path is unchanged - it was the model for the rest. Overwrite is gated behind ROLL_ENV_INIT_FORCE=1 rather than a --force flag: env-init is not on ROLL_CMD_ANYARGS, so bin/roll rejects any dash-prefixed argument before the command file is sourced, and joining that list to carry one boolean would mean hand-rolling --help handling too. The four local promptPassword copies set a "PROMPT" sentinel, which the shared implementation would have read as an already-supplied value, so each call site clears it first. Quiet-mode hard errors still run ahead of the prompt. No raw read -p or read -s remains outside interact.sh. Interactive paths cannot be exercised without a TTY and are unverified: the env-init name/type/overwrite prompts, the invalid-name re-prompt loop, and the password prompts including the mismatch path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both found while converting prompts, both predating this release, both invisible to CI because it only ran Ubuntu. roll db/env/redis/svc/restore/restore-full/duplicate --help re-invoked roll <self> --help. Those commands are on ROLL_CMD_ANYARGS, so roll's parser stops at --help and hands it to the command with ROLL_PARAMS empty, landing straight back on the same branch: measured at 8 processes within 3 seconds before being killed. backup.cmd already carried a comment warning about exactly this and did the right thing; the others now do too. config and registry were never affected - they are not on that list, so bin/roll renders their help. redis additionally keyed its help branch on ROLL_PARAMS[0] == "help", which --help never satisfies, so its help was unreachable rather than recursive. It cannot use the empty-params form the others use, because bare roll redis opens a session. backup.help and status.help failed to parse under bash 3.2, so roll backup --help and roll status --help exited 2 on macOS - every user's primary platform - while parsing cleanly under bash 5. bash 3.2 mis-handles an apostrophe inside a heredoc nested in $( ), even with a quoted delimiter. The text is reworded to avoid lone apostrophes rather than escaped, which would have shown in the rendered output. test-syntax.sh guards both classes and runs in the smoke suite, so the macOS leg exercises the bash 3.2 parse that Ubuntu cannot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS H1, milestone 16. --wait itself needed no code change: env is already on ROLL_CMD_ANYARGS, so the flag reaches docker compose up on its own. The healthchecks are what make it meaningful. Every probe was executed inside the real running container before being committed, which is how the two surprises turned up. varnish ships neither curl nor bash, so it uses varnishadm ping. nginx cannot require a 200: a project with no code deployed legitimately serves 403, so the probe accepts 2xx/3xx/4xx and fails only on a connection error or 5xx. Elasticsearch and OpenSearch probe /_cluster/health rather than a port. Their containers run a fixed heap and have historically looked healthy while actually dead, which a TCP probe cannot detect. The db probe tries mariadb-admin then mysqladmin, because MariaDB 11 ships only the former and the mysql images only the latter. It is deliberately unauthenticated: verified on both engines that ping exits 0 with no credentials and with wrong ones alike, so passing the root password would add nothing but a credential in argv on every interval - which mysqladmin itself warns about. The rendered compose for a real project is unchanged apart from the healthcheck blocks: 54 lines added, none removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS M4, milestone 14. Stable keys, no ANSI, and no credentials: describe prints a database hint in its human output, and that path is not reachable from the JSON one. Values go through a new jsonEscape in utils/core.sh rather than being concatenated raw, so a project directory containing a quote, backslash, tab, newline or non-ASCII still produces parseable output. Verified against exactly such a value. status and describe had to join ROLL_CMD_ANYARGS for --format to reach them at all, which changes how --help behaves for both: roll stops parsing at the first dash and no longer renders help itself. Both therefore source usage.cmd rather than re-invoking themselves, the mistake that made seven other commands fork until killed. test-syntax.sh covers them. Also replaces status.cmd's scrape of the compose YAML for the network name with the literal constant that file actually defines. Human output is byte-identical to before for status, describe and env describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS H2 and M1, milestone 18. Both exist for running many environments unattended on one host. ROLL_BROWSERSYNC=1 publishes host ports on php-fpm, which is a known cause of "port is already allocated" that no .roll/roll-env.yml override can fix. ROLL_PUBLISH_PORTS=0 now selects a browsersync.noports variant that keeps the environment variables and drops the ports block. A variant fragment rather than a conditional, because Compose YAML cannot drop a ports block by interpolation, and because env.cmd already selects partials by name this way for tests and magepack. ELASTICSEARCH_JAVA_OPTS and OPENSEARCH_JAVA_OPTS replace the hardcoded 512MB heap, defaulting to the same value. Registered in the schema rather than left as a YAML fallback, which would be dead code since the exported schema value wins. Rendered compose for a real project that sets none of these is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Boundary finding MB2 and M6, FEATURE-REQUESTS M5, milestone 15, decision D6 (implement the metadata layer rather than delete it - the machine-readable interface needs real descriptions). .help files now carry ## @description: and ## @category: headers, added to all 51 built-in files across seven categories. extractCommandMetadata parses them with one grep per file and only when metadata is requested: it is never reached from initializeRegistry or scanCommandDirectory, which run on every dispatch. roll version measures 0.25s, unchanged. RECLU help files predate the convention and have no headers; they degrade to an empty description and the general category rather than erroring. Nine commands are currently in that state, all of them from that repository. registry list --format json emits {command, category, description, priority, source}, escaped through the shared jsonEscape. Source is now a separate field from category, which the old code conflated. has-command exits 0 or 1 with no output, and resolves through the registry, so it works for RECLU commands too. registry joined ROLL_CMD_ANYARGS for --format, so its help branch was rewritten to source usage.cmd - it was one of the commands that recursed until killed. One real bug fixed in passing: a function ending on a while-read loop whose last guard evaluates false returns that nonzero status, which under set -e kills the CLI at a bare call site. An explicit return 0 closes it, documented inline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FEATURE-REQUESTS M2, milestone 17. Seventeen checks on a magento2 project: config loads, Docker reachable, per-container state, the shared traefik and dnsmasq ports, search-engine health, and disk headroom on the Docker data root. Container state reuses milestone 16's healthchecks rather than re-probing, and reads them from a single docker ps keyed by the compose service label - the per-service lookup pattern costs a full CLI re-entry each and was already reverted once from describe. A container started before those healthcheck definitions existed reports no health at all, so that is a distinct running-but-unverified state rather than a failure; every container on a project that has not been recreated is in it. The search engine gets two checks, because a cluster can report non-red and still refuse writes: the second one PUTs a throwaway roll-doctor-probe index and deletes it again. Verified against a real project that the index is gone afterwards and the existing indices are untouched. Disk headroom is probed from inside a running container, since the Docker data root is not reachable from the host under Docker Desktop or OrbStack. Exit 0 when everything passes, 1 when anything fails, in both output formats. Not exercised: the elasticsearch branch (the verification project runs OpenSearch), browsersync ports, and the disk-low and daemon-down paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 21. A user-facing guide at docs/machine-interface.md plus a contributor summary in the README: the JSON surfaces, has-command, env up --wait, the flag-first prompt convention with a table of every non-interactive form, the exit-code contract, and the two settings for running many environments on one host. Every example was run against a real project rather than written from the source. That caught two errors in my own draft: doctor returns an object with ok and checks rather than a bare array, so the jq example was wrong, and status nests its fields under projects[] while describe returns them directly - the draft had claimed the two shapes were interchangeable. The README section also records the rule that giving a command a flag means adding it to ROLL_CMD_ANYARGS, which changes how --help reaches it, since that is the trap that produced seven fork bombs in this release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 22, including RECLU finding H1. Both are general-purpose commands with nothing Epartment-specific in them, so they belong in the public CLI. H1: copytocontainer's --all was dead code. The flag check sat after the ROLL_PARAMS/help check, and since the command is on ROLL_CMD_ANYARGS a leading dash leaves ROLL_PARAMS empty, so the branch was unreachable. It is now checked first, mirroring copyfromcontainer. Also in transit: the bare docker compose ps becomes roll env ps; copyfromcontainer gains the loadEnvConfig and assertDockerRunning it was missing; and the --cachegrind/--traces pickers go through promptChoose, so without a terminal they name the filename argument instead of hanging. The RECLU originals also re-invoked their own --help, which forks until killed for an ANYARGS command. The ported copies source usage.cmd instead. The RECLU originals are deliberately left in place this phase and still shadow these at registry priority 3, so the fix does not reach users until the cut-over. Verified against a throwaway environment, not the production-data project: --all now copies and runs fixowns and fixperms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 23, including RECLU findings M2 and M3. dialog was the last non-gum UI
tool in the stack; the theme and action pickers now use promptChoose, and no
dialog reference remains anywhere in rolldev.
M3: theme discovery is rooted at ${ROLL_ENV_PATH}/app/design/frontend rather
than a path relative to the working directory, so it finds the same themes from
anywhere in the project.
M2: boilerplate detection iterates every discovered theme. The original echoed
the array, which only ever examined the first element - verified against a
fixture whose boilerplate theme is not first, where the old form misses it.
Non-interactive forms: roll theme all, and roll theme <vendor/name> build|watch.
Without a terminal and without them, it names the positional form rather than
hanging.
The build and watch invocations themselves are an unexecuted port: no theme with
a Gulpfile.js exists on this machine to build against, and the production-data
project must not be built into. The discovery, selection and dispatch logic that
M2 and M3 touch is verified against fixtures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 25, including RECLU finding L4. Converts a Warden project to RollDev. L4: the original rewrote ELASTICSEARCH_VERSION to a hardcoded, by now outdated value. That substitution is gone, so whatever the project pins survives the conversion. Also: sed_inplace replaces the sed -i.warden calls, so no backup files are left behind; warden env down is guarded on the binary existing, since a machine being converted away from Warden may not have it; and the result is validated with roll config validate before anything is started. The previously silent already-converted and not-a-Warden-project paths now say so. Verified against a fixture: .env.roll and .roll/roll-env.yml are produced, no backup files remain, the distinctive ELASTICSEARCH_VERSION is unchanged, and config validate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Findings MB1 and M4, milestone 27. Eleven of the sixteen functions restore and restore-full defined were byte-identical, and three of the four commands defined their own logMessage. utils/backup.sh is now the single definition. Four more functions differed only cosmetically or by one guard and are unified on the richer version; stopEnvironment gains restore-full's missing-config guard, which is what let it be shared at all. The staging directory gets one resolver. All four commands keyed it on $(pwd) rather than ROLL_ENV_PATH, and backup.cmd carried a comment warning that changing it in one place would make backups land where restore does not look. That contract is preserved exactly and now lives in one place so it cannot drift. restore-full is why it must stay cwd-keyed: it has no configured environment, so it cds into its target first and the resolver answers relative to that. restore-full becomes restore --include-source, with the old command kept as a ten-line alias since its name is the documented interface. Its unique logic - extracting from an archive file rather than a backup id, and restoring source code - moves into restore.cmd behind the flag, which has to be detected before the preamble because a full restore has no .env.roll to load. restore 962 -> 615, restore-full 1125 -> 10, against 755 shared lines. Also fixes a pre-existing errexit trap surfaced by the round trip: findLatestBackup returns 1 when no backup exists, and the unguarded command substitution killed the CLI on the ERR trap instead of printing "No backups found". Verified the same failure on the pre-refactor code. Verified by a real round trip on a throwaway environment: marker row, backup, drop, restore, marker back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found by the milestone 27 round trip. Elasticsearch exits 1 after every restore because the volume root comes back owned by uid 0 while the image runs as 1000. Pre-existing and reproducible on the pre-refactor code, so it is recorded rather than fixed inside a refactor. Worth noting how it surfaced: restore reports success and, before this release, env up returned 0 as well, so the environment looked healthy. It is milestone 16's healthchecks and env up --wait that expose it - --wait correctly exits 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Boundary finding LB1, milestone 28. The command was a long linear script with no way to resume after a failure part-way through. Its phases move into utils/magento2-init.sh as m2init* functions and the command file becomes orchestration, 784 lines down to 42. Sourced from the command rather than bin/roll, matching install.cmd and svc.cmd: bin/roll sources every top-level util on every invocation, and this one is needed by a single command. Every phase function ends with an explicit return 0, since several end on an if or while whose status would otherwise become the function's own and kill the CLI under set -e. Behaviour-preserving. A structural diff of the original linear body against the extracted functions caught one real reordering mistake - version resolution had moved ahead of directory creation - and shows no command-sequence differences after the fix. A live run reached the composer create-project step and stopped there on a pre-existing security-advisory block that the original hits identically, so phases 9 to 12 are covered by the structural check rather than by execution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… of ps Milestone 24, rolldev side, including RECLU finding H2. The connection URI carries the database password, and open "$uri" -a TablePlus makes it a process argument, so anything able to read ps could see it. The internal command pack was worse: its staging branch wrapped that in gum spin -- open, putting the password in two argument lists at once, and the production URI holds real live credentials. openInTablePlus in utils/core.sh pipes an AppleScript open location into osascript -, which reads its script from stdin, so only "osascript -" appears in the process list. It falls back to open on failure and warns when it does, rather than exposing the password silently. Verified end to end on a machine with TablePlus installed: sampling ps eight times through a real launch never shows the URI, and the connection opens without the fallback warning firing. The AppleScript string escaping was checked against a URI containing both a double quote and a backslash. The builtin also absorbs the discovery the command pack had and this did not - Setapp installs TablePlus somewhere else entirely - and gains a macOS-only guard that points Linux users at roll db connect instead of failing obscurely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Milestone 32, documentation half. MB1 and M4 were fixed by the backup shared library but I left them in the README; M5's marker is present and the finding itself said so. The boundary-findings section is now empty, and the environment table reflects vuejs and local having init.env files. The only finding still open is H1, the restored Elasticsearch volume, which is recorded deliberately rather than fixed inside a refactor. Release notes are a draft file rather than a changelog entry, because docs/changelog.md points at the GitHub releases page instead of carrying one. They lead with the two upgrade notes that matter - version pins warning now and erroring in 0.9.0, and gum becoming a dependency - and carry the known issue. Tagging and publishing are not done here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Covers all 45 tags from 0.1.0-beta1 in July 2022 to 0.7.2, plus an Unreleased section for the 0.8.0 work. Each version quotes the notes attached to its GitHub release where there are any - 19 of the 45 tags have a release, and 12 of those carry notes - and lists the commits in that range beneath. Everything before 0.3.0 predates the releases and is reconstructed from commits alone; those sections are marked as tag-only so it is clear the summary was not written by a human at the time. Filtered out: merge commits, the automated Tagged <version> commits, version bumps, documentation-sync commits, and any bullet that only restates the release note above it. Two tags have nothing left after that and say so rather than appearing empty. The Unreleased section is written rather than generated, since raw commit subjects for in-progress work read as internal bookkeeping. It groups the changes as fixed/added and carries the one known issue. docs/changelog.md now includes the file instead of linking out to the releases page, so the published documentation carries the history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The curated summary was too thin: it named about two thirds of what changed and omitted several things a reader would want, including two security fixes. Now grouped as Added / Changed / Fixed / Security / Internal, with 43 entries against the 28 substantive commits on the branch, and audited so every one of those commits is represented. Previously missing: myloader --normalize-source-ddl, ROLL_PUBLISH_PORTS and the search-engine heap options, ROLL_ENV_INIT_FORCE, the registry descriptions, the env-name validation, the redundant compose pass on every env up, the Portainer and Startpage defaults, the unsupported-distro CA warning, vuejs and local gaining init.env, and under Security the TablePlus password no longer reaching ps, the ssh-agent socket going from chmod 777 to 600, and the six eval sites replaced with schema-validated loading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 141 out of 142 changed files in this pull request and generated 9 comments.
Suppressed comments (2)
commands/convert.cmd:42
- This global lowercase replacement rewrites arbitrary YAML values, not just Warden identifiers—for example image names, bind-mount paths, labels, and command strings containing
warden. That can silently corrupt an otherwise valid custom compose file. Restrict conversion to the known variable references and Warden-specific keys that actually require renaming.
sed_inplace "s/WARDEN/ROLL/g;s/warden/roll/g" "${CURRENT_DIR}/.roll/roll-env.yml"
commands/convert.cmd:63
- Conversion silently removes the existing
<env>_appdatavolume before starting RollDev. On macOS this is the named/var/www/htmlvolume and can contain project files that are not present on the host, so this operation can cause irreversible data loss despite not being mentioned in the command contract. Preserve/reuse the volume, or require an explicit confirmed migration step with a backup.
if [[ -n "$(docker volume ls -q --filter "name=^${DATA_VOLUME}\$")" ]]; then
docker volume rm -- "${DATA_VOLUME}"
| reachable and writable, disk headroom on the Docker data root. See \033[31m'roll doctor -h'\033[0m for the | ||
| full check list and \033[31m'--format json'\033[0m. | ||
|
|
||
| roll env up --wait |
Comment on lines
+269
to
+279
| local probeIndex="roll-doctor-probe" | ||
| local writeCode="" | ||
| writeCode="$(curl -sk -m 5 -o /dev/null -w '%{http_code}' -X PUT "${baseUrl}/${probeIndex}" \ | ||
| -H 'Content-Type: application/json' -d '{}' 2>/dev/null)" || true | ||
| curl -sk -m 5 -o /dev/null -X DELETE "${baseUrl}/${probeIndex}" 2>/dev/null || true | ||
|
|
||
| if [[ "${writeCode}" == "200" || "${writeCode}" == "201" ]]; then | ||
| recordCheck "search-engine-write:${engine}" 1 "${engineLabel} accepted a throwaway index write at ${baseUrl}/${probeIndex}." | ||
| else | ||
| recordCheck "search-engine-write:${engine}" 0 "${engineLabel} rejected a throwaway index write at ${baseUrl}/${probeIndex} (HTTP ${writeCode:-no response})." | ||
| fi |
Comment on lines
+31
to
+35
| if [[ -f "${FILE_OR_FOLDER}" ]]; then | ||
| docker cp "${ROLL_ENV_PATH}/${FILE_OR_FOLDER}" "$(roll env ps -q php-fpm)":/var/www/html/"${FILE_OR_FOLDER}" | ||
| else | ||
| docker cp "${ROLL_ENV_PATH}/$(dirname -- "${FILE_OR_FOLDER}")" "$(roll env ps -q php-fpm)":/var/www/html/"${FILE_OR_FOLDER}" | ||
| fi |
Comment on lines
+61
to
+66
| if [[ -f "${REALPATH_FILE}" ]]; then | ||
| docker cp "$(roll env ps -q php-fpm)":"${REALPATH_FILE}" "${ROLL_ENV_PATH}/tmp/${REALPATH_FILE}" | ||
| else | ||
| docker cp "$(roll env ps -q php-fpm)":"${REALPATH_FILE}" "${ROLL_ENV_PATH}/tmp/$(dirname -- "${REALPATH_FILE}")" | ||
| fi | ||
| success "Completed copying ${REALPATH_FILE} from container to host tmp folder: ${ROLL_ENV_PATH}/tmp/${REALPATH_FILE}" |
Comment on lines
+35
to
+38
| if (( ${#BOILERPLATE_THEME[@]} > 0 )); then | ||
| YARN_INSTEAD_OF_GULP=1 | ||
| else | ||
| YARN_INSTEAD_OF_GULP=${ROLL_YARN_INSTEAD_OF_GULP:-0} |
Comment on lines
+539
to
+543
| eval "toggle_value=\${${toggle}:-0}" | ||
| eval "version_value=\${${version_key}:-}" | ||
|
|
||
| if [[ "${toggle_value}" == "1" && -z "${version_value}" ]]; then | ||
| ROLL_MISSING_PINS+=("${version_key}") |
Comment on lines
+32
to
+35
| if grep -q 'ROLL_NO_STATIC_CACHING' "${CURRENT_DIR}/.env"; then | ||
| sed_inplace "s/.*ROLL_NO_STATIC_CACHING.*$/ROLL_NO_STATIC_CACHING=1/g" "${CURRENT_DIR}/.env" | ||
| else | ||
| echo "ROLL_NO_STATIC_CACHING=1" >> "${CURRENT_DIR}/.env" |
| - 'commands/**/*.help' | ||
| - 'utils/*.sh' | ||
| - 'environments/**' | ||
| - '.github/scripts/smoke.sh' |
| ROLL_USAGE=$(cat <<EOF | ||
| \033[33mUsage:\033[0m | ||
| tableplus Launches tableplus with current database | ||
| tableplus Open the current project database in TablePlus (macOS only) Launches tableplus with current database |
Copilot reviewed PR #15 and raised nine findings. Eight were real and are fixed here; the ninth is rejected below. copytocontainer applied dirname to the SOURCE rather than the destination, so `copytocontainer vendor` ran `docker cp <project>/. <container>:/var/www/html/vendor` - the entire project root into the wrong path, followed by fixowns and fixperms over it. dirname now names the destination parent, which one expression handles for both a file and a directory, and that parent is created inside the container first because docker cp will not create it. copyfromcontainer's non-flag branch already had this right; it is the mirror of that. copyfromcontainer --realpath tested a CONTAINER path against the host filesystem, then built destinations like <project>/tmp//tmp/file whose parent was never created, so the normal case - a file that only exists in the container - always failed. Both branches were wrong, the -f one too for any nested path. Everything now lands flat in the project tmp folder, as --cachegrind and --traces already do, and the success line names where the file actually went. convert wrote ROLL_NO_STATIC_CACHING, a key no schema defines, so it only ever produced an unknown-key warning and had no effect. The supported key is positive-form, so the value has to be inverted rather than renamed: writing ROLL_MAGENTO_STATIC_CACHING=1 for a legacy NO_STATIC_CACHING=1 would have moved converted projects onto the production nginx template. Nothing is written when the legacy key is absent, since the schema default already means what the old forced value meant. utils/backup.sh carried the same rewrite in its legacy migration path and is fixed with it - Copilot did not flag that copy. Version-pin detection judged a pin on the resolved value, but global config is loaded into the same cache first, so a version in ~/.roll/.env made check-pins report success while the project file stayed unpinned and a colleague without that line resolved a different image - the exact failure the pin mechanism exists to prevent. loadConfigFromFile now records which file each key came from, pins are judged on the project's own keys, and the recommended value is the one being inherited rather than the legacy literal, so fix-pins still never changes which image comes up. DB_DISTRIBUTION_VERSION honours the older per-distribution spellings in both directions. Verified end to end: before, check-pins reported "Every enabled service already has its version pinned" with no PHP_VERSION line in the project file; after, it reports it missing and fix-pins writes the inherited 8.3, not the built-in 8.1. roll theme decided Gulp-vs-Yarn once for the whole project from whether any discovered theme was a boilerplate one, so a project holding both kinds ran `yarn run dev` against themes that only have a Gulp build. The choice is now made per theme, with ROLL_YARN_INSTEAD_OF_GULP still overriding in both directions. That override and ROLL_NODE_PACKAGE_MANAGER were registered in no schema, so setting either in .env.roll warned about an unknown key; both are now registered as optional, which keeps "unset means decide per theme" alive where a 0/1 default would not have. tableplus.help had two descriptions concatenated on the usage line, a regression from giving the command a @description header without removing the old text. doctor's search-engine write probe used a fixed index name and deleted it unconditionally, so a leftover from an interrupted run made the PUT fail as resource_already_exists and the check report a false negative. The name now carries the invocation's PID and the delete only runs when the write succeeded. The ShellCheck workflow ran test-syntax.sh and test-interact.sh through smoke.sh but listed neither in its path filters, so a PR touching only those skipped the workflow, and the lint step did not cover them either. Both filters and the lint scope now take .github/scripts. Rejected: the claim that `env up --wait` is unavailable across the supported Compose range because --wait needs 2.20.0. It was introduced in 2.1.1 (docker/compose#8777) and only --wait-timeout came later, in 2.17.0, so DOCKER_COMPOSE_REQUIRE=2.2.3 already covers it. No change made. Verified with the CI ShellCheck command (clean before and after), the smoke suite in both native and bash32 modes, the pin logic across four config scenarios plus the errexit path through bin/roll with every pin absent, and the convert translation across all four legacy values on bash 3.2.57. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The changelog and release notes described the branch as it stood before the review fixes, and two passages in CLAUDE.md had gone stale as the branch closed the findings they pointed at. Changelog: the moved-in-commands entry named one bug per command where there were several, and now lists the destination-path bugs in copytocontainer and copyfromcontainer, the per-theme build choice, and the static-caching key translation. The pin entry gained the global-config rule - a version in ~/.roll/.env counts as unpinned, and fix-pins writes the inherited value - since anyone keeping versions in global config will start seeing warnings they did not see before. The restore-full line said it "becomes" restore --include-source, which reads as a removal; the old name is kept as an alias and now says so. The ShellCheck line covers utils/ and the CI scripts, which is what the gate actually lints. Release notes carry the same two corrections, with the global-config rule under the upgrade note where it belongs rather than in a later section. CLAUDE.md said ShellCheck already fails on a clean checkout and does not cover commands/magento2/, commands/wordpress/ or the .help files, citing findings H1 and L9. Both statements are now wrong - the gate is green and covers all of those - and neither finding number means that any more: H1 is the restored search volume and there is no L9. A future session reading "a red run is not evidence that your change broke something" would have ignored real findings, so the bullet now states that any finding is the author's, gives the real command, and documents the smoke suite, which was missing from the verification section entirely. The known-defects bullet enumerated five defects fixed for 0.8.0 as though they still reproduce; it now points at the one open finding and sends the reader to the changelog for the rest. README's Medium and Low finding sections were left as bare headings when the last entries were closed, and now say so explicitly. FEATURE-REQUESTS gains L4: roll config schema renders seven hand-maintained category filters with no catch-all, so a registered key matching none of them is invisible - ROLL_ADMIN_AUTOLOGIN and ROLL_MAGENTO_STATIC_CACHING are hidden today. Recorded rather than fixed, since deriving the category from the key is a refactor of that display and not part of the review response. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It had been red for most of the 0.8.0 cycle - the same failure is on f38f8e4 and earlier - while the macOS leg was green, so nothing in the branch caught it. Two independent causes, both platform-dependent in a way a local macOS run cannot see. ShellCheck version skew. Ubuntu 24.04 installs 0.9.0 from apt; Homebrew ships 0.11.0. The older version raises four findings the newer one no longer does: SC2002 on a `cat file | jq` in backup.cmd, SC2015 twice on `lsof … && occupied=1 || true` in doctor.cmd, and SC2236 on `[[ ! -z … ]]` in env-init.cmd. Fixed in code rather than added to .shellcheckrc - the jq call reads the file directly, the lsof probes became plain `if` blocks, which is clearer and errexit-safe anyway, and the test is now `[[ -n "…" ]]` with the expansion quoted. `box` aborting under `set -u` on bash 5. It declared `local s=("$@") b w use_tput=0`, leaving b and w unset, then compared `((w < ${#l}))`. bash 4.4 and later apply set -u inside arithmetic contexts and abort; bash 3.2 substitutes 0. The prompt harness runs with `set -u`, so on Linux styledBox produced nothing and both the plain-box fallback test and the survives-set-e test failed, while macOS passed all 17. Initialising b and w fixes both. `roll` itself never sets -u, so no shipped command was affected. Verified by reproducing the Ubuntu job in an ubuntu:24.04 container - bash 5.2.21 and ShellCheck 0.9.0 - which now passes both steps, with the macOS combination (bash 3.2.57, ShellCheck 0.11.0) still green. The container recipe is recorded in CLAUDE.md along with the version-skew warning, since a clean local run on macOS demonstrably says nothing about the gate. Co-Authored-By: Claude Opus 5 <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
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.