Skip to content

Optimise orb dependency caching and setup diagnostics - #1773

Open
3kh0 wants to merge 2 commits into
mainfrom
orb-setup-caching
Open

3kh0 wants to merge 2 commits into
mainfrom
orb-setup-caching

Conversation

@3kh0

@3kh0 3kh0 commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary of the problem

Frontend dependency changes invalidate expensive native gem builds during orb setup. Existing dependency volumes can also hide rebuilt image contents and setup logs lack timings for major steps.

Describe your changes

Keep Ruby layers cached when JavaScript dependencies change and reuse Bun downloads through BuildKit with a frozen lockfile. Refresh persistent dependencies before building assets and wait for container readiness to prevent overlapping route generation.

Rebuild client and SSR assets when dependency patches change. Add a standalone regression check covering an existing orb upgrade and unchanged-input reuse. Add setup timings and stop overwriting global Amp settings.

Screenshots / Media

Not applicable. No visual changes.

AI Usage

Implemented with Amp in medium mode. The underlying model identifier and reasoning effort are not exposed in this session.

The human sent four messages: the investigation request, approval to run full CI and open a PR, a request to review Greptile's findings and approval to implement and push the fixes. They requested the follow-up after review but did not directly critique the implementation.

Amp thread

Co-authored-by: Echo <github@3kh0.net>
@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; both previous findings are addressed and no new blocking issue was found.

Summary

This PR keeps Ruby dependency layers cached when JavaScript dependencies change and adds setup timings.

  • Development image builds reuse Ruby layers and Bun downloads.
  • Development containers refresh mounted dependencies before setup continues.
  • Setup rebuilds both Vite bundles when their inputs change.
  • Orb setup times long steps and leaves Amp settings untouched.

Both previous findings are addressed. No new actionable issues were found. The regression test was reviewed but not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Setup checks container inputs] --> B[Build or reuse development image]
  B --> C[Refresh dependencies in persistent volumes]
  C --> D[Finish startup work]
  D --> E[Mark web container ready]
  E --> F[Setup checks asset inputs and outputs]
  F -->|Changed or missing| G[Force client and SSR builds]
  F -->|Unchanged| H[Reuse assets]
  G --> I[Prepare databases when needed]
  H --> I
Loading

Reviews (2) · Last reviewed commit: "Refresh persistent dependencies before o..." · Reviewed by Greptile

Comment thread .agents/setup
fi

container_fingerprint="$(fingerprint Dockerfile.dev docker-compose.yml entrypoint.dev.sh Gemfile Gemfile.lock package.json bun.lock bunfig.toml)"
container_fingerprint="$(fingerprint Dockerfile.dev docker-compose.yml entrypoint.dev.sh Gemfile Gemfile.lock package.json bun.lock bunfig.toml patches)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Patch changes do not apply

On an existing orb, adding patches to container_fingerprint triggers an image rebuild but leaves the running container using old dependencies. The saved node_modules volume hides the image's newly patched files. Neither setup nor the entrypoint reinstalls dependencies.

Refresh dependencies in that volume when patches change, then rebuild the affected assets.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .agents/setup
Line: 82

Comment:
**Patch changes do not apply**

On an existing orb, adding `patches` to `container_fingerprint` triggers an image rebuild but leaves the running container using old dependencies. The saved `node_modules` volume hides the image's newly patched files. Neither setup nor the entrypoint reinstalls dependencies.

Refresh dependencies in that volume when patches change, then rebuild the affected assets.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment thread Dockerfile.dev
Comment on lines +46 to +47
RUN --mount=type=cache,target=/root/.bun/install/cache \
bun install --frozen-lockfile && \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Required setup tests are missing

The PR changes setup caching and dependency installation without adding tests. The repository guide requires adding tests when making a change, so this requirement must be satisfied before merging.

Add a regression check that changes a patch after initial setup and confirms the next setup uses the changed dependency. Also check that unchanged inputs reuse cached work.

Context Used: AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: Dockerfile.dev
Line: 46-47

Comment:
**Required setup tests are missing**

The PR changes setup caching and dependency installation without adding tests. The repository guide requires adding tests when making a change, so this requirement must be satisfied before merging.

Add a regression check that changes a patch after initial setup and confirms the next setup uses the changed dependency. Also check that unchanged inputs reuse cached work.

**Context Used:** AGENTS.md ([source](https://github.com/hackclub/hackatime/blob/main/AGENTS.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Co-authored-by: Echo <github@3kh0.net>

This branch has not been deployed

No deployments
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