Skip to content

Conversation

@llama90
Copy link
Contributor

@llama90 llama90 commented Dec 30, 2025

Summary

  • Implement BuildLockManager for distributed lock management
  • Integrate distributed locking in build worker to prevent duplicate builds
  • Add command configuration for lock requirements
  • Fix GitHub PAT retrieval from Parameter Store with caching

Key Changes

  • BuildLockManager: DynamoDB-based distributed locking with conditional writes
  • Build Worker Integration: Acquire lock before processing, release on completion/failure
  • User Notifications: Inform users when build is already in progress
  • GitHub PAT Fix: Use Parameter Store instead of environment variable with proper caching
  • Command Config: Centralized configuration for lock/cache requirements per command

Implementation Details

  • Lock key format: {command}-{component}-{environment} (e.g., build-router-plt)
  • Lock TTL: 10 minutes (auto-cleanup for stale locks)
  • Lock statuses: ACQUIRED, COMPLETED, FAILED
  • Conditional writes prevent race conditions

Test Plan

  • Test single user triggering build - should succeed
  • Test concurrent users triggering same build - second should see "already in progress"
  • Verify lock is released on successful build
  • Verify lock is released on failed build
  • Confirm TTL cleans up stale locks after 10 minutes

🤖 Generated with Claude Code

llama90 and others added 2 commits December 30, 2025 09:37
… var

- Add getGitHubToken() function to retrieve PAT from common environment
- GitHub PAT stored at /laco/cmn/github/pat/cloud-apps (cross-environment)
- Support local development with GITHUB_PAT_CLOUD_APPS env var
- Remove TODO comments and temporary GITHUB_TOKEN workaround

This fixes the SSM parameter access issue where the build worker
couldn't access the GitHub PAT needed for repository_dispatch API.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
## Changes

### New Shared Modules
- build-lock.ts: BuildLockManager with DynamoDB-based distributed locking
  - acquireLock(): Conditional write to acquire lock
  - releaseLock(): Mark as COMPLETED/FAILED
  - getLock(): Get current lock status
  - isLocked(): Check if operation in progress

- command-config.ts: Centralized command configuration
  - Define lock requirements per command
  - /build: requiresLock=true, TTL=10min
  - /deploy: requiresLock=true, TTL=30min
  - /status, /echo: requiresLock=false

### Updated Build Worker
- Import buildLockManager
- Acquire lock before processing
- If locked: notify user (ephemeral), skip processing
- If acquired: proceed with build
- Release lock on success/failure

## User Experience

Lock acquired:
  "🔨 Building router..." (visible to channel)

Lock held by another user:
  "⚠️ Build already in progress
   Started by: alice
   Started: 30s ago
   Please wait for the current build to complete." (ephemeral)

## Problem Solved
Prevents duplicate GitHub Actions triggers when multiple users
request the same build simultaneously.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
private tableName: string;

