Skip to content

fix: correct the partial-success guidance and the changelog's Foundation split - #225

Merged
kuny0707 merged 1 commit into
tronprotocol:developfrom
barbatos2011:fix/partial-success-guidance
Aug 23, 2026
Merged

kuny0707 merged 1 commit into
tronprotocol:developfrom
barbatos2011:fix/partial-success-guidance

Conversation

@barbatos2011

Copy link
Copy Markdown

Two mistakes in #224, found reviewing it after it had already merged.

The partial-success guidance was wrong in the direction that matters

#224 added this to AGENTS.md:

Re-running the whole command is usually wrong: the nodes that succeeded are already in the requested state.

Only one command emits PARTIAL_SUCCESS — network destroy — and for that one the reasoning is backwards. Its message is state cleaned up regardless (cmd/network/destroy.go:189): trond has already dropped every node from state before returning. So:

  • a retry returns NETWORK_NOT_FOUND, not a partial re-run;
  • the entries in failed are containers or systemd units still running on the target with nothing tracking them.

An agent following the old text would retry, get NETWORK_NOT_FOUND, and reasonably conclude the teardown had completed — while the containers kept running. That is a worse failure than the missing-code gap #224 set out to fix.

It now says: do not retry, surface the failed list for manual cleanup. It also records what the other multi-node commands do instead, so "multi-node" is no longer implied to mean "partial-reporting":

command on per-node failure
network destroy exit 1, error_code: PARTIAL_SUCCESS, removed + failed arrays, state already clean
network create stops at the first failed node, DEPLOY_ERROR, exit 1
network upgrade UPGRADE_FAILED, exit 1

The same over-generalisation is corrected in internal/output/exitcode.go and contracts/cli-contract.md, which #224 worded the same way.

The changelog's 40-command list sat under the wrong heading

#223 split ### Added into Foundation (what the never-published alpha section described) and Since (everything after). #224 replaced Foundation's 32 CLI commands bullet with an enumeration of all 40 — including recipe, schema, mcp, doctor, verify-config, build and shadow-fork, every one of which is itemised again below under Since.

Release notes that say "Foundation: everything" and then "Since: some of that again" are worse than the wrong count they replaced. The total is now a lead paragraph under ### Added, where it describes the release; Foundation goes back to naming the command groups the project launched with.

Checked while reviewing, no change needed

  • Only the root command defines PersistentPreRunE, so nothing shadows fix: reconcile the docs with what trond actually does #224's new --output validation or paths.SetBaseDir.
  • SchemaVersion governs the output-JSON schema manifest, not the CLI flag surface, so removing --config needs no version bump — cmd/schema_coverage_test.go and version_test.go agree.
  • shadow-fork mutate --config binds its own local flag and still works. Removing the root persistent --config in fact took away a latent shadowing hazard between the two.
  • trond completion bash is fine — an earlier report of exit 1 was my own shell quoting, not a regression.

No behaviour change here; docs plus one code comment. make test 25/25, gofmt and vet clean, knowledge/ and internal/knowledge/files/ still byte-identical.

…ion split

Two mistakes in tronprotocol#224, found reviewing it after it merged.

**The partial-success guidance was wrong in the direction that matters.** It
told agents "re-running the whole command is usually wrong: the nodes that
succeeded are already in the requested state." Only one command emits
PARTIAL_SUCCESS — `network destroy` — and there the reasoning is backwards.
Its message is `state cleaned up regardless`: trond has already dropped every
node from state, so a retry returns NETWORK_NOT_FOUND, and the entries in
`failed` are containers or units still running on the target with nothing
tracking them. An agent following the old text would retry, get
NETWORK_NOT_FOUND, and conclude the teardown was fine while the containers
kept running.

It now says: do not retry, surface the `failed` list for manual cleanup. It
also records what the other multi-node commands actually do instead —
`network create` stops at the first failure with DEPLOY_ERROR, `network
upgrade` with UPGRADE_FAILED — so "multi-node" is no longer implied to mean
"partial-reporting". Same over-generalisation corrected in exitcode.go and
contracts/cli-contract.md.

**The changelog's 40-command enumeration sat under the wrong heading.** tronprotocol#223
split `### Added` into **Foundation** (what the never-published alpha
described) and **Since** (everything after). tronprotocol#224 replaced Foundation's
"32 CLI commands" bullet with a list of all 40 — including recipe, schema,
mcp, doctor, verify-config, build and shadow-fork, every one of which is
itemised again below under Since. Release notes that say "Foundation:
everything" and then "Since: some of that again" are worse than the wrong
count they replaced. The total is now a lead paragraph under `### Added`,
describing the release; Foundation goes back to naming the command groups the
project launched with.

No behaviour change; docs and one comment. make test 25/25, gofmt and vet
clean, knowledge/ and internal/knowledge/files/ still byte-identical.
@kuny0707
kuny0707 merged commit 0c7451c into tronprotocol:develop Aug 23, 2026
10 checks passed
@barbatos2011
barbatos2011 deleted the fix/partial-success-guidance branch August 23, 2026 02:23
warku123 added a commit to warku123/tron-deployment that referenced this pull request Aug 27, 2026
…semantics

The runtime error claimed "state cleaned up regardless" while the
implementation keeps failed entries in tracked state and a re-run
retries exactly those nodes. That lie misled PR tronprotocol#225's docs correction
into encoding it in AGENTS.md; fixing the message at the source closes
the chain (root cause of tronprotocol#226 doc findings).
warku123 added a commit to warku123/tron-deployment that referenced this pull request Aug 27, 2026
…tions

First wave of the full-repo documentation audit (council + oracle
reviewed); zero behavior changes.

- AGENTS.md: truthful partial-state semantics for network destroy and
  create per code archaeology (create aggregates failures after trying
  every node; upgrade is genuinely first-failure-stop), wait example
  fixed to the real --json-path/--json-gt flags, versioning policy
  aligned with internal/schema/embed.go (additive = patch)
- schemas/output + embed copies: declare status.monitoring and
  inspect.intent_hash as already emitted (additive, SchemaVersion
  1.16.1); version baseline regenerated; synthetic contract assertions
  added in cmd/output_schema_contract_test.go
- internal/mcp/server.go: serverInstructions now list exactly the 23
  registered tools and prompt chains match their bodies
- knowledge/cloud-deployment.md (+ embedded mirror): drop the
  nonexistent --target flag, SSH setup via the intent target block
- examples/dev-local.yaml / README.md: real build prune flags, verify
  usage with required --intent
- CHANGELOG: user-visible fixes plus tronprotocol#225->tronprotocol#226 correction-chain
  provenance
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