Conversation
The page claimed all seven listed spellings print a notice naming v0.1.0-rc17, and stated a rigid two-releases-then-removed window. Both were wrong. deprecated() prints a rename with no version, so brig agents and brig template name no release; forward: and statePaths: warn only in the exported profile header; BRIG_TEMPLATE_DIR is not deprecated at all, it is a working alias for BRIG_PROFILE_DIR. Only BRIG_CREDENTIALS_CMD and hostCredential: name rc17, and they were deprecated in rc16, so the rigid two-release rule would put removal at rc18 and contradict their own notices. State the window as a minimum with the named release as the authority, and describe what each deprecation actually does rather than asserting a uniform schedule none of them follow. Refs: #75 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
The page claimed all seven listed spellings print a notice naming v0.1.0-rc17, and stated a rigid two-releases-then-removed window. Both were wrong. deprecated() prints a rename with no version, so brig agents and brig template name no release; forward: and statePaths: warn only in the exported profile header; BRIG_TEMPLATE_DIR is not deprecated at all, it is a working alias for BRIG_PROFILE_DIR. Only BRIG_CREDENTIALS_CMD and hostCredential: name rc17, and they were deprecated in rc16, so the rigid two-release rule would put removal at rc18 and contradict their own notices. State the window as a minimum with the named release as the authority, and describe what each deprecation actually does rather than asserting a uniform schedule none of them follow. Refs: #75 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
fd1c8ce to
335981c
Compare
The page claimed all seven listed spellings print a notice naming v0.1.0-rc17, and stated a rigid two-releases-then-removed window. Both were wrong. deprecated() prints a rename with no version, so brig agents and brig template name no release; forward: and statePaths: warn only in the exported profile header; BRIG_TEMPLATE_DIR is not deprecated at all, it is a working alias for BRIG_PROFILE_DIR. Only BRIG_CREDENTIALS_CMD and hostCredential: name rc17, and they were deprecated in rc16, so the rigid two-release rule would put removal at rc18 and contradict their own notices. State the window as a minimum with the named release as the authority, and describe what each deprecation actually does rather than asserting a uniform schedule none of them follow. Refs: #75 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
335981c to
331686e
Compare
There was no statement of what a version number promises: which surfaces a script or profile can depend on, which are free to change, and how long a deprecated spelling survives once its replacement ships. Deprecation notices told the reader a setting went "next release" without saying which one, so the deadline was a version number nobody could read. Add docs/compatibility.md. It states honestly that the project is prerelease-only, enumerates the covered surfaces from the source (the CLI verbs and flags, the documented BRIG_* settings, the profile schema, the exit status and the filesystem layout) and the ones that are unstable on purpose (the brigd line protocol, the JSON output, human-readable text, undocumented settings), fixes the deprecation window at two releases with removal in the third, and points at where breaking changes and the support matrix are recorded. Link it once from the README's documentation list. Name the release in every deprecation message that said "next release": the BRIG_CREDENTIALS_CMD usage line, the hostCredential: profile warning and the BRIG_CREDENTIALS_CMD run-time warning now say v0.1.0-rc17, which is the next tag after the current v0.1.0-rc16 and matches what docs/security.md already commits to. The existing tests for those two warnings assert the release is named. Fixes: #60 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
The page claimed all seven listed spellings print a notice naming v0.1.0-rc17, and stated a rigid two-releases-then-removed window. Both were wrong. deprecated() prints a rename with no version, so brig agents and brig template name no release; forward: and statePaths: warn only in the exported profile header; BRIG_TEMPLATE_DIR is not deprecated at all, it is a working alias for BRIG_PROFILE_DIR. Only BRIG_CREDENTIALS_CMD and hostCredential: name rc17, and they were deprecated in rc16, so the rigid two-release rule would put removal at rc18 and contradict their own notices. State the window as a minimum with the named release as the authority, and describe what each deprecation actually does rather than asserting a uniform schedule none of them follow. Refs: #75 Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
331686e to
fd9a7c4
Compare
ananos
left a comment
There was a problem hiding this comment.
Worth having -- naming the removal release in the notice, and making that named release the authority, is the part that turns a deprecation into a promise someone can plan against.
Two bits have gone stale while it sat in draft, inline.
|
|
||
| ## What 0.x means | ||
|
|
||
| brig has no stable release yet. There are twelve `v0.1.0-rc` tags and nothing |
There was a problem hiding this comment.
v0.1.0-rc17 shipped on 28 Aug, so the count is out of date. Perhaps say "every version to date is a release candidate" without the number -- then it cannot go stale again between drafting and merging.
| removed; a run that still sets it fails and names the replacement, rather | ||
| than warning and carrying on. | ||
| - The `hostCredential:` profile key prints a runtime notice naming | ||
| `v0.1.0-rc17` as the release that removes it. It was deprecated in |
There was a problem hiding this comment.
This names v0.1.0-rc17 as the release that removes hostCredential:, and rc17 has already shipped -- the field is still in internal/profile (it has left the shipped profiles, which is the other half). So the page would land stating a deadline that has passed.
Either remove the field and keep the promise, or move the removal to the next release and say so here. The first is truer to the policy this page is introducing; the second is fine too, as long as the notice in main.go moves with it. The test pins the string, so it will keep passing either way -- which is why it is worth catching by eye now.
ananos
left a comment
There was a problem hiding this comment.
Still worth having -- naming the removal release and making it the authority is the part that turns a deprecation into something someone can plan against.
The difficulty is that this branch is 30 commits behind main and has been open since 4 September, and this is the one page where being behind is worse than being absent. Every other doc that goes stale is merely wrong; a compatibility page that goes stale promises coverage for surfaces that are not there and omits the ones that are, and a reader has no way to tell. Most of what follows is not a flaw in the writing -- it is main having moved under it.
I checked each of these against the current tree rather than reading, and inlined them. Nothing here needs re-thinking, but the page wants a pass over its lists before it comes out of draft.
The underlying tension is worth a thought too: four of the six are hand-kept lists that duplicate something the binary already knows, which is the same drift #145 is about for verbs and flags. Where a list can be derived -- the verbs and the run-line flags both come off brigFlags and the dispatch table -- deriving it in a test would keep this page honest the way 6ed914b just did for the completion test.
|
|
||
| ## What 0.x means | ||
|
|
||
| brig has no stable release yet. There are twelve `v0.1.0-rc` tags and nothing |
There was a problem hiding this comment.
There are 13 v0.1.0-rc tags now, not twelve. This is the second time this number has gone stale on this branch -- it was eleven when I first read it.
So I would drop the count rather than correct it: "every version to date is a release candidate, and nothing without the -rc has shipped" says the same thing and cannot rot. The number adds no information a reader needs, and it is the one sentence on the page guaranteed to be wrong again by the next release.
| change only through a deprecation, and across a `1.0` they will not change | ||
| without a major bump. | ||
|
|
||
| - **The CLI verbs.** `run`, `create`, `exec`, `shell`, `stop`, `rm`, `ls`, |
There was a problem hiding this comment.
This list and brig's actual verbs have diverged a long way. Asked the binary built from current main:
agent completion doctor info ls policy rm run secret sh stop telemetry version
So the page covers create, exec, shell, reset, env, help and profiles, and does not mention agent, completion, doctor, info, policy, telemetry or sh. Some of that is renaming (shell to sh, env deprecated toward info), but doctor, policy, telemetry and completion are simply newer than the branch.
That matters more here than on an ordinary page: a promise that names the covered verbs, and omits half of them, tells a script author their brig policy usage is uncovered when it is not.
| `profile ls|export|import|edit|rm`, plus the top-level `export` and `import` | ||
| aliases), and the secret verbs (`secret create|read|update|delete|ls|import`). | ||
| The list is `cmd/brig/main.go`. | ||
| - **The run-line flags.** `-n`/`--name`, `-t`/`--image`, `-w`/`--workspace`, |
There was a problem hiding this comment.
Same drift, and this one closes the set explicitly -- "the set brig owns is the set here and no more" -- which makes an omission a wrong promise rather than an incomplete one.
What the run line actually offers today:
--cpus --detach --home --image --json --mem --network --no-project --offline --skills
Against the seven here: --name, --workspace and --memory are not among them, and --home, --json, --network, --no-project, --offline and --mem are missing from the page. --network is the one I would least want left out, since it is the flag that selects an isolation posture.
This is the list I would most like to see derived rather than typed: the rows are already in brigFlags with a position each, so a test could assert the page names exactly the posRun set.
| `hypervisor`, `runtimeBin`, `rootfsType`, `genericBoot` and `reserved`. The | ||
| types are `internal/profile`. The parser refuses an unknown field rather than | ||
| ignoring it, so adding one is a change you will see, not a silent no-op. | ||
| - **The exit status, coarsely.** `0` is success and non-zero is failure. On |
There was a problem hiding this comment.
This says a specific non-zero number is not a contract. README now says the opposite, under "Exit codes" at line 222:
brig returns a small, stable set of exit codes so a script can tell what went [wrong]
followed by a table giving each of 0-6 its own meaning. And #154 is building on that: internal/exitcode produces the same six for brigd, so brigd's clients will branch on them too.
Of the two, README's is the promise that is actually being kept, so I would bring this into line with it rather than the other way round -- otherwise the compatibility page is the more conservative of two documents that disagree, which is the confusing direction.
The sentence above it needs the same exception docs/security.md just gained on main: brig no longer always replaces itself with the runtime -- under --json it stays as the agent's parent to print one status line. The exit status is still the agent's own, so the promise holds; the mechanism described does not.
| appends `-<slug>`); brig's own runtime sockets under `~/.brig`; and custom | ||
| profiles under the config directory, `$XDG_CONFIG_HOME/brig` or | ||
| `~/.config/brig` when that is unset. `brig secret` stores each item in the | ||
| macOS login keychain under the service `sh.brig.secret`. |
There was a problem hiding this comment.
No longer macOS-only: #153 merged this morning, so brig secret also stores into a freedesktop Secret Service keyring on Linux (internal/secret/secretservice_linux.go), one item per secret in the default collection under the same sh.brig.secret service attribute.
Since this bullet is about the filesystem layout a user can depend on, it is worth saying both, and that the service name is the same on each.
Summary
Nothing said what a version number promised. Which surfaces were covered, how
long a deprecated spelling survived, and what a patch release could change were
all answered by whatever the last commit did, and three deprecation messages in
the tool said a thing "goes in the next release" without naming one.
docs/compatibility.mdstates it, short enough to stay true. Every publicsurface is classified from the source as covered or explicitly not covered, the
deprecation window is two releases with removal in the third, and breaking
changes are recorded where the changelog is generated from. The
brigdlineprotocol, the JSON output and the human-readable text are declared unstable on
purpose, which is the more useful half of the page.
The three "next release" messages now name
v0.1.0-rc17, with tests assertingthe release name so a later edit cannot slide back to "next".
Related issues
Closes #60
Refs #23, #20
Changes
docs/compatibility.md, linked once from the README's Documentation list.BRIG_CREDENTIALS_CMDandhostCredential:warnings, and the help-text line,name
v0.1.0-rc17instead of "next release".cmd/brig/deprecated_test.goand
internal/wrap/config_test.goassert it.Two things to know before merging
BRIG_CREDENTIALS_CMDis being removed in #23, in flight now. If that lands inrc17 the message is exactly right; if it slips, the policy's own rule is what
decides the number, not this text.
The policy says the support matrix lives in the README's platform sections. #20
is putting it in
docs/support.md; whichever merges second fixes the pointer.Checklist
make allpasses (vet, test, build)script/smoke.shpassesgo test ./... -race, if the change touches concurrency, subprocesses or the daemonI have exercised the change against a real runtime (brig run <agent>), if it touches the run, exec or credential pathdocs/security.md,docs/profiles.md,docs/brigd.md)Credentials and the sandbox boundary
Message text only. No behaviour changes on either promise.