Skip to content

Commit 8b03306

Browse files
CoderCococlaude
andauthored
chore(build): decommission embed-tfstate.mjs predev/prebuild hooks (#241)
Closes #164 ## Summary - Deletes `app/scripts/embed-tfstate.mjs` and removes the `predev`/`prebuild` npm hooks that invoked it — the desktop app reads tfstate directly at runtime via the AWS SDK, making the embed step unnecessary - Removes the `EMBEDDED_TFSTATE` fallback path from `ConfigService` (including the generated `src/generated/tfstate.ts` artifact and `.gitignore` entry for it) and updates the associated unit tests - Cleans up stale references in `CLAUDE.md`, `package.json`, `.github/workflows/package.yml`, `docs/`, and `scripts/init-parent.ts` ## Changes ``` .github/workflows/package.yml | 8 +- .gitignore | 4 - CLAUDE.md | 2 - app/package.json | 2 - app/packages/desktop-main/src/main.test.ts | 4 +- app/packages/desktop-main/src/services/ConfigService.test.ts | 39 --------- app/packages/desktop-main/src/services/ConfigService.ts | 10 +-- app/scripts/embed-tfstate.mjs | 87 ---------------------- docs/docs/components/management-app.md | 8 +- docs/docs/guides/submodule.md | 2 +- package.json | 3 +- scripts/init-parent.ts | 5 +- 12 files changed, 17 insertions(+), 157 deletions(-) ``` ## Test plan - [ ] `npm run app:test` — all unit tests pass (`ConfigService` tests no longer exercise the embedded-tfstate path) - [ ] `npm run app:lint` — 0 errors - [ ] `app/scripts/embed-tfstate.mjs` no longer exists in the repository - [ ] `app/package.json` contains no `predev` or `prebuild` scripts referencing `embed-tfstate` - [ ] `ConfigService` contains no reference to `EMBEDDED_TFSTATE` or `src/generated/tfstate.ts` - [ ] `npm run desktop:dev` starts without the embed step (no `embed-tfstate.mjs` execution) - [ ] `npm run desktop:build` completes without the embed step 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent aabc216 commit 8b03306

12 files changed

Lines changed: 71 additions & 162 deletions

File tree

.github/workflows/package.yml

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,18 @@ jobs:
4848
# terraform/terraform.tfstate is gitignored and will never exist in a
4949
# clean CI checkout. electron-builder silently skips a missing
5050
# extraResources 'from' path, which would produce an installer that
51-
# lacks the tfstate file entirely. Create an empty placeholder so
52-
# electron-builder always includes the file. ConfigService falls back
53-
# to the build-time-embedded tfstate.ts when the runtime file is empty,
54-
# so shipping an empty state file is safe for CI-produced installers.
51+
# lacks the tfstate file entirely. Create a null placeholder so
52+
# electron-builder always includes the file. ConfigService.getTfOutputs()
53+
# returns null for any state file that lacks an 'outputs' key (including
54+
# 'null' and '{}'), so the app correctly shows "not deployed" until the
55+
# operator provides a real state file. Using 'null' here makes the intent
56+
# explicit and avoids any risk of the placeholder being mistaken for a
57+
# valid (all-defaults) state.
5558
shell: bash
5659
run: |
5760
if [ ! -f terraform/terraform.tfstate ]; then
58-
echo "terraform/terraform.tfstate not found — creating empty placeholder for packaging."
59-
echo '{}' > terraform/terraform.tfstate
61+
echo "terraform/terraform.tfstate not found — creating null placeholder for packaging."
62+
echo 'null' > terraform/terraform.tfstate
6063
fi
6164
6265
- name: Build workspace packages

.gitignore

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ Thumbs.db
3030
.claude/worktrees/
3131
.worktrees/
3232

33-
# Auto-generated at dev/build time — committed as null, real state must not
34-
# be committed (contains AWS ARNs/account IDs).
35-
app/packages/desktop-main/src/generated/tfstate.ts
36-
3733
# Playwright artifacts (traces, videos, screenshots) and HTML report.
3834
app/packages/web/test-results/
3935
app/packages/web/playwright-report/

CLAUDE.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ When adding a game, only edit `terraform.tfvars`. Don't hand-write new resources
109109

110110
`ConfigService.getTfOutputs()` (in `app/packages/desktop-main/src/services/ConfigService.ts`) parses `terraform.tfstate` as JSON and caches it in-memory. `invalidateCache()` is called on `/api/games` and `/api/status` to pick up new deploys. The app's container mounts `./terraform:/app/terraform:ro` — this path coupling matters if directory structure changes. The parsed `TfOutputs` shape now also exposes `discord_table_name`, `discord_bot_token_secret_arn`, `discord_public_key_secret_arn`, and `interactions_invoke_url` so `DiscordConfigService` can reach the Discord stores without extra env-var plumbing.
111111

112-
**Build-time state embedding**: `app/scripts/embed-tfstate.mjs` (runs via `predev`/`prebuild` hooks) writes `app/packages/desktop-main/src/generated/tfstate.ts`; `ConfigService` uses it as a fallback when the runtime `terraform.tfstate` is absent (Docker/CI). The file is committed as `null` and overwritten at dev/build time.
113-
114112
### API authentication
115113

116114
Every `/api/*` route is gated behind a bearer token via `ApiTokenGuard` in `app/packages/desktop-main/src/guards/api-token.guard.ts`, registered globally in `AppModule` as an `APP_GUARD` provider so it applies to every controller automatically. The token comes from env `API_TOKEN` (wins, even when set to empty to deliberately disable) or `api_token` in `server_config.json`. In production (`NODE_ENV=production`), boot aborts in `main.ts` if no token is configured. In dev, a warning is logged and unauthenticated requests are allowed for convenience. The web client stores the token in `localStorage` under key `apiToken` and sends it as `Authorization: Bearer`. Don't remove the guard or bypass it on individual controllers — Copilot flagged the unauthenticated surface as a security issue and this is the fix.

app/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
"private": true,
55
"type": "module",
66
"scripts": {
7-
"predev": "node scripts/embed-tfstate.mjs",
87
"dev": "electron-vite dev --config ../electron.vite.config.ts",
9-
"prebuild": "node scripts/embed-tfstate.mjs",
108
"build": "npm run build -w @hyveon/shared && npm run build -w @hyveon/desktop-main && npm run build -w @hyveon/web",
119
"build:lambdas": "npm run build -w @hyveon/shared && npm run build -w @hyveon/lambda-interactions -w @hyveon/lambda-followup -w @hyveon/lambda-update-dns -w @hyveon/lambda-watchdog -w @hyveon/lambda-efs-seeder",
1210
"start": "electron ../out/main/index.js",

app/packages/desktop-main/src/main.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ vi.mock('@nestjs/core', () => ({
4141
}));
4242

4343
/**
44-
* Stub AppModule so the deep Nest module graph (which requires the generated
45-
* tfstate file and AWS service dependencies) is never traversed during this
44+
* Stub AppModule so the deep Nest module graph (which pulls in AWS service
45+
* dependencies and their transitive imports) is never traversed during this
4646
* unit test.
4747
*/
4848
vi.mock('./app.module.js', () => ({

app/packages/desktop-main/src/services/ConfigService.test.ts

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,6 @@ vi.mock('../logger.js', () => ({
1717
},
1818
}));
1919

20-
/**
21-
* Mutable holder for the build-time embedded Terraform state.
22-
* Tests that exercise the EMBEDDED_TFSTATE fallback path set this before
23-
* calling `getTfOutputs()`; all other tests leave it as `null` so they
24-
* don't accidentally exercise the fallback.
25-
*/
26-
let mockEmbeddedState: Record<string, unknown> | null = null;
27-
28-
vi.mock('../generated/tfstate.js', () => ({
29-
get EMBEDDED_TFSTATE() {
30-
return mockEmbeddedState;
31-
},
32-
}));
33-
3420
import { readFileSync, writeFileSync, existsSync } from 'fs';
3521
import { ConfigService } from './ConfigService.js';
3622

@@ -53,35 +39,24 @@ describe('ConfigService', () => {
5339

5440
beforeEach(() => {
5541
service = new ConfigService();
56-
mockEmbeddedState = null;
5742
});
5843

5944
describe('getTfOutputs', () => {
60-
it('should return null when both the state file and embedded state are absent', () => {
45+
it('should return null when the state file is absent', () => {
6146
mockExists.mockReturnValue(false);
6247
expect(service.getTfOutputs()).toBeNull();
6348
});
6449

65-
it('should use EMBEDDED_TFSTATE as fallback when the state file is absent', () => {
66-
mockEmbeddedState = {
67-
outputs: {
68-
aws_region: { value: 'us-west-1' },
69-
game_names: { value: ['minecraft'] },
70-
},
71-
};
72-
mockExists.mockReturnValue(false);
73-
const outputs = service.getTfOutputs();
74-
expect(outputs).not.toBeNull();
75-
expect(outputs!.aws_region).toBe('us-west-1');
76-
expect(outputs!.game_names).toEqual(['minecraft']);
77-
expect(outputs!.subnet_ids).toBe('');
50+
it('should return null when the state file contains the literal null', () => {
51+
mockExists.mockReturnValue(true);
52+
mockRead.mockReturnValue('null');
53+
expect(service.getTfOutputs()).toBeNull();
7854
});
7955

80-
it('should prefer the runtime state file over EMBEDDED_TFSTATE when both are present', () => {
81-
mockEmbeddedState = { outputs: { aws_region: { value: 'embedded-region' } } };
56+
it('should return null when the state file has no outputs key', () => {
8257
mockExists.mockReturnValue(true);
83-
mockRead.mockReturnValue(makeState({ aws_region: { value: 'runtime-region' } }));
84-
expect(service.getTfOutputs()!.aws_region).toBe('runtime-region');
58+
mockRead.mockReturnValue('{}');
59+
expect(service.getTfOutputs()).toBeNull();
8560
});
8661

8762
it('should parse outputs and fill defaults for missing keys', () => {
@@ -120,6 +95,26 @@ describe('ConfigService', () => {
12095
expect(mockRead).toHaveBeenCalledTimes(1);
12196
});
12297

98+
it('should cache a null result so an undeployed stack is not re-read on every call', () => {
99+
mockExists.mockReturnValue(false);
100+
101+
expect(service.getTfOutputs()).toBeNull();
102+
expect(service.getTfOutputs()).toBeNull();
103+
104+
expect(mockExists).toHaveBeenCalledTimes(1);
105+
});
106+
107+
it('should re-read after invalidateCache when the previous result was null', () => {
108+
mockExists.mockReturnValue(false);
109+
expect(service.getTfOutputs()).toBeNull();
110+
111+
service.invalidateCache();
112+
mockExists.mockReturnValue(true);
113+
mockRead.mockReturnValue(makeState({ aws_region: { value: 'eu-west-1' } }));
114+
115+
expect(service.getTfOutputs()!.aws_region).toBe('eu-west-1');
116+
});
117+
123118
it('should force a re-read after invalidateCache', () => {
124119
mockExists.mockReturnValue(true);
125120
mockRead.mockReturnValue(makeState({ aws_region: { value: 'a' } }));

app/packages/desktop-main/src/services/ConfigService.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { join, dirname } from 'path';
44
import { fileURLToPath } from 'url';
55
import { createRequire } from 'module';
66
import { logger } from '../logger.js';
7-
import { EMBEDDED_TFSTATE } from '../generated/tfstate.js';
87

98
/** Absolute path to the `dist/services/` directory at runtime. */
109
const _dirname = dirname(fileURLToPath(import.meta.url));
@@ -79,26 +78,31 @@ const DEFAULT_CONFIG: WatchdogConfig = {
7978
*/
8079
@Injectable()
8180
export class ConfigService {
82-
private tfCache: TfOutputs | null = null;
81+
/**
82+
* Memoised tfstate projection. Tri-state: `undefined` means "not loaded yet",
83+
* `null` means "loaded, but no usable state" (absent/empty/placeholder), and
84+
* an object is a parsed projection. Caching `null` matters because callers on
85+
* polling paths (e.g. status) hit this every tick — without it, an undeployed
86+
* stack would re-read the file and re-log a warning on every call.
87+
*/
88+
private tfCache: TfOutputs | null | undefined = undefined;
8389

8490
/**
8591
* Drop the cached tfstate parse. Called from the `/api/games` and
8692
* `/api/status` handlers so a fresh `terraform apply` is picked up without
8793
* a server restart; tests also call it between scenarios.
8894
*/
8995
invalidateCache(): void {
90-
this.tfCache = null;
96+
this.tfCache = undefined;
9197
}
9298

9399
/**
94100
* Parse `terraform/terraform.tfstate` (once, then memoised) and project the
95-
* pieces the app cares about. Falls back to the state embedded at build time
96-
* by `scripts/embed-tfstate.mjs` when the runtime file is absent. Returns
97-
* `null` when neither source is available — callers treat that as "infra not
98-
* deployed yet" and degrade gracefully.
101+
* pieces the app cares about. Returns `null` when the runtime file is absent
102+
* — callers treat that as "infra not deployed yet" and degrade gracefully.
99103
*/
100104
getTfOutputs(): TfOutputs | null {
101-
if (this.tfCache) return this.tfCache;
105+
if (this.tfCache !== undefined) return this.tfCache;
102106

103107
type RawState = { outputs?: Record<string, { value: unknown }> };
104108
let raw: RawState;
@@ -109,18 +113,24 @@ export class ConfigService {
109113
raw = JSON.parse(readFileSync(tfStatePath, 'utf-8')) as RawState;
110114
} catch (err) {
111115
logger.error('Failed to parse Terraform state', { err, path: tfStatePath });
112-
return null;
116+
return (this.tfCache = null);
117+
}
118+
if (raw === null || raw === undefined) {
119+
logger.warn('Terraform state file is empty or null', { path: tfStatePath });
120+
return (this.tfCache = null);
113121
}
114-
} else if (EMBEDDED_TFSTATE) {
115-
logger.debug('Using build-time embedded Terraform state');
116-
raw = EMBEDDED_TFSTATE as unknown as RawState;
117122
} else {
118123
logger.warn('Terraform state not found', { path: tfStatePath });
119-
return null;
124+
return (this.tfCache = null);
120125
}
121126

122127
try {
123-
const out = raw.outputs ?? {};
128+
if (!raw.outputs) {
129+
logger.warn('Terraform state has no outputs — infra not yet deployed', { path: tfStatePath });
130+
return (this.tfCache = null);
131+
}
132+
133+
const out = raw.outputs;
124134
const get = <T>(key: string, fallback: T): T =>
125135
key in out ? (out[key]!.value as T) : fallback;
126136

@@ -148,7 +158,7 @@ export class ConfigService {
148158
return this.tfCache;
149159
} catch (err) {
150160
logger.error('Failed to parse Terraform state', { err });
151-
return null;
161+
return (this.tfCache = null);
152162
}
153163
}
154164

app/scripts/embed-tfstate.mjs

Lines changed: 0 additions & 87 deletions
This file was deleted.

docs/docs/components/management-app.md

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,8 @@ Every route is under `/api/*` and gated by `ApiTokenGuard`.
8787
on list/status so a new `terraform apply` is picked up without a server
8888
restart. Also resolves the bearer token from `API_TOKEN` (wins) or
8989
`server_config.json:api_token`. State resolution order: (1) runtime
90-
`terraform/terraform.tfstate`; (2) build-time embedded state from
91-
`app/packages/desktop-main/src/generated/tfstate.ts` (written by
92-
`app/scripts/embed-tfstate.mjs` via the `predev`/`prebuild` npm hooks
93-
— useful in Docker/CI where the Terraform directory isn't mounted);
94-
(3) `null` — callers degrade gracefully so the dashboard can render
95-
even pre-apply.
90+
`terraform/terraform.tfstate`; (2) `null` — callers degrade gracefully
91+
so the dashboard can render even pre-apply.
9692
- **`DiscordConfigService`** — persistence facade over DynamoDB
9793
(`CONFIG#discord`) + Secrets Manager. Concurrent reads are coalesced via
9894
an inflight-promise pattern. `getRedacted()` returns

docs/docs/guides/submodule.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ Five targets, no surprises:
116116
| `make plan` | Copies `terraform.tfvars` into `Hyveon/terraform/terraform.tfvars`, then runs `make -C Hyveon tf-plan` — which itself rebuilds the Lambda bundles before `terraform plan`. |
117117
| `make apply` | Same as `plan`, but delegates to `tf-apply`. The submodule's `tf-apply` recipe prints a post-deploy checklist with the Discord interactions URL when it finishes. |
118118
| `make update` | Bumps the submodule to the tip of `main` (`git submodule update --remote --merge`). If the new `setup.sh` differs from the recorded sha, clears `.terraform/` and re-runs `setup.sh` automatically; otherwise leaves it alone. Reminds you to commit the new submodule pointer. |
119-
| `make dev` | Pulls live tfstate into `.make/tfstate.json` (so the embed step has something to read), wipes stale TS build info under the submodule's `app/packages/*/`, then runs `make -C Hyveon dev`, exporting `API_TOKEN` and `TF_STATE_PATH` to the child make. |
119+
| `make dev` | Pulls live tfstate into `.make/tfstate.json` (so ConfigService can read it via `TF_STATE_PATH`), wipes stale TS build info under the submodule's `app/packages/*/`, then runs `make -C Hyveon dev`, exporting `API_TOKEN` and `TF_STATE_PATH` to the child make. |
120120

121121
The `tfvars` copy is **always fresh** on plan/apply — the recipe `cp`s
122122
unconditionally, not just when the file is older than the destination. This

0 commit comments

Comments
 (0)