EBP-2889: Address session-analysis feedback on solace-application-development

[aelsammak]

Changes

Rules moved to where every session reads them

• Promoted the buried mandates to SKILL.md invariants: AI-assisted disclaimer header on every generated source file, always-emitted verification artifacts, and a live-broker heads-up line before any run that connects.
• Surfaced the Implement Step 0 door question (Quickstart / Solace Suggested / Custom) and the environment-discovery rule (a running broker or found config.json is a fact to report, never an answer) at the jcsmp.md front door and in the Design mode handoff.

File restructuring

• Split implement-mode.md (previously too large for one Read, so later steps were unreachable) into a core file plus three per-leaf wiring files: guaranteed-pubsub, direct-pubsub, request-reply. VERIFY: marker strings are unchanged character for character.

Verification made shape-agnostic

• verify.sh gains an app stage driven by a generated verify-hooks.sh, so web and embedded apps verify with the same script as the two-CLI shape. Added a warn-only conformance preflight. The six existing stages are untouched.

Logging and versioning fixes

• Removed the log4j2 version pin (it contradicted the no-pins invariant). Replaced with outcome rules: resolve the highest stable 2.x at generation time, one backend only, config file required, and the 2.17.1 Log4Shell floor enforced as a check.

Doc grounding

• Grounding is bound per design question, and the summary's Grounding docs field may only list pages actually fetched in the session. Removed the never-followed Core Concepts intro mandate.

Design-first routing

• Mode routing now keys on the design contract, not phrasing: every build request without a confirmed design enters Design mode first, web and embedded shapes included. Implement Step 2 makes the eight-field summary a hard precondition.
• A fully specified prompt resolves in ONE confirm (summary echoed at close-out). Edits to existing apps split by a topology rule: structural changes re-enter Design, mechanical edits go straight to Implement.
• The door question stays its own isolated step; Implement no longer re-asks whether a broker exists when Design already confirmed it.

Comment preservation

• New comments-follow-their-code contract in Step 4: a construct carried from a sample keeps that construct's best-practice comment, dropped demo harness takes its comments with it, and fresh messaging code names the practice it applies. Backed by a new binary item in the checklist's Generation conforma

Door simplification: Non-Secure removed

• Solace Suggested is TLS-secure by definition: tcps:// with server-certificate validation, requiring a TLS-configured broker. The Secure / Non-Secure sub-question is gone, so the door question is a clean three-way choice.
• On a no-TLS broker, the skill states the requirement and offers Custom with the equivalent knobs minus the secure session, or Quickstart. It never generates a plaintext session under the Solace Suggested name.
• Quickstart's plaintext posture is now explicit: no TLS requirement, and a tcp:// dev brokerrget, so the Suggested TLS rule cannot be over-generalized.

[mtite-sol]

Added some of my personal comments inline but overall looks good to me, I think splitting things out is the right choice!

I have some general feedback of things to test / look out for like fail-fast scenarios around docs being down, deprecated features discovered while polling latest on active dev, etc but we can take that offline it doesn't gate this PR!

I got Claude to take a second look afterwards here are the results (non-blocking unless you agree with its feedback):

1. implement-mode.md Step 6 mis-describes its own preflight. It lists the preflight-reported items as "disclaimer header, logging floor, version freshness, the recorded verify stage and exit code" — the last one can't be a preflight check (the preflight runs before
   the stage) — and it omits "this tailored checklist exists at every project root", which the preflight does check. It also says only the comments item is non-preflight, but the new Generation-conformance group has three (comments, stage/exit code,
   grounding-docs-fetched). The group heading "(checked by the verify.sh preflight)" over-promises for 3 of 7 items.

2. The log4j conformance item claims far more than the preflight implements. The checklist asserts "exactly one logging backend … any log4j-core on the classpath (including transitive) at or above 2.17.1 … the com.solacesystems loggers are not silenced". The
   preflight regexes a literal two lines under log4j-core in pom.xml — it cannot see a transitive log4j-core, a property-driven version, a second backend, or a logger level. mvn dependency:list would actually close it. Separately,
   sort -V is GNU-only, so the floor check silently no-ops on macOS — the one platform the new group-kill comment goes out of its way to support.

3. The app stage's trigger is the only unbounded wait in a script whose stated rule is that nothing waits unbounded. bash -c "$TRIGGER_CMD" runs in the foreground with no deadline. Every other wait uses a SECONDS deadline, and the header says "NO teardown path may
   use a bare unbounded wait". A curl against a wedged web app — precisely the shape this stage exists for — hangs verify.sh forever with the app still up.

4 . ./verify-hooks.sh sources generated shell straight into the observer's scope. The design promise is "the hooks carry the ONLY app-specific facts; the observer logic stays in this script", but a hooks file can cd, set SUB_PID/SUB_LOG/status/rc/TIMEOUT_S, or
redefine cleanup, and silently break the observer. Cheap fix: pull the four values out of a subshell, or document reserved names in the generated template.

5. The group-kill conversion is incomplete. cleanup and shutdown_long_running now signal the group, but the publisher/requestor timeout escalations still do bare-PID kill -TERM "$PUB_PID" / kill -KILL "$PUB_PID" (three sites). Harmless for mvn exec:java today, but
   it's the same orphan hole the round was closing.

6. Solace Suggested's new single-deliverable carve-out contradicts the rule two lines above it. Option 2 — "one Maven build with separate publisher and consumer modules under a parent pom" — is offered as keeping the decoupling, directly under "there is NO parent
   POM and NO aggregator" and against a checklist item reading "its own pom and NO parent POM". Only option 3 is described as recorded under Developer-owned items, so choosing option 2 leaves the checklist item unsatisfied and unrecorded.

7. Minor: preflight and the live-run banner fire before the stage name is validated. verify.sh bogus announces "Live-run heads-up: stage 'bogus' starts processes that connect to broker …" and makes a network call to repo1.maven.org, then errors with "unknown
   stage".

8. Minor: the sol-jcsmp freshness warn conflates two things. It compares the pom to on every run, so a project that resolved correctly at generation time starts warning the moment Solace ships a release. The checklist item is scoped "at generation time";
   the check isn't.

9. Question count before any code. The body sells "a fully specified prompt resolves in ONE confirm" — that's only the design confirm. A fully-specified "build me X" now walks Design Step 0 (do you have a broker?) → design close-out confirm → Implement Step 0 door
   question → Implement Step 1 (which broker?). Three or four turns before generation, on a PR whose stated purpose is addressing session-analysis feedback. The two broker questions especially read as one question split across modes.

One follow-up on the SIGKILL residual: worth asking whether the agent harness's abort sends TERM (covered) or KILL (not). If KILL, an orphaned subscriber JVM keeps the EXCLUSIVE queue flow and breaks every later verify run — the script's own header says exactly
that — so a stale-process check at startup would be the real close-out.

[mtite-sol]

Changes look good to me!

[cjwmorgan-sol]

LGTM