constructor() {
const config = getConfig();

Check failure

Code scanning / CodeQL

Invocation of non-function Error

Callee is not a function: it has type undefined.

Copilot Autofix

AI 15 days ago

In general, to fix “invocation of non‑function” errors, you either (1) ensure the imported/assigned value is actually a function, or (2) guard before calling it and handle the error path explicitly. Since we are constrained to changes within this file and cannot alter the ./config module or change existing imports, the appropriate approach is to validate getConfig at the call site and avoid calling it if it is not a function.

Concretely, in applications/chatops/slack-bot/src/shared/build-lock.ts, update the BuildLockManager constructor to:

  1. Check that getConfig is a function before invoking it.
  2. If it is not a function, throw a descriptive error (or log and throw) instead of calling it, preventing a cryptic “is not a function” crash.
  3. Optionally give a type-safe fallback (e.g., never after throwing) so TypeScript remains happy, but that is not strictly necessary if we just throw and then use the result.

The minimal change is to replace:

constructor() {
  const config = getConfig();
  this.tableName = `${config.orgPrefix}-${config.environment}-chatbot-build-locks`;
}

with a constructor that:

  • Validates typeof getConfig === 'function'.
  • Throws an Error if the validation fails.
  • Calls getConfig() only in the safe branch and uses the returned config as before.

No new imports are required; we can use the built‑in Error and existing logger if desired (but using Error alone is sufficient and keeps changes minimal).


Suggested changeset 1
applications/chatops/slack-bot/src/shared/build-lock.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/applications/chatops/slack-bot/src/shared/build-lock.ts b/applications/chatops/slack-bot/src/shared/build-lock.ts
--- a/applications/chatops/slack-bot/src/shared/build-lock.ts
+++ b/applications/chatops/slack-bot/src/shared/build-lock.ts
@@ -38,6 +38,9 @@
   private tableName: string;
 
   constructor() {
+    if (typeof getConfig !== 'function') {
+      throw new Error('getConfig is not a function. Ensure ./config exports a callable getConfig.');
+    }
     const config = getConfig();
     this.tableName = `${config.orgPrefix}-${config.environment}-chatbot-build-locks`;
   }
EOF
@@ -38,6 +38,9 @@
private tableName: string;

constructor() {
if (typeof getConfig !== 'function') {
throw new Error('getConfig is not a function. Ensure ./config exports a callable getConfig.');
}
const config = getConfig();
this.tableName = `${config.orgPrefix}-${config.environment}-chatbot-build-locks`;
}
Copilot is powered by AI and may make mistakes. Always verify output.
llama90 added a commit that referenced this pull request Jan 10, 2026
Replace command-specific workers with unified quadrant workers for
better scalability and maintainability.

**Workers (routing layer):**
- Remove: echo, build, deploy, status workers (command-specific)
- Add: SR (short-read) and LW (long-write) unified workers
- SR worker handles: /echo and future fast read commands
- LW worker handles: /build, /deploy and future write commands

**Handlers (business logic layer):**
- Extract command logic into reusable handlers
- handlers/echo.ts - Echo command logic
- handlers/build.ts - Build command logic
- Workers route to handlers based on command registry

1. **Extensibility**: New commands just need handler registration
2. **DRY**: Shared worker infrastructure for similar command types
3. **Performance**: Optimized timeouts and concurrency per quadrant
4. **Maintainability**: Clear separation of routing vs business logic

- Build system: package.sh, Makefile, component-config.sh
- CI/CD: slack-build.yml workflow
- Local dev: LocalStack setup, .env.local.example
- Documentation: CONFIGURATION.md, LOCAL-TESTING.md

Aligns with cloud-sandbox PR #14 which provisions:
- laco-plt-chatbot-command-sr-sqs queue
- laco-plt-chatbot-command-lw-sqs queue
- laco-plt-chatbot-command-sr-worker Lambda
- laco-plt-chatbot-command-lw-worker Lambda
llama90 added a commit that referenced this pull request Jan 10, 2026
Replace command-specific workers with unified quadrant workers for
better scalability and maintainability.

**Workers (routing layer):**
- Remove: echo, build, deploy, status workers (command-specific)
- Add: SR (short-read) and LW (long-write) unified workers
- SR worker handles: /echo and future fast read commands
- LW worker handles: /build, /deploy and future write commands

**Handlers (business logic layer):**
- Extract command logic into reusable handlers
- handlers/echo.ts - Echo command logic
- handlers/build.ts - Build command logic
- Workers route to handlers based on command registry

1. **Extensibility**: New commands just need handler registration
2. **DRY**: Shared worker infrastructure for similar command types
3. **Performance**: Optimized timeouts and concurrency per quadrant
4. **Maintainability**: Clear separation of routing vs business logic

- Build system: package.sh, Makefile, component-config.sh
- CI/CD: slack-build.yml workflow
- Local dev: LocalStack setup, .env.local.example
- Documentation: CONFIGURATION.md, LOCAL-TESTING.md

Aligns with cloud-sandbox PR #14 which provisions:
- laco-plt-chatbot-command-sr-sqs queue
- laco-plt-chatbot-command-lw-sqs queue
- laco-plt-chatbot-command-sr-worker Lambda
- laco-plt-chatbot-command-lw-worker Lambda
llama90 added a commit that referenced this pull request Jan 10, 2026
Replace command-specific workers with unified quadrant workers for
better scalability and maintainability.

**Workers (routing layer):**
- Remove: echo, build, deploy, status workers (command-specific)
- Add: SR (short-read) and LW (long-write) unified workers
- SR worker handles: /echo and future fast read commands
- LW worker handles: /build, /deploy and future write commands

**Handlers (business logic layer):**
- Extract command logic into reusable handlers
- handlers/echo.ts - Echo command logic
- handlers/build.ts - Build command logic
- Workers route to handlers based on command registry

1. **Extensibility**: New commands just need handler registration
2. **DRY**: Shared worker infrastructure for similar command types
3. **Performance**: Optimized timeouts and concurrency per quadrant
4. **Maintainability**: Clear separation of routing vs business logic

- Build system: package.sh, Makefile, component-config.sh
- CI/CD: slack-build.yml workflow
- Local dev: LocalStack setup, .env.local.example
- Documentation: CONFIGURATION.md, LOCAL-TESTING.md

Aligns with cloud-sandbox PR #14 which provisions:
- laco-plt-chatbot-command-sr-sqs queue
- laco-plt-chatbot-command-lw-sqs queue
- laco-plt-chatbot-command-sr-worker Lambda
- laco-plt-chatbot-command-lw-worker Lambda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants