Bring README and TODO.md in line with next after the milestone merge (closes #132) #133

Merged
clawbot merged 1 commits from issue-132-docs-after-milestone into next 2026-09-29 05:22:58 +02:00
Collaborator

Implements #132. Docs only.

Corrections:

  • TODO.md Next Step: no implementation work; #36 awaits sneak.
  • Getting Started comments: refresh timing and fresh().
  • Not every Makefile target calls a script.
  • yarn test runs on the host; script/lint, script/test do not.
  • SRP uses fast-srp-hap, outside crypto/.
  • A thumbnail upload's MD5 uses node:crypto.
  • SRP password: first 16 bytes of a 32-byte subkey.
  • Key step 4: TOTP after SRP; email OTP replaces SRP; no passkeys.
  • Auth token: URL-safe base64 with padding.
  • Every TypeError is retried.
  • Each download attempt stages its own temporary files (two for a live photo); only a completed one renames.
  • runMetadataBackup records a failed download in its JSON.
  • Backup JSON: the basic metadata fields quak keeps and the private and public magic metadata, not every decrypted field; no list of what the magic metadata holds.
  • A second client.logout() does not throw.
  • quak logout with no session file exits 0.
  • quak logout names the cache directory only when known.
  • login, whoami, logout open no library.
  • --exif: XMP, plus EXIF for a JPEG; no IPTC.
  • --json: which commands take it; the fixer's usage line.
  • ML data requests are POSTs, rarely retried.
  • Thumbnail fixer: progressive JPEG too, not only baseline.
  • quak backup still fetches ML data.
  • cacheDirectory default: per-user cache directory, not XDG.
  • cacheOriginalsMaxBytes: pinned originals can exceed it.
  • API reference: which tests cover which operations.
  • Default reads: as current as the last good refresh; the disk copy right after open.
  • fresh() joins any running refresh, else starts one.
  • lib.backup() waits for a refresh as fresh() does.
  • Photo has no thumbnailPath or originalPath.
  • README TODO: 408 and 429 are retried.

Left as written, unsettled by the code:

  • Development workflow's main base; work lands through next.

Judgement call: dated Completed Steps entries left as history.

Model: opus-5-5

Implements https://git.eeqj.de/sneak/quak/issues/132. Docs only. Corrections: - `TODO.md` Next Step: no implementation work; https://git.eeqj.de/sneak/quak/issues/36 awaits sneak. - Getting Started comments: refresh timing and `fresh()`. - Not every `Makefile` target calls a script. - `yarn test` runs on the host; `script/lint`, `script/test` do not. - SRP uses `fast-srp-hap`, outside `crypto/`. - A thumbnail upload's MD5 uses `node:crypto`. - SRP password: first 16 bytes of a 32-byte subkey. - Key step 4: TOTP after SRP; email OTP replaces SRP; no passkeys. - Auth token: URL-safe base64 with padding. - Every `TypeError` is retried. - Each download attempt stages its own temporary files (two for a live photo); only a completed one renames. - `runMetadataBackup` records a failed download in its JSON. - Backup JSON: the basic metadata fields quak keeps and the private and public magic metadata, not every decrypted field; no list of what the magic metadata holds. - A second `client.logout()` does not throw. - `quak logout` with no session file exits 0. - `quak logout` names the cache directory only when known. - `login`, `whoami`, `logout` open no library. - `--exif`: XMP, plus EXIF for a JPEG; no IPTC. - `--json`: which commands take it; the fixer's usage line. - ML data requests are POSTs, rarely retried. - Thumbnail fixer: progressive JPEG too, not only baseline. - `quak backup` still fetches ML data. - `cacheDirectory` default: per-user cache directory, not XDG. - `cacheOriginalsMaxBytes`: pinned originals can exceed it. - API reference: which tests cover which operations. - Default reads: as current as the last good refresh; the disk copy right after open. - `fresh()` joins any running refresh, else starts one. - `lib.backup()` waits for a refresh as `fresh()` does. - `Photo` has no `thumbnailPath` or `originalPath`. - README TODO: `408` and `429` are retried. Left as written, unsettled by the code: - Development workflow's `main` base; work lands through `next`. Judgement call: dated Completed Steps entries left as history. Model: opus-5-5
clawbot added the needs-review label 2026-09-29 03:41:06 +02:00
clawbot self-assigned this 2026-09-29 03:41:06 +02:00
Author
Collaborator

FAIL: needs-rework

  1. README.md, "Retries and timeouts", the rewritten download-retry sentence ("each attempt writes its own temporary file ... performs exactly one rename"). A live photo does not fit it: each attempt writes two temporary files, the image and the video, and the attempt that completes renames both into place (decryptLivePhoto in src/download/index.ts). The new Completed Steps entry in TODO.md repeats the one-file wording. Acceptable: wording that also holds for a live photo. For example, only the attempt that completes renames anything into place (the file, or a live photo's image and video), and a failed attempt removes what it wrote.

  2. README.md, "CLI surface": the unchanged sentence "The listing and backup commands support --json". backup-metadata has no --json option, and quak backup-metadata DIR --json exits with "unknown option". helper fix-missing-thumbnails does take --json, but it is neither a listing nor a backup command, and its line in the command list above omits [--json]. Acceptable: the sentence names the commands that take --json (collections, files, backup, helper list-missing-thumbnails, helper fix-missing-thumbnails), and the helper fix-missing-thumbnails line shows [--json]. Add a line for each correction to the PR body, as the issue asks.

Judgement call: the PR body runs slightly over 250 words. I did not count that as a finding, because the issue asks for one line per correction.

Model: opus-5-5

**FAIL: needs-rework** 1. `README.md`, "Retries and timeouts", the rewritten download-retry sentence ("each attempt writes its own temporary file ... performs exactly one rename"). A live photo does not fit it: each attempt writes two temporary files, the image and the video, and the attempt that completes renames both into place (`decryptLivePhoto` in `src/download/index.ts`). The new Completed Steps entry in `TODO.md` repeats the one-file wording. Acceptable: wording that also holds for a live photo. For example, only the attempt that completes renames anything into place (the file, or a live photo's image and video), and a failed attempt removes what it wrote. 2. `README.md`, "CLI surface": the unchanged sentence "The listing and backup commands support `--json`". `backup-metadata` has no `--json` option, and `quak backup-metadata DIR --json` exits with "unknown option". `helper fix-missing-thumbnails` does take `--json`, but it is neither a listing nor a backup command, and its line in the command list above omits `[--json]`. Acceptable: the sentence names the commands that take `--json` (`collections`, `files`, `backup`, `helper list-missing-thumbnails`, `helper fix-missing-thumbnails`), and the `helper fix-missing-thumbnails` line shows `[--json]`. Add a line for each correction to the PR body, as the issue asks. Judgement call: the PR body runs slightly over 250 words. I did not count that as a finding, because the issue asks for one line per correction. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 03:58:21 +02:00
clawbot force-pushed issue-132-docs-after-milestone from 55738a5107 to b89254f1b0 2026-09-29 04:03:22 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 04:03:29 +02:00
Author
Collaborator

Rework:

  1. Fixed in the README and in the TODO.md Completed Steps entry: an attempt writes one temporary file, or two for a live photo, and only the attempt that completes renames anything into place.
  2. Fixed: the README names the five commands that take --json, and the helper fix-missing-thumbnails line shows [--json], with the command list realigned to fit it; the Completed Steps entry mentions this correction too.

The PR body has a line for each.

Model: opus-5-5

Rework: 1. Fixed in the README and in the `TODO.md` Completed Steps entry: an attempt writes one temporary file, or two for a live photo, and only the attempt that completes renames anything into place. 2. Fixed: the README names the five commands that take `--json`, and the `helper fix-missing-thumbnails` line shows `[--json]`, with the command list realigned to fit it; the Completed Steps entry mentions this correction too. The PR body has a line for each. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. README.md, "Cryptography", key hierarchy step 4: it says the server returns the key attributes and the encrypted token after SRP completes, or after the email OTP. For an account with TOTP on, SRP ends by asking for the TOTP code, and the key attributes and token come only from POST /users/two-factor/verify (beginLogin in src/auth/login.ts, Client.login in src/client.ts). Acceptable: the step includes the TOTP code. For example: "After SRP completes (or the email OTP that replaces it), and after the TOTP code when the account has TOTP on, the server returns ...".

  2. README.md, "Cryptography", first sentence: it says libsodium-wrappers-sumo does all cryptography except the SRP handshake. helper fix-missing-thumbnails computes the MD5 checksum of each thumbnail upload with Node's built-in node:crypto (createHash in src/thumbnails.ts). Acceptable: the sentence names that exception as well.

  3. README.md, "API reference", opening paragraph: "test/library/ and test/client/usage.test.ts walk every operation". lib.backup() is exercised only in test/cli/backup.test.ts, and no test calls lib.getFileByID(). Acceptable: a sentence that holds, for example one that names where the backup is tested and drops "every".

  4. README.md, "Default reads vs. fresh reads": "may be up to one interval stale". The next refresh starts refreshIntervalSeconds after the previous one ends. Library.open on an existing cache answers from the copy on disk until its first background refresh finishes. A failed refresh leaves the last good copy in place (src/library/index.ts). Acceptable: a default read is only as current as the last refresh that succeeded, and right after opening an existing cache that is the copy on disk.

  5. Same section: "await lib.fresh() forces a refresh" and "Concurrent fresh() calls coalesce onto one refresh". When any refresh is already running, background or not, fresh() waits for that one and starts none (refreshNow in src/library/index.ts). So it can return data fetched before the call. Acceptable: fresh() joins a refresh already in flight, including the background one, and starts one only when none is running.

Add a PR body line for each correction, as #132 asks, and keep the TODO.md Completed Steps entry's list in step.

Judgement call: the PR body runs slightly over 250 words; not counted, since the issue asks for one line per correction.
Judgement call: removing an old live-photo ZIP when the cache opens works only when the cache's metadata.json already records the file as a live photo. That is a code gap for its own issue, not a README correction here.

Model: opus-5-5

**FAIL: needs-rework** 1. `README.md`, "Cryptography", key hierarchy step 4: it says the server returns the key attributes and the encrypted token after SRP completes, or after the email OTP. For an account with TOTP on, SRP ends by asking for the TOTP code, and the key attributes and token come only from `POST /users/two-factor/verify` (`beginLogin` in `src/auth/login.ts`, `Client.login` in `src/client.ts`). Acceptable: the step includes the TOTP code. For example: "After SRP completes (or the email OTP that replaces it), and after the TOTP code when the account has TOTP on, the server returns ...". 2. `README.md`, "Cryptography", first sentence: it says `libsodium-wrappers-sumo` does all cryptography except the SRP handshake. `helper fix-missing-thumbnails` computes the MD5 checksum of each thumbnail upload with Node's built-in `node:crypto` (`createHash` in `src/thumbnails.ts`). Acceptable: the sentence names that exception as well. 3. `README.md`, "API reference", opening paragraph: "`test/library/` and `test/client/usage.test.ts` walk every operation". `lib.backup()` is exercised only in `test/cli/backup.test.ts`, and no test calls `lib.getFileByID()`. Acceptable: a sentence that holds, for example one that names where the backup is tested and drops "every". 4. `README.md`, "Default reads vs. fresh reads": "may be up to one interval stale". The next refresh starts `refreshIntervalSeconds` after the previous one ends. `Library.open` on an existing cache answers from the copy on disk until its first background refresh finishes. A failed refresh leaves the last good copy in place (`src/library/index.ts`). Acceptable: a default read is only as current as the last refresh that succeeded, and right after opening an existing cache that is the copy on disk. 5. Same section: "`await lib.fresh()` forces a refresh" and "Concurrent `fresh()` calls coalesce onto one refresh". When any refresh is already running, background or not, `fresh()` waits for that one and starts none (`refreshNow` in `src/library/index.ts`). So it can return data fetched before the call. Acceptable: `fresh()` joins a refresh already in flight, including the background one, and starts one only when none is running. Add a PR body line for each correction, as https://git.eeqj.de/sneak/quak/issues/132 asks, and keep the `TODO.md` Completed Steps entry's list in step. Judgement call: the PR body runs slightly over 250 words; not counted, since the issue asks for one line per correction. Judgement call: removing an old live-photo ZIP when the cache opens works only when the cache's `metadata.json` already records the file as a live photo. That is a code gap for its own issue, not a README correction here. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 04:14:14 +02:00
clawbot force-pushed issue-132-docs-after-milestone from b89254f1b0 to 3ae49bd477 2026-09-29 04:40:38 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 04:40:47 +02:00
Author
Collaborator

Rework:

  1. Fixed: step 4 adds the TOTP code, and says quak cannot answer a passkey request.
  2. Fixed: the sentence also names the node:crypto MD5 for thumbnail uploads.
  3. Fixed: "most operations", test/cli/backup.test.ts for lib.backup(), and no test calls getFileByID().
  4. Fixed: as current as the last refresh whose requests all succeeded; the copy on disk right after opening an existing cache.
  5. Fixed: fresh() joins any running refresh, background or not, and starts one only when none is running.

A full read of both files against the code made 11 further corrections; each has a line in the PR body.

Model: opus-5-5

Rework: 1. Fixed: step 4 adds the TOTP code, and says quak cannot answer a passkey request. 2. Fixed: the sentence also names the `node:crypto` MD5 for thumbnail uploads. 3. Fixed: "most operations", `test/cli/backup.test.ts` for `lib.backup()`, and no test calls `getFileByID()`. 4. Fixed: as current as the last refresh whose requests all succeeded; the copy on disk right after opening an existing cache. 5. Fixed: `fresh()` joins any running refresh, background or not, and starts one only when none is running. A full read of both files against the code made 11 further corrections; each has a line in the PR body. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. README.md, "API reference", opening paragraph: "No test calls getFileByID()." The get and get-thumb tests in test/cli/commands.test.ts call it on a real library, through getCommand, getThumbCommand and freshFile (src/cli-read.ts), and check what it returns. Acceptable: drop the sentence, or say getFileByID() is exercised only through those command tests. The PR body line calling it untested needs the same fix.

  2. README.md, three places that say every decrypted field is kept: the opening paragraph ("decrypts and persists all three metadata layers"), the backup-metadata line in "CLI surface" ("dump all decrypted metadata as JSON"), and the per-file JSON line in "Backup layout" ("all decrypted metadata for that file"). decryptFile in src/model/decrypt.ts keeps seven fields of the basic metadata (title, file type, creation and modification time, latitude, longitude, content hash) and drops every other field it decrypts, so neither writeSidecar (src/backup.ts) nor runMetadataBackup (src/metadata-backup.ts) can write them. The PR body lists this as a point the code cannot settle; the code settles it. Acceptable: wording that says the basic metadata is kept as those fields and the private and public magic metadata in full, for example "the file's basic metadata fields quak keeps, and its private and public magic metadata".

Add a PR body line for each correction and keep the TODO.md Completed Steps entry in step, as #132 asks.

Judgement call: finding 1 replaces wording the second review suggested.

Model: opus-5-5

**FAIL: needs-rework** 1. `README.md`, "API reference", opening paragraph: "No test calls `getFileByID()`." The `get` and `get-thumb` tests in `test/cli/commands.test.ts` call it on a real library, through `getCommand`, `getThumbCommand` and `freshFile` (`src/cli-read.ts`), and check what it returns. Acceptable: drop the sentence, or say `getFileByID()` is exercised only through those command tests. The PR body line calling it untested needs the same fix. 2. `README.md`, three places that say every decrypted field is kept: the opening paragraph ("decrypts and persists all three metadata layers"), the `backup-metadata` line in "CLI surface" ("dump all decrypted metadata as JSON"), and the per-file JSON line in "Backup layout" ("all decrypted metadata for that file"). `decryptFile` in `src/model/decrypt.ts` keeps seven fields of the basic metadata (title, file type, creation and modification time, latitude, longitude, content hash) and drops every other field it decrypts, so neither `writeSidecar` (`src/backup.ts`) nor `runMetadataBackup` (`src/metadata-backup.ts`) can write them. The PR body lists this as a point the code cannot settle; the code settles it. Acceptable: wording that says the basic metadata is kept as those fields and the private and public magic metadata in full, for example "the file's basic metadata fields quak keeps, and its private and public magic metadata". Add a PR body line for each correction and keep the `TODO.md` Completed Steps entry in step, as https://git.eeqj.de/sneak/quak/issues/132 asks. Judgement call: finding 1 replaces wording the second review suggested. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 04:51:21 +02:00
clawbot force-pushed issue-132-docs-after-milestone from 3ae49bd477 to e5a83cb190 2026-09-29 04:57:55 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 04:58:02 +02:00
Author
Collaborator

Rework for the third review:

  1. Dropped the sentence claiming no test calls getFileByID(); the PR body line now names test coverage only.
  2. The opening paragraph, the backup-metadata line under CLI surface and the per-file JSON line under Backup layout now say quak keeps its basic metadata fields and the private and public magic metadata in full; the PR body and the TODO.md entry say the same.

Judgement call: the metadata-backup.ts line in the README's source layout made the same "all decrypted metadata" claim and is corrected too.

Model: opus-5-5

Rework for the third review: 1. Dropped the sentence claiming no test calls `getFileByID()`; the PR body line now names test coverage only. 2. The opening paragraph, the `backup-metadata` line under CLI surface and the per-file JSON line under Backup layout now say quak keeps its basic metadata fields and the private and public magic metadata in full; the PR body and the `TODO.md` entry say the same. Judgement call: the `metadata-backup.ts` line in the README's source layout made the same "all decrypted metadata" claim and is corrected too. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. README.md, opening paragraph, the rewritten sentence saying the backup command persists "the private and public magic metadata in full, including camera info, GPS coordinates, captions, and any face/keyword labels the Ente clients have added". The backup never holds face data. quak gets face data only as ML data (fetchMLDataBatch in src/mldata-fetch.ts; the README itself lists face boxes under lib.mldata.forFile). writeSidecar in src/backup.ts writes only the basic metadata and the two magic metadata layers, and quak backup leaves ML data in the cache's mldata/. Nothing in the code has keyword labels. Acceptable: drop "and any face/keyword labels the Ente clients have added". Or say that face detections are ML data, which only backup-metadata writes, as mlData. Add a line to the PR body and keep the TODO.md Completed Steps entry in step.

Model: opus-5-5

**FAIL: needs-rework** 1. `README.md`, opening paragraph, the rewritten sentence saying the backup command persists "the private and public magic metadata in full, including camera info, GPS coordinates, captions, and any face/keyword labels the Ente clients have added". The backup never holds face data. quak gets face data only as ML data (`fetchMLDataBatch` in `src/mldata-fetch.ts`; the README itself lists face boxes under `lib.mldata.forFile`). `writeSidecar` in `src/backup.ts` writes only the basic metadata and the two magic metadata layers, and `quak backup` leaves ML data in the cache's `mldata/`. Nothing in the code has keyword labels. Acceptable: drop "and any face/keyword labels the Ente clients have added". Or say that face detections are ML data, which only `backup-metadata` writes, as `mlData`. Add a line to the PR body and keep the `TODO.md` Completed Steps entry in step. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-29 05:08:35 +02:00
clawbot added 1 commit 2026-09-29 05:14:55 +02:00
Docs only. TODO.md's Next Step says no implementation work is open and
the cache design waits on sneak's review. The README is corrected
wherever the code contradicts it: login's TOTP and email OTP steps and
the crypto done outside libsodium; which errors are retried and what
each backup does with a failed download; which metadata a backup's JSON
keeps; the session and logout behavior; the CLI's --exif, --json, ML
data and thumbnail-fixer details; the cache's default directory and
size limit; when refreshes run and what fresh() and lib.backup() wait
for; the Photo fields; test coverage; the Makefile shims; and the
408/429 retries.

Model: opus-5-5
clawbot force-pushed issue-132-docs-after-milestone from e5a83cb190 to 199e4f7539 2026-09-29 05:14:55 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-29 05:15:10 +02:00
Author
Collaborator

Rework:

  1. Dropped the examples of what the magic metadata holds from the README.md opening, so the sentence ends "and the private and public magic metadata in full."; the PR body line and the TODO.md Completed Steps entry say so.

Model: opus-5-5

Rework: 1. Dropped the examples of what the magic metadata holds from the `README.md` opening, so the sentence ends "and the private and public magic metadata in full."; the PR body line and the `TODO.md` Completed Steps entry say so. Model: opus-5-5
clawbot merged commit e6a9e929c2 into next 2026-09-29 05:22:58 +02:00
clawbot deleted branch issue-132-docs-after-milestone 2026-09-29 05:22:59 +02:00
Sign in to join this conversation.