fix: make config constants env-configurable, fix deploy re-sync gap#1677
Draft
simple-agent-manager[bot] wants to merge 9 commits into
Draft
fix: make config constants env-configurable, fix deploy re-sync gap#1677simple-agent-manager[bot] wants to merge 9 commits into
simple-agent-manager[bot] wants to merge 9 commits into
Conversation
- Fix deploy-reusable.yml re-sync step missing 16 optional env vars (container/sandbox overrides silently lost on first deploys) - Make NODE_LIFECYCLE_ALARM_RETRY_MS env-configurable (was documented as configurable but was a compile-time constant) - Make WORKSPACE_IDLE_CHECK_INTERVAL_MS env-configurable (all sibling constants had env overrides, this one did not) - Make MAX_NOTIFICATION_PAGE_SIZE env-configurable (was documented as configurable but was a compile-time constant) - Move CF container constants to packages/shared/src/constants/cf-container.ts (were defined locally in DO file, bypassing shared constants pattern) - Update configuration.md to document WORKSPACE_IDLE_CHECK_INTERVAL_MS Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nc parity - config-env-resolution: verifies DEFAULT_* constant values and parseInt-with-fallback env var resolution pattern including computeIdleAlarmTimes env override - deploy-resync-env-parity: regression test ensuring the re-sync step in deploy-reusable.yml forwards every optional env var from the initial sync step (prevents the 16-var gap from recurring) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Notification DO has its own local Env type separate from the Worker Env. The new env var override was added to the Worker Env but not the DO's local type, causing a typecheck failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ating Assert the exact resolved value (baseTime + envOverrideMs) instead of just asserting non-null. This discriminates between the env override (600000ms / 10min) and the default (300000ms / 5min). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- HIGH: add WORKSPACE_IDLE_CHECK_INTERVAL_MS and MAX_NOTIFICATION_PAGE_SIZE to .env.example (the documented 'complete list') - MEDIUM: align MAX_NOTIFICATION_PAGE_SIZE resolution with the isFinite && > 0 guard pattern used by sibling resolvers - LOW: fix Prettier table formatting in configuration.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ioral DO test - Fix import sort lint errors in notification.ts and idle-cleanup.ts - Replace tautological resolveMs() hand-copy test with real computeIdleAlarmTimes edge case tests (NaN, zero, negative, valid override) that exercise actual code - Add NodeLifecycle DO integration test proving alarm retry interval uses DEFAULT_NODE_LIFECYCLE_ALARM_RETRY_MS through the real alarm handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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.



Summary
NODE_LIFECYCLE_ALARM_RETRY_MS,WORKSPACE_IDLE_CHECK_INTERVAL_MS,MAX_NOTIFICATION_PAGE_SIZEDEFAULT_*prefix per Constitution Principle XIChanges
Deploy Pipeline
.github/workflows/deploy-reusable.yml: Add 16 missing env vars to re-sync step so first-deploy and subsequent deploys forward the same operator configurationRuntime Config
apps/api/src/durable-objects/node-lifecycle.ts: AddgetAlarmRetryMs()with env var resolution (same pattern as existinggetWarmTimeoutMs())apps/api/src/durable-objects/project-data/idle-cleanup.ts: Add env param tocomputeIdleAlarmTimes()forWORKSPACE_IDLE_CHECK_INTERVAL_MSresolutionapps/api/src/durable-objects/notification.ts: AlignMAX_NOTIFICATION_PAGE_SIZEresolution withisFinite && > 0guard patternapps/api/src/env.ts: Add 3 new optional env var fields to Worker Env typeNodeLifecycleandProjectDataandNotificationShared Constants
packages/shared/src/constants/cf-container.ts: New file — CF container constants moved from DO filepackages/shared/src/constants/node-pooling.ts: RenameWORKSPACE_IDLE_CHECK_INTERVAL_MS→DEFAULT_WORKSPACE_IDLE_CHECK_INTERVAL_MSpackages/shared/src/constants/notifications.ts: RenameMAX_NOTIFICATION_PAGE_SIZE→DEFAULT_MAX_NOTIFICATION_PAGE_SIZETests
config-env-resolution.test.ts: Default value assertions + realcomputeIdleAlarmTimesedge cases (NaN, zero, negative, valid override)deploy-resync-env-parity.test.ts: Regression test parsing deploy YAML to ensure re-sync step forwards every env var from initial syncnode-lifecycle-do.test.ts: Behavioral DO test verifying alarm retry usesDEFAULT_NODE_LIFECYCLE_ALARM_RETRY_MSthrough real alarm handlerDocs
configuration.md: AddWORKSPACE_IDLE_CHECK_INTERVAL_MSto documented env vars.env.example: Add example entries for new env varsBackward Compatibility
All changes are backward-compatible:
Agent Preflight (Required)
Classification
External References
N/A: Internal config/deploy remediation only. Referenced existing codebase patterns (getWarmTimeoutMs in node-lifecycle.ts) and Constitution Principle XI from .specify/memory/constitution.md.
Codebase Impact Analysis
apps/api/src/durable-objects/— NodeLifecycle, Notification, ProjectData DOs updated with env var resolutionapps/api/src/env.ts— 3 new optional env var fields added to Worker Env interfacepackages/shared/src/constants/— New cf-container.ts, renamed constants in node-pooling.ts and notifications.ts.github/workflows/deploy-reusable.yml— 16 env vars added to re-sync step for deploy parityapps/www/src/content/docs/docs/reference/configuration.md— New env var documented in operator referenceapps/api/.env.example— Added example entries for new configurable env varsDocumentation & Specs
apps/www/src/content/docs/docs/reference/configuration.md— AddedWORKSPACE_IDLE_CHECK_INTERVAL_MSto Warm Node Pooling tableapps/api/.env.example— Added example entries forWORKSPACE_IDLE_CHECK_INTERVAL_MSandMAX_NOTIFICATION_PAGE_SIZEConstitution & Risk Check
DEFAULT_*fallbacksSpecialist Review Evidence
Staging Verification
Skipped — config/constants/deploy changes only, no runtime behavior or UI changes. Deploy pipeline regression is covered by
deploy-resync-env-parity.test.ts.Test Plan
pnpm lintpasses (0 errors)pnpm typecheckpassespnpm testpasses (6362+ tests)pnpm buildpasses🤖 Generated with Claude Code
Task ID: 01KYC73DQFH22WH4AF09R9861C