Fix agent confusion about app commands (#30) #4045
Closed
Conversation
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
Establishes CLAUDE.md as single source of truth for operational commands (npm run dev, validate, deploy). Updates validation to dynamically discover package.json scripts instead of hardcoding. Apps.tmpl now references CLAUDE.md for complete guidance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
keugenek
requested review from
a team,
lennartkats-db and
pietern
as code owners
keugenek
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
arsenyinfo
reviewed
Dec 1, 2025
|
|
||
| **⚠️ CRITICAL**: Use ONLY these commands. Do not improvise alternatives. | ||
|
|
||
| ### Local Development |
Contributor
There was a problem hiding this comment.
are you sure it is agent's concern? imo it should not know anything but validate
Contributor
Author
There was a problem hiding this comment.
template must set the way it should be executed, dont you think?
Contributor
There was a problem hiding this comment.
This is certainly the agent's concern. I regularly tell the agent to run the app locally and open it in the browser so I can inspect the progress of the app.
The agent needs to know how to build and run the app locally.
arsenyinfo
reviewed
Dec 1, 2025
| What do you want to do? | ||
| │ | ||
| ├─ "Test/run locally" → npm run dev | ||
| ├─ "Check ready to deploy" → databricks experimental apps-mcp tools validate . |
Contributor
There was a problem hiding this comment.
i'd hint invoke_databricks_cli reference here
Collaborator
|
Commit: e5ed0b2
17 failing tests:
Top 31 slowest tests (at least 2 minutes):
|
keugenek
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
reviewed
Dec 1, 2025
| errorPrefix: "Failed to run tests", | ||
| displayName: "Tests", | ||
| }, | ||
| {name: "install", command: "npm install", errorPrefix: "Failed to install dependencies", displayName: "Install"}, |
Contributor
There was a problem hiding this comment.
Why do we need to parse package.json?
AFAIK, this is functionally equivalent to the old implementation. The old implementation will always work even if thescrips are missing since we are running with --if-present
Contributor
Author
There was a problem hiding this comment.
wanted to have a single source of truth, but I agree the code looks less clean , so will roll this back until I have cleaner idea of implementing this
fjakobs
reviewed
Dec 2, 2025
experimental/apps-mcp/templates/appkit/template/{{.project_name}}/CLAUDE.md
Outdated
Show resolved
Hide resolved
Resolved conflicts in:
- experimental/apps-mcp/lib/prompts/apps.tmpl
- experimental/apps-mcp/templates/appkit/template/{{.project_name}}/CLAUDE.md
Kept main branch versions for consistency with updated documentation style.
keugenek
requested review from
andrewnester,
anton-107,
denik and
shreyas-goenka
as code owners
keugenek
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
requested changes
Dec 2, 2025
|
|
||
| >>> [CLI] bundle deploy | ||
| Building python_artifact... | ||
| Error: build failed python_artifact, error: exit status 127, output: bash: uv: command not found |
Contributor
There was a problem hiding this comment.
This seems to be specific output to your local run and will cause tests to fail on CI, could you revert?
|
|
||
| >>> find my_pydabs -mindepth 1 ! -name pyproject.toml ! -regex .*/resources.* -delete | ||
|
|
||
| >>> ruff format --quiet --diff --check my_pydabs |
Contributor
There was a problem hiding this comment.
same as above, but you can just install ruff and it should succeed
Contributor
Author
|
closing due to feedback |
keugenek
added a commit
that referenced
this pull request
Dec 2, 2025
…irmation Two key issues addressed: 1. Agent was opening deployed URLs instead of localhost when user asked to "open a dev copy" or see the app during development. 2. Agent was deploying too early without user confirmation. Changes: - apps.tmpl: Add localhost guidance and deployment confirmation requirement - CLAUDE.md: Add "Deployment Procedures" section with pre-deployment checklist - CLAUDE.md: Add "Local Development vs Deployed Apps" section with decision tree The agent will now: - Prefer localhost:8000 during active development - Ask "Ready to deploy to [environment]?" before deploying - Only use deployed URLs after deployment with user approval Addresses feedback from PR #4045 discussion.
keugenek
added a commit
that referenced
this pull request
Dec 2, 2025
…rmation Three key issues addressed: 1. Agent was opening deployed URLs instead of localhost when user asked to "open a dev copy" or see the app during development. 2. Agent was deploying too early without user confirmation. 3. Need to clarify npm run dev (development) vs npm start (production). Changes: - apps.tmpl: Add localhost guidance and deployment confirmation requirement - apps.tmpl: Clarify "npm run dev" ALWAYS for development, NEVER "npm start" - CLAUDE.md: Add "Deployment Procedures" section with pre-deployment checklist - CLAUDE.md: Add "Local Development vs Deployed Apps" section with decision tree - CLAUDE.md: Add visual box explaining "npm run dev vs npm start" The agent will now: - ALWAYS use "npm run dev" during development (NEVER npm start) - Prefer localhost:8000 during active development - Ask "Ready to deploy to [environment]?" before deploying - Only use deployed URLs after deployment with user approval Addresses feedback from PR #4045 discussion.
pietern
pushed a commit
that referenced
this pull request
Dec 2, 2025
#4056) Fix apps-mcp agent behavior: localhost preference and deployment confirmation Two key issues addressed: 1. Agent was opening deployed URLs instead of localhost when user asked to "open a dev copy" or see the app during development. 2. Agent was deploying too early without user confirmation. Changes: - apps.tmpl: Add localhost guidance and deployment confirmation requirement - CLAUDE.md: Add "Deployment Procedures" section with pre-deployment checklist - CLAUDE.md: Add "Local Development vs Deployed Apps" section with decision tree The agent will now: - Prefer localhost:8000 during active development - Ask "Ready to deploy to [environment]?" before deploying - Only use deployed URLs after deployment with user approval Addresses feedback from PR #4045 discussion. ## Changes Prompt updates with checklist how to deploy when and how to run app. ## Why issue 21 | Victor | The deployment was really early, maybe a confirmation step first. issue 30 | Lennart | Agent is opening deployed apps rather than “dev copy” (unless I ask about localhost very explicitly) ## Tests make test
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.
Establishes CLAUDE.md as single source of truth for operational commands (npm run dev, validate, deploy). Updates validation to dynamically discover package.json scripts instead of hardcoding. Apps.tmpl now references CLAUDE.md for complete guidance.