3c46db56d5968bff71acda906be6be4f384d3318
12
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3c46db56d5 |
Carry file size, thumbnail size, and deletion flag through decryptFile (closes #37)
check / check (push) Successful in 24s
Foundation unit for the cache/API design. Three fields arrived on the wire but decryptFile dropped them: - Live files now set `file.size` from `info.fileSize` and `thumbnail.size` from `info.thumbSize`, left `undefined` when the server omits `info`. - A deleted file is returned as a new `EnteFileTombstone` (`id`, `collectionID`, `updationTime`, `isDeleted: true`) with no decryption, since the server no longer holds ciphertext for it. decryptFile now returns `EnteFile | EnteFileTombstone`. Rather than making EnteFile's structural fields optional — which would force `?.`/guards across every consumer under strict tsc and break the build — a tombstone is a distinct minimal type, and `EnteFile.isDeleted?: false` is the discriminant (a live file is never deleted). This keeps all existing consumers untouched; they still receive fully-populated `EnteFile` values. client.ts is the only direct caller: it now routes every row through decryptFile and drops results whose `isDeleted` is set. listFiles still returns live files only, so its observable behaviour is unchanged. Model: opus-4-8 |
||
|
|
2bfa11c10c |
Walk the path CI runs in lint-once, and pin the branch the container installs (closes #33)
check / check (push) Successful in 35s
The header of test/packaging/lint-once.test.ts claimed a duplicate prettier pass is caught wherever it is added. It was not: the walk started at `make check`, which never reads `Dockerfile`, so appending `RUN yarn run prettier --check .` to the image that `script/cibuild` builds left the suite green — two prettier passes on the one path where it matters most. The walk now also starts at `.gitea/workflows/check.yml` and follows its `run:` steps into `script/cibuild` and from there into both images, so the graph under test is the one CI executes rather than the one it was assumed to execute. Reaching `script/cibuild` and `Dockerfile` is asserted, and the test and build image is asserted to invoke prettier zero times. The lockfile assertion was a substring check against the whole of `script/bootstrap`. That script has two install sites, and the containers take the second, because the pinned node image ships yarn; changing that site to a bare `yarn install` kept the suite green while the container's install stopped being pinned. `install_js_deps` is now resolved out of the script and split at its `missing yarn` guard, and every `yarn install` occurrence in each branch is required to carry `--frozen-lockfile`. That the container runs `script/bootstrap` at all is asserted too, so the lockfile assertions cannot end up describing a script the image never executes. Prettier is counted per occurrence instead of per line: `prettier --check . && prettier --check src` was one invocation by the old count. The `continue` that followed a counted line also dropped every script, make, yarn and docker edge sharing that line, so a subtree could be hidden behind a single `&&`; edges are now extracted from every line. Undercounting is what would make this file worthless, so every way of reaching nothing is a thrown error rather than a quiet zero: an unknown Makefile target, an unknown package.json script, a missing script file, a node that resolves to no commands, and an unknown node kind. All five are tested, as is a walk that legitimately counts zero, and the cycle guard. Every assertion in the file was mutation-tested: changed to assert something else, run, and confirmed to fail for its own named reason. The two mutations above were reproduced and both now turn the suite red. test/packaging/entrypoints.test.ts said `make check` runs test, lint and fmt-check. Formatting has been part of the lint container since the duplicate host pass was removed, so the comment now says what it does. |
||
|
|
a73f0abbe8 |
Check formatting once per make check, in the container (closes #29)
check / check (push) Successful in 59s
script/check ran script/test, script/lint and script/fmt-check. Since linting moved into Docker, script/lint is a build of Dockerfile.lint, which runs `prettier --check .` as a build step — so make check checked formatting twice over the same tree: once in the container and once on the host. script/precommit had the same pair. Drop the script/fmt-check call from both. The container keeps the check, because a successful Dockerfile.lint build is what CI treats as proof of a clean tree, and it is the stronger of the two verdicts: its prettier is digest-pinned and installed under --frozen-lockfile, while the host's is whatever the working tree happens to have. The pre-commit hook is unchanged in what it catches — script/lint still fails a badly formatted tree, and therefore the commit. script/fmt-check survives as a standalone entrypoint, as REPO_POLICIES.md requires, for asking the formatting question by itself without docker. Its verdict cannot drift from the container's: prettier is pinned to an exact version, installed from yarn.lock in both places, and reads .gitignore as its default ignore file, which is why .dockerignore keeps .gitignore in the build context. The count is asserted rather than promised. test/packaging/lint-once.test.ts walks the invocation graph from each entrypoint — through the Makefile shims, the script/ calls, the package.json scripts and the docker build into Dockerfile.lint's RUN steps — and counts prettier invocations: one per make check, one per script/precommit, and one each for make lint and make fmt-check alone, so neither can become a no-op that satisfies the count trivially. The walk also asserts which nodes it reached, so a restructure that defeats the resolver fails the test instead of quietly counting zero. Observed: 2 prettier invocations per make check before, 1 after. |
||
|
|
fed39d19cf |
Run all linting in Docker via Dockerfile.lint (closes #30)
check / check (push) Successful in 1m2s
Linting now happens in one place only: a new root Dockerfile.lint copies the repo into the digest-pinned node image already used by Dockerfile and runs eslint and prettier as build steps, so a successful build is a clean lint. script/lint is reduced to building it, which also works where the docker daemon is remote and bind mounts are impossible. No host lint path survives: the "lint" script is gone from package.json, so there is no second, unpinned way to get a lint verdict. Caching is waived for lint, because a lint build over an unchanged tree returns success in well under a second having linted nothing. LINT_EPOCH is the cache buster and it fails closed exactly as CHECK_EPOCH does: an unset ARG is the empty string, which is a perfectly stable cache key, so the guard rejects it and a bare `docker build -f Dockerfile.lint .` errors out instead of serving a green it did not earn. Both linters sit below the guard, so a fresh epoch forces them to execute while the bootstrap and dependency layers above stay cached. That makes script/lint a docker build, which nothing inside a container may call. script/check calls script/lint, so the Dockerfile image can no longer run make check: the lint stage and its COPY --from=lint ordering hack are deleted, and the remaining stage runs make test and make build under the existing CHECK_EPOCH guard. script/cibuild is now the composite gate and builds the lint image first, so a lint failure is reported before the slower suite runs. The .dockerignore exclusions are unchanged and still apply to the lint build, including the .claude/ exclusion (eslint's flat config does not ignore dot-directories, so a nested worktree in the context would be linted) and the deliberate exception that keeps .gitignore in the context for prettier. A new test asserts no per-Dockerfile ignore file shadows the root one for either image, and test/packaging/lint-docker.test.ts asserts the whole shape: the docker-only lint path, the digest pin, manifests copied before sources, the fail-closed guard with both linters below it, the absence of a lint stage or make check in Dockerfile, and the build order in script/cibuild. |
||
|
|
156fe871e8 |
Make the image build multi-stage and cache-proof (closes #4)
check / check (push) Successful in 23s
The image build reported a green it had not earned. `script/cibuild` is a bare `docker build .`, and with `COPY . .` followed by `RUN make check`, an unchanged tree served that layer from cache: the suite never ran and the build still exited 0, while the script's header comment asserted the opposite. CHECK_EPOCH, passed by `script/cibuild` and `script/docker`, changes the cache key of the check and build layers on every invocation. It is guarded, because an unset ARG is the empty string and therefore a stable key: without the guard a plain `docker build .` — the command the policy names, and the one anyone debugging types — would still get the false green. A missing argument is now a hard failure rather than a silent degradation to the behaviour the epoch was added to prevent. The Dockerfile is now two stages: `fmt-check` and `lint` run first, and the check stage takes a `COPY --from=lint` dependency on them, so a formatting mistake fails the build in seconds instead of racing the suite to the finish. Both stages stay pinned to the same digest. The remaining fixes are one-liners that had made the target unusable: `script/projectname` still printed the pre-rename name, so `make docker` tagged its image after a name this project dropped in May; `script/bootstrap` installed without fetching apt's package lists, which cannot work on a Debian base; and `.dockerignore` had drifted far enough from `.gitignore` to ship a ~100 MB compiled binary and any agent worktree under `.claude/` into the build context. The second of those is a correctness problem, not a size one — vitest globs a copied worktree's tests alongside the real ones and runs the suite twice over. `.gitignore` itself stays in the context, because prettier reads it as a default ignore file and dropping it would change what `make fmt-check` sees. |
||
|
|
69bd6d1539 |
Compile bin/ alongside src/ so the package can be built (closes #3)
check / check (push) Successful in 5s
rootDir was ./src while include also matched bin/**/*, which is TS6059: tsc refuses to emit at all when a compiled file sits outside rootDir. rootDir is now the repository root, which is the smallest change that makes the two agree and leaves the source layout the README documents alone. Output keeps the shape of the source tree, so main and types move to dist/src/index.js and dist/src/index.d.ts while bin.quak stays at dist/bin/quak.js. The alternative, moving the CLI body into src/ behind a shim in bin/, would hold main at dist/index.js at the cost of churning the CLI and contradicting the layout diagram in the README. Clearing TS6059 exposed two type errors that had never been reached, because the config error aborts before checking: StateAddress was read as a namespace member off the default import, and the secretstream pull was called without the additional-data argument, which libsodium does not make optional. The type is now taken from the module's named export and the pull passes null for ad, matching the null already passed on the push side in encryptBlob. Neither changes what runs. noEmitOnError stops a failed build from leaving output behind. It emitted despite the error before, which is how a stale bin/quak.js came to sit next to bin/quak.ts in a working tree, where eslint then read it and failed make check on a generated file. script/build compiles and then checks that the files package.json advertises are among the ones the compiler wrote, since tsc knows nothing about the manifest and a green build could still ship a package whose main resolves to nothing. It also sets the executable bit on the bin entries, which tsc does not carry over from the source even though it does copy the shebang. The Makefile target is now a shim over it, as the other targets are, and package.json's build script points at it so yarn build gets the same checks. The Dockerfile runs make build after make check, so a branch that does not compile cannot reach main. What make check itself runs is unchanged. package.json gains a quak script, so the yarn quak commands the README's Getting Started block has always listed resolve to the built CLI. |
||
|
|
f3cf4af833 |
Retry transient network failures with exponential backoff (closes #2)
check / check (push) Successful in 20s
No retry on 4xx, backoff on 5xx and transport failures, and a deadline on every request. Before this, one transient 503 or TCP reset failed a file for good, and a CDN connection that went quiet after accepting the request blocked `quak backup` forever, because there was no timeout anywhere. src/retry.ts holds the policy: a classifier that decides whether another attempt could produce a different answer, and a loop that acts on it with exponential backoff and full jitter. Retried: 5xx, 408, 429, transport failures (the errno is read out of the cause chain, which is where Node's fetch puts it), deadline aborts, and truncated transfers. Not retried: every other 4xx, and anything unrecognised — a wrongly retried permanent failure delays every remaining file, while a wrongly abandoned transient one costs a single file the next run picks up. Attempt count, delays, sleep and jitter source are all configurable through ApiClientOptions; sleep being injectable is what lets the suite exercise the policy without waiting. Truncation needed a type before it could be classified. streamDecrypt threw plain Errors whose messages began "download: stream truncated", and classifying on message text would mean the next reword silently turned every truncated download into a permanent failure. It now throws TruncatedStreamError, which lives in src/errors.ts alongside ApiError so the classifier can recognise both without importing the modules that import it; api/client.ts re-exports ApiError, so it stays one class and every existing import path still resolves. Downloads retry the request, the stream consumption and the decryption together. Only the first of those happens inside ApiClient: a socket reset after the headers arrived throws in streamDecrypt, and retrying the request alone would never see it. The client's own retry is switched off for those two calls so the budgets do not multiply into sixteen requests per file, and the atomic write stays outside the loop so a download that took three attempts still performs one write and one rename. Non-idempotent requests are not blindly replayed. postJSON and putJSON reach create-session, two-factor/verify — which burns one of a few second-factor attempts — and files/thumbnail, so they retry only when the connection was never established and the server provably never saw the request. putFile is exempt and retries fully: a presigned PUT stores one whole object at one key, with no partial state to damage. It now throws ApiError with the status, as do the two null-body paths, which previously threw bare Errors that nothing could classify. Timeouts come from AbortSignal.timeout(), renewed per attempt: 30s for JSON and upload calls, 10 minutes for file bodies, since a value short enough to keep a hung API call from stalling a backup would cancel a legitimate multi-gigabyte download. The download deadline is enforced over the body rather than only the headers, by racing each read against the signal, so the guarantee does not depend on the fetch implementation tearing down a stream it already handed over. listMissingThumbnails now separates a genuine 404 from an exhausted retry. Its bare catch reported both as missing, which after this change would have let a few minutes of 500s talk fix-missing-thumbnails into regenerating and re-uploading thumbnails that were fine. runBackup and runMetadataBackup are untouched: the retry sits below them and their per-file resilience is unchanged. |
||
|
|
99905277a3 |
Verify secretstream TAG_FINAL and write downloads atomically (closes #1)
check / check (push) Failing after 1m19s
streamDecrypt discarded the secretstream tag, so a download cut short by a dropped connection decrypted cleanly up to the last whole chunk and was returned as a success. downloadFile and downloadThumbnail then wrote straight to the destination, and runBackup skips any existing non-empty file, so a truncated original was treated as complete on every subsequent run and never repaired. streamDecrypt now tracks the tag of each chunk it pulls and throws if the stream ended on anything other than TAG_FINAL, or if the body carried no chunks at all — Ente always emits at least one chunk, as encryptBlob shows by producing a TAG_FINAL chunk even for zero-length plaintext, so an empty body is a failed transfer rather than an empty file. Both error messages say the stream was truncated. Plaintext is now staged in a temporary sibling file (same directory, so the rename cannot cross a filesystem boundary; random UUID suffix, so concurrent downloads cannot collide) and renamed into place only after the whole stream has decrypted and verified. On any error the temporary file is removed and the original error is rethrown unchanged, so a cleanup failure never masks the real diagnosis. A failed download therefore leaves the destination exactly as it was. Public signatures and the DownloadResult shape are unchanged. The download layer keeps its no-direct-sodium-import shape: TAG_FINAL is re-exported from src/crypto as STREAM_TAG_FINAL, which decryptBlob now uses too. Also moves the pullStreamChunk doc comment off decryptBlob, where it had been sitting. Retry and backoff remain out of scope; they stay the Next Step in TODO.md and are tracked separately. |
||
|
|
4f506b0155 | Adopt scripts-to-rule-them-all: script/ entrypoints, Makefile shims | ||
|
|
dc0dd11f19 |
Format TODO.md with prettier (make fmt)
check / check (push) Failing after 5s
|
||
|
|
84554a85ad |
Add standard Workflow section to TODO.md
check / check (push) Failing after 6s
|
||
|
|
88510a3ff5 |
Add TODO.md
check / check (push) Failing after 5s
|