Add Dagster run task detection to container entrypoint

[jfrench9]

Summary

This bugfix enhances the container entrypoint script to properly detect and handle Dagster run tasks, improving the reliability of workflow execution in containerized environments.

Key Accomplishments

• Implemented detection logic for Dagster run tasks in the entrypoint script
• Added 12 lines of new functionality to handle task identification
• Enhanced container startup behavior to properly recognize Dagster execution contexts

Breaking Changes

None - This is a backwards-compatible enhancement to existing functionality.

Testing Notes

• Verify that Dagster runs are properly detected during container startup
• Test both standard container operations and Dagster-specific task execution
• Confirm that existing non-Dagster container usage remains unaffected
• Validate that the detection logic correctly identifies different types of Dagster operations

Infrastructure Considerations

This change affects container initialization behavior and may impact how Dagster jobs are orchestrated in containerized deployments. The enhancement should improve reliability when running Dagster workflows in container orchestration platforms, particularly for distinguishing between different types of container startup scenarios.

---

🤖 Generated with Claude Code

Branch Info:

• Source: bugfix/dagster-run-entrypoint
• Target: main
• Type: bugfix

Co-Authored-By: Claude noreply@anthropic.com

[jfrench9]

@claude please review this PR

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @jfrench9's task in 1m 8s —— View job

---

PR Review: Dagster Run Task Detection

Tasks

• Read the modified entrypoint.sh file
• Analyze the git diff to understand the changes
• Review code for correctness and potential issues
• Provide comprehensive feedback

---

Summary

This PR adds critical logic to detect when the container is being launched as a Dagster run task (via EcsRunLauncher) and handles it appropriately by executing the Dagster command directly instead of falling through to the profile-based startup logic.

✅ Strengths

1. Correct placement: The detection logic is placed before DOCKER_PROFILE handling (lines 14-17), which is essential since Dagster run tasks won't have/need that variable.

2. Proper execution: Uses exec uv run "$@" which:

   • Replaces the shell process (correct for containerized apps)
   • Properly uses uv run per the project's CLAUDE.md requirements
   • Correctly expands all arguments with "$@"

3. Good documentation: The comment block clearly explains the purpose and context (EcsRunLauncher behavior).

4. Backwards compatible: The check only triggers when arguments are provided AND the first argument is "dagster", so existing usage is unaffected.

🔍 Observations & Potential Issues

1. Signal trap ineffectiveness (bin/entrypoint.sh:5)

The signal trap defined on line 5 references $PID, but this variable is never set in the script:

trap 'echo "Received shutdown signal"; kill -TERM $PID 2>/dev/null || true; wait $PID; exit 0' SIGTERM SIGINT

When the Dagster detection path executes exec on line 16, the trap won't apply to the new process anyway since exec replaces the shell. However, this is a pre-existing issue not introduced by this PR.

Impact: Low (the || true prevents failures, but the trap isn't doing what it intends)

2. Environment variable validation skipped

The new Dagster run path bypasses the validate_env_vars() check (line 44) and the LocalStack wait (lines 64-67).

Questions to consider:

• Do Dagster run tasks need ENVIRONMENT to be set for secret fetching?
• Should run tasks wait for LocalStack in dev environments?

Likely okay because: Dagster run tasks are ephemeral and EcsRunLauncher likely sets up the environment properly. But worth confirming this matches the actual behavior.

3. Test coverage consideration

The PR description mentions testing, but given this is infrastructure/startup logic, it would be valuable to know:

• How will this be tested in practice? (Manual testing in ECS?)
• Are there unit tests or integration tests that can verify this behavior?

🎯 Recommendation

Approve with minor observations noted. The implementation is correct for its stated purpose. The issues identified are either:

• Pre-existing (signal trap)
• By design and likely correct (skipping validation)
• Process-related (testing strategy)

Optional Follow-up Items

If you want to be extra thorough (not blocking for this PR):

1. Fix the signal trap (separate PR): Either set $PID when running background processes, or simplify the trap since exec replaces the process anyway.

2. Add integration test: Consider adding a test that validates the entrypoint behaves correctly when invoked with dagster api execute_run arguments (could be a simple Docker test or shell script test).

3. Document the environment assumptions: If Dagster run tasks have different environment requirements, document this in the script or in infrastructure docs.

Code-Specific Feedback

bin/entrypoint.sh:14-17 ✅

if [[ $# -gt 0 && "$1" == "dagster" ]]; then
    echo "Dagster run task detected, executing: $*"
    exec uv run "$@"
fi

• Correct use of [[ ]] for bash conditionals
• Good defensive check for $# -gt 0 before checking $1
• Proper quoting of "$@" for argument expansion
• Follows project standards (uses uv run)

Minor nit: The echo uses $* which joins arguments with spaces. Consider "$@" for more accurate representation of what will be executed, though this is purely cosmetic.

---

Overall: This is a well-implemented fix that correctly addresses the Dagster run task detection requirement. The code is clean, well-documented, and follows project conventions. ✅

---