Repository navigation
e2e: fall back to legacy zip archive on darwin in releaseBinary - #51
Merged
Merged
Conversation
The working plan for the command grammar, direct client launch and serve. Decisions live in ADRs 0007 and 0008; this file is the step list and the watch items, so it sits in plans/ rather than docs/.
The TUI is the only interface today, so nothing about an aperture setup composes with a shell, and the team runs ts-unplug as a second fixed-port sidecar. 0007 takes cobra over hand-rolled dispatch and rejects Viper; 0008 puts the fixed port on the Bridge record and keeps serve a foreground process, since a daemon is ADR 0006's rejected shape.
The command line becomes the API contract in connection-contracts.md. The context map gains the Name and Port language rows and resolves the bridge command ambiguity toward serve. The domain model records the Route fixed-port invariant and the RouteTo port parameter.
ParseEndpointURL accepts any bare host, so URL-first resolution would make every saved name unreachable and the add-time URL refusal would reject names like e1. Names resolve first; only names containing :// are refused. The endpoint list element is its own noun because endpoint equality stays URL plus bridge.
The command line addresses endpoints and bridges by name (ADR 0007) and serve binds a stored port (ADR 0008). The Name lives on a new Endpoint Entry row rather than on the Endpoint value, because endpoint equality is URL plus bridge and every dedup in Global relies on it. Names resolve before URLs in EndpointFromFlags: ParseEndpointURL accepts any bare host, so URL-first would orphan every saved name. Bridge names are now unique at add; hand-edited duplicates still fail at resolution as before. --config gets UseSettingsFile, a process-wide path set once before Load.
Every command's --help carries example invocations, and a test will hold them to the contract grammar. Recorded before the cobra step so the CLI work is built to it.
Guy ruled the flag changes strictly additive. The single-dash long forms keep parsing through a normalization pass before cobra, marked deprecated in docs and source. ADR 0007 decision 2 amended in place with the date and the reason.
The TUI was the only interface, so nothing about an aperture setup could live in a script (ADR 0007). cobra now owns dispatch: a bare invocation still starts the TUI through the same code path, and bridge add, list, describe and delete plus endpoint add, list and delete work headless. Deletion reuses CheckRemovable, WillDestroyMachine and Machines.Destroy rather than restating their rules. Every flag has a shorthand, and the single-dash long forms keep parsing through gnuForm, which rewrites and warns; nothing that parsed before stops parsing. Help carries examples because the contract makes help part of the surface, and a test walks the tree to hold it there.
The team runs ts-unplug as a second fixed-port sidecar (ADR 0008). serve is Machines.For plus Open plus one Route held until SIGTERM or SIGINT, with the port from --port or the bridge record and no ephemeral fallback. RouteTo takes the port; existing callers pass zero and keep free ports. A signal during bring-up exits zero, because a supervisor restarting on non-zero would flap a serve the operator asked to stop. bridgeByName never creates: a typo must not register a device.
go test -count=3 exposed it: the --config test sets UseSettingsFile and nothing cleared it, so later tests in the process read and wrote that file. scratchHome now clears it, the same place the scratch home itself is set.
The headless CLI needs a menu-free way to build a launch (ADR 0007), so the Client interface gains Spec: flags win, LastLaunch fills what they leave, and the first compatible provider covers the rest. buildSpec carries what launch always did, so the menu path is byte-identical. Codex was the reference; claudecode, copilot, gemini, hermes, opencode and pilike followed it, each through an adversarial review. Opencode keeps model selection inside OpenCode after review caught a synthesized model ref the binary rejects: only --model a user typed passes through. Claude Cowork refuses Spec, since a desktop app has no headless launch.
aperture codex --endpoint e1 -- --search replaces the menu walk, and an alias like codex='aperture codex' works because the tail parser consumes only aperture's flags: the first unknown flag, or --, goes to the client verbatim (ADR 0007). The child's exit code becomes aperture's own, so the alias behaves like the binary. The connect half reuses BeginAttempt, Run and Commit rather than restating them. Help documents the flags by hand because DisableFlagParsing hides them from cobra.
An adversarial review of the branch found three real gaps and two hardening misses. endpoint add created a bridge from a typo, which a management command must not do; it resolves with bridgeByName now, and the launcher flags keep the creating path that makes a first run scriptable. --debug=false enabled debug, and --debug=banana now belongs to the client. Names with control characters are refused at the door, since they reach tabbed stdout a script parses. The run log opens for every run again, not only the TUI one: the original main always logged, the cobra refactor moved it into runTUI and headless runs silently lost their diagnostics to stderr. A serve's loopback exposure is recorded as a consequence in ADR 0008, same trust as the ts-unplug it replaces. A child killed by a signal exits 1 rather than wrapping to 255.
The GLM-5.3 security pass caught the passthrough contract broken twice over: gnuForm rewrote single-dash long flags inside a client command's tail, so a client's own -config became aperture's --config and the launch read and wrote a settings file the user never named, and a positional token ended flag consumption, which the contract never said. Rewriting now stops at the client command's name, the tail takes only GNU forms and single-letter shorthands as aperture's, and positionals pass through without ending consumption. --version is answered before dispatch, so a client command after it no longer swallows the flag and launches. OpenCode's per-launch config gets a unique name, so two concurrent launches stop overwriting and deleting each other's, and the pilike orphan sweep leaves files inside a grace window so a sibling launch keeps its extension. ADR 0007 and the contract record the tail boundary.
A --config or APERTURE_CONFIG override hands a launch an endpoint and a yolo flag with no screen to show either, so the resolved endpoint and the yolo state go to stderr before the client starts. Additive only: nothing is refused, and CI logs carry the source of the configuration the run used. Approved by Guy after the security review raised it.
The flag inventory had four gaps. flagOrEnv lost its precedence test when main_test.go died in the cobra rewrite, and --bridge= beating APERTURE_BRIDGE is a documented contract, so it comes back against pflag. The tail shorthands -e, -b, -c and -d had no parser cases. Bare -v had no pre-scan case. And a deprecated single-dash flag on a subcommand (bridge add -port) had nothing pinning it through the real binary, the one corner of the tail boundary that could silently break.
Two suites make check does not run, because they download the latest release and, with TS_AUTHKEY set, log a bridge into the local tailnet. compat has the released binary write settings, state and a bridge login, then the branch binary reads and reuses them, which is the only test that proves a real old install upgrades cleanly. contract pins the scripted surface byte for byte: the columns a script parses, the stderr a user reads, the exit codes a supervisor checks. make test-tailnet runs both, and the README documents the suites and the -update regeneration. Driving the release through a real Aperture needed screen reading, not row guesses: its picker shifts with whatever clients the box has.
A disk-full or close failure left the CreateTemp file behind in ~/.opencode instead of being cleaned up like the analogous writer in pilike/extension.go.
Spec resolved the provider from LastLaunch but still passed the caller's (possibly empty) model straight to buildSpec, so a model an earlier headless launch recorded was never replayed on the next one.
SetActiveEndpoint looked for the new URL among existing rows to carry its name forward, but a URL edit's new row never matches anything already saved; only the replaced row does. The name was silently dropped instead.
…-port test free+1 could already be taken or overflow past 65535 when free itself was the top of the range, failing the test before the behavior under test ever ran.
The scanner matched any token named "version" or "v" regardless of a leading dash, so a positional argument like `bridge add version` was mistaken for --version; it also ignored an attached value, so --version=false still printed the version.
The flag branch of resolve returned the caller's model unchanged, so --provider without --model sent Codex no --model at all instead of the provider's default, unlike the unflagged and LastLaunch paths.
…odel Same gap as Codex: the flag branch of resolve skipped the default-model fallback the unflagged path uses, so COPILOT_MODEL went unset and Copilot could pick a model outside the chosen provider's set.
signal.NotifyContext's handler consumes SIGTERM on aperture's own process; nothing forwarded it to the child, so a SIGTERM aimed at aperture left the client running while Run kept waiting on it. runChild now signals the child on ctx.Done and still waits for its real exit status.
The manual client-command parser bypassed cobra's flag/environment merging entirely, so $APERTURE_ENDPOINT, $APERTURE_BRIDGE and $APERTURE_CONFIG were ignored whenever the matching tail flag was absent. launchSelection now tracks whether each flag was supplied at all, even with an empty value, so an explicit --bridge= can still mean "no bridge" while an absent one reads the environment.
TestMain always built from source, so nothing exercised what make install or a release actually put on disk. APERTURE_E2E_BIN points the harness at a binary instead, and the README shows it with make test-tailnet.
The testing story spans four places (unit style, the e2e harness, the tailnet-tagged suites, the installed-binary override) and lived in none. docs/testing.md collects them with working examples: a table test, a headless launch through the stub, a PTY session, a signal test, golden regeneration and APERTURE_E2E_BIN. The README's Test suites section shrinks to a summary and links to it.
The tailnet-tagged suites were local-only, but only the bridge-login half of compat actually needs a tailnet: contract needs nothing external and the state-read half needs only GitHub. A parallel job runs make test-tailnet on ubuntu-latest, where the login half skips unless a TS_AUTHKEY secret exists. Same Makefile target, so a green local run still means a green CI.
Setting RLIMIT_FSIZE in the test runner process breaks testlog writes under 'go test' caching (write testlog.txt: file too large). Once bufio.Writer hits EFBIG during the limit window, its error is sticky and fails StopTestLog on exit even after Setrlimit restores the original limit. Running the failure path in a child process isolates the limit from the runner's log.
reportFailure points users to details: <run-log>, but neither main nor the headless subcommands logged the returning error to slog, leaving the run log empty of the failure that ended the process. Similarly, bridge proxy/dial failures in headless serve and client launch, as well as asynchronous TUI failures, were dropped or sent only to the UI sink. Routing all failure paths through slog ensures aperture.log contains the diagnostic trail across both headless and interactive modes.
…mands root.PersistentPreRun selected APERTURE_CONFIG before client commands' tail parser ran. When an explicit empty override (--config=) was passed, PersistentPreRun fell back to the environment variable because flags were not parsed by Cobra, and launch's nonempty guard never cleared it. Skipping PersistentPreRun when DisableFlagParsing is true leaves config selection to the tail parser.
NameEndpoint trims whitespace from names before saving them in settings. Echoing raw args[0] in the confirmation output preserved leading and trailing padding from input into the table. Reading the entry from settings after saving ensures the canonical name is printed.
…checks gnuForm and versionRequested treated any token starting with '-' as a flag. When a value-taking flag was followed by a value that started with '-' (e.g. '--config -debug' or '--endpoint -v'), gnuForm rewrote the value as a flag and emitted a deprecation warning, while versionRequested mistook the value for a version request. Tracking value-taking flags in both scanners preserves following values verbatim.
When an unflagged headless launch restored LastModel, it checked only that the provider was compatible, not that the saved model still existed in the provider's advertised models. If a model was removed or an endpoint switch served different models under the same provider ID, the launch passed the stale model instead of falling back. Validating LastModel against fqnModels(p) falls back to the default model across Codex, Copilot, Hermes and Pi-like.
EditAttempt calls BeginAttempt, which appends the new candidate URL to settings before verification. In SetActiveEndpoint, iterating through existing endpoints matched the replacement original first, but then overwrote head with the subsequent candidate entry, losing the copied name. Selecting the candidate row first and then copying the replacement name preserves the command-line handle when both rows are present.
A bridge in use by endpoints is refused by CheckRemovable, but unnamed endpoints created by flag-based connects or older releases have no handle to delete individually. Adding --force bypasses the guard, logs out and destroys every machine slot from the tailnet, and cleans up the associated endpoints from settings.
TestCompatOldBridgeLoginReusedByNewBinary registered a bridge on the real tailnet with TS_AUTHKEY without destroying it, leaving active nodes behind. A t.Cleanup hook now force-deletes the bridge to log out its machines. Also fixes macOS e2e test failures: - apertureConfigDir resolves ~/Library/Application Support/aperture on darwin - detailsPattern in golden normalization matches paths containing spaces - releaseBinary handles darwin .zip archive releases alongside .tar.gz - terminal synchronization eliminates a data race between waitExit and kill - TestLaunchPi waits for the quick select row before sending quit
Adds the --force example and explanation to aperture bridge delete's Long and Example descriptions so the newly added flag is documented in its help output.
CI produces .tar.gz archives for darwin, but v0.0.13 was uploaded as a .zip archive via the legacy manual flow. Trying the tarball first and falling back to the zip on darwin matches install.sh and ensures compat tests can download both current and legacy releases.
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.
CI produces
.tar.gzarchives for darwin, but v0.0.13 was uploaded as a.zipvia the old manual release flow.releaseBinaryonly tried the tarball, so the compat suite couldn't download that older darwin release.Trying the tarball first and falling back to the
.zipon a 404 (darwin only) matches whatinstall.shalready does, and lets the compat tests download both current and legacy darwin releases.Test-only change, scoped to the e2e black-box harness. No production code or install path changes.
Risk: a 404 on darwin now costs a second round trip to GitHub releases before the fallback resolves, so a compat run against an old darwin tag is a bit slower and has one more network call that can fail or get rate-limited.