celilo/lunacycle

Fork 0

Hooks write smoke artifacts to `/tmp/lunacycle-smoke`, which the hook jail turns into a lying diagnostic — move them to `state/` #64

New issue

Closed

opened 2026-08-22 15:23:07 +00:00 by forgejo-admin · 2 comments

forgejo-admin commented

2026-08-22 15:23:07 +00:00

Owner

celilo/scripts/health-check.ts:57 and celilo/scripts/smoke-handler.ts:49 both define:

const SPA_ARTIFACT_DIR = '/tmp/lunacycle-smoke';

and pass it as artifactDir (health-check.ts:420, smoke-handler.ts:103). When the smoke check fails, health-check.ts:433 tells the operator where to look:

(post-mortem screenshot/DOM/requests in /tmp/lunacycle-smoke on the celilo host)

Why this breaks, and why it breaks silently

celilo is putting module hooks inside a jail (celilo/celilo#1001, openspec/changes/hook-process-boundary). Stage 2 derives each hook's mount set from its hook context, and /tmp becomes a fresh tmpfs per run.

Nothing errors. The mkdir succeeds, the screenshot writes, the hook reports success, and the sentence at health-check.ts:433 still prints. The artifacts are gone the moment the hook process exits, so an operator follows a message naming a directory that no longer exists and finds nothing.

That is worse than a failure. A diagnostic path that lies is consulted exactly when something is already wrong, and it costs the reader their first hour.

The sanctioned destination now exists

Until today a hook had nowhere to write. Anything it left beside itself failed module audit as extra, and the only reason screenshots/ and cookies.json did not was a hardcoded allow-list of paths someone had already been bitten by.

celilo/celilo#1000 closed that. A module now has a state/ directory in its own root, classified derived, which means writable, survives module update, not audited, not pruned. It landed on main in apps/celilo/src/module/packaging/package-rules.ts:105.

This module is the first real consumer of it.

Scope

celilo/scripts/health-check.ts:57 — SPA_ARTIFACT_DIR.
celilo/scripts/health-check.ts:420 — where it is passed as artifactDir.
celilo/scripts/health-check.ts:433 — the operator-facing sentence, which has to name the new location.
celilo/scripts/smoke-handler.ts:49,103 — the same constant and the same use. Two files define it independently, which is its own small defect: fix that by defining it once.

Acceptance

Both files resolve the artifact directory from the hook context's state/ rather than a hardcoded /tmp path, and the constant is defined once rather than twice.
health-check.ts:433's message names the real location, so it stays true under the jail.
Artifacts from a failed run survive the hook process exiting, and a following module audit is clean. Both halves matter: the point of state/ is that it is writable and not reported as extra.
Decide deliberately whether these artifacts should be retained or bounded. state/ survives module update by construction, which is right for a cursor and arguably wrong for an unbounded pile of screenshots. If they need bounding, bound them here rather than discovering it at restore, since celilo's backup picks state/ up through its existing per-module walk.

celilo/celilo#1000 — the state/ decision and the one-line change that shipped it.
celilo/celilo#1001 — the hook process boundary, whose stage 2 makes /tmp a fresh tmpfs.

Found while answering task 1.5 of hook-process-boundary, which asks whether any lunacycle hook reads outside its module tree. Two other findings from that sweep, recorded here so they are not re-derived:

Environment variables: none. Zero process.env and zero Bun.env across every non-test hook script. The jail's environment allow-list costs this module nothing.

build_artifacts_dir is a separate problem and is celilo's to solve, not lunacycle's. setup-web.ts:125 reads it from module config and then reads and uploads from under it. It is an arbitrary operator-supplied absolute path on the management host, and the jail's mount set is derived from the hook context, so no rule can predict it. celilo is adding a path type to variable declarations so a declared path config value becomes a mount input. Nothing to change here until that lands.

`celilo/scripts/health-check.ts:57` and `celilo/scripts/smoke-handler.ts:49` both define: ```ts const SPA_ARTIFACT_DIR = '/tmp/lunacycle-smoke'; ``` and pass it as `artifactDir` (`health-check.ts:420`, `smoke-handler.ts:103`). When the smoke check fails, `health-check.ts:433` tells the operator where to look: ``` (post-mortem screenshot/DOM/requests in /tmp/lunacycle-smoke on the celilo host) ``` ## Why this breaks, and why it breaks silently celilo is putting module hooks inside a jail (celilo/celilo#1001, `openspec/changes/hook-process-boundary`). Stage 2 derives each hook's mount set from its hook context, and **`/tmp` becomes a fresh tmpfs per run**. Nothing errors. The `mkdir` succeeds, the screenshot writes, the hook reports success, and the sentence at `health-check.ts:433` still prints. The artifacts are gone the moment the hook process exits, so an operator follows a message naming a directory that no longer exists and finds nothing. That is worse than a failure. A diagnostic path that lies is consulted exactly when something is already wrong, and it costs the reader their first hour. ## The sanctioned destination now exists Until today a hook had nowhere to write. Anything it left beside itself failed `module audit` as `extra`, and the only reason `screenshots/` and `cookies.json` did not was a hardcoded allow-list of paths someone had already been bitten by. celilo/celilo#1000 closed that. A module now has a **`state/`** directory in its own root, classified `derived`, which means writable, survives `module update`, not audited, not pruned. It landed on `main` in `apps/celilo/src/module/packaging/package-rules.ts:105`. This module is the first real consumer of it. ## Scope - `celilo/scripts/health-check.ts:57` — `SPA_ARTIFACT_DIR`. - `celilo/scripts/health-check.ts:420` — where it is passed as `artifactDir`. - `celilo/scripts/health-check.ts:433` — the operator-facing sentence, which has to name the new location. - `celilo/scripts/smoke-handler.ts:49,103` — the same constant and the same use. Two files define it independently, which is its own small defect: fix that by defining it once. ## Acceptance - [ ] Both files resolve the artifact directory from the hook context's `state/` rather than a hardcoded `/tmp` path, and the constant is defined once rather than twice. - [ ] `health-check.ts:433`'s message names the real location, so it stays true under the jail. - [ ] Artifacts from a failed run survive the hook process exiting, and a following `module audit` is clean. Both halves matter: the point of `state/` is that it is writable **and** not reported as `extra`. - [ ] Decide deliberately whether these artifacts should be retained or bounded. `state/` survives `module update` by construction, which is right for a cursor and arguably wrong for an unbounded pile of screenshots. If they need bounding, bound them here rather than discovering it at restore, since celilo's backup picks `state/` up through its existing per-module walk. ## Related - celilo/celilo#1000 — the `state/` decision and the one-line change that shipped it. - celilo/celilo#1001 — the hook process boundary, whose stage 2 makes `/tmp` a fresh tmpfs. Found while answering task 1.5 of `hook-process-boundary`, which asks whether any lunacycle hook reads outside its module tree. Two other findings from that sweep, recorded here so they are not re-derived: **Environment variables: none.** Zero `process.env` and zero `Bun.env` across every non-test hook script. The jail's environment allow-list costs this module nothing. **`build_artifacts_dir` is a separate problem and is celilo's to solve, not lunacycle's.** `setup-web.ts:125` reads it from module config and then reads and uploads from under it. It is an arbitrary operator-supplied absolute path on the management host, and the jail's mount set is derived from the hook context, so no rule can predict it. celilo is adding a path type to variable declarations so a declared path config value becomes a mount input. Nothing to change here until that lands.

forgejo-admin commented

2026-08-22 15:38:20 +00:00

Author

Owner

Already fixed on main, four days before this issue was filed. No code change needed.

bc8ea9b ("feat(smoke): launch celilo's provisioned browser, and keep the artifacts", 2026-08-18) deleted both copies of SPA_ARTIFACT_DIR and moved both call sites onto the framework. This issue was written against the line numbers from before that commit, which is why health-check.ts:57 and smoke-handler.ts:49 no longer say what it quotes.

Where the artifacts go now

The hook takes the directory from its context. health-check.ts:602 destructures screenshotDir off HookContext and passes it in as artifactDir:

handler: async ({ config, systems, logger, secrets, screenshotDir }) =>
  lunacycleHealthCheck({ ..., artifactDir: screenshotDir }),

The bus subscriber genuinely has no HookContext, so it calls celilo's own definition of that layout directly (smoke-handler.ts:122):

const runKey = `smoke-${process.argv[2] ?? Date.now()}`;
const artifactDir = moduleArtifactDir(MODULE_PATH, runKey);

moduleArtifactDir is a public export of @celilo/capabilities (packages/capabilities/src/index.ts:85), and it is the same function celilo's own hook executor calls to build screenshotDir (apps/celilo/src/hooks/executor.ts:630). Both sites therefore land in one place: <modulePath>/screenshots/<runKey>. Nothing is hand-rolled and nothing is hardcoded.

Acceptance, item by item

Resolved from the hook context, constant defined once. Met, and more strongly than asked. The constant is not defined once, it is defined zero times. Both sites derive the path from the framework, so there is no literal left to drift.

The operator-facing sentence names the real location. Met. Both messages interpolate the resolved value rather than a literal, so the sentence is true by construction:

(post-mortem screenshot/DOM/requests in ${artifactDir} on the celilo host)

Artifacts survive the process exiting, and module audit stays clean. Met on both halves, but via screenshots/ rather than state/.

Survives: <modulePath>/screenshots/ is in the module store on disk, not /tmp. hook-process-boundary task 4.2 already names this run's screenshots/<run> in the read-write mount set alongside state/ and generated/, so the jail carries it.
Audit clean: package-rules.ts:105 classifies screenshots as derived, in the same condition as state.

Deliberate decision on retention. Bounded, and celilo does the bounding — this module needs no policy of its own. apps/celilo/src/hooks/artifact-retention.ts prunes by mtime: ARTIFACT_RETENTION_MS is 24 hours, with a 64 MB per-module size ceiling as a backstop. The executor calls pruneModuleArtifacts() on every hook run and discards the run directory entirely when the hook wrote nothing. The subscriber's directories share the same artifact root, so a health-check run prunes them too.

This is the part where screenshots/ is the better destination and state/ would have been the wrong one. The issue flags the risk itself: state/ is "not pruned", which is right for a cursor and wrong for an unbounded pile of screenshots, and celilo's backup walks state/ per module. Putting failure screenshots there would have put an unbounded pile into every backup. screenshots/ is the directory that already has the age-based retention this data wants.

Gates

From a clean worktree at 644bf61:

bun test in celilo/scripts — 69 pass, 0 fail
tsc -p celilo/scripts/tsconfig.json --noEmit — exit 0

The only surviving mention of the old path is a doc comment at health-check.ts:95, which describes it accurately in the past tense as the thing that was replaced.

One note for celilo, not actionable here

There is still no stateDir on HookContext. celilo#1000 shipped the state/ classification without a surface, so module audit tolerates the directory but nothing tells a hook its path. That did not block this issue, because the data in question is artifacts and screenshotDir already covers artifacts. It would block the first hook that wants durable cursor-like state. Raised at celilo/celilo#1000, not lunacycle's to fix.

Closing as already resolved.

Already fixed on `main`, four days before this issue was filed. No code change needed. `bc8ea9b` ("feat(smoke): launch celilo's provisioned browser, and keep the artifacts", 2026-08-18) deleted both copies of `SPA_ARTIFACT_DIR` and moved both call sites onto the framework. This issue was written against the line numbers from before that commit, which is why `health-check.ts:57` and `smoke-handler.ts:49` no longer say what it quotes. ## Where the artifacts go now The hook takes the directory from its context. `health-check.ts:602` destructures `screenshotDir` off `HookContext` and passes it in as `artifactDir`: ```ts handler: async ({ config, systems, logger, secrets, screenshotDir }) => lunacycleHealthCheck({ ..., artifactDir: screenshotDir }), ``` The bus subscriber genuinely has no `HookContext`, so it calls celilo's own definition of that layout directly (`smoke-handler.ts:122`): ```ts const runKey = `smoke-${process.argv[2] ?? Date.now()}`; const artifactDir = moduleArtifactDir(MODULE_PATH, runKey); ``` `moduleArtifactDir` is a public export of `@celilo/capabilities` (`packages/capabilities/src/index.ts:85`), and it is the same function celilo's own hook executor calls to build `screenshotDir` (`apps/celilo/src/hooks/executor.ts:630`). Both sites therefore land in one place: `<modulePath>/screenshots/<runKey>`. Nothing is hand-rolled and nothing is hardcoded. ## Acceptance, item by item **Resolved from the hook context, constant defined once.** Met, and more strongly than asked. The constant is not defined once, it is defined zero times. Both sites derive the path from the framework, so there is no literal left to drift. **The operator-facing sentence names the real location.** Met. Both messages interpolate the resolved value rather than a literal, so the sentence is true by construction: ``` (post-mortem screenshot/DOM/requests in ${artifactDir} on the celilo host) ``` **Artifacts survive the process exiting, and `module audit` stays clean.** Met on both halves, but via `screenshots/` rather than `state/`. - *Survives:* `<modulePath>/screenshots/` is in the module store on disk, not `/tmp`. `hook-process-boundary` task 4.2 already names `this run's screenshots/<run>` in the read-write mount set alongside `state/` and `generated/`, so the jail carries it. - *Audit clean:* `package-rules.ts:105` classifies `screenshots` as `derived`, in the same condition as `state`. **Deliberate decision on retention.** Bounded, and celilo does the bounding — this module needs no policy of its own. `apps/celilo/src/hooks/artifact-retention.ts` prunes by mtime: `ARTIFACT_RETENTION_MS` is 24 hours, with a 64 MB per-module size ceiling as a backstop. The executor calls `pruneModuleArtifacts()` on every hook run and discards the run directory entirely when the hook wrote nothing. The subscriber's directories share the same artifact root, so a health-check run prunes them too. This is the part where `screenshots/` is the better destination and `state/` would have been the wrong one. The issue flags the risk itself: `state/` is "not pruned", which is right for a cursor and wrong for an unbounded pile of screenshots, and celilo's backup walks `state/` per module. Putting failure screenshots there would have put an unbounded pile into every backup. `screenshots/` is the directory that already has the age-based retention this data wants. ## Gates From a clean worktree at `644bf61`: - `bun test` in `celilo/scripts` — 69 pass, 0 fail - `tsc -p celilo/scripts/tsconfig.json --noEmit` — exit 0 The only surviving mention of the old path is a doc comment at `health-check.ts:95`, which describes it accurately in the past tense as the thing that was replaced. ## One note for celilo, not actionable here There is still no `stateDir` on `HookContext`. `celilo#1000` shipped the `state/` *classification* without a *surface*, so `module audit` tolerates the directory but nothing tells a hook its path. That did not block this issue, because the data in question is artifacts and `screenshotDir` already covers artifacts. It would block the first hook that wants durable cursor-like state. Raised at celilo/celilo#1000, not lunacycle's to fix. Closing as already resolved.

forgejo-admin closed this issue

2026-08-22 15:38:24 +00:00

forgejo-admin referenced this issue from a commit

2026-08-22 15:57:21 +00:00

feat(hooks): ctx.stateDir, the surface celilo#1000 was missing

forgejo-admin referenced this issue

2026-08-22 20:26:36 +00:00

(disregard — created in error, closed immediately) #65

forgejo-admin commented

2026-08-22 20:27:01 +00:00

Author

Owner

Closing: already fixed before this was filed. PR #62, "feat(smoke): launch celilo's provisioned browser, and keep the artifacts", landed the change and I did not check main before writing the issue.

On origin/main today:

celilo/scripts/health-check.ts:604   handler: async ({ config, systems, logger, secrets, screenshotDir }) =>
celilo/scripts/health-check.ts:612       artifactDir: screenshotDir,

The hook takes screenshotDir from the HookContext and passes it through as artifactDir. SPA_ARTIFACT_DIR is gone. The only surviving mention of /tmp/lunacycle-smoke is a doc comment at health-check.ts:95 describing it in the past tense, which is accurate.

And the destination it chose is better than the one this issue proposed. The issue said move to state/. screenshots/ is the right home, for reasons that are properties of the two directories rather than of which name is newer:

screenshots/ is classified derived in the same condition as state (package-rules.ts:105), so module audit is equally clean either way.
It is already in the hook jail's read-write mount set, which lists screenshots/<run> beside state/ and generated/.
It carries the retention this data needs: celilo prunes by age, 24h by mtime with a 64 MB per-module ceiling, on every hook run.
state/ carries none of that. It is explicitly never pruned, and celilo's backup was believed to walk it per module. An unbounded pile of failure screenshots and DOM dumps is exactly the wrong thing to put somewhere with no ceiling.

That last point got sharper after this issue was written. state/ turns out not to be backed up at all — see celilo/celilo#1102. A module backup's envelope is manifest.json plus data/, and data/ holds only what the module's own on_backup hook wrote. Nothing walks the install tree. So state/ currently loses its contents on restore, silently. Sending smoke artifacts there would have been wrong twice over.

The two findings from the sweep that filed this are still worth keeping

They came from answering task 1.5 of celilo/celilo#1001, which asks whether any lunacycle hook reads outside its module tree or reads environment variables. Neither is affected by #62.

Environment variables: none. Zero process.env and zero Bun.env across every non-test script in celilo/scripts. The hook jail's environment allow-list costs this module nothing.

build_artifacts_dir is still an open problem, and it is celilo's, not lunacycle's. setup-web.ts:125 reads it from module config, then reads join(buildArtifactsDir, 'dist', 'apps', 'lunacycle-web', 'config.js') at :140 and uploads from :148. It is an arbitrary operator-supplied absolute path on the management host, and the jail derives its mount set from the hook context, so no rule can predict it. Under the jail as currently specified, on_install fails. celilo is fixing that by giving variable declarations a path type so a declared path config value becomes a mount input. Nothing to change here until that lands.

The remote paths are fine and need no action: /var/lib/lunacycle/db.sqlite (backup.ts:31, restore.ts:40) and /opt/lunacycle/{migrate.sh,migrations} (restore.ts:41-42) are all on the deployed system, reached over remote ops. The jail does not span the remote box.

**Closing: already fixed before this was filed.** PR #62, "feat(smoke): launch celilo's provisioned browser, and keep the artifacts", landed the change and I did not check `main` before writing the issue. On `origin/main` today: ``` celilo/scripts/health-check.ts:604 handler: async ({ config, systems, logger, secrets, screenshotDir }) => celilo/scripts/health-check.ts:612 artifactDir: screenshotDir, ``` The hook takes `screenshotDir` from the `HookContext` and passes it through as `artifactDir`. `SPA_ARTIFACT_DIR` is gone. The only surviving mention of `/tmp/lunacycle-smoke` is a doc comment at `health-check.ts:95` describing it in the past tense, which is accurate. **And the destination it chose is better than the one this issue proposed.** The issue said move to `state/`. `screenshots/` is the right home, for reasons that are properties of the two directories rather than of which name is newer: - `screenshots/` is classified `derived` in the same condition as `state` (`package-rules.ts:105`), so `module audit` is equally clean either way. - It is already in the hook jail's read-write mount set, which lists `screenshots/<run>` beside `state/` and `generated/`. - It carries the retention this data needs: celilo prunes by age, 24h by mtime with a 64 MB per-module ceiling, on every hook run. - `state/` carries none of that. It is explicitly never pruned, and celilo's backup was believed to walk it per module. An unbounded pile of failure screenshots and DOM dumps is exactly the wrong thing to put somewhere with no ceiling. That last point got sharper after this issue was written. `state/` turns out **not** to be backed up at all — see celilo/celilo#1102. A module backup's envelope is `manifest.json` plus `data/`, and `data/` holds only what the module's own `on_backup` hook wrote. Nothing walks the install tree. So `state/` currently loses its contents on restore, silently. Sending smoke artifacts there would have been wrong twice over. ## The two findings from the sweep that filed this are still worth keeping They came from answering task 1.5 of celilo/celilo#1001, which asks whether any lunacycle hook reads outside its module tree or reads environment variables. Neither is affected by #62. **Environment variables: none.** Zero `process.env` and zero `Bun.env` across every non-test script in `celilo/scripts`. The hook jail's environment allow-list costs this module nothing. **`build_artifacts_dir` is still an open problem, and it is celilo's, not lunacycle's.** `setup-web.ts:125` reads it from module config, then reads `join(buildArtifactsDir, 'dist', 'apps', 'lunacycle-web', 'config.js')` at `:140` and uploads from `:148`. It is an arbitrary operator-supplied absolute path on the management host, and the jail derives its mount set from the hook context, so no rule can predict it. Under the jail as currently specified, `on_install` fails. celilo is fixing that by giving variable declarations a path type so a declared path config value becomes a mount input. Nothing to change here until that lands. The remote paths are fine and need no action: `/var/lib/lunacycle/db.sqlite` (`backup.ts:31`, `restore.ts:40`) and `/opt/lunacycle/{migrate.sh,migrations}` (`restore.ts:41-42`) are all on the deployed system, reached over remote ops. The jail does not span the remote box.