⚠️ Rewrite harness as thin single-stage runner with SkillCard-based skills#53
⚠️ Rewrite harness as thin single-stage runner with SkillCard-based skills#53savitharaghunathan wants to merge 8 commits into
Conversation
Replace multi-step CLI harness with thin ACP wrapper that runs a single migration stage (plan/execute/verify) per pod invocation, managed by the AgentPlaybookRun reconciler. Add per-stage skill OCI images, full javaee-to-quarkus skill from migration-skills repo (SKILL.md + 6 modules + 6 references), handoff.md generation with agent summaries and plan steps, filesystem watcher for incremental commits, and e2e test scripts with timestamped branch names. Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe harness is replaced with a single-stage, SkillCard-driven runner. It adds Hub credential and analysis integration, incremental filesystem commits, language-specific agent images, Java migration skills, and AgentPlaybook integration resources. ChangesThin runner architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AgentPlaybookRun
participant migrationHarness
participant Hub
participant GitRepository
participant gooseServe
participant ACP
participant Watcher
AgentPlaybookRun->>migrationHarness: Start stage with environment
migrationHarness->>Hub: Resolve application and git credentials
migrationHarness->>GitRepository: Clone and checkout target branch
migrationHarness->>Hub: Fetch analysis insights
migrationHarness->>gooseServe: Start ACP service
migrationHarness->>ACP: Discover skills and send stage prompt
ACP-->>Watcher: Produce repository changes
Watcher->>GitRepository: Stage and commit changes after quiet period
ACP-->>migrationHarness: Return stage completion status
migrationHarness->>GitRepository: Write handoff and push final commit
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: build linters: plugin(logcheck): plugin "logcheck" not found Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
6f8e1cc to
3a926f6
Compare
The old agent-java-goose-build target was replaced by agent-images-build which builds all three stage images (plan, execute, verify). Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
3a926f6 to
4c53e82
Compare
Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
Replace per-stage images (plan, execute, verify) with per-language images (java, go, csharp, nodejs). Skills come via SkillCards at runtime. Closes konveyor#40. Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (27)
hack/harness-test/setup.sh-82-92 (1)
82-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLoad skill images when
CONTAINER_TOOL=docker.
docker image existsis not a Docker CLI subcommand, and the conditional redirects stderr, so Docker users always skip skill-image loading. Useimage inspect, which both Docker and Podman support for local-image checks.Proposed fix
- if $CONTAINER_TOOL image exists "${SKILL_IMAGE}:${SKILL}" 2>/dev/null; then + if $CONTAINER_TOOL image inspect "${SKILL_IMAGE}:${SKILL}" >/dev/null 2>&1; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/harness-test/setup.sh` around lines 82 - 92, Update the image-existence check in the SKILL loop to use the shared `image inspect` command instead of `image exists`, preserving the existing container-tool-specific image loading behavior and output.harness/internal/git/git.go-129-136 (1)
129-136: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTreat fully filtered changes as a no-op.
When all status entries are excluded untracked files, no paths are staged before
wt.Commit; go-git returnsErrEmptyCommitfor a clean index and the watcher treats this as a failed commit+push. Track whether anywt.Add(path)succeeds and returnplumbing.ZeroHash, nilwhen nothing was staged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/internal/git/git.go` around lines 129 - 136, Update the status-processing loop around wt.Add in the git commit flow to track whether at least one path is staged. When every entry is an excluded untracked file and no wt.Add succeeds, return plumbing.ZeroHash, nil before attempting wt.Commit; preserve existing add-error propagation and normal commit behavior when files are staged.harness/internal/git/git.go-20-24 (1)
20-24: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPrevent bypassing the destructive-delete boundary.
/workspace/../etcand/workspace-otherpass the prefix check, so this can recursively remove an unintended existing directory. Normalize the path and usefilepath.Relto require a proper child of an allowed root; also returnos.RemoveAllfailures.Proposed fix
+ "path/filepath" + func Clone(ctx context.Context, cred *Credentials, destDir string) (*gogit.Repository, error) { + var err error + destDir, err = filepath.Abs(destDir) + if err != nil { + return nil, fmt.Errorf("resolve destination: %w", err) + } + if _, err := os.Stat(destDir); err == nil { - if !strings.HasPrefix(destDir, "/workspace") && !strings.HasPrefix(destDir, os.TempDir()) { + if !isChildOf(destDir, "/workspace") && !isChildOf(destDir, os.TempDir()) { return nil, fmt.Errorf("refusing to remove %s: not under /workspace or temp", destDir) } - os.RemoveAll(destDir) + if err := os.RemoveAll(destDir); err != nil { + return nil, fmt.Errorf("remove %s: %w", destDir, err) + } } } + +func isChildOf(path, root string) bool { + rel, err := filepath.Rel(root, path) + return err == nil && rel != "." && rel != ".." && + !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/internal/git/git.go` around lines 20 - 24, Update the destructive-delete guard around os.Stat and os.RemoveAll to cleanly normalize destDir and use filepath.Rel against /workspace and os.TempDir(), allowing only paths that are proper descendants of either root, not the roots themselves or similarly prefixed paths. Return any error from os.RemoveAll instead of ignoring it, while preserving the existing refusal error for paths outside the allowed roots.skills/javaee-to-quarkus/modules/cleanup.md-48-54 (1)
48-54: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake dev-mode verification bounded in every module. A foreground
mvn quarkus:devcommand can block the stage indefinitely.
skills/javaee-to-quarkus/modules/cleanup.md#L48-L54: use a timeout/readiness probe and explicitly stop the process.skills/javaee-to-quarkus/modules/app-config.md#L61-L68: apply the same bounded startup check instead of relying on “briefly.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/cleanup.md` around lines 48 - 54, Make dev-mode verification bounded in both skills/javaee-to-quarkus/modules/cleanup.md (lines 48-54) and skills/javaee-to-quarkus/modules/app-config.md (lines 61-68): replace the unbounded or “briefly” run of mvn quarkus:dev with a timeout/readiness probe, and explicitly stop the launched process after verification.skills/javaee-to-quarkus/modules/cleanup.md-9-12 (1)
9-12: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not recursively delete every directory named
weblogic.This can remove unrelated application packages or vendor content. Enumerate exact tracked stub paths first, then delete only those paths with
git rmor a narrowly scoped command.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/cleanup.md` around lines 9 - 12, Update the cleanup instructions to enumerate the exact tracked WebLogic stub directory paths before removal, then delete only those approved paths using git rm or an equivalently narrow command; remove the broad find-based recursive deletion.skills/plan/references/migration-phases.md-125-130 (1)
125-130: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not replace
createCipheras a one-token migration tocreateCipheriv.
createCipherivrequires an explicit cipher, key, and IV, so this replacement is cryptographic redesign. Add a separate crypto migration task covering algorithm, key management, IV handling, and whether authenticated encryption is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/plan/references/migration-phases.md` around lines 125 - 130, Update the “Key Changes” migration guidance to remove the direct require('crypto').createCipher → createCipheriv replacement. Add a separate crypto migration task that explicitly covers cipher algorithm selection, key management, IV generation/handling, and whether authenticated encryption is required; retain the unrelated Buffer, URL, and fs migration entries.skills/plan/SKILL.md-54-62 (1)
54-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGenerate
graph.jsoninto an ignored graph directory.
PLAN.mdis the only expected plan-stage output, and the repo-levelgraph.jsonremains staged by the harness unless the generated graph is written to an internal path such asgraphify-out/,detect/, or.konveyor/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/plan/SKILL.md` around lines 54 - 62, Update the “Phase 1 — Generate Code Graph” instructions so the graphify update command writes graph.json under an ignored internal directory such as graphify-out/, detect/, or .konveyor/, rather than the repository root. Preserve PLAN.md as the only expected plan-stage output and ensure the documented command and output location are consistent.skills/javaee-to-quarkus/references/dependency-map.md-16-17 (1)
16-17: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake JMS/MDB connector selection broker-specific across all messaging guidance. The current JMS examples all default SmallRye Reactive Messaging to AMQP, but Java EE queue/topic mappings can also target JMS or Kafka brokers with different ack/fail/transaction semantics. Keep AMQP as one option and require explicit connector/mapping for JMS and Kafka destinations, including channel names, acknowledgement mode, and transaction handling where applicable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/dependency-map.md` around lines 16 - 17, Make JMS/MDB connector guidance broker-specific across all affected sites: skills/javaee-to-quarkus/references/dependency-map.md lines 16-17, skills/plan/references/migration-phases.md lines 60-63, and skills/javaee-to-quarkus/modules/app-config.md lines 46-59. Keep AMQP as an explicit option, and add distinct JMS and Kafka mappings with broker-appropriate channel names, acknowledgement/failure behavior, and transaction handling where applicable.skills/javaee-to-quarkus/modules/build-config.md-20-30 (1)
20-30: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove Java EE umbrella APIs and legacy build plugins consistently.
references/dependency-map.mdcalls out removal ofjavax:javaee-api,javax:javaee-web-api,maven-war-plugin, andmaven-ejb-plugin, but onlyjavaee-apiandmaven-war-pluginare listed here. Ensure the build Config step documents removal of the full incompatible set so legacy Java EE WAR/EJB artifacts don’t remain in the Quarkus build.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/build-config.md` around lines 20 - 30, Update the build Config step to document removal of all incompatible legacy artifacts: javax:javaee-api, javax:javaee-web-api, maven-war-plugin, and maven-ejb-plugin. Preserve the existing removal guidance while adding the missing dependency and plugin entries so the full set matches references/dependency-map.md.skills/javaee-to-quarkus/modules/app-config.md-18-27 (1)
18-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not hardcode datasource values into migrated applications.
These values lock every migration to PostgreSQL/localhost and allow fallback credentials (
${DB_USER:myuser}/${DB_PASS:mypass}) when secrets are absent. Use project-specific placeholder syntax here, make production credentials fail closed when missing, and add Flyway settings only when the project actually performs migrations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/app-config.md` around lines 18 - 27, Replace the hardcoded datasource and fallback credential values in the application.properties example with the project-specific placeholder syntax, ensuring required production credentials fail when absent. Remove PostgreSQL-specific and Flyway settings unless the migrated project actually uses PostgreSQL and performs Flyway migrations; otherwise generate only the applicable configuration.skills/javaee-to-quarkus/references/dependency-map.md-35-36 (1)
35-36: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not map JAAS directly to
quarkus-oidc.JAAS is a Java Authentication/Authorization Service mechanism, while
quarkus-oidcis only for OIDC/OAuth2 providers. This mapping can send JAAS-based auth to an OIDC provider and silently change or remove the original authentication behavior; make it mechanism-specific and require security review.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/dependency-map.md` around lines 35 - 36, Update the JAAS / Security entry in the dependency map to avoid directly mapping JAAS to quarkus-oidc. Make the recommendation mechanism-specific, distinguish OIDC/OAuth2 use from other JAAS-based authentication, and require security review before selecting an alternative dependency; leave the separate Servlet Filter mapping unchanged.skills/plan/references/migration-phases.md-78-83 (1)
78-83: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the byte-vs-text distinction in the Python mappings.
unicode(x) → str(x)andbasestring → stronly preserve semantics whenx/basestringare guaranteed to be text. For byte values, replace with explicit conversion with an encoding policy, e.g.bytes.decode()orstr.encode(), instead of blindly mapping tostr.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/plan/references/migration-phases.md` around lines 78 - 83, Update the Python 2-to-3 mappings in the migration reference to preserve byte-versus-text semantics: do not present unicode(x) → str(x) or basestring → str as universal replacements. Specify explicit encoding or decoding using bytes.decode() or str.encode() when values may be bytes, while retaining direct str mappings only for guaranteed text values.skills/javaee-to-quarkus/references/pattern-map.md-153-167 (1)
153-167: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not use
ContainerRequestFilteras a blanket replacement for@WebFilter("/*").A servlet
/*filter intercepts all HTTP requests routed through the servlet layer, including non-JAX-RS servlet endpoints and static resources. A JAX-RS@ProviderContainerRequestFilteronly runs for JAX-RS requests, so authentication, header injection, or request/servlet-state transformations can be skipped on those paths. Preserve equivalent HTTP-layer coverage, for example with Quarkus HTTP/Vert.x filters, or explicitly migrate the security policy to apply to non-REST traffic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/pattern-map.md` around lines 153 - 167, Update the “Servlet Filter → JAX-RS Filter” mapping so it does not present ContainerRequestFilter as a blanket replacement for `@WebFilter`("/*"). Document that ContainerRequestFilter covers only JAX-RS requests, and direct migrations requiring all HTTP, servlet, or static-resource coverage toward Quarkus HTTP/Vert.x filters or an explicitly expanded security policy.skills/javaee-to-quarkus/references/config-map.md-9-12 (1)
9-12: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse secret references instead of literal credentials.
The mapping instructs the agent to write
username=userandpassword=passinto configuration. Use environment/secret references such as${DB_USERNAME}and${DB_PASSWORD}, and explicitly prohibit committing resolved credentials.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/config-map.md` around lines 9 - 12, Update the datasource credential mappings in config-map.md to use secret/environment references such as ${DB_USERNAME} and ${DB_PASSWORD} instead of literal username and password values, and explicitly instruct that resolved credentials must never be committed.skills/verify/SKILL.md-44-49 (1)
44-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow minimal fixes in related build/configuration files.
A compiler error can require updating
pom.xml, generated configuration, or another directly related file. Restricting changes to only the reported source file can make the verification stage fail on otherwise fixable dependency/configuration errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/verify/SKILL.md` around lines 44 - 49, Update the “Fix Rules” section in SKILL.md to permit minimal changes to directly related build or configuration files, such as pom.xml or generated configuration, when required to resolve a compiler error. Retain the existing constraints to fix only compiler errors, avoid refactoring, and limit changes to files necessary for the reported error.skills/javaee-to-quarkus/references/config-map.md-7-18 (1)
7-18: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not infer the database vendor from the JNDI name.
A
<jta-data-source>identifies a datasource, not whether it is PostgreSQL. Hardcodingdb-kind=postgresqlcan select the wrong driver, URL, dialect, and transaction configuration. Require inspection of the actual JDBC URL/driver and preserve named/XA datasource semantics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/config-map.md` around lines 7 - 18, The datasource mapping table must not infer PostgreSQL from the JNDI name in <jta-data-source>. Update the entries around quarkus.datasource.db-kind and JDBC settings to require determining the vendor from the actual JDBC URL or driver, while preserving named and XA datasource semantics instead of presenting generic values as universal mappings.skills/javaee-to-quarkus/modules/lifecycle.md-48-55 (1)
48-55: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not delete custom migration bootstrap unconditionally.
quarkus.flyway.migrate-at-start=trueruns configured Flyway migrations only when the Flyway extension, driver, and migration resources are present; it does not reproduce custom tenant selection, pre/post hooks, locking, or conditional logic. Preserve or explicitly migrate that behavior before deleting the startup class.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/lifecycle.md` around lines 48 - 55, Revise the Flyway/DB startup guidance to avoid unconditionally deleting custom migration bootstrap classes. State that removal is safe only after verifying Quarkus has the required Flyway extension, driver, migration resources, and equivalent tenant selection, hooks, locking, and conditional behavior; otherwise preserve or explicitly migrate that custom logic before relying on quarkus.flyway.migrate-at-start.Source: MCP tools
skills/javaee-to-quarkus/references/annotation-map.md-19-20 (1)
19-20: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve EJB timer semantics in the scheduler migration
Mapping
@Scheduleto@Scheduledwithout carrying over schedule expression, persistence, overlap/concurrency, timezone, and transaction behavior can change timer lifetimes or execution behavior; the same applies to@Timeoutlifecycle handling. Document/configure the exact Quarkus Scheduler settings needed instead of a bare annotation rename.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/annotation-map.md` around lines 19 - 20, Expand the `@Schedule` and `@Timeout` mappings in annotation-map.md to document how schedule expressions, persistence, overlap/concurrency, timezone, transactions, and `@Timeout` lifecycle behavior must be preserved with Quarkus Scheduler. Specify the required `@Scheduled` attributes and related configuration or code changes instead of presenting a bare annotation rename or removal.Source: MCP tools
skills/javaee-to-quarkus/modules/messaging.md-72-76 (1)
72-76: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle the emitter completion result.
Emitter.send(payload)returns aCompletionStagecompleted only after downstream acknowledgement and exceptionally on nack. Ignoring it makessend()report success before delivery succeeds and hides broker/processing failures; return, await, or handle the completion result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/messaging.md` around lines 72 - 76, Update the send method to handle the CompletionStage returned by emitter.send(payload) instead of discarding it. Propagate, await, or explicitly handle completion and exceptional nack outcomes so callers do not observe success before downstream acknowledgement and failures are not hidden.Source: MCP tools
skills/javaee-to-quarkus/modules/ejb-to-cdi.md-64-66 (1)
64-66: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve remote contracts before deleting interfaces.
@Remoteinterfaces define EJB remote calls that may be consumed outside the migrating project. In this deletion step, either verify no external consumers exist or replace the remote contract with a versioned REST, messaging, or client API before removing the interface file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/ejb-to-cdi.md` around lines 64 - 66, Before applying “Remove Remote interfaces,” verify that each `@Remote` contract has no external consumers; where consumers exist, replace it with a versioned REST, messaging, or client API and migrate callers before deleting the interface and removing its implementations’ implements clauses.skills/javaee-to-quarkus/modules/ejb-to-cdi.md-16-18 (1)
16-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
@Statefulconversational scope instead of mapping it to@ApplicationScoped.
@ApplicationScopedbeans are application-wide, so replacing@Statefulthis way moves client-specific conversational state to a shared instance and can cause cross-client data leakage/thread-safety issues. Update both this migration step and the annotation reference table with a scope-preserving migration rule based on the actual client state ownership (for example, externalizing state or choosing the narrowest CDI scope that matches the bean’s lifetime).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/ejb-to-cdi.md` around lines 16 - 18, Replace the direct `@Stateful-to-`@ApplicationScoped mapping with guidance to preserve conversational state based on client ownership, such as externalizing state or selecting the narrowest matching CDI scope. Apply this migration rule in the EJB-to-CDI migration step and update the `@Stateful` entry in skills/javaee-to-quarkus/references/annotation-map.md (lines 9-12); leave the `@Stateless` and `@Singleton` mappings unchanged.Source: MCP tools
skills/javaee-to-quarkus/modules/messaging.md-27-43 (1)
27-43: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftConfirm the messaging contract before converting to String payloads.
onMessage(Message)can preserveTextMessage, but narrowing it to@Incoming("orders") String bodydiscards non-text payloads, headers/properties, transaction behavior, and JMS/Lifecycle delivery results. Use the reactiveMessagepayload when metadata or acknowledgement/failure handling matters, and handle failures explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/messaging.md` around lines 27 - 43, Update the OrderServiceMDB.onMessage reactive messaging contract to use org.eclipse.microprofile.reactive.messaging.Message when metadata, acknowledgement, failure handling, or non-text payloads must be preserved; otherwise explicitly validate and handle the TextMessage-to-String conversion. Preserve required headers/properties and delivery semantics, and add explicit acknowledgement or failure handling where applicable.Source: MCP tools
skills/javaee-to-quarkus/references/config-map.md-52-58 (1)
52-58: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not use
durable=trueas the queue/topic discriminator.Durability controls AMQP subscription persistence; it does not preserve JMS queue-versus-topic topology. The migration needs broker topology details such as address, container, fully-qualified queue names, link names, or multicast routing rather than this one-line mapping.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/config-map.md` around lines 52 - 58, The Messaging config mapping should not present durable=true as the Queue vs Topic discriminator. Update the relevant table row to state that topology must be derived from broker-specific details such as address, container, fully-qualified queue names, link names, or multicast routing, while retaining durable as an AMQP subscription-persistence setting.Source: MCP tools
skills/javaee-to-quarkus/SKILL.md-27-32 (1)
27-32: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMigrate
web.xmlcontents before removing the descriptor.
web.xmlis listed for removal inskills/javaee-to-quarkus/SKILL.md:28,modules/app-config.md:44, andmodules/cleanup.md:18, butskills/javaee-to-quarkus/references/config-map.mddefines migrations forcontext-param,<welcome-file-list>,<error-page>,<servlet-mapping>,<filter-mapping>, and<security-constraint>. Element-by-element migration is required before deleting the file to avoid dropping application behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/SKILL.md` around lines 27 - 32, Update the migration workflow in the relevant app-config and cleanup guidance to require reviewing and migrating every web.xml element before deletion, using the mappings in references/config-map.md for context-param, welcome-file-list, error-page, servlet-mapping, filter-mapping, and security-constraint. Make web.xml removal conditional on completing those migrations rather than presenting it as an immediate step.skills/javaee-to-quarkus/references/annotation-map.md-23-24 (1)
23-24: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMap datasource resources to Quarkus datasource beans, not
@ConfigProperty.Quarkus datasources are injected via CDI, e.g.
@Inject DataSourcefor the default datasource or@Inject AgroalDataSourcewith@DataSource("name")for named datasources.@ConfigPropertyis for application configuration values, not the datasource bean itself, so this mapping is misleading for migration output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/annotation-map.md` around lines 23 - 24, Update the `@Resource` (DataSource) entry in annotation-map.md to map datasource resources to CDI-injected DataSource or named AgroalDataSource beans using the appropriate `@DataSource` qualifier, rather than `@ConfigProperty`; leave the separate `@Resource` (JMS) mapping unchanged.Source: MCP tools
skills/javaee-to-quarkus/references/config-map.md-23-29 (1)
23-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve session timeout and security behavior during migration.
The app-config/cleanup flow removes
WEB-INF/web.xmlunconditionally, but<session-config><session-timeout>and<security-constraint>are application behaviors that must not be discarded as “not applicable.” Confirm whether the target Quarkus app should remain stateless, then migrate session timeout to an equivalent (for examplequarkus.http.auth.form.timeout/cookie expiry) and ensure existing security constraints are covered by the new HTTP auth policies or annotations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/config-map.md` around lines 23 - 29, Update the app-config/cleanup flow that removes WEB-INF/web.xml to preserve behaviors from session-config and security-constraint before deletion: confirm the target Quarkus application’s intended statefulness, configure an equivalent session timeout when sessions are required, and migrate every security constraint to matching HTTP auth policies or authorization annotations such as `@RolesAllowed`. Do not treat these web.xml entries as unconditionally “not applicable.”skills/javaee-to-quarkus/modules/ejb-to-cdi.md-68-82 (1)
68-82: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not replace arbitrary JNDI lookups with CDI injection.
ejb:/...lookups bind to a WildFly/JBoss-style EJB view that may be remote or outside the target Quarkus module. CDI injection only resolves beans managed in the Quarkus application; keep a remote-client adapter or migrate the caller to an explicit REST/messaging client for external/deployed-away EJBs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/modules/ejb-to-cdi.md` around lines 68 - 82, Revise the “Replace JNDI lookups with injection” guidance to avoid blanket replacement of arbitrary JNDI lookups. For ejb:/ lookups targeting remote or external EJB views, retain a remote-client adapter or direct callers to an explicit REST or messaging client; recommend `@Inject` FooService only when the service is a CDI-managed bean within the same Quarkus application.
🟡 Minor comments (9)
docs/adr/0006-harness-thin-runner-and-skillcard-skills.md-20-24 (1)
20-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the thin-runner scope statement with the responsibilities listed below.
Saying the harness handles “only git plumbing and goose lifecycle” omits Hub resolution, analysis persistence, skill discovery, filesystem watching, and final push behavior. Replace that wording with the actual boundary.
Proposed wording
-The harness needed to become a thin, uniform wrapper — identical in -every stage image — that handles only git plumbing and goose lifecycle. +The harness needed to become a thin, uniform wrapper — identical in +every stage image — that handles git/Hub integration, skill discovery, +incremental persistence, and goose lifecycle, while leaving migration +intelligence to mounted skills.Also applies to: 30-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0006-harness-thin-runner-and-skillcard-skills.md` around lines 20 - 24, Update the thin-runner scope statement in the ADR to match the responsibilities described in the subsequent sections: include Hub resolution, analysis persistence, skill discovery, filesystem watching, and final push behavior alongside git plumbing and Goose lifecycle handling. Apply the same boundary wording consistently to the related statement covering the additional referenced section.docs/adr/0006-harness-thin-runner-and-skillcard-skills.md-117-123 (1)
117-123: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the fenced diagram.
Use
text(or another appropriate language) after the opening fence to satisfy MD040.Proposed fix
-``` +```text🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0006-harness-thin-runner-and-skillcard-skills.md` around lines 117 - 123, Specify the fenced diagram’s language by changing its opening fence to use the text language, while leaving the diagram content and closing fence unchanged.Source: Linters/SAST tools
docs/adr/0006-harness-thin-runner-and-skillcard-skills.md-114-129 (1)
114-129: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the shipped image matrix, not the Java-only hierarchy.
This ADR says there are five images and treats Go as future, while this PR adds language-specific variants for Java, Go, C#, and Node.js. Update the diagram, image count, and consequences so the architecture record matches the released build matrix.
Also applies to: 304-307
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0006-harness-thin-runner-and-skillcard-skills.md` around lines 114 - 129, Update the image hierarchy and image count in this ADR to document the shipped matrix: shared agent-base and agent-plan images plus language-specific base, execute, and verify images for Java, Go, C#, and Node.js. Revise the surrounding consequences text to describe all four supported languages and remove the statement that Go is future or requires adding new images.hack/harness-test/setup.sh-73-75 (1)
73-75: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winUse unique temporary archive paths. Predictable
/tmpfilenames permit symlink/TOCTOU interference by another local user.
hack/harness-test/setup.sh#L73-L75: create the agent archive withmktempbefore saving and loading it.hack/harness-test/setup.sh#L85-L87: create the skill archive withmktempbefore saving and loading it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/harness-test/setup.sh` around lines 73 - 75, Create unique temporary archive paths with mktemp before each save/load sequence, and use the resulting path for both the $CONTAINER_TOOL save and kind load commands. Apply this to both the agent archive sequence at hack/harness-test/setup.sh lines 73-75 and the skill archive sequence at lines 85-87, preserving cleanup of the generated temporary files.Source: Linters/SAST tools
harness/internal/watcher/patterns.go-8-12 (1)
8-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAllow Kotlin DSL Gradle files.
New
build.gradle.ktsandsettings.gradle.ktsfiles resolve to.kts, which is not allowlisted and therefore will never be committed.Proposed fix
- ".gradle": true, ".kt": true, ".groovy": true, + ".gradle": true, ".kts": true, ".kt": true, ".groovy": true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/internal/watcher/patterns.go` around lines 8 - 12, Update the sourceExts allowlist in patterns.go to include the .kts extension, so Kotlin DSL Gradle files such as build.gradle.kts and settings.gradle.kts are recognized and committed.harness/internal/hub/client.go-49-56 (1)
49-56: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn the real
ListInsights()failure inFetchAnalysis.
FetchAnalysisreturnsnil, nilfor every error, so transport/auth/read failures become “no analysis” and the non-fatal caller warning becomes dead. Returnnil, errunless the API has a documented not-found/empty-success contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/internal/hub/client.go` around lines 49 - 56, Update Client.FetchAnalysis so failures from selected.Analysis.ListInsights() return nil together with the original err instead of nil, nil; preserve the existing successful insights return path.skills/javaee-to-quarkus/references/dependency-map.md-29-30 (1)
29-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSeparate MicroProfile Config from YAML support.
quarkus-config-yamlonly adds YAML config support; MicroProfile Config is built into Quarkus. Mark Config support as built-in and add the YAML extension only when the application uses YAML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/dependency-map.md` around lines 29 - 30, Update the MicroProfile Config entry in the dependency map to identify Config as built into Quarkus rather than mapping it to quarkus-config-yaml, and add a separate YAML configuration entry that lists quarkus-config-yaml as conditional on the application using YAML.skills/javaee-to-quarkus/references/verify-errors.md-9-9 (1)
9-9: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon’t add
quarkus-arcas a compiler workaround for missing EJB types.
quarkus-arcsupplies CDI, notjavax.ejb; the compiler error is expected until EJB code is refactored to CDI/Jakarta types in the EJB-to-CDI phase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/references/verify-errors.md` at line 9, Update the `package javax.ejb does not exist` verification guidance to remove the suggestion to add `quarkus-arc` temporarily; state that the error is expected until the EJB-to-CDI migration replaces EJB types with CDI/Jakarta equivalents.skills/plan/references/migration-plan-skill.md-53-58 (1)
53-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake reference discovery safe when no files match.
The unexpanded glob causes
lsto fail when no reference files exist, so the agent may stop before reaching the generic-planning fallback.- ls /opt/skills/*/references/*.md + find /opt/skills -type f -path '*/references/*.md' -print🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/plan/references/migration-plan-skill.md` around lines 53 - 58, Update the reference discovery command in the loaded migration skills check so an unmatched glob does not cause the command to fail or halt execution. Preserve discovery of matching reference files and ensure the flow continues to the generic planning fallback using migration-phases.md when none are found.
🧹 Nitpick comments (3)
.github/workflows/images.yml (1)
67-80: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBuild the image matrix only once on publishing jobs.
agent-images-pushalready depends onagent-images-build, so non-PR jobs build all images at Line 68 and again at Line 80. Limit the standalone build step to PRs; the push target can build once before publishing.Proposed fix
- name: Build images + if: github.event_name == 'pull_request' run: make agent-images-build🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/images.yml around lines 67 - 80, Update the “Build images” step in the workflow to run only for pull-request events, while leaving the “Push images” step and its existing condition unchanged. Since agent-images-push already depends on agent-images-build, publishing jobs must rely on that dependency and build the image matrix only once.harness/cmd/migration-harness/main.go (1)
187-193: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
os.Exit(1)on the failure path skips all deferred cleanup.
defer cancel(),defer srv.Stop(),defer wsClient.Close(), anddefer w.Stop()do not run whenos.Exitis called, so on stage failure goose serve and the websocket are not gracefully torn down (the success path at Line 193 runs them). Prefer surfacing the exit code frommain()instead of exiting mid-function.Sketch
- // 13. Exit - if stageFailed { - logging.Err("stage failed") - os.Exit(1) - } - logging.Ok("stage succeeded") - return nil + // 13. Exit + if stageFailed { + logging.Err("stage failed") + return errStageFailed // deferred cleanup runs; main() maps this to exit code 1 + } + logging.Ok("stage succeeded") + return nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/cmd/migration-harness/main.go` around lines 187 - 193, Update the stage-failure handling in main so it returns an error or otherwise propagates a nonzero exit status to the top-level process instead of calling os.Exit(1) mid-function. Preserve the existing success and failure logging while ensuring deferred cleanup from cancel, srv.Stop, wsClient.Close, and w.Stop executes before main exits.skills/javaee-to-quarkus/SKILL.md (1)
40-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the Maven-only build contract.
The skill advertises Java EE migration generally but hardcodes
mvn compile. Either explicitly scope this SkillCard to Maven projects or add Gradle detection and build commands.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/javaee-to-quarkus/SKILL.md` around lines 40 - 44, The build instructions after each phase must support the advertised project scope: either explicitly restrict the SkillCard to Maven projects and retain the pom.xml/mvn compile flow, or add Gradle project detection with its appropriate compile command alongside Maven detection. Ensure the selected build tool is used consistently before proceeding or stopping on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 80d6f163-f427-4ad4-842a-f9961220414b
⛔ Files ignored due to path filters (1)
harness/go.sumis excluded by!**/*.sum
📒 Files selected for processing (98)
.github/workflows/images.ymlMakefilechanges/unreleased/53-harness-thin-runner.yamlconfig/samples/kustomization.yamlconfig/samples/skillcard_javaee_to_quarkus.yamlconfig/samples/skillcard_javax_to_jakarta_ee.yamlconfig/samples/skillcollection_java_migration.yamldocs/adr/0006-harness-thin-runner-and-skillcard-skills.mdhack/harness-test/playbook-resources.yamlhack/harness-test/resources.yamlhack/harness-test/setup.shharness/cmd/migration-harness/main.goharness/go.modharness/internal/acp/session.goharness/internal/config/config.goharness/internal/config/config_test.goharness/internal/detect/detect.goharness/internal/detect/detect_test.goharness/internal/execute/execute.goharness/internal/execute/execute_test.goharness/internal/fixloop/fixloop.goharness/internal/fixloop/fixloop_test.goharness/internal/git/credentials.goharness/internal/git/git.goharness/internal/git/git_test.goharness/internal/goose/acp_runner.goharness/internal/goose/acp_runner_test.goharness/internal/goose/goose.goharness/internal/goose/lifecycle.goharness/internal/goose/recipe.goharness/internal/goose/recipe_test.goharness/internal/handoff/handoff.goharness/internal/handoff/handoff_test.goharness/internal/hub/client.goharness/internal/metrics/metrics.goharness/internal/metrics/metrics_test.goharness/internal/plan/approval.goharness/internal/plan/context.goharness/internal/plan/context_test.goharness/internal/plan/hints.goharness/internal/plan/hints_test.goharness/internal/plan/parser.goharness/internal/plan/parser_test.goharness/internal/plan/plan.goharness/internal/plan/recipe.goharness/internal/plan/turns.goharness/internal/plan/turns_test.goharness/internal/plan/types.goharness/internal/rundir/rundir.goharness/internal/rundir/rundir_test.goharness/internal/verify/verify.goharness/internal/verify/verify_test.goharness/internal/watcher/patterns.goharness/internal/watcher/patterns_test.goharness/internal/watcher/watcher.goharness/internal/watcher/watcher_test.goharness/recipes/execute.yamlharness/recipes/fix.yamlharness/recipes/verify.yamlharness/skill-bundle/goose-migration/SKILL.mdharness/skill-bundle/goose-migration/references/README.mdharness/skill-bundle/goose-migration/references/REFERENCES_SUMMARY.mdharness/skill-bundle/goose-migration/references/dotnet-framework-to-core.mdharness/skill-bundle/goose-migration/references/javaee-quarkus.mdharness/skill-bundle/goose-migration/references/python2-to-python3.mdharness/skill-bundle/goose-migration/references/springboot-2-to-3.mdharness/skill-bundle/goose-migration/skills/javaee-quarkus/SKILL.mdharness/skill-bundle/goose-migration/skills/migration-plan/MIGRATION_PLAN_SKILL_UPDATES.mdharness/skill-bundle/goose-migration/skills/python2-to-python3/SKILL.mdimages/README.mdimages/agent-base-goose/Containerfileimages/agent-base/Containerfileimages/agent-csharp/Containerfileimages/agent-go/Containerfileimages/agent-java-goose/Containerfileimages/agent-java/Containerfileimages/agent-nodejs/Containerfileskills/execute/SKILL.mdskills/execute/skill.yamlskills/javaee-to-quarkus/SKILL.mdskills/javaee-to-quarkus/modules/app-config.mdskills/javaee-to-quarkus/modules/build-config.mdskills/javaee-to-quarkus/modules/cleanup.mdskills/javaee-to-quarkus/modules/ejb-to-cdi.mdskills/javaee-to-quarkus/modules/lifecycle.mdskills/javaee-to-quarkus/modules/messaging.mdskills/javaee-to-quarkus/references/annotation-map.mdskills/javaee-to-quarkus/references/config-map.mdskills/javaee-to-quarkus/references/dependency-map.mdskills/javaee-to-quarkus/references/pattern-map.mdskills/javaee-to-quarkus/references/sources.mdskills/javaee-to-quarkus/references/verify-errors.mdskills/plan/SKILL.mdskills/plan/references/migration-phases.mdskills/plan/references/migration-plan-skill.mdskills/plan/skill.yamlskills/verify/SKILL.mdskills/verify/skill.yaml
💤 Files with no reviewable changes (47)
- harness/skill-bundle/goose-migration/SKILL.md
- images/agent-base-goose/Containerfile
- harness/internal/rundir/rundir.go
- harness/internal/plan/types.go
- harness/internal/plan/hints_test.go
- harness/recipes/fix.yaml
- harness/skill-bundle/goose-migration/references/javaee-quarkus.md
- harness/skill-bundle/goose-migration/references/README.md
- harness/skill-bundle/goose-migration/references/dotnet-framework-to-core.md
- harness/internal/plan/turns.go
- harness/internal/goose/goose.go
- harness/skill-bundle/goose-migration/references/REFERENCES_SUMMARY.md
- harness/skill-bundle/goose-migration/skills/javaee-quarkus/SKILL.md
- harness/internal/rundir/rundir_test.go
- harness/internal/goose/recipe_test.go
- harness/internal/plan/parser.go
- harness/internal/plan/recipe.go
- harness/skill-bundle/goose-migration/references/springboot-2-to-3.md
- harness/internal/plan/approval.go
- harness/internal/verify/verify.go
- harness/internal/verify/verify_test.go
- harness/internal/plan/parser_test.go
- images/agent-java-goose/Containerfile
- harness/internal/fixloop/fixloop_test.go
- harness/recipes/execute.yaml
- harness/internal/plan/hints.go
- harness/internal/execute/execute.go
- harness/internal/handoff/handoff.go
- harness/internal/detect/detect_test.go
- harness/internal/handoff/handoff_test.go
- harness/internal/metrics/metrics.go
- harness/skill-bundle/goose-migration/skills/migration-plan/MIGRATION_PLAN_SKILL_UPDATES.md
- harness/internal/plan/plan.go
- harness/internal/goose/acp_runner_test.go
- harness/internal/metrics/metrics_test.go
- harness/skill-bundle/goose-migration/skills/python2-to-python3/SKILL.md
- harness/internal/detect/detect.go
- harness/internal/goose/acp_runner.go
- harness/internal/execute/execute_test.go
- harness/internal/plan/context.go
- harness/recipes/verify.yaml
- harness/internal/fixloop/fixloop.go
- harness/internal/goose/recipe.go
- harness/internal/plan/turns_test.go
- harness/internal/git/credentials.go
- harness/skill-bundle/goose-migration/references/python2-to-python3.md
- harness/internal/plan/context_test.go
There was a problem hiding this comment.
@coderabbitai
Re: the skill content findings (migration guidance for Java EE → Quarkus, messaging semantics, security mappings, etc.) — these are valid improvements but will be addressed separately in a follow-up PR. The current skill content is a baseline to get the pipeline working end-to-end; refinements to migration accuracy and edge-case handling come next.
- Path traversal in Clone guard: use filepath.Abs + filepath.Rel instead of strings.HasPrefix; check os.RemoveAll error - docker image exists is not a Docker CLI command: use image inspect - Empty staging commit: track whether anything was staged, return early if not - Missing .kts in watcher allowlist: Kotlin DSL Gradle files ignored - FetchAnalysis swallows errors: return nil, err instead of nil, nil - os.Exit(1) skips deferred cleanup: return error so defers run Assisted-By: Claude Code <noreply@anthropic.com> Signed-off-by: Savitha Raghunathan <saveetha13@gmail.com>
2 participants
Replace multi-step CLI harness with thin ACP wrapper that runs a single migration stage (plan/execute/verify) per pod invocation, managed by the AgentPlaybookRun reconciler. Add per-stage skill OCI images, full javaee-to-quarkus skill from migration-skills repo, handoff.md generation with agent summaries and plan steps, filesystem watcher for incremental commits, and e2e test scripts with timestamped branch names.
Assisted-By: Claude Code noreply@anthropic.com
Summary by CodeRabbit
New Features
Enhancements
Documentation