From bb5b1d78073229d9d92ea8ee2ddb90088b672990 Mon Sep 17 00:00:00 2001 From: dnth Date: Fri, 16 Oct 2026 17:09:29 +0800 Subject: [PATCH 1/8] fix(ext): preserve questions on settled decision posts Keep the original last chunk beneath a bold outcome, reserve room for settlement headers, and highlight a unique choice on disabled buttons. --- bin/fm-ext-ask.sh | 46 ++-- bin/fm-ext-decision.sh | 153 ++++++++++- bin/fm-ext-emit.sh | 50 +++- bin/fm-ext-lib.sh | 16 +- .../decisions.py | 23 +- .../outbox_poster.py | 91 +++++-- docs/configuration.md | 14 +- tests/fm-ext-ask.test.sh | 14 +- tests/fm-ext-decision.test.sh | 247 +++++++++++++++--- 9 files changed, 552 insertions(+), 102 deletions(-) diff --git a/bin/fm-ext-ask.sh b/bin/fm-ext-ask.sh index c50405ffc40..1384df49a69 100755 --- a/bin/fm-ext-ask.sh +++ b/bin/fm-ext-ask.sh @@ -5,7 +5,7 @@ # fm-ext-ask.sh --request-id discord:::: # --kind answer|followup|final --generation # --task [--title ] [--repo <repo>] --reason <reason> -# (--text-file <path> | --text-file -) +# (--text-file <path> | --text-file -) [--explanation-file <path>] # --option <label> [--recommended] [--confirm] # [--option <label> [--recommended] [--confirm]] ... (1-5 options) # --mode done|release [--thread-title <text>] @@ -19,13 +19,13 @@ # 2. writes the decision file (key hold:<task-id>, the mode, the options); # 3. posts through bin/fm-ext-emit.sh --decision-file with # --thread-state needs-you. -# The post text is the --text-file body followed by the numbered option lines, -# one per option in button order, and a one-line cue, so the text and the buttons -# always match. A recommended option shows "⭐ " in both. --recommended marks the -# option just before it as the recommended choice (at most one); --confirm asks -# for a second click on the option just before it. Options whose label matches -# FM_EXT_DECISION_CONFIRM_PATTERN in bin/fm-ext-lib.sh (merge, deploy, delete, -# and other risky words) get the confirmation click automatically. +# The post text keeps the question, optional explanation, numbered option +# lines, and reply cue as separate sections for safe chunking and settlement. +# The numbered option lines follow button order, and a recommended option shows +# "⭐ " in both. --recommended marks the option just before it (at most one); +# --confirm asks for a second click on the option just before it. Options whose +# label matches FM_EXT_DECISION_CONFIRM_PATTERN in bin/fm-ext-lib.sh (merge, +# deploy, delete, and other risky words) get the confirmation click automatically. # --mode is required and has no default: it chooses how a chosen option closes # the call, as in bin/fm-captain-hold.sh answers. done closes a question; # release lifts the hold so held work proceeds. A held build task that has not @@ -56,7 +56,7 @@ usage() { usage: fm-ext-ask.sh --request-id discord:<guild>:<channel>:<thread>:<message> --kind answer|followup|final --generation <n> --task <task-id> [--title <title>] [--repo <repo>] --reason <reason> - (--text-file <path> | --text-file -) + (--text-file <path> | --text-file -) [--explanation-file <path>] --option <label> [--recommended] [--confirm] [--option <label> ...] --mode done|release [--thread-title <text>] EOF @@ -76,10 +76,10 @@ TITLE= REPO= REASON= TEXT_FILE= +EXPLANATION_FILE= MODE= THREAD_TITLE= OPTIONS_JSON='[]' - case "${1:-}" in --help|-h) help; exit 0 ;; '') usage; exit 2 ;; @@ -96,6 +96,7 @@ while [ "$#" -gt 0 ]; do --repo) shift; REPO=${1:-} ;; --reason) shift; REASON=${1:-} ;; --text-file) shift; TEXT_FILE=${1:-} ;; + --explanation-file) shift; EXPLANATION_FILE=${1:-} ;; --mode) shift; MODE=${1:-} ;; --thread-title) shift; THREAD_TITLE=${1:-} ;; --option) @@ -143,6 +144,7 @@ jq -e --argjson max "$FM_EXT_DECISION_MAX_OPTIONS" --argjson label_max "$FM_EXT_ ' <<<"$OPTIONS_JSON" >/dev/null 2>&1 \ || die "give 1-$FM_EXT_DECISION_MAX_OPTIONS distinct --option labels of 1-$FM_EXT_DECISION_LABEL_MAX characters, at most one --recommended" +KEY="hold:$TASK" if [ "$TEXT_FILE" = '-' ]; then TEXT=$(cat) else @@ -150,10 +152,15 @@ else TEXT=$(cat -- "$TEXT_FILE") fi [ -n "$TEXT" ] || die "text is empty" +EXPLANATION= +if [ -n "$EXPLANATION_FILE" ]; then + [ -f "$EXPLANATION_FILE" ] && [ ! -L "$EXPLANATION_FILE" ] \ + || die "explanation file not found: $EXPLANATION_FILE" + EXPLANATION=$(cat -- "$EXPLANATION_FILE") +fi -KEY="hold:$TASK" -# The key and task id ride only in the button ids; refuse before holding. -if { printf '%s\n' "$TEXT"; jq -r '.[].label' <<<"$OPTIONS_JSON"; } | grep -qiF -e "$TASK" -e "$KEY"; then +if { printf '%s\n' "$TEXT" "$EXPLANATION"; jq -r '.[].label' <<<"$OPTIONS_JSON"; } \ + | grep -qiF -e "$TASK" -e "$KEY"; then die "the text and labels must not name the task id" fi @@ -161,12 +168,19 @@ fi LINES=$(jq -r '. as $o | range(0; length) as $i | $o[$i] | "\($i + 1). " + (if (.recommended // false) and ((.label | startswith("⭐")) | not) then "⭐ " + .label else .label end)' \ <<<"$OPTIONS_JSON") || die "could not number the options" 1 -FULL_TEXT=$(printf '%s\n\n%s\nTap a button, or reply with the number.' "$TEXT" "$LINES") +CUE='Tap a button, or reply with the number.' +BODY=$TEXT +[ -z "$EXPLANATION" ] || BODY=$(printf '%s\n\n%s' "$TEXT" "$EXPLANATION") +FULL_TEXT=$(printf '%s\n\n%s\n%s' "$BODY" "$LINES" "$CUE") WORK=$(mktemp -d "${TMPDIR:-/tmp}/fm-ext-ask.XXXXXX") || die "could not make a scratch directory" 1 trap 'rm -rf "$WORK"' EXIT -jq -n --arg key "$KEY" --arg mode "$MODE" --argjson options "$OPTIONS_JSON" \ - '{key: $key, mode: $mode, options: $options}' > "$WORK/decision.json" \ +jq -n --arg key "$KEY" --arg mode "$MODE" --arg question "$TEXT" \ + --arg explanation "$EXPLANATION" --arg option_text "$LINES" --arg cue "$CUE" \ + --argjson options "$OPTIONS_JSON" \ + '{key: $key, mode: $mode, options: $options, + sections: {question: $question, explanation: $explanation, + options: $option_text, cue: $cue}}' > "$WORK/decision.json" \ || die "could not write the decision file" 1 emit=("$SCRIPT_DIR/fm-ext-emit.sh" --request-id "$REQUEST_ID" --kind "$KIND" diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index 3ccb64a57c0..a1cc07faad4 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -238,10 +238,12 @@ post_content() { fi } -# settle_post <stem> <kind> <text>: the post's final outcome line, written once. +# settle_post <stem> <kind> <text> [selected option id]: write the final +# outcome once, including the option whose button should remain highlighted. settle_post() { - jq -cn --arg kind "$2" --arg text "$3" --argjson at "$NOW" \ - '{kind:$kind, text:$text, at:$at}' \ + jq -cn --arg kind "$2" --arg text "$3" --arg selected "${4:-}" --argjson at "$NOW" \ + '{kind:$kind, text:$text, at:$at} + + if $selected == "" then {} else {selected_option_id:($selected | tonumber)} end' \ | fm_ext_private_artifact_publish_stdin_once "$OUTBOX" "$1.settled.json" 600 [ "$?" -le 1 ] } @@ -262,6 +264,55 @@ newer_post() { return 1 } +# newer_post_link <stem>: the newest newer button post's jump URL, when known. +newer_post_link() { + local stem=$1 key posted file candidate candidate_posted best_stem= + local request guild thread message best_posted=0 + key=$(post_field "$stem" '.decision.key') + posted=$(post_field "$stem" '.posted_at') + while IFS= read -r file; do + candidate=$(basename "$file" .progress.json) + [ "$candidate" = "$stem" ] && continue + if jq -e --arg key "$key" --argjson posted "${posted:-0}" --argjson best "$best_posted" \ + '.decision.key == $key and .buttons == true + and ((.posted_at // 0) > $posted) and ((.posted_at // 0) > $best)' \ + "$file" >/dev/null 2>&1; then + candidate_posted=$(jq -r '.posted_at // 0' "$file" 2>/dev/null) + best_posted=$candidate_posted + best_stem=$candidate + fi + done < <(progress_files) + [ -n "$best_stem" ] || return 0 + file="$OUTBOX/$best_stem.progress.json" + request=$(jq -r '.request_id // ""' "$file") + guild=${request#discord:} + guild=${guild%%:*} + message=$(jq -r '.discord_message_ids[-1] // empty' "$file") + case "$guild" in ''|*[!0-9]*) return 0 ;; esac + case "$message" in ''|*[!0-9]*) return 0 ;; esac + thread=$(post_thread "$best_stem") || return 0 + printf 'https://discord.com/channels/%s/%s/%s\n' "$guild" "$thread" "$message" +} + +# selected_option_id <stem>: the option chosen for this settled or sent post. +selected_option_id() { + local stem=$1 kind outcome option + if settled "$stem"; then + kind=$(jq -r '.kind // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) + [ "$kind" = decided ] || return 0 + option=$(jq -r '.selected_option_id // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) + if [ -z "$option" ]; then + outcome=$(post_outcome "$stem") || return 0 + option=$(answer_option "$stem" "${outcome#*$'\t'}") + fi + elif sent "$stem"; then + option=$(sent_field "$stem" '.option_id') + else + return 0 + fi + case "$option" in [1-9]) printf '%s\n' "$option" ;; esac +} + # answer_option <stem> <answer>: ignore the audit suffix, then accept a unique # longest full label prefix immediately. Only without that prefix, collect # standalone option numbers and whole-word labels and require one distinct id. @@ -343,7 +394,7 @@ post_outcome() { # or whose own lifecycle closed. Returns 0 when the post is settled after it, # 1 while the decision is still open, and 2 when its state cannot be read. settle_if_closed() { - local stem=$1 outcome state + local stem=$1 outcome state option settled "$stem" && return 0 if newer_post "$stem"; then settle_post "$stem" replaced "Replaced by a newer post $(discord_time "$NOW")" || return 2 @@ -354,7 +405,10 @@ settle_if_closed() { case "$state" in open) return 1 ;; decided|superseded) - settle_post "$stem" "$state" "$(outcome_text "$stem" "$state" "${outcome#*$'\t'}")" || return 2 + option= + [ "$state" != decided ] || option=$(answer_option "$stem" "${outcome#*$'\t'}") + settle_post "$stem" "$state" "$(outcome_text "$stem" "$state" "${outcome#*$'\t'}")" "$option" \ + || return 2 return 0 ;; esac @@ -552,8 +606,64 @@ command_click() { notice 0 "Sent: $label. Firstmate will record it, and the post updates when it does." } +# fit_post_content <header> <original text> <progress file>: preserve the +# original last chunk when it fits; structured posts trim only explanation, +# while legacy overflow keeps the header and option labels before the question prefix. +fit_post_content() { + jq -njr --arg header "$1" --arg original "$2" --slurpfile progress "$3" ' + def compose($header; $question; $explanation; $options; $cue; $trim_question): + ($header + "\n\n") as $base + | (if $options == "" then "" else "\n\n" + $options end) as $option_suffix + | (if $cue == "" then "" else "\n" + $cue end) as $cue_suffix + | ($option_suffix + $cue_suffix) as $suffix + | "\n\n(trimmed)" as $marker + | (($base | length) + ($suffix | length)) as $fixed + | if ($fixed + ($question | length)) > 2000 then + if $trim_question then + (2000 - $fixed - ($marker | length)) as $room + | $base + ($question[0:(if $room < 0 then 0 else $room end)]) + $marker + $suffix + else + error("structured decision header, question, and options exceed the Discord limit") + end + else + (2000 - $fixed - ($question | length)) as $room + | if $explanation == "" then + $base + $question + $suffix + elif (2 + ($explanation | length)) <= $room then + $base + $question + "\n\n" + $explanation + $suffix + elif $room >= (2 + ($marker | length)) then + ($room - 2 - ($marker | length)) as $explanation_room + | $base + $question + "\n\n" + + $explanation[0:$explanation_room] + $marker + $suffix + else + if $trim_question then + (2000 - $fixed - ($marker | length)) as $question_room + | $base + ($question[0:(if $question_room < 0 then 0 else $question_room end)]) + $marker + $suffix + else + error("structured decision header leaves no room for the explanation marker") + end + end + end; + $progress[0] as $post + | ($header + "\n\n" + $original) as $whole + | if ($whole | length) <= 2000 then + $whole + else + ($post.decision.sections // {}) as $sections + | if $sections.structured == true then + compose($header; $sections.question; $sections.explanation; + $sections.options; $sections.cue; false) + else + compose($header; $original; ""; + ([$post.decision.options[] | "\(.id). \(.label)"] | join("\n")); ""; true) + end + end + ' +} + command_posts() { - local settled_only=0 file stem thread content shown final + local settled_only=0 file stem thread content original displayed shown final + local kind header replacement_url selected [ "${1:-}" != --settled-only ] || settled_only=1 while IFS= read -r file; do stem=$(basename "$file" .progress.json) @@ -563,16 +673,39 @@ command_posts() { fi content=$(post_content "$stem") [ -n "$content" ] || continue - shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null) || shown= - [ "$content" != "$shown" ] || continue + original=$(jq -j '.texts[-1] // ""' "$file"; printf '\001') + original=${original%$'\001'} + selected=$(selected_option_id "$stem") + replacement_url= + if settled "$stem"; then + kind=$(jq -r '.kind // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) + else + kind=waiting + fi + if [ "$kind" = replaced ]; then + replacement_url=$(newer_post_link "$stem") + if [ -n "$replacement_url" ]; then + header="♻️ **Replaced by a newer question** ([open]($replacement_url))" + else + header="**Replaced by a newer question**" + fi + else + header="**$content**" + fi + displayed=$(fit_post_content "$header" "$original" "$file") || continue + shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null; printf '\001') || shown= + shown=${shown%$'\001'} + [ "$displayed" != "$shown" ] || continue final=false settled "$stem" && final=true thread=$(post_thread "$stem") || continue jq -cn --arg post "$stem" --arg channel "$thread" \ --arg message "$(post_field "$stem" '.discord_message_ids[-1]')" \ - --arg content "$content" --argjson final "$final" \ + --arg content "$displayed" --arg selected "$selected" --argjson final "$final" \ --slurpfile progress "$file" \ - '{post:$post, channel_id:$channel, message_id:$message, content:$content, final:$final, + '{post:$post, channel_id:$channel, message_id:$message, content:$content, + final:$final, + selected_option_id:(if $selected == "" then null else ($selected | tonumber) end), key:$progress[0].decision.key, options:$progress[0].decision.options}' done < <(progress_files) } diff --git a/bin/fm-ext-emit.sh b/bin/fm-ext-emit.sh index a74463a22ec..62990b36483 100755 --- a/bin/fm-ext-emit.sh +++ b/bin/fm-ext-emit.sh @@ -34,25 +34,32 @@ # The file is JSON: # {"key": "hold:<task-id>" | "key:<task-id>:<slug>", # "mode": "done" | "release", -# "options": [{"label": "<text>", "confirm": true|false, "recommended": true|false}, ...]} +# "options": [{"label": "<text>", "confirm": true|false, "recommended": true|false}, ...], +# "sections": {"question": "<text>", "explanation": "<text>", +# "options": "<numbered list>", "cue": "<reply cue>"}} # with 1-5 options whose input and displayed labels are distinct, 1-80 characters, and free of # control characters, in the same order as the post text numbers them; option # ids are 1-based in that order. "mode" (captain calls only, default done) # chooses how a chosen option closes the call, as in fm-captain-hold.sh # answers. "confirm" and "recommended" are optional booleans, default false. +# "sections" is optional; when present, its question, explanation, option list, +# and cue must reproduce the post text exactly. Without it, the whole post is +# protected question text. fm-ext-ask.sh writes these boundaries explicitly. +# A decision post reserves room for its later outcome header and keeps the +# option list on the final button-carrying chunk. # At most one option may set "recommended": true; emit adds "⭐ " unless the # label already starts with ⭐, and stores the displayed label in the payload. -# The added prefix counts toward the 80-character label limit. # The key must name a decision that is open right now (a held captain # call, or a worker's open keyed status decision) and must fit Discord's # 100-character custom_id as fmd:<key>:<n>. An option needs a confirmation # click when it declares confirm, and always when its label matches # FM_EXT_DECISION_CONFIRM_PATTERN in bin/fm-ext-lib.sh. The post text and labels # are refused when they name the key or its task id, because only the button -# custom_ids may carry the key. The payload's decision field carries the key, -# mode, numbered options, the post text cut to its first 1000 characters, and -# the decision's current lifecycle (fm_ext_decision_key_occurrence in -# bin/fm-ext-lib.sh), so the post can only ever answer that lifecycle. +# custom_ids may carry the key. The payload's decision field also carries +# the mode, numbered options, post text cut to its first 1000 characters, +# structured post sections, and current lifecycle +# (fm_ext_decision_key_occurrence in bin/fm-ext-lib.sh), so a post can only ever +# answer that lifecycle. # # CHOICE GUARD. An answer, followup, or final whose text has the shape of a choice is refused unless --decision-file is given, so a choice cannot go out as plain text by mistake. # Detection is case-insensitive, cues match within one line, and it uses these rules: @@ -230,6 +237,7 @@ esac # A decision is validated in full before anything is published, so a refused # decision posts nothing at all. DECISION_JSON=null +DECISION_SECTIONS=null if [ -n "$DECISION_FILE" ]; then [ -f "$DECISION_FILE" ] && [ ! -L "$DECISION_FILE" ] || die "decision file not found: $DECISION_FILE" jq -e \ @@ -249,8 +257,15 @@ if [ -n "$DECISION_FILE" ]; then and ([.options[] | select(.recommended // false)] | length <= 1) and ([.options[].label | ascii_downcase] | length == (unique | length)) and ([.options[] | (if (.recommended // false) and ((.label | startswith("⭐")) | not) then ("⭐ " + .label) else .label end) | ascii_downcase] | length == (unique | length)) + and (if has("sections") then + (.sections | type == "object" + and (.question | type == "string") + and (.explanation | type == "string") + and (.options | type == "string") + and (.cue | type == "string")) + else true end) ' "$DECISION_FILE" >/dev/null 2>&1 \ - || die "decision file must hold {key, mode, options:[{label, confirm, recommended}]} with 1-$FM_EXT_DECISION_MAX_OPTIONS distinct labels of 1-$FM_EXT_DECISION_LABEL_MAX characters and at most one recommended option" + || die "decision file must hold {key, mode, options:[{label, confirm, recommended}], sections?} with 1-$FM_EXT_DECISION_MAX_OPTIONS distinct labels of 1-$FM_EXT_DECISION_LABEL_MAX characters and at most one recommended option" DECISION_KEY=$(jq -r '.key' "$DECISION_FILE") DECISION_MODE=$(jq -r '.mode // "done"' "$DECISION_FILE") DECISION_TASK=$(fm_ext_decision_key_task "$DECISION_KEY") \ @@ -277,13 +292,30 @@ if [ -n "$DECISION_FILE" ]; then # call is re-held or the key reopened is a no-op. DECISION_OCCURRENCE=$(fm_ext_decision_key_occurrence "$DECISION_KEY") \ || die "could not read which lifecycle of decision $DECISION_KEY is open" 1 + if jq -e 'has("sections")' "$DECISION_FILE" >/dev/null 2>&1; then + jq -e --arg text "$TEXT" ' + .sections as $s + | ($s.question + + (if $s.explanation == "" then "" else "\n\n" + $s.explanation end) + + (if $s.options == "" then "" else "\n\n" + $s.options end) + + (if $s.cue == "" then "" else "\n" + $s.cue end)) == $text + ' "$DECISION_FILE" >/dev/null \ + || die "decision sections must reproduce the post text exactly" 2 + DECISION_SECTIONS=$(jq -c '.sections + {structured: true}' "$DECISION_FILE") \ + || die "could not read decision sections" 1 + else + DECISION_SECTIONS=$(jq -cn --arg question "$TEXT" \ + '{question:$question, explanation:"", options:"", cue:"", structured:false}') \ + || die "could not build default decision sections" 1 + fi DECISION_JSON=$(jq -c \ --arg pattern "$FM_EXT_DECISION_CONFIRM_PATTERN" \ --arg mode "$DECISION_MODE" \ --arg occurrence "$DECISION_OCCURRENCE" \ --arg text "$TEXT" \ - --argjson text_max "$FM_EXT_DECISION_TEXT_MAX" ' - {key, mode: $mode, occurrence: $occurrence, + --argjson text_max "$FM_EXT_DECISION_TEXT_MAX" \ + --argjson sections "$DECISION_SECTIONS" ' + {key, mode: $mode, occurrence: $occurrence, sections: $sections, options: [.options | to_entries[] | ({"id": (.key + 1), "label": (if (.value.recommended // false) and ((.value.label | startswith("⭐")) | not) then ("⭐ " + .value.label) else .value.label end), "confirm": ((.value.confirm // false) or (.value.label | test($pattern; "i")))} diff --git a/bin/fm-ext-lib.sh b/bin/fm-ext-lib.sh index f964aae02d2..bbcf6069a8c 100644 --- a/bin/fm-ext-lib.sh +++ b/bin/fm-ext-lib.sh @@ -501,9 +501,9 @@ fm_ext_allowlisted() { # button's Discord custom_id carries that key and a 1-based option id # (fmd:<key>:<n>, or fmd:<key>:d for Discuss). The decision itself lives only # in those ledgers, and only Firstmate records an answer there: a click files a -# request like a typed reply. The bridge keeps just what a post needs to be -# found and redrawn - the decision in the post's chunk-progress record, its sent -# click, its final line, and what Discord shows (bin/fm-ext-decision.sh). +# request like a typed reply. The bridge keeps only what a post needs to be +# found and redrawn: decision options and structured question, explanation, and +# numbered-list sections in chunk progress, its sent click, outcome, and display. # Most options one decision post carries: one Discord action row. FM_EXT_DECISION_MAX_OPTIONS=5 @@ -1301,7 +1301,15 @@ fm_ext_outbox_schema_valid() { and (.id | type == "number") and (.label | type == "string" and length >= 1 and length <= $label_max) and (.confirm | type == "boolean") - and ((.recommended // false) | type == "boolean")))) + and ((.recommended // false) | type == "boolean"))) + and (if has("sections") then + (.sections | type == "object" + and (.question | type == "string") + and (.explanation | type == "string") + and (.options | type == "string") + and (.cue | type == "string") + and (.structured | type == "boolean")) + else true end)) else true end) ' "$file" >/dev/null 2>&1 } diff --git a/contrib/hermes-gateway-firstmate-comms/decisions.py b/contrib/hermes-gateway-firstmate-comms/decisions.py index 087d6541e3e..2f5e01acae6 100644 --- a/contrib/hermes-gateway-firstmate-comms/decisions.py +++ b/contrib/hermes-gateway-firstmate-comms/decisions.py @@ -89,32 +89,39 @@ def confirm_id(post_id: str, option: int) -> str: return f"fmc:{post_id}:{option}" -def components_for(decision: dict, disabled: bool = False) -> list[dict]: - """The decision's buttons: one row of options, then the Discuss button. - Recommendation styling does not waive confirmation; the click handler - still requires it before filing a request. - """ +def components_for( + decision: dict, + disabled: bool = False, + selected_option_id: int | str | None = None, +) -> list[dict]: + """Build decision buttons, showing the recorded choice on a settled post.""" key = decision["key"] options = [] for option in decision["options"]: - if option.get("recommended"): + selected = disabled and str(option["id"]) == str(selected_option_id) + if disabled: + style = _STYLE_SUCCESS if selected else _STYLE_SECONDARY + elif option.get("recommended"): style = _STYLE_SUCCESS elif option.get("confirm"): style = _STYLE_DANGER else: style = _STYLE_SECONDARY + label = option["label"] + if selected: + label = label[:78].rstrip() + " ✓" options.append( { "type": _BUTTON, "style": style, - "label": option["label"], + "label": label, "custom_id": custom_id(key, option["id"]), "disabled": disabled, } ) discuss = { "type": _BUTTON, - "style": _STYLE_PRIMARY, + "style": _STYLE_SECONDARY if disabled else _STYLE_PRIMARY, "label": DISCUSS_LABEL, "custom_id": custom_id(key, "d"), "disabled": disabled, diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index 0f38c4ab45f..a786aaeb72e 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -34,13 +34,15 @@ request, which is how ``firstmate_post_for`` recognises a reply to one of Firstmate's own messages. -A payload carrying a ``decision`` posts its option buttons on its last chunk, -but only while ``decisions.buttons_ready()`` says the gateway's Discord client -routes the clicks; otherwise the decision posts as plain text that the captain -can still answer by replying to it. Its chunk-progress record also keeps the -decision, whether buttons were posted, and when, which is how a click finds -its post. ``reconcile_decision_posts`` edits each settled decision post to its -outcome line with its buttons disabled, and ``reconcile_thread_states`` then +A decision payload carries buttons only on the final chunk and only when +``decisions.buttons_ready()`` confirms click routing; otherwise it stays plain +text. Progress keeps the decision, button state, chunk ids, and posting time. +Structured sections keep options together on the final chunk and reserve room +for a later outcome header. +``reconcile_decision_posts`` edits only that final message, prepending a bold +outcome line and preserving the original last-chunk text whenever it fits. +It disables every button, highlights a unique choice in success style, and +grays the remaining options and Discuss. ``reconcile_thread_states`` then clears answered needs-you threads and retries failed renames. """ @@ -87,6 +89,9 @@ REACT_DONE = "\u2705" # check mark: the answer has posted REACT_FAILED = "\u26a0\ufe0f" # warning: a reply could not be delivered _SNOWFLAKE = re.compile(r"^[0-9]+$") + +DISCORD_MESSAGE_MAX = 2000 +DECISION_OUTCOME_HEADER_RESERVE = 256 # A request slug is the SHA-256 hex of its canonical id, and the request-wide # topic-thread record beside its payload is named "<slug>.thread.json". _SLUG = re.compile(r"^[0-9a-f]{64}$") @@ -376,6 +381,46 @@ def split_reply( } +def split_decision_reply( + payload: dict, + home: Path, + limit: int | None = None, + cap: int | None = None, + legacy: bool = False, +) -> dict: + """Split decision sections while keeping their option list on the button post.""" + text = payload.get("text") or "" + decision = payload.get("decision") or {} + sections = decision.get("sections") or {} + resolved = reply_limits(home) if limit is None or cap is None else None + used_limit = limit if limit is not None else resolved["limit"] + used_cap = cap if cap is not None else resolved["cap"] + budget = min(used_limit, DISCORD_MESSAGE_MAX - DECISION_OUTCOME_HEADER_RESERVE) + if legacy or not sections.get("structured"): + return split_reply(text, home=home, limit=budget, cap=used_cap, legacy=legacy) + + question = sections.get("question", "") + explanation = sections.get("explanation", "") + options = sections.get("options", "") + cue = sections.get("cue", "") + prefix = question + (("\n\n" + explanation) if explanation else "") + suffix = (("\n\n" + options) if options else "") + (("\n" + cue) if cue else "") + if prefix + suffix != text: + raise ValueError("decision sections do not reproduce the post text") + budget = min(DISCORD_MESSAGE_MAX - DECISION_OUTCOME_HEADER_RESERVE, + max(budget, len(suffix) + 1)) + prefix_budget = budget - len(suffix) + if prefix_budget < 1: + raise ValueError("decision option list leaves no room for question text") + chunks = discord_split.split_discord(prefix, prefix_budget, used_cap) + if not chunks: + chunks = [""] + chunks[-1] += suffix + if any(len(chunk) > budget for chunk in chunks): + raise RuntimeError("decision chunk exceeded its reserved message budget") + return {"limit": budget, "cap": used_cap, "texts": chunks} + + def progress_path(payload: dict, home: Path) -> Path: name = f"{payload['slug']}.{payload['kind']}.{payload['generation']}.progress.json" return state_dir(home) / "ext-outbox" / name @@ -1499,7 +1544,11 @@ def reconcile_decision_posts( for line in result.stdout.splitlines(): try: item = json.loads(line) - components = decisions.components_for(item, disabled=True) + components = decisions.components_for( + item, + disabled=True, + selected_option_id=item.get("selected_option_id"), + ) (edit or discord_edit)(item["channel_id"], item["message_id"], item["content"], components) except DiscordSendError as err: if err.outcome != "permanent": @@ -1770,17 +1819,29 @@ def deliver_one( return status sender = send or discord_send stored = load_progress(payload, home) + decision = isinstance(payload.get("decision"), dict) try: if stored and "texts" in stored: split = {"texts": stored["texts"], "limit": stored["limit"], "cap": stored["cap"]} else: - split = split_reply( - payload.get("text") or "", - home=home, - limit=stored.get("limit") if stored else None, - cap=stored.get("cap") if stored else None, - legacy=bool(stored), - ) + limit = stored.get("limit") if stored else None + cap = stored.get("cap") if stored else None + if decision: + split = split_decision_reply( + payload, + home=home, + limit=limit, + cap=cap, + legacy=bool(stored), + ) + else: + split = split_reply( + payload.get("text") or "", + home=home, + limit=limit, + cap=cap, + legacy=bool(stored), + ) except Exception: if stored and int(stored.get("posted_count") or 0) > 0: release_inflight(payload, home=home) diff --git a/docs/configuration.md b/docs/configuration.md index 4a25a0d402e..5a5c9d6e7b7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -980,9 +980,12 @@ Reactions are best effort and use the same bot token and User-Agent as replies: Firstmate can attach option buttons to an `answer`, `followup`, or `final`, so the captain sends a choice to Firstmate from the post. Each decision post is bound to exactly one decision Firstmate already tracks, named by its key: `hold:<task-id>` for a captain call held on the board, or `key:<task-id>:<slug>` for a worker's keyed status decision. -`bin/fm-ext-ask.sh` is the one command that asks the captain a choice: it places or refreshes the captain hold for the task, writes the decision file, appends the numbered option lines to the post text in button order so text and buttons always match, and posts through `bin/fm-ext-emit.sh --decision-file` with the thread marked needs-you; its header owns the flags. +`bin/fm-ext-ask.sh` is the one command that asks the captain a choice: it places or refreshes the captain hold, writes the decision file, and posts matching numbered options. +Its `--text-file` is the question, optional `--explanation-file` is the explanation, and it records those sections separately from the numbered option list and reply cue for safe chunking and later edits. +The gateway reserves room for the outcome header on the final button-carrying chunk and keeps the numbered options together there; its header and the [emit header](../bin/fm-ext-emit.sh) own the section format and flags. Its `--mode done|release` is required and never inferred: `done` closes a question and `release` lifts the hold so held work proceeds. For a worker's existing keyed status decision, use `bin/fm-ext-emit.sh --decision-file` directly; its [header](../bin/fm-ext-emit.sh) owns the file format and validation. +If its file omits `sections`, the whole post is treated as protected question text. The [emit header's choice guard](../bin/fm-ext-emit.sh) owns the matching rules, including digit-selection cues and standalone choice headings followed by at least two option lines, and routes refused plain-text choices to `bin/fm-ext-ask.sh`. `--no-buttons "<reason>"` posts a deliberate plain list anyway and records the reason in the outbox payload as `no_buttons_reason`; the emit header owns the exact detection rule. Each option button's `custom_id` carries the key and the option id, `fmd:<key>:<n>`, and every decision post also carries a Discuss button, `fmd:<key>:d`. @@ -1010,7 +1013,14 @@ A recommended option that also needs confirmation still needs its second click. A confirmed click from a standing-rule author is that author's explicit word for that one decision only, never standing authority, and merge authority stays where `AGENTS.md` puts it. Each post is bound to the lifecycle of its decision that was open when it was posted, so a captain call re-held or a worker key reopened for a new question is a different decision to it. -When Firstmate records an answer on any channel, its posts are disabled and edited to show the outcome, read from the same ledgers rather than from any step Firstmate must remember: `✅ <option> chosen by <user> <time>` names the clicker when the recorded answer is the option that post's click sent, and the captain for any other recorded answer that names exactly one option by number or label; `✅ Decided <time>` covers answers that cannot be matched to one option, `Closed - no longer needed <time>` covers a decision closed with no captain answer or replaced by a later lifecycle, and `Replaced by a newer post <time>` covers an older post for the same decision. +When Firstmate records an answer on any channel, its posts are disabled and edited with a bold outcome header above the exact original last-chunk text whenever both fit Discord's 2,000-character limit. +The uniquely chosen option is success-colored and gains a `✓`; every other option and Discuss are grey, and all buttons are disabled. +When no single option matches, every button is grey; a click waiting for Firstmate to record it still highlights the clicked option. +The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. +When a newer post replaces an older one for the same decision, the older outcome header links to the newest post when its message id is known and otherwise says `Replaced by a newer question`. +Structured posts reserve space for that header, so a normal settlement edit needs no truncation. +If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; for an older unstructured post that still overflows, it keeps the header, as much question text as fits, the marker, and the complete option labels. +Both paths remain within Discord's character limit, and the outcome is read from the same ledgers rather than from any step Firstmate must remember. Option matching ignores the appended Discord-button audit and the `Decision answer: ` prefix. For a recommended option, matching accepts its label with or without the leading star, while the outcome keeps the displayed label. A unique longest full option label at the start of the answer wins immediately, regardless of trailing text; otherwise the matcher collects standalone option numbers and whole-word labels and accepts only one distinct option. diff --git a/tests/fm-ext-ask.test.sh b/tests/fm-ext-ask.test.sh index 8bc5202bc5d..cc8ae3aed58 100755 --- a/tests/fm-ext-ask.test.sh +++ b/tests/fm-ext-ask.test.sh @@ -80,10 +80,11 @@ test_1_ask_holds_and_posts_buttons_in_text_order() { ln -sf "$(command -v tasks-axi)" "$home/fakebin/tasks-axi" fm_fake_exit0 "$home/fakebin" treehouse no-mistakes gh gh-axi printf 'The fix can go two ways.' > "$home/body.txt" + printf 'North takes less time; South keeps the existing API.' > "$home/explanation.txt" slug=$(home_env "$home" "$ASK" --request-id "$RID" --kind followup --generation 1 \ --task "$TASK" --title "Pick the route" --repo sample --reason "route pending" --mode "done" \ - --text-file "$home/body.txt" \ + --text-file "$home/body.txt" --explanation-file "$home/explanation.txt" \ --option "North" --recommended --option "South" --option "Merge it" \ --option "Hold" --confirm) || fail "fm-ext-ask refused a valid choice" [ -n "$slug" ] || fail "fm-ext-ask printed no slug" @@ -99,8 +100,15 @@ test_1_ask_holds_and_posts_buttons_in_text_order() { and ([.decision.options[] | .id] == [1,2,3,4]) and ([.decision.options[] | .label] == ["⭐ North","South","Merge it","Hold"]) and ([.decision.options[] | .confirm] == [false,false,true,true]) - and .decision.options[0].recommended == true' "$payload" >/dev/null \ - || fail "wrong decision block: $(jq -c .decision "$payload")" + and .decision.options[0].recommended == true + and .decision.sections == { + "question":"The fix can go two ways.", + "explanation":"North takes less time; South keeps the existing API.", + "options":"1. ⭐ North\n2. South\n3. Merge it\n4. Hold", + "cue":"Tap a button, or reply with the number.", + "structured":true + }' "$payload" >/dev/null \ + || fail "wrong decision block or structured post sections: $(jq -c .decision "$payload")" # The numbered lines in the text follow the buttons, label for label. jq -r '.text' "$payload" | grep -E '^[0-9]+\. ' | sed -E 's/^[0-9]+\. //' > "$home/text-labels" jq -r '.decision.options[].label' "$payload" > "$home/button-labels" diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 51e9ebe7b20..0d50a8d75e3 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -130,9 +130,10 @@ import sys from pathlib import Path import decisions, outbox_poster decisions.buttons_ready = lambda: True -outbox_poster.drain_outbox( - send=lambda p: {"ok": True, "discord_message_id": sys.argv[3], "buttons": bool(p.get("decision"))}, - home=Path(sys.argv[1])) +def send(p): + return {"ok": True, "discord_message_id": str(int(sys.argv[3]) + int(p.get("chunk_index", 0))), + "buttons": bool(p.get("decision"))} +outbox_poster.drain_outbox(send=send, home=Path(sys.argv[1])) PY } @@ -383,22 +384,31 @@ test_2_standing_click_files_one_request_for_a_worker_key() { # The post's buttons go off at once with the sent line. out=$(reconcile "$home" False) - printf '%s' "$out" | jq -e --arg key "$KEY" --arg msg "$POST" ' - .message == $msg - and .content == "⏳ North sent by Dana <t:1791720000:f> - waiting for Firstmate to record it" + printf '%s' "$out" | jq -e --arg key "$KEY" --arg msg "$POST" \ + --arg expected $'**⏳ North sent by Dana <t:1791720000:f> - waiting for Firstmate to record it**\n\nWhich way should the fix go?\n1. North ⭐\n2. South\nReply to this message with the number.' ' + .message == $msg and .content == $expected and ([.components[].components[] | .disabled] | all) + and .components[0].components[0].style == 3 + and .components[0].components[0].label == "North ✓" + and .components[0].components[1].style == 2 + and .components[1].components[0].style == 2 and (.components[0].components[0].custom_id == ("fmd:" + $key + ":1"))' >/dev/null \ - || fail "a sent click must turn the post's buttons off with the sent line: $out" + || fail "a sent click must keep its question visible and highlight only its disabled choice: $out" [ -z "$(reconcile "$home" True)" ] || fail "an unchanged sent post was edited again" # Firstmate records the request through its normal path; the post follows. [ "$(record_as_firstmate "$home" "$record")" = open ] || fail "Firstmate found the clicked decision no longer open" [ "$(resolved_lines "$home" pick)" = 1 ] || fail "Firstmate's normal path did not record the answer" out=$(FM_EXT_NOW_OVERRIDE=1791720100 reconcile "$home" True) - printf '%s' "$out" | jq -e --arg msg "$POST" ' - .message == $msg and .content == "✅ North chosen by Dana <t:1791720000:f>" - and ([.components[].components[] | .disabled] | all)' >/dev/null \ - || fail "the post must show the recorded choice with its buttons off: $out" + printf '%s' "$out" | jq -e --arg msg "$POST" \ + --arg expected $'**✅ North chosen by Dana <t:1791720000:f>**\n\nWhich way should the fix go?\n1. North ⭐\n2. South\nReply to this message with the number.' ' + .message == $msg and .content == $expected + and ([.components[].components[] | .disabled] | all) + and .components[0].components[0].style == 3 + and .components[0].components[0].label == "North ✓" + and .components[0].components[1].style == 2 + and .components[1].components[0].style == 2' >/dev/null \ + || fail "the post must preserve its question and mark only the recorded choice: $out" [ -z "$(reconcile "$home" True)" ] || fail "a post Discord already shows was edited again" pass "2 a standing click files one request with key, option, text, and audit, turns the buttons off, and shows the choice once Firstmate records it" } @@ -517,11 +527,11 @@ test_3_captain_call_click_is_a_request_the_board_records() { out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) printf '%s\n' "$out" | jq -se ' (map({(.message): .content}) | add) as $shown - | $shown["700000000000000001"] == "✅ South chosen by Dana <t:1791720000:f>" - and $shown["700000000000000006"] == "✅ South chosen by the captain <t:1791720000:f>" - and $shown["700000000000000007"] == "Closed - no longer needed <t:1791720000:f>" - and $shown["700000000000000008"] == "Closed - no longer needed <t:1791720000:f>" - and $shown["700000000000000010"] == "Closed - no longer needed <t:1791720000:f>"' \ + | ($shown["700000000000000001"] | startswith("**✅ South chosen by Dana <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000006"] | startswith("**✅ South chosen by the captain <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000007"] | startswith("**Closed - no longer needed <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000008"] | startswith("**Closed - no longer needed <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000010"] | startswith("**Closed - no longer needed <t:1791720000:f>**\n\nWhich way"))' \ >/dev/null || fail "button, board, unanswered, and re-held calls must each show their own outcome: $out" for pair in digits overlap; do case "$pair" in @@ -538,8 +548,9 @@ test_3_captain_call_click_is_a_request_the_board_records() { home_env "$home" "$ROOT/bin/fm-captain-hold.sh" answer "$id-$pair" --decision-file "$home/answer.txt" >/dev/null \ || fail "could not record the undecorated captain-call answer" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - jq -e --arg expected "✅ ⭐ $label chosen by the captain <t:1791720000:f>" '.content == $expected' <<<"$out" >/dev/null \ - || fail "the recommended captain-call outcome picked the wrong label: $out" + jq -e --arg expected "✅ ⭐ $label chosen by the captain <t:1791720000:f>" \ + '.content | startswith("**" + $expected + "**\n\nWhich way")' <<<"$out" >/dev/null \ + || fail "the recommended captain-call outcome picked the wrong label or lost its question: $out" done pass "3 a captain-call click files a request the board records; board answers, unanswered closes, and re-holds show their own outcome" } @@ -704,13 +715,20 @@ test_6_answered_anywhere_settles_posts_and_clicks_are_no_ops() { out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) printf '%s\n' "$out" | jq -se ' (map({(.message): .content}) | add) as $shown - | $shown["700000000000000001"] == "✅ North chosen by Dana <t:1791720000:f>" - and $shown["700000000000000002"] == "✅ South chosen by the captain <t:1791720000:f>" - and $shown["700000000000000003"] == "✅ South chosen by the captain <t:1791720000:f>" - and $shown["700000000000000004"] == "✅ Decided <t:1791720000:f>" - and $shown["700000000000000005"] == "Closed - no longer needed <t:1791720000:f>" - and $shown["700000000000000006"] == "✅ South chosen by the captain <t:1791720000:f>" - and $shown["700000000000000007"] == "✅ Decided <t:1791720000:f>" + | ($shown["700000000000000001"] | startswith("**✅ North chosen by Dana <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000002"] | startswith("**✅ South chosen by the captain <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000003"] | startswith("**✅ South chosen by the captain <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000004"] | startswith("**✅ Decided <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000005"] | startswith("**Closed - no longer needed <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000006"] | startswith("**✅ South chosen by the captain <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000007"] | startswith("**✅ Decided <t:1791720000:f>**\n\nWhich way")) + and (map(select(.message == "700000000000000002"))[0] + | .components[0].components[0].style == 2 + and .components[0].components[1].style == 3 + and .components[0].components[1].label == "South ✓" + and .components[1].components[0].style == 2) + and (map(select(.message == "700000000000000004"))[0].components + | [.[].components[] | .style] | all(. == 2)) and all(.[]; [.components[].components[].disabled] | all)' >/dev/null \ || fail "every answered or superseded post must be disabled and show its outcome from the ledger: $out" out=$(click "$home" "fmd:key:$TASK:typed:1" "$CAPTAIN" 880000000000000041 700000000000000002) @@ -750,11 +768,11 @@ test_7_stale_posts_are_no_ops() { assert_contains "$out" "Sent: South." "the replacement post's option 1 must send its own label" [ "$(record_as_firstmate "$home" "$(button_request "$home" option)")" = open ] || fail "Firstmate found the clicked decision no longer open" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - printf '%s\n' "$out" | jq -se ' + printf '%s\n' "$out" | jq -se --arg url "https://discord.com/channels/$GUILD/$CHANNEL/700000000000000011" ' (map({(.message): .content}) | add) as $shown - | $shown["700000000000000011"] == "✅ South chosen by Dana <t:1791720000:f>" - and $shown["700000000000000010"] == "Replaced by a newer post <t:1791720000:f>"' >/dev/null \ - || fail "a replaced post must show it was replaced, not the winner's index read through its own options: $out" + | ($shown["700000000000000011"] | startswith("**✅ South chosen by Dana <t:1791720000:f>**\n\nWhich way")) + and ($shown["700000000000000010"] | startswith("♻️ **Replaced by a newer question** ([open](" + $url + "))\n\nWhich way"))' >/dev/null \ + || fail "the newer post must show its own choice and the replaced post must link to it: $out" # A worker key answered and then reopened for a new question: the old post # belongs to the first question and must not answer the second. @@ -1160,7 +1178,8 @@ test_14_audit_metadata_does_not_match_options() { deliver "$home" 700000000000000011 home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key ambiguous-audit 'Decision answer: Merge or Hold (Discord button: option 2 Hold)' >/dev/null 2>&1 || fail "could not record the ambiguous answer" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - jq -e '.content == "✅ Merge chosen by the captain <t:1791720000:f>"' <<<"$out" >/dev/null || fail "a full label prefix must win after audit removal" + jq -e '.content | startswith("**✅ Merge chosen by the captain <t:1791720000:f>**\n\nWhich way")' \ + <<<"$out" >/dev/null || fail "a full label prefix must win after audit removal and preserve the question" pass "14 option matching ignores appended audit metadata and gives the full label prefix precedence" } @@ -1223,7 +1242,9 @@ test_15_full_labels_keep_digits_and_overlaps_together() { home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key "$slug" "$answer" >/dev/null 2>&1 || fail "could not record the typed full-label answer" fi out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - jq -e --arg expected "$expected" '.content == $expected and ([.components[].components[].disabled] | all)' <<<"$out" >/dev/null || fail "the full-label outcome lost its choice, attribution, or fallback ambiguity: $out" + jq -e --arg expected "$expected" \ + '.content | startswith("**" + $expected + "**\n\nWhich way")' <<<"$out" >/dev/null \ + || fail "the full-label outcome lost its choice, attribution, fallback ambiguity, or original question: $out" done done done @@ -1252,12 +1273,29 @@ PY .[0].components[0] == {"type":2,"style":3,"label":"⭐ North","custom_id":("fmd:" + $key + ":1"),"disabled":false} and .[0].components[1] == {"type":2,"style":4,"label":"Merge it","custom_id":("fmd:" + $key + ":2"),"disabled":false}' \ >/dev/null || fail "the recommended button must be green and the other keeps its style: $out" + out=$(plugin_py "$home" <<'PY' +import json, decisions +decision = {"key": "key:route-fix:limit", "options": [ + {"id": 1, "label": "x" * 80}, {"id": 2, "label": "Other"}]} +print(json.dumps(decisions.components_for( + decision, disabled=True, selected_option_id=1))) +PY + ) || fail "the renderer refused a maximum-length selected label" + printf '%s' "$out" | jq -e ' + .[0].components[0].style == 3 + and .[0].components[0].label == (("x" * 78) + " ✓") + and (.[0].components[0].label | length == 80) + and .[0].components[1].style == 2 + and .[1].components[0].style == 2 + and all(.[].components[]; .disabled)' >/dev/null \ + || fail "the selected marker must fit the Discord label limit and leave other buttons grey: $out" deliver "$home" "$POST" home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key pick 'Decision answer: Choose North' >/dev/null 2>&1 \ || fail "could not record the undecorated recommended label fallback" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - jq -e '.content == "✅ ⭐ North chosen by the captain <t:1791720000:f>"' <<<"$out" >/dev/null \ - || fail "the recommended label fallback lost its display label: $out" + jq -e '.content | startswith("**✅ ⭐ North chosen by the captain <t:1791720000:f>**\n\nWhich way")' \ + <<<"$out" >/dev/null \ + || fail "the recommended label fallback lost its display label or original question: $out" open_key "$home" confirm-rec payload=$(emit_decision "$home" 2 '{"key":"key:'"$TASK"':confirm-rec","options":[ {"label":"Merge it","confirm":true,"recommended":true},{"label":"Hold"}]}') @@ -1279,8 +1317,9 @@ PY record=$(button_request "$home" option) [ "$(record_as_firstmate "$home" "$record")" = open ] || fail "Firstmate found the recommended decision no longer open" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - printf '%s' "$out" | jq -e --arg expected "✅ ⭐ Merge it chosen by Dana <t:1791720000:f>" '.content == $expected' >/dev/null \ - || fail "the recorded answer label must match the shown label: $out" + printf '%s' "$out" | jq -e --arg expected "✅ ⭐ Merge it chosen by Dana <t:1791720000:f>" \ + '.content | startswith("**" + $expected + "**\n\nWhich way")' >/dev/null \ + || fail "the recorded answer label or original question was lost: $out" open_key "$home" pre-starred deliver "$home" 700000000000000032 payload=$(emit_decision "$home" 3 '{"key":"key:'"$TASK"':pre-starred","options":[ @@ -1311,6 +1350,143 @@ PY pass "16 the recommended option renders green with a single star, still confirms when risky, and emit refuses two recommended options" } +# --- 17. settled edits preserve sections and fit legacy posts --------------- + +test_17_outcome_headers_fit_reserved_and_legacy_posts() { + local home question explanation option_text cue full slug progress payload original out repeat legacy last_message + local old_payload new_payload old_progress new_progress + home="$TMP_ROOT/c17" + setup_home "$home" + open_request "$home" + + open_key "$home" near-limit + question=$(printf '%*s' 1800 '' | tr ' ' Q) + option_text=$'1. North\n2. South' + cue='Tap a button, or reply with the number.' + full=$(printf '%s\n\n%s\n%s' "$question" "$option_text" "$cue") + printf '%s' "$full" > "$home/post.txt" + jq -n --arg key "key:$TASK:near-limit" --arg question "$question" \ + --arg options "$option_text" --arg cue "$cue" \ + '{key:$key, options:[{label:"North"},{label:"South"}], + sections:{question:$question, explanation:"", options:$options, cue:$cue}}' \ + > "$home/decision.json" + slug=$(home_env "$home" "$EMIT" --request-id "discord:$GUILD:$CHANNEL:$CHANNEL:$REQUEST_MESSAGE" \ + --kind followup --generation 1 --text-file "$home/post.txt" \ + --decision-file "$home/decision.json") || fail "emit refused structured near-limit decision" + payload="$home/state/ext-outbox/$slug.followup.1.json" + deliver "$home" 700000000000000101 + progress="${payload%.json}.progress.json" + jq -e '.decision.sections.structured == true + and .decision.sections.explanation == "" + and (.texts | length == 2) + and .discord_message_ids == ["700000000000000101", "700000000000000102"] + and (.limit as $limit | all(.texts[]; length <= $limit)) + and (.texts[-1] | contains("1. North\n2. South\nTap a button, or reply with the number."))' \ + "$progress" >/dev/null || fail "the decision split lost its options or header reserve" + original=$(jq -r '.texts[-1]' "$progress") + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key near-limit North >/dev/null 2>&1 \ + || fail "could not settle the near-limit decision" + out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) + printf '%s\n' "$out" | jq -se --arg original "$original" ' + length == 1 + and .[0].message == "700000000000000102" + and .[0].content == ("**✅ North chosen by the captain <t:1791720000:f>**\n\n" + $original) + and (.[0].content | length <= 2000) + and ([.[0].components[].components[] | .disabled] | all) + and .[0].components[0].components[0].style == 3 + and .[0].components[0].components[0].label == "North ✓" + and .[0].components[0].components[1].style == 2 + and .[0].components[1].components[0].style == 2' >/dev/null \ + || fail "only the last chunk must be edited, with its original text and selected button preserved: $out" + + + open_key "$home" explanation-overflow + question='Which option should the team record?' + explanation=$(printf '%*s' 2300 '' | tr ' ' E) + full=$(printf '%s\n\n%s\n\n%s\n%s' "$question" "$explanation" "$option_text" "$cue") + printf '%s' "$full" > "$home/post.txt" + jq -n --arg key "key:$TASK:explanation-overflow" --arg question "$question" \ + --arg explanation "$explanation" --arg options "$option_text" --arg cue "$cue" \ + '{key:$key, options:[{label:"North"},{label:"South"}], + sections:{question:$question, explanation:$explanation, options:$options, cue:$cue}}' \ + > "$home/decision.json" + slug=$(home_env "$home" "$EMIT" --request-id "discord:$GUILD:$CHANNEL:$CHANNEL:$REQUEST_MESSAGE" \ + --kind followup --generation 2 --text-file "$home/post.txt" \ + --decision-file "$home/decision.json") || fail "emit refused the structured explanation fixture" + payload="$home/state/ext-outbox/$slug.followup.2.json" + deliver "$home" 700000000000000110 + progress="${payload%.json}.progress.json" + last_message=$(jq -r '.discord_message_ids[-1]' "$progress") + if ! jq --arg full "$full" '.texts[-1] = $full' "$progress" \ + > "$home/explanation-progress.json"; then + fail "could not force the explanation overflow fixture" + fi + mv "$home/explanation-progress.json" "$progress" \ + || fail "could not install the explanation overflow fixture" + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key explanation-overflow North \ + >/dev/null 2>&1 || fail "could not settle the explanation overflow" + out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) + printf '%s' "$out" | jq -e --arg message "$last_message" --arg question "$question" \ + --arg option_text "$option_text" --arg cue "$cue" ' + .message == $message + and (.content | startswith("**✅ North chosen by the captain <t:1791720000:f>**\n\n" + $question + "\n\nE")) + and (.content | contains("(trimmed)\n\n" + $option_text + "\n" + $cue))' >/dev/null \ + || fail "overflow must trim only the explanation tail and retain the full question and options: $out" + + open_key "$home" legacy-overflow + payload=$(emit_decision "$home" 3 '{"key":"key:'"$TASK"':legacy-overflow","options":[{"label":"North"},{"label":"South"}]}') + deliver "$home" 700000000000000120 + progress="${payload%.json}.progress.json" + legacy=$(printf '%*s' 2300 '' | tr ' ' L) + if ! jq --arg legacy "$legacy" '.texts[-1] = $legacy | del(.decision.sections)' \ + "$progress" > "$home/legacy-progress.json"; then + fail "could not build the legacy overflow fixture" + fi + mv "$home/legacy-progress.json" "$progress" \ + || fail "could not install the legacy overflow fixture" + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy-overflow \ + "No preference yet" >/dev/null 2>&1 || fail "could not settle the legacy decision" + out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) + printf '%s' "$out" | jq -e ' + .message == "700000000000000120" + and (.content | length <= 2000) + and (.content | startswith("**✅ Decided <t:1791720000:f>**\n\nL")) + and (.content | contains("(trimmed)\n\n1. North\n2. South")) + and ([.components[].components[] | .disabled] | all) + and ([.components[].components[] | .style] | all(. == 2))' >/dev/null \ + || fail "the legacy fallback must keep its header, question prefix, marker, option labels, and grey buttons: $out" + + open_key "$home" replace-fallback + old_payload=$(emit_decision "$home" 4 \ + '{"key":"key:'"$TASK"':replace-fallback","options":[{"label":"North"}]}') + deliver "$home" 700000000000000130 + new_payload=$(emit_decision "$home" 5 \ + '{"key":"key:'"$TASK"':replace-fallback","options":[{"label":"South"}]}') + deliver "$home" 700000000000000140 + old_progress="${old_payload%.json}.progress.json" + new_progress="${new_payload%.json}.progress.json" + if ! jq '.posted_at = 1000' "$old_progress" > "$home/old-progress.json"; then + fail "could not order the old post fixture" + fi + mv "$home/old-progress.json" "$old_progress" || fail "could not install old-post ordering" + if ! jq '.posted_at = 1001 | .discord_message_ids = []' "$new_progress" \ + > "$home/new-progress.json"; then + fail "could not hide the newer message id" + fi + mv "$home/new-progress.json" "$new_progress" || fail "could not install missing newer message id" + out=$(reconcile "$home" True) + printf '%s' "$out" | jq -e ' + .message == "700000000000000130" + and (.content | startswith("**Replaced by a newer question**\n\nWhich way should the fix go?")) + and (.content | length <= 2000) + and ([.components[].components[] | .disabled] | all) + and ([.components[].components[] | .style] | all(. == 2))' >/dev/null \ + || fail "the replacement fallback must keep the question and disable the old buttons: $out" + repeat=$(reconcile "$home" True) + [ -z "$repeat" ] || fail "settled overflow and replacement edits must not retry unchanged posts" + pass "17 reserved chunks settle without trimming, only the last chunk is edited, and legacy overflow retains its header, question prefix, option labels, and one-time replacement state" +} + test_1_buttons_carry_key_and_option_ids_in_the_thread test_2_standing_click_files_one_request_for_a_worker_key test_3_captain_call_click_is_a_request_the_board_records @@ -1330,3 +1506,4 @@ test_14_audit_metadata_does_not_match_options test_15_full_labels_keep_digits_and_overlaps_together test_16_recommended_option_renders_green_with_a_single_star +test_17_outcome_headers_fit_reserved_and_legacy_posts From 6228a03b83cb6a56016a0d20f9005ca6352b31c1 Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 17:29:53 +0800 Subject: [PATCH 2/8] no-mistakes(review): Preserve decision questions and legacy delivery boundaries --- bin/fm-ext-decision.sh | 24 +--- .../outbox_poster.py | 14 ++- docs/configuration.md | 5 +- tests/fm-ext-decision.test.sh | 107 ++++++++++++++++-- 4 files changed, 115 insertions(+), 35 deletions(-) diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index a1cc07faad4..e90bd6ad310 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -607,11 +607,10 @@ command_click() { } # fit_post_content <header> <original text> <progress file>: preserve the -# original last chunk when it fits; structured posts trim only explanation, -# while legacy overflow keeps the header and option labels before the question prefix. +# original last chunk when it fits; structured posts trim only explanation. fit_post_content() { jq -njr --arg header "$1" --arg original "$2" --slurpfile progress "$3" ' - def compose($header; $question; $explanation; $options; $cue; $trim_question): + def compose($header; $question; $explanation; $options; $cue): ($header + "\n\n") as $base | (if $options == "" then "" else "\n\n" + $options end) as $option_suffix | (if $cue == "" then "" else "\n" + $cue end) as $cue_suffix @@ -619,12 +618,7 @@ fit_post_content() { | "\n\n(trimmed)" as $marker | (($base | length) + ($suffix | length)) as $fixed | if ($fixed + ($question | length)) > 2000 then - if $trim_question then - (2000 - $fixed - ($marker | length)) as $room - | $base + ($question[0:(if $room < 0 then 0 else $room end)]) + $marker + $suffix - else - error("structured decision header, question, and options exceed the Discord limit") - end + error("decision header, question, and options exceed the Discord limit") else (2000 - $fixed - ($question | length)) as $room | if $explanation == "" then @@ -636,12 +630,7 @@ fit_post_content() { | $base + $question + "\n\n" + $explanation[0:$explanation_room] + $marker + $suffix else - if $trim_question then - (2000 - $fixed - ($marker | length)) as $question_room - | $base + ($question[0:(if $question_room < 0 then 0 else $question_room end)]) + $marker + $suffix - else - error("structured decision header leaves no room for the explanation marker") - end + error("decision header leaves no room for the explanation marker") end end; $progress[0] as $post @@ -652,10 +641,9 @@ fit_post_content() { ($post.decision.sections // {}) as $sections | if $sections.structured == true then compose($header; $sections.question; $sections.explanation; - $sections.options; $sections.cue; false) + $sections.options; $sections.cue) else - compose($header; $original; ""; - ([$post.decision.options[] | "\(.id). \(.label)"] | join("\n")); ""; true) + error("legacy decision text cannot safely fit its outcome header") end end ' diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index a786aaeb72e..d43db24c96e 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -395,9 +395,15 @@ def split_decision_reply( resolved = reply_limits(home) if limit is None or cap is None else None used_limit = limit if limit is not None else resolved["limit"] used_cap = cap if cap is not None else resolved["cap"] + if legacy: + return split_reply(text, home=home, limit=used_limit, cap=used_cap, legacy=True) budget = min(used_limit, DISCORD_MESSAGE_MAX - DECISION_OUTCOME_HEADER_RESERVE) - if legacy or not sections.get("structured"): - return split_reply(text, home=home, limit=budget, cap=used_cap, legacy=legacy) + if not sections.get("structured"): + split = split_reply(text, home=home, limit=budget, cap=max(used_cap, len(text) + 1)) + if len(split["texts"]) > used_cap: + raise ValueError("decision question exceeds the configured chunk cap") + split["cap"] = used_cap + return split question = sections.get("question", "") explanation = sections.get("explanation", "") @@ -412,7 +418,9 @@ def split_decision_reply( prefix_budget = budget - len(suffix) if prefix_budget < 1: raise ValueError("decision option list leaves no room for question text") - chunks = discord_split.split_discord(prefix, prefix_budget, used_cap) + chunks = discord_split.split_discord(prefix, prefix_budget, max(used_cap, len(prefix) + 1)) + if len(chunks) > used_cap: + raise ValueError("decision question and explanation exceed the configured chunk cap") if not chunks: chunks = [""] chunks[-1] += suffix diff --git a/docs/configuration.md b/docs/configuration.md index 5a5c9d6e7b7..32dbb3632e2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1019,8 +1019,9 @@ When no single option matches, every button is grey; a click waiting for Firstma The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. When a newer post replaces an older one for the same decision, the older outcome header links to the newest post when its message id is known and otherwise says `Replaced by a newer question`. Structured posts reserve space for that header, so a normal settlement edit needs no truncation. -If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; for an older unstructured post that still overflows, it keeps the header, as much question text as fits, the marker, and the complete option labels. -Both paths remain within Discord's character limit, and the outcome is read from the same ledgers rather than from any step Firstmate must remember. +If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; if those protected sections cannot fit, or an older unstructured post overflows, it refuses the edit and leaves the original message intact. +New decision plans that exceed the configured chunk cap are refused before posting rather than truncating their text. +The outcome is read from the same ledgers rather than from any step Firstmate must remember. Option matching ignores the appended Discord-button audit and the `Decision answer: ` prefix. For a recommended option, matching accepts its label with or without the leading star, while the outcome keeps the displayed label. A unique longest full option label at the start of the answer wins immediately, regardless of trailing text; otherwise the matcher collects standalone option numbers and whole-word labels and accepts only one distinct option. diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 0d50a8d75e3..050a2c66c66 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -1437,24 +1437,22 @@ test_17_outcome_headers_fit_reserved_and_legacy_posts() { payload=$(emit_decision "$home" 3 '{"key":"key:'"$TASK"':legacy-overflow","options":[{"label":"North"},{"label":"South"}]}') deliver "$home" 700000000000000120 progress="${payload%.json}.progress.json" - legacy=$(printf '%*s' 2300 '' | tr ' ' L) - if ! jq --arg legacy "$legacy" '.texts[-1] = $legacy | del(.decision.sections)' \ + legacy=$(printf '%*s' 1900 '' | tr ' ' L) + if ! jq --arg legacy "$legacy" '.texts[-1] = $legacy + | .decision.options[0].label = ("North" * 16) | del(.decision.sections)' \ "$progress" > "$home/legacy-progress.json"; then fail "could not build the legacy overflow fixture" fi mv "$home/legacy-progress.json" "$progress" \ || fail "could not install the legacy overflow fixture" home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy-overflow \ - "No preference yet" >/dev/null 2>&1 || fail "could not settle the legacy decision" + "$(printf 'North%.0s' {1..16})" >/dev/null 2>&1 || fail "could not settle the legacy decision" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - printf '%s' "$out" | jq -e ' - .message == "700000000000000120" - and (.content | length <= 2000) - and (.content | startswith("**✅ Decided <t:1791720000:f>**\n\nL")) - and (.content | contains("(trimmed)\n\n1. North\n2. South")) - and ([.components[].components[] | .disabled] | all) - and ([.components[].components[] | .style] | all(. == 2))' >/dev/null \ - || fail "the legacy fallback must keep its header, question prefix, marker, option labels, and grey buttons: $out" + [ -z "$out" ] || fail "legacy overflow must refuse the edit rather than truncate protected text: $out" + jq -e --arg legacy "$legacy" '.texts[-1] == $legacy' "$progress" >/dev/null \ + || fail "legacy overflow changed the original text" + [ -f "${payload%.json}.settled.json" ] || fail "legacy overflow lost its settlement" + [ ! -f "${payload%.json}.finalized" ] || fail "refused legacy edit was finalized" open_key "$home" replace-fallback old_payload=$(emit_decision "$home" 4 \ @@ -1484,7 +1482,91 @@ test_17_outcome_headers_fit_reserved_and_legacy_posts() { || fail "the replacement fallback must keep the question and disable the old buttons: $out" repeat=$(reconcile "$home" True) [ -z "$repeat" ] || fail "settled overflow and replacement edits must not retry unchanged posts" - pass "17 reserved chunks settle without trimming, only the last chunk is edited, and legacy overflow retains its header, question prefix, option labels, and one-time replacement state" + pass "17 reserved chunks settle without trimming, only the last chunk is edited, and legacy overflow refuses truncation while replacement state remains one-time" +} + +test_18_decision_plans_preserve_questions_and_legacy_boundaries() { + local home mode length slug payload + for mode in structured unstructured; do + home="$TMP_ROOT/c18-$mode" + setup_home "$home" + open_request "$home" + open_key "$home" capped + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':capped","options":[{"label":"North"},{"label":"South"}]}') + plugin_py "$home" "$payload" "$mode" <<'PYTEST' || fail "capped $mode decision lost protected text" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path, mode = Path(sys.argv[1]), Path(sys.argv[3]), sys.argv[4] +payload = json.loads(path.read_text()) +question = "Q" * 1800 +options = "1. North\n2. South" +cue = "Tap a button, or reply with the number." +text = question + "\n\n" + options + "\n" + cue +payload["text"] = text +payload["decision"]["sections"] = dict(question=question, explanation="", options=options, + cue=cue, structured=mode == "structured") +path.write_text(json.dumps(payload)) +poster.reply_limits = lambda home: {"limit": 1900, "cap": 1} +sent = [] +def send(chunk): + sent.append(chunk["text"]) + return {"discord_message_id": str(1800 + chunk["chunk_index"]), "buttons": True} + +assert poster.deliver_one(path, send=send, home=home) == "failed" +assert sent == [] +assert poster.load_progress(payload, home) is None +poster.reply_limits = lambda home: {"limit": 1900, "cap": 2} +assert poster.deliver_one(path, send=send, home=home) == "sent" +assert len(sent) == 2 +assert "".join(sent) == text +assert sent[-1].endswith(options + "\n" + cue) +assert all(len(chunk) <= 1744 for chunk in sent) +PYTEST + done + for length in 3200 3600; do + home="$TMP_ROOT/c18-resume-$length" + setup_home "$home" + open_request "$home" + open_key "$home" resume + slug=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':resume","options":[{"label":"North"},{"label":"South"}]}') + plugin_py "$home" "$slug" "$length" <<'PYTEST' || fail "legacy decision resume changed chunk boundaries" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path, length = Path(sys.argv[1]), Path(sys.argv[3]), int(sys.argv[4]) +payload = json.loads(path.read_text()) +payload["text"] = "0123456789" * (length // 10) +payload["decision"].pop("sections", None) +path.write_text(json.dumps(payload)) +plan = poster.split_reply(payload["text"], home=home, limit=1900, cap=25, legacy=True) +assert len(plan["texts"]) == 2 +original = poster.split_decision_reply +poster.split_decision_reply = lambda payload, **kwargs: plan +sent = [] +def partial(chunk): + if chunk["chunk_index"]: + raise poster.DiscordSendError("transient", "retry later") + sent.append(chunk["text"]) + return {"discord_message_id": "1801"} +assert poster.deliver_one(path, send=partial, home=home) == "failed" +progress = poster.load_progress(payload, home) +assert progress["posted_count"] == 1 +assert progress["limit"] == 1900 +progress.pop("texts") +poster.write_progress(payload, progress, home) +poster.split_decision_reply = original +def resume(chunk): + sent.append(chunk["text"]) + return {"discord_message_id": "1802", "buttons": True} +assert poster.deliver_one(path, send=resume, home=home) == "sent" +assert sent == plan["texts"], sent +assert poster.load_progress(payload, home)["posted_count"] == 2 +PYTEST + done + pass "18 capped decisions refuse text loss and legacy resumes retain original chunks" } test_1_buttons_carry_key_and_option_ids_in_the_thread @@ -1507,3 +1589,4 @@ test_15_full_labels_keep_digits_and_overlaps_together test_16_recommended_option_renders_green_with_a_single_star test_17_outcome_headers_fit_reserved_and_legacy_posts +test_18_decision_plans_preserve_questions_and_legacy_boundaries From 6d5fd127bdd0f397521a25ad89913098e6c3ee15 Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 17:48:28 +0800 Subject: [PATCH 3/8] no-mistakes(review): Recover legacy decision text; overflow remains unresolved --- bin/fm-ext-decision.sh | 25 +++-- .../outbox_poster.py | 36 +++++++ docs/configuration.md | 2 + tests/fm-ext-decision.test.sh | 99 +++++++++++++++++++ 4 files changed, 153 insertions(+), 9 deletions(-) diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index e90bd6ad310..1dd48fc7b1e 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -651,7 +651,7 @@ fit_post_content() { command_posts() { local settled_only=0 file stem thread content original displayed shown final - local kind header replacement_url selected + local kind header replacement_url selected missing_original [ "${1:-}" != --settled-only ] || settled_only=1 while IFS= read -r file; do stem=$(basename "$file" .progress.json) @@ -661,8 +661,6 @@ command_posts() { fi content=$(post_content "$stem") [ -n "$content" ] || continue - original=$(jq -j '.texts[-1] // ""' "$file"; printf '\001') - original=${original%$'\001'} selected=$(selected_option_id "$stem") replacement_url= if settled "$stem"; then @@ -680,18 +678,27 @@ command_posts() { else header="**$content**" fi - displayed=$(fit_post_content "$header" "$original" "$file") || continue - shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null; printf '\001') || shown= - shown=${shown%$'\001'} - [ "$displayed" != "$shown" ] || continue + missing_original=true + displayed= + if jq -e '.texts[-1] | type == "string" and length > 0' "$file" >/dev/null 2>&1; then + missing_original=false + original=$(jq -j '.texts[-1]' "$file"; printf '\001') + original=${original%$'\001'} + displayed=$(fit_post_content "$header" "$original" "$file") || continue + shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null; printf '\001') || shown= + shown=${shown%$'\001'} + [ "$displayed" != "$shown" ] || continue + fi final=false settled "$stem" && final=true thread=$(post_thread "$stem") || continue jq -cn --arg post "$stem" --arg channel "$thread" \ --arg message "$(post_field "$stem" '.discord_message_ids[-1]')" \ - --arg content "$displayed" --arg selected "$selected" --argjson final "$final" \ + --arg content "$displayed" --arg header "$header" --argjson missing_original "$missing_original" \ + --arg selected "$selected" --argjson final "$final" \ --slurpfile progress "$file" \ - '{post:$post, channel_id:$channel, message_id:$message, content:$content, + '{post:$post, channel_id:$channel, message_id:$message, + content:(if $missing_original then null else $content end), header:$header, final:$final, selected_option_id:(if $selected == "" then null else ($selected | tonumber) end), key:$progress[0].decision.key, options:$progress[0].decision.options}' diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index d43db24c96e..8b4d4ad2c1c 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -1511,6 +1511,24 @@ def discord_edit(channel_id: str, message_id: str, content: str, components: lis raise DiscordSendError("ambiguous", f"discord transport: {err}") from err +def discord_message_text(channel_id: str, message_id: str) -> str: + token = _bot_token() + if not token: + raise DiscordSendError("transient", "missing DISCORD_BOT_TOKEN") + if not (_SNOWFLAKE.fullmatch(channel_id or "") and _SNOWFLAKE.fullmatch(message_id or "")): + raise ValueError("read target is not a Discord message id") + request = urllib.request.Request( + f"{DISCORD_API_BASE}/channels/{channel_id}/messages/{message_id}", + method="GET", + headers={"Authorization": f"Bot {token}", "User-Agent": USER_AGENT}, + ) + with urllib.request.urlopen(request, timeout=15) as response: + text = json.load(response).get("content") + if not isinstance(text, str) or not text: + raise ValueError("Discord message has no original text") + return text + + def _decision_cli() -> Path: return firstmate_root() / "bin" / "fm-ext-decision.sh" @@ -1552,6 +1570,24 @@ def reconcile_decision_posts( for line in result.stdout.splitlines(): try: item = json.loads(line) + if item["content"] is None: + try: + outbox = state_dir(home) / "ext-outbox" + progress = json.loads((outbox / (item["post"] + ".progress.json")).read_text()) + if (outbox / (item["post"] + ".finalized")).exists(): + raise ValueError("original decision text is unavailable after an earlier edit") + ids = progress["discord_message_ids"] + if len(ids) != progress["total"] or progress["posted_count"] != progress["total"]: + raise ValueError("decision delivery is incomplete") + progress["texts"] = [discord_message_text(item["channel_id"], message) for message in ids] + slug, kind, generation = item["post"].rsplit(".", 2) + write_progress({"slug": slug, "kind": kind, "generation": int(generation)}, progress, home) + item["content"] = item["header"] + "\n\n" + progress["texts"][-1] + if len(item["content"]) > DISCORD_MESSAGE_MAX: + raise ValueError("legacy decision text cannot safely fit its outcome header") + except Exception as err: + print(f"fm-ext-poster: original decision text recovery failed: {err}", file=sys.stderr) + continue components = decisions.components_for( item, disabled=True, diff --git a/docs/configuration.md b/docs/configuration.md index 32dbb3632e2..3bc18e55feb 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1019,6 +1019,8 @@ When no single option matches, every button is grey; a click waiting for Firstma The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. When a newer post replaces an older one for the same decision, the older outcome header links to the newest post when its message id is known and otherwise says `Replaced by a newer question`. Structured posts reserve space for that header, so a normal settlement edit needs no truncation. +Completed legacy posts without persisted chunks recover their original text from Discord before editing; a failed recovery leaves the message and finalize receipt untouched. +An earlier edit that already removed the original text cannot be recovered from Discord. If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; if those protected sections cannot fit, or an older unstructured post overflows, it refuses the edit and leaves the original message intact. New decision plans that exceed the configured chunk cap are refused before posting rather than truncating their text. The outcome is read from the same ledgers rather than from any step Firstmate must remember. diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 050a2c66c66..01dcc688854 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -1569,6 +1569,104 @@ PYTEST pass "18 capped decisions refuse text loss and legacy resumes retain original chunks" } +test_19_completed_legacy_posts_recover_original_chunks_before_editing() { + local home payload + home="$TMP_ROOT/c19" + setup_home "$home" + open_request "$home" + open_key "$home" recover-original + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':recover-original","options":[{"label":"North"},{"label":"South"}]}') + plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare completed legacy chunks" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +payload = json.loads(path.read_text()) +payload["text"] = "Earlier explanation. " * 90 + "\n\n" + payload["text"] +payload["decision"].pop("sections", None) +path.write_text(json.dumps(payload)) +messages = {} +def send(chunk): + message = str(700000000000000190 + chunk["chunk_index"]) + messages[message] = chunk["text"] + return {"discord_message_id": message, "buttons": True} +assert poster.deliver_one(path, send=send, home=home) == "sent" +progress = poster.load_progress(payload, home) +assert progress["total"] == 2 +assert progress["posted_count"] == 2 +progress.pop("texts") +poster.write_progress(payload, progress, home) +(home / "messages.json").write_text(json.dumps(messages)) +PYTEST + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key recover-original North \ + >/dev/null 2>&1 || fail "could not settle the completed legacy post" + plugin_py "$home" "$payload" <<'PYTEST' || fail "completed legacy recovery lost original text or retry semantics" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +stem = path.stem +outbox = path.parent +progress_path = outbox / (stem + ".progress.json") +finalized = outbox / (stem + ".finalized") +messages = json.loads((home / "messages.json").read_text()) +import io +from unittest.mock import patch +sample_id, sample_text = next(iter(messages.items())) +with patch.object(poster, "_bot_token", return_value="test-token"), patch.object( + poster.urllib.request, "urlopen", + return_value=io.BytesIO(json.dumps({"content": sample_text}).encode()), +) as http: + assert poster.discord_message_text("222222222222222222", sample_id) == sample_text + request = http.call_args.args[0] + assert request.get_method() == "GET" + assert request.full_url.endswith("/channels/222222222222222222/messages/" + sample_id) + assert request.get_header("Authorization") == "Bot test-token" +reads, edits = [], [] +def read(channel, message): + assert channel == "222222222222222222" + reads.append(message) + if message.endswith("191"): + raise OSError("temporary read failure") + return messages[message] +def edit(channel, message, content, components): + edits.append((message, content)) + assert message.endswith("191") + assert content.split("\n\n", 1)[1] == messages[message] + assert all(button["disabled"] for row in components for button in row["components"]) + +poster.discord_message_text = read +assert poster.reconcile_decision_posts(home=home, edit=edit) == [] +assert edits == [] +assert "texts" not in json.loads(progress_path.read_text()) +assert not finalized.exists() +poster.discord_message_text = lambda channel, message: messages[message] +def failed_edit(*args): + raise poster.DiscordSendError("transient", "temporary edit failure") +assert poster.reconcile_decision_posts(home=home, edit=failed_edit) == [] +assert json.loads(progress_path.read_text())["texts"] == list(messages.values()) +assert not finalized.exists() +def unexpected_read(*args): + raise AssertionError("persisted originals must not be fetched again") +poster.discord_message_text = unexpected_read +assert poster.reconcile_decision_posts(home=home, edit=edit) == [stem] +assert len(edits) == 1 +assert finalized.read_text() == edits[0][1] +assert poster.reconcile_decision_posts(home=home, edit=edit) == [] +assert len(edits) == 1 +progress = json.loads(progress_path.read_text()) +progress.pop("texts") +payload = {"slug": stem.rsplit(".", 2)[0], "kind": "followup", "generation": 1} +poster.write_progress(payload, progress, home) +assert poster.reconcile_decision_posts(home=home, edit=edit) == [] +assert len(edits) == 1 +PYTEST + pass "19 completed legacy posts recover original chunks before editing and preserve retries and receipts" +} + + test_1_buttons_carry_key_and_option_ids_in_the_thread test_2_standing_click_files_one_request_for_a_worker_key test_3_captain_call_click_is_a_request_the_board_records @@ -1590,3 +1688,4 @@ test_15_full_labels_keep_digits_and_overlaps_together test_16_recommended_option_renders_green_with_a_single_star test_17_outcome_headers_fit_reserved_and_legacy_posts test_18_decision_plans_preserve_questions_and_legacy_boundaries +test_19_completed_legacy_posts_recover_original_chunks_before_editing From 33363f2dc88c77edf91c9cf3f1a74136b8f3244e Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 18:11:05 +0800 Subject: [PATCH 4/8] no-mistakes(review): Recover retained legacy originals; overflow remains unresolved --- bin/fm-ext-lib.sh | 10 +- .../outbox_poster.py | 16 +++- docs/configuration.md | 3 +- tests/fm-ext-decision.test.sh | 92 +++++++++++++++++++ 4 files changed, 115 insertions(+), 6 deletions(-) diff --git a/bin/fm-ext-lib.sh b/bin/fm-ext-lib.sh index bbcf6069a8c..66094ecb33f 100644 --- a/bin/fm-ext-lib.sh +++ b/bin/fm-ext-lib.sh @@ -1498,10 +1498,10 @@ fm_ext_outbox_fail() { # that is genuinely still pending instead of every reply ever sent. The terminal # marker itself stays in place, so a duplicate emit of the same generation is # still recognized as already delivered until retention expires it. Returns 0 -# when the generation is terminal and its payload is gone, 1 when it is not +# when the generation is terminal, 1 when it is not # terminal, and 2 on an unsafe path or a removal failure. fm_ext_outbox_retire() { - local dir=$1 slug=$2 kind=$3 generation=$4 payload receipt failed + local dir=$1 slug=$2 kind=$3 generation=$4 payload receipt failed progress payload=$(fm_ext_outbox_basename "$slug" "$kind" "$generation") || return 2 receipt=$(fm_ext_outbox_receipt_basename "$slug" "$kind" "$generation") || return 2 failed=$(fm_ext_outbox_failed_basename "$slug" "$kind" "$generation") || return 2 @@ -1509,6 +1509,12 @@ fm_ext_outbox_retire() { && ! fm_ext_private_artifact_file_valid "$dir" "$failed" 600; then return 1 fi + progress=$(fm_ext_outbox_progress_basename "$slug" "$kind" "$generation") || return 2 + if fm_ext_private_artifact_file_valid "$dir" "$progress" 600 \ + && jq -e '.decision.key and .buttons == true + and (.texts[-1] | type != "string" or length == 0)' "$dir/$progress" >/dev/null 2>&1; then + return 0 + fi fm_ext_private_artifact_remove "$dir" "$payload" 600 || return 2 return 0 } diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index 8b4d4ad2c1c..28fef7bd547 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -1574,12 +1574,22 @@ def reconcile_decision_posts( try: outbox = state_dir(home) / "ext-outbox" progress = json.loads((outbox / (item["post"] + ".progress.json")).read_text()) - if (outbox / (item["post"] + ".finalized")).exists(): - raise ValueError("original decision text is unavailable after an earlier edit") ids = progress["discord_message_ids"] if len(ids) != progress["total"] or progress["posted_count"] != progress["total"]: raise ValueError("decision delivery is incomplete") - progress["texts"] = [discord_message_text(item["channel_id"], message) for message in ids] + payload_path = outbox / (item["post"] + ".json") + if payload_path.is_file(): + payload = json.loads(payload_path.read_text()) + progress["texts"] = split_reply( + payload["text"], home=home, limit=progress["limit"], + cap=progress["cap"], legacy=True, + )["texts"] + if len(progress["texts"]) != progress["total"]: + raise ValueError("recovered decision chunk count does not match delivery") + else: + if (outbox / (item["post"] + ".finalized")).exists(): + raise ValueError("original decision text is unavailable after an earlier edit") + progress["texts"] = [discord_message_text(item["channel_id"], message) for message in ids] slug, kind, generation = item["post"].rsplit(".", 2) write_progress({"slug": slug, "kind": kind, "generation": int(generation)}, progress, home) item["content"] = item["header"] + "\n\n" + progress["texts"][-1] diff --git a/docs/configuration.md b/docs/configuration.md index 3bc18e55feb..2c5a4f78201 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1019,7 +1019,8 @@ When no single option matches, every button is grey; a click waiting for Firstma The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. When a newer post replaces an older one for the same decision, the older outcome header links to the newest post when its message id is known and otherwise says `Replaced by a newer question`. Structured posts reserve space for that header, so a normal settlement edit needs no truncation. -Completed legacy posts without persisted chunks recover their original text from Discord before editing; a failed recovery leaves the message and finalize receipt untouched. +Completed legacy posts without persisted chunks recover their original text from a retained payload using the stored legacy split limit and cap, or from Discord when the post has never been edited; a failed recovery leaves the message and finalize receipt untouched. +A completed legacy decision payload remains available until its original chunks are persisted, within the existing retention window. An earlier edit that already removed the original text cannot be recovered from Discord. If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; if those protected sections cannot fit, or an older unstructured post overflows, it refuses the edit and leaves the original message intact. New decision plans that exceed the configured chunk cap are refused before posting rather than truncating their text. diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 01dcc688854..9b287df09b7 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -1612,6 +1612,7 @@ outbox = path.parent progress_path = outbox / (stem + ".progress.json") finalized = outbox / (stem + ".finalized") messages = json.loads((home / "messages.json").read_text()) +path.unlink(missing_ok=True) import io from unittest.mock import patch sample_id, sample_text = next(iter(messages.items())) @@ -1667,6 +1668,96 @@ PYTEST } +test_20_previously_edited_legacy_posts_recover_retained_payloads() { + local home payload + home="$TMP_ROOT/c20" + setup_home "$home" + open_request "$home" + open_key "$home" retained-original + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':retained-original","options":[{"label":"North"},{"label":"South"}]}') + plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare previously edited legacy post" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +payload = json.loads(path.read_text()) +payload["text"] = "Earlier explanation. " * 90 + "\n\n" + payload["text"] +payload["decision"].pop("sections", None) +path.write_text(json.dumps(payload)) +(home / "retained.json").write_text(json.dumps(payload)) +plan = poster.split_reply(payload["text"], home=home, limit=1900, cap=25, legacy=True) +assert len(plan["texts"]) == 2 +poster.split_decision_reply = lambda *args, **kwargs: plan +def send(chunk): + return {"discord_message_id": str(700000000000000200 + chunk["chunk_index"]), "buttons": True} +assert poster.deliver_one(path, send=send, home=home) == "sent" +progress = poster.load_progress(payload, home) +assert progress["total"] == progress["posted_count"] == 2 +progress.pop("texts") +poster.write_progress(payload, progress, home) +finalized = path.parent / (path.stem + ".finalized") +finalized.write_text("⏳ North sent by Dana - waiting for Firstmate to record it") +finalized.chmod(0o600) +(home / "plan.json").write_text(json.dumps(plan["texts"])) +PYTEST + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key retained-original North \ + >/dev/null 2>&1 || fail "could not settle the previously edited legacy post" + plugin_py "$home" "$payload" <<'PYTEST' || fail "retained legacy payload did not restore the final decision display" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +path.write_text((home / "retained.json").read_text()) +path.chmod(0o600) +plan = json.loads((home / "plan.json").read_text()) +finalized = path.parent / (path.stem + ".finalized") +old_receipt = finalized.read_text() +edits = [] +def read(*args): + raise AssertionError("retained payload must recover originals without reading the edited Discord message") +poster.discord_message_text = read +progress_path = path.parent / (path.stem + ".progress.json") +progress = json.loads(progress_path.read_text()) +for texts in (None, [], [""]): + candidate = dict(progress) + if texts is not None: + candidate["texts"] = texts + poster.write_progress(json.loads(path.read_text()), candidate, home) + assert path not in poster.list_pending(home=home) + assert path.is_file() +def failed_edit(*args): + raise poster.DiscordSendError("transient", "temporary edit failure") +assert poster.reconcile_decision_posts(home=home, edit=failed_edit) == [] +assert finalized.read_text() == old_receipt +progress = json.loads((path.parent / (path.stem + ".progress.json")).read_text()) +assert progress["texts"] == plan +assert progress["limit"] == 1900 and progress["cap"] == 25 +assert path not in poster.list_pending(home=home) +assert not path.exists() +def edit(channel, message, content, components): + edits.append(dict(message=message, content=content, components=components)) +result = poster.reconcile_decision_posts(home=home, edit=edit) +assert result == [path.stem], dict(result=result, edits=edits) +assert len(edits) == 1 +message = edits[0] +assert message["message"] == "700000000000000201", message +assert "chosen by the captain" in message["content"].split("\n\n", 1)[0], message +assert message["content"].split("\n\n", 1)[1] == plan[-1], message +buttons = [button for row in message["components"] for button in row["components"]] +assert all(button["disabled"] for button in buttons), buttons +assert buttons[0]["style"] == 3 and buttons[0]["label"] == "North ✓", buttons +assert all(button["style"] == 2 for button in buttons[1:]), buttons +assert finalized.read_text() == message["content"] + +assert poster.reconcile_decision_posts(home=home, edit=edit) == [] +assert len(edits) == 1 +PYTEST + pass "20 previously edited legacy posts recover retained split plans and finalize the recorded decision" +} + + test_1_buttons_carry_key_and_option_ids_in_the_thread test_2_standing_click_files_one_request_for_a_worker_key test_3_captain_call_click_is_a_request_the_board_records @@ -1689,3 +1780,4 @@ test_16_recommended_option_renders_green_with_a_single_star test_17_outcome_headers_fit_reserved_and_legacy_posts test_18_decision_plans_preserve_questions_and_legacy_boundaries test_19_completed_legacy_posts_recover_original_chunks_before_editing +test_20_previously_edited_legacy_posts_recover_retained_payloads From 1363fdaa0ed6e2ee666041404364189be3991fb4 Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 18:32:11 +0800 Subject: [PATCH 5/8] no-mistakes(review): Settle legacy buttons without editing message text --- bin/fm-ext-decision.sh | 77 +++-- bin/fm-ext-lib.sh | 10 +- .../outbox_poster.py | 70 +--- docs/configuration.md | 9 +- tests/fm-ext-decision.test.sh | 318 +++++++++--------- 5 files changed, 222 insertions(+), 262 deletions(-) diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index 1dd48fc7b1e..4153728bafa 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -28,7 +28,7 @@ # <stem>.sent.json the click that filed this post's request, written # once, so a second click files nothing # <stem>.settled.json the post's final outcome line, written once -# <stem>.finalized the post content Discord shows now +# <stem>.finalized the displayed content or legacy button-state receipt # # What a post shows, where <time> is a Discord timestamp: # ⏳ <label> sent by <name> <time> - waiting for Firstmate to record it @@ -78,7 +78,7 @@ # # posts # Prints one JSON line per button post whose current content Discord does -# not show yet: {post, channel_id, message_id, content, final, key, +# not show yet: {post, channel_id, message_id, content, receipt, final, key, # options}; a listed post always shows with its buttons off. Without # --settled-only it first settles every live post that a newer post for the # same key replaced, or whose own lifecycle the ledgers report closed. @@ -96,8 +96,8 @@ # is open. Read-only. # # finalized -# Records the content Discord now shows for a post, so posts lists it only -# when that content changes. +# Records the displayed content or legacy button state, so posts lists it +# only when that receipt changes. # # still-open # The check Firstmate runs before recording a button request: is the @@ -643,15 +643,15 @@ fit_post_content() { compose($header; $sections.question; $sections.explanation; $sections.options; $sections.cue) else - error("legacy decision text cannot safely fit its outcome header") + compose($header; $original; ""; ""; "") end end ' } command_posts() { - local settled_only=0 file stem thread content original displayed shown final - local kind header replacement_url selected missing_original + local settled_only=0 file stem thread content original displayed shown final receipt legacy + local kind header replacement_url selected [ "${1:-}" != --settled-only ] || settled_only=1 while IFS= read -r file; do stem=$(basename "$file" .progress.json) @@ -659,46 +659,55 @@ command_posts() { if [ "$settled_only" = 0 ]; then settle_if_closed "$stem" || true fi - content=$(post_content "$stem") - [ -n "$content" ] || continue - selected=$(selected_option_id "$stem") - replacement_url= - if settled "$stem"; then - kind=$(jq -r '.kind // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) - else - kind=waiting + if ! settled "$stem"; then + sent "$stem" && request_published "$(sent_field "$stem" '.request_id')" || continue fi - if [ "$kind" = replaced ]; then - replacement_url=$(newer_post_link "$stem") - if [ -n "$replacement_url" ]; then - header="♻️ **Replaced by a newer question** ([open]($replacement_url))" + selected=$(selected_option_id "$stem") + final=false + settled "$stem" && final=true + legacy=true + displayed= + if jq -e '(.decision.sections | type == "object") + and (.texts[-1] | type == "string" and length > 0)' "$file" >/dev/null 2>&1; then + legacy=false + content=$(post_content "$stem") + [ -n "$content" ] || continue + replacement_url= + if settled "$stem"; then + kind=$(jq -r '.kind // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) else - header="**Replaced by a newer question**" + kind=waiting + fi + if [ "$kind" = replaced ]; then + replacement_url=$(newer_post_link "$stem") + if [ -n "$replacement_url" ]; then + header="♻️ **Replaced by a newer question** ([open]($replacement_url))" + else + header="**Replaced by a newer question**" + fi + else + header="**$content**" fi - else - header="**$content**" - fi - missing_original=true - displayed= - if jq -e '.texts[-1] | type == "string" and length > 0' "$file" >/dev/null 2>&1; then - missing_original=false original=$(jq -j '.texts[-1]' "$file"; printf '\001') original=${original%$'\001'} displayed=$(fit_post_content "$header" "$original" "$file") || continue - shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null; printf '\001') || shown= - shown=${shown%$'\001'} - [ "$displayed" != "$shown" ] || continue + receipt=$displayed + else + receipt=$(jq -cn --arg selected "$selected" --argjson final "$final" ' + {components_only:true, final:$final, + selected_option_id:(if $selected == "" then null else ($selected | tonumber) end)}') || continue fi - final=false - settled "$stem" && final=true + shown=$(cat "$OUTBOX/$stem.finalized" 2>/dev/null; printf '\001') || shown= + shown=${shown%$'\001'} + [ "$receipt" != "$shown" ] || continue thread=$(post_thread "$stem") || continue jq -cn --arg post "$stem" --arg channel "$thread" \ --arg message "$(post_field "$stem" '.discord_message_ids[-1]')" \ - --arg content "$displayed" --arg header "$header" --argjson missing_original "$missing_original" \ + --arg content "$displayed" --arg receipt "$receipt" --argjson legacy "$legacy" \ --arg selected "$selected" --argjson final "$final" \ --slurpfile progress "$file" \ '{post:$post, channel_id:$channel, message_id:$message, - content:(if $missing_original then null else $content end), header:$header, + content:(if $legacy then null else $content end), receipt:$receipt, final:$final, selected_option_id:(if $selected == "" then null else ($selected | tonumber) end), key:$progress[0].decision.key, options:$progress[0].decision.options}' diff --git a/bin/fm-ext-lib.sh b/bin/fm-ext-lib.sh index 66094ecb33f..bbcf6069a8c 100644 --- a/bin/fm-ext-lib.sh +++ b/bin/fm-ext-lib.sh @@ -1498,10 +1498,10 @@ fm_ext_outbox_fail() { # that is genuinely still pending instead of every reply ever sent. The terminal # marker itself stays in place, so a duplicate emit of the same generation is # still recognized as already delivered until retention expires it. Returns 0 -# when the generation is terminal, 1 when it is not +# when the generation is terminal and its payload is gone, 1 when it is not # terminal, and 2 on an unsafe path or a removal failure. fm_ext_outbox_retire() { - local dir=$1 slug=$2 kind=$3 generation=$4 payload receipt failed progress + local dir=$1 slug=$2 kind=$3 generation=$4 payload receipt failed payload=$(fm_ext_outbox_basename "$slug" "$kind" "$generation") || return 2 receipt=$(fm_ext_outbox_receipt_basename "$slug" "$kind" "$generation") || return 2 failed=$(fm_ext_outbox_failed_basename "$slug" "$kind" "$generation") || return 2 @@ -1509,12 +1509,6 @@ fm_ext_outbox_retire() { && ! fm_ext_private_artifact_file_valid "$dir" "$failed" 600; then return 1 fi - progress=$(fm_ext_outbox_progress_basename "$slug" "$kind" "$generation") || return 2 - if fm_ext_private_artifact_file_valid "$dir" "$progress" 600 \ - && jq -e '.decision.key and .buttons == true - and (.texts[-1] | type != "string" or length == 0)' "$dir/$progress" >/dev/null 2>&1; then - return 0 - fi fm_ext_private_artifact_remove "$dir" "$payload" 600 || return 2 return 0 } diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index 28fef7bd547..62f04786bca 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -1479,8 +1479,8 @@ def discord_send(payload: dict) -> dict: } -def discord_edit(channel_id: str, message_id: str, content: str, components: list) -> None: - """Replace one posted message's text and buttons over REST. Mentions are +def discord_edit(channel_id: str, message_id: str, content: str | None, components: list) -> None: + """Edit buttons and optional text over REST; None preserves text. Mentions are never parsed, so a name in the outcome line cannot ping anyone. Raises DiscordSendError on any failure. """ @@ -1489,9 +1489,10 @@ def discord_edit(channel_id: str, message_id: str, content: str, components: lis raise DiscordSendError("transient", "missing DISCORD_BOT_TOKEN") if not (_SNOWFLAKE.fullmatch(channel_id or "") and _SNOWFLAKE.fullmatch(message_id or "")): raise DiscordSendError("permanent", "edit target is not a Discord message id") - body = json.dumps( - {"content": content, "components": components, "allowed_mentions": {"parse": []}} - ).encode("utf-8") + body = {"components": components, "allowed_mentions": {"parse": []}} + if content is not None: + body["content"] = content + body = json.dumps(body).encode("utf-8") request = urllib.request.Request( f"{DISCORD_API_BASE}/channels/{channel_id}/messages/{message_id}", data=body, @@ -1511,24 +1512,6 @@ def discord_edit(channel_id: str, message_id: str, content: str, components: lis raise DiscordSendError("ambiguous", f"discord transport: {err}") from err -def discord_message_text(channel_id: str, message_id: str) -> str: - token = _bot_token() - if not token: - raise DiscordSendError("transient", "missing DISCORD_BOT_TOKEN") - if not (_SNOWFLAKE.fullmatch(channel_id or "") and _SNOWFLAKE.fullmatch(message_id or "")): - raise ValueError("read target is not a Discord message id") - request = urllib.request.Request( - f"{DISCORD_API_BASE}/channels/{channel_id}/messages/{message_id}", - method="GET", - headers={"Authorization": f"Bot {token}", "User-Agent": USER_AGENT}, - ) - with urllib.request.urlopen(request, timeout=15) as response: - text = json.load(response).get("content") - if not isinstance(text, str) or not text: - raise ValueError("Discord message has no original text") - return text - - def _decision_cli() -> Path: return firstmate_root() / "bin" / "fm-ext-decision.sh" @@ -1551,7 +1534,7 @@ def _unshown_settled_posts(home: Path | None = None) -> bool: def reconcile_decision_posts( home: Path | None = None, full: bool = True, - edit: Callable[[str, str, str, list], None] | None = None, + edit: Callable[[str, str, str | None, list], None] | None = None, ) -> list[str]: """Show each decision post's current line on Discord with its buttons off: the sent line once a click filed its request, then the outcome once @@ -1570,34 +1553,6 @@ def reconcile_decision_posts( for line in result.stdout.splitlines(): try: item = json.loads(line) - if item["content"] is None: - try: - outbox = state_dir(home) / "ext-outbox" - progress = json.loads((outbox / (item["post"] + ".progress.json")).read_text()) - ids = progress["discord_message_ids"] - if len(ids) != progress["total"] or progress["posted_count"] != progress["total"]: - raise ValueError("decision delivery is incomplete") - payload_path = outbox / (item["post"] + ".json") - if payload_path.is_file(): - payload = json.loads(payload_path.read_text()) - progress["texts"] = split_reply( - payload["text"], home=home, limit=progress["limit"], - cap=progress["cap"], legacy=True, - )["texts"] - if len(progress["texts"]) != progress["total"]: - raise ValueError("recovered decision chunk count does not match delivery") - else: - if (outbox / (item["post"] + ".finalized")).exists(): - raise ValueError("original decision text is unavailable after an earlier edit") - progress["texts"] = [discord_message_text(item["channel_id"], message) for message in ids] - slug, kind, generation = item["post"].rsplit(".", 2) - write_progress({"slug": slug, "kind": kind, "generation": int(generation)}, progress, home) - item["content"] = item["header"] + "\n\n" + progress["texts"][-1] - if len(item["content"]) > DISCORD_MESSAGE_MAX: - raise ValueError("legacy decision text cannot safely fit its outcome header") - except Exception as err: - print(f"fm-ext-poster: original decision text recovery failed: {err}", file=sys.stderr) - continue components = decisions.components_for( item, disabled=True, @@ -1609,7 +1564,7 @@ def reconcile_decision_posts( continue except Exception: continue - with _temp_text(item["content"]) as content_file: + with _temp_text(item["receipt"]) as content_file: done = subprocess.run( [str(_decision_cli()), "finalized", "--post", item["post"], "--content-file", content_file], check=False, @@ -1912,10 +1867,15 @@ def deliver_one( "limit": split["limit"], "cap": split["cap"], } - progress["texts"] = chunks if isinstance(payload.get("decision"), dict): # The post record a click and a later edit find this decision by. - progress["decision"] = payload["decision"] + post_decision = dict(payload["decision"]) + if stored and ("texts" not in stored or not isinstance( + (stored.get("decision") or {}).get("sections"), dict + )): + post_decision.pop("sections", None) + progress["decision"] = post_decision + progress["texts"] = chunks if int(progress.get("total") or 0) != len(chunks): release_inflight(payload, home=home) return "mid-delivery" diff --git a/docs/configuration.md b/docs/configuration.md index 2c5a4f78201..3e185dfea99 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1013,16 +1013,15 @@ A recommended option that also needs confirmation still needs its second click. A confirmed click from a standing-rule author is that author's explicit word for that one decision only, never standing authority, and merge authority stays where `AGENTS.md` puts it. Each post is bound to the lifecycle of its decision that was open when it was posted, so a captain call re-held or a worker key reopened for a new question is a different decision to it. -When Firstmate records an answer on any channel, its posts are disabled and edited with a bold outcome header above the exact original last-chunk text whenever both fit Discord's 2,000-character limit. +When Firstmate records an answer on any channel, new posts are disabled and edited with a bold outcome header above the exact original last-chunk text whenever both fit Discord's 2,000-character limit. The uniquely chosen option is success-colored and gains a `✓`; every other option and Discuss are grey, and all buttons are disabled. When no single option matches, every button is grey; a click waiting for Firstmate to record it still highlights the clicked option. The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. When a newer post replaces an older one for the same decision, the older outcome header links to the newest post when its message id is known and otherwise says `Replaced by a newer question`. Structured posts reserve space for that header, so a normal settlement edit needs no truncation. -Completed legacy posts without persisted chunks recover their original text from a retained payload using the stored legacy split limit and cap, or from Discord when the post has never been edited; a failed recovery leaves the message and finalize receipt untouched. -A completed legacy decision payload remains available until its original chunks are persisted, within the existing retention window. -An earlier edit that already removed the original text cannot be recovered from Discord. -If an edit cannot fit, the bridge trims the explanation tail with `(trimmed)` while preserving the header, question, and option list; if those protected sections cannot fit, or an older unstructured post overflows, it refuses the edit and leaves the original message intact. +Legacy posts, identified by missing original chunks or legacy decision records without section metadata, receive only button updates in every outcome state; their message text stays byte for byte unchanged, even if an earlier version replaced it with a waiting line. +Legacy posts need no original-text recovery or retained payload, and a recorded answer updates their buttons even when an older waiting receipt exists. +For new posts, an edit that cannot fit trims the explanation tail with `(trimmed)` while preserving the header, question, and option list. New decision plans that exceed the configured chunk cap are refused before posting rather than truncating their text. The outcome is read from the same ledgers rather than from any step Firstmate must remember. Option matching ignores the appended Discord-button audit and the `Decision answer: ` prefix. diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 9b287df09b7..7436182e431 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -1448,11 +1448,19 @@ test_17_outcome_headers_fit_reserved_and_legacy_posts() { home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy-overflow \ "$(printf 'North%.0s' {1..16})" >/dev/null 2>&1 || fail "could not settle the legacy decision" out=$(FM_EXT_NOW_OVERRIDE=1791720000 reconcile "$home" True) - [ -z "$out" ] || fail "legacy overflow must refuse the edit rather than truncate protected text: $out" + printf '%s' "$out" | jq -e ' + .message == "700000000000000120" and .content == null + and ([.components[].components[] | .disabled] | all) + and .components[0].components[0].style == 3 + and (.components[0].components[0].label | endswith(" ✓")) + and .components[0].components[1].style == 2 + and .components[1].components[0].style == 2' >/dev/null \ + || fail "legacy overflow must settle its buttons without a text edit: $out" jq -e --arg legacy "$legacy" '.texts[-1] == $legacy' "$progress" >/dev/null \ || fail "legacy overflow changed the original text" [ -f "${payload%.json}.settled.json" ] || fail "legacy overflow lost its settlement" - [ ! -f "${payload%.json}.finalized" ] || fail "refused legacy edit was finalized" + jq -e '.components_only == true and .final == true and .selected_option_id == 1' \ + "${payload%.json}.finalized" >/dev/null || fail "legacy button settlement was not receipted" open_key "$home" replace-fallback old_payload=$(emit_decision "$home" 4 \ @@ -1482,7 +1490,7 @@ test_17_outcome_headers_fit_reserved_and_legacy_posts() { || fail "the replacement fallback must keep the question and disable the old buttons: $out" repeat=$(reconcile "$home" True) [ -z "$repeat" ] || fail "settled overflow and replacement edits must not retry unchanged posts" - pass "17 reserved chunks settle without trimming, only the last chunk is edited, and legacy overflow refuses truncation while replacement state remains one-time" + pass "17 reserved chunks settle without trimming, only the last chunk is edited, and legacy overflow settles buttons without changing text" } test_18_decision_plans_preserve_questions_and_legacy_boundaries() { @@ -1539,7 +1547,8 @@ import outbox_poster as poster home, path, length = Path(sys.argv[1]), Path(sys.argv[3]), int(sys.argv[4]) payload = json.loads(path.read_text()) payload["text"] = "0123456789" * (length // 10) -payload["decision"].pop("sections", None) +if length == 3600: + payload["decision"].pop("sections", None) path.write_text(json.dumps(payload)) plan = poster.split_reply(payload["text"], home=home, limit=1900, cap=25, legacy=True) assert len(plan["texts"]) == 2 @@ -1564,197 +1573,186 @@ def resume(chunk): assert poster.deliver_one(path, send=resume, home=home) == "sent" assert sent == plan["texts"], sent assert poster.load_progress(payload, home)["posted_count"] == 2 +assert "sections" not in poster.load_progress(payload, home)["decision"] PYTEST done pass "18 capped decisions refuse text loss and legacy resumes retain original chunks" } -test_19_completed_legacy_posts_recover_original_chunks_before_editing() { - local home payload - home="$TMP_ROOT/c19" - setup_home "$home" - open_request "$home" - open_key "$home" recover-original - payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':recover-original","options":[{"label":"North"},{"label":"South"}]}') - plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare completed legacy chunks" -import json, sys +assert_legacy_edit() { + local home=$1 payload=$2 selected=$3 final=$4 + plugin_py "$home" "$payload" "$selected" "$final" <<'PYTEST' || fail "legacy buttons did not settle without changing text" +import io, json, sys from pathlib import Path +from unittest.mock import patch import outbox_poster as poster home, path = Path(sys.argv[1]), Path(sys.argv[3]) -payload = json.loads(path.read_text()) -payload["text"] = "Earlier explanation. " * 90 + "\n\n" + payload["text"] -payload["decision"].pop("sections", None) -path.write_text(json.dumps(payload)) -messages = {} -def send(chunk): - message = str(700000000000000190 + chunk["chunk_index"]) - messages[message] = chunk["text"] - return {"discord_message_id": message, "buttons": True} -assert poster.deliver_one(path, send=send, home=home) == "sent" -progress = poster.load_progress(payload, home) -assert progress["total"] == 2 -assert progress["posted_count"] == 2 -progress.pop("texts") -poster.write_progress(payload, progress, home) -(home / "messages.json").write_text(json.dumps(messages)) +selected = None if sys.argv[4] == "none" else int(sys.argv[4]) +final = sys.argv[5] == "true" +state_path = home / "discord.json" +state = json.loads(state_path.read_text()) +original = state["content"].encode() +earlier = state["earlier"].encode() +requests = [] +settled = path.parent / (path.stem + ".settled.json") +ledger = settled.read_bytes() if settled.exists() else None +def http(request, timeout): + body = json.loads(request.data) + requests.append((request.get_method(), request.full_url, body)) + if "content" in body: + state["content"] = body["content"] + state["components"] = body["components"] + return io.BytesIO(b"{}") +with patch.object(poster, "_bot_token", return_value="test-token"), patch.object( + poster.urllib.request, "urlopen", side_effect=http, +): + result = poster.reconcile_decision_posts(home=home) + assert result == [path.stem], (result, requests) + assert len(requests) == 1, requests + method, url, body = requests[0] + assert method == "PATCH" and url.endswith("/messages/700000000000000001"), requests + assert set(body) == {"components", "allowed_mentions"}, body + assert state["content"].encode() == original + assert state["earlier"].encode() == earlier + buttons = [button for row in body["components"] for button in row["components"]] + assert all(button["disabled"] for button in buttons), buttons + for index, button in enumerate(buttons, 1): + chosen = index == selected + assert button["style"] == (3 if chosen else 2), buttons + assert button["label"].endswith(" ✓") == chosen, buttons + receipt = json.loads((path.parent / (path.stem + ".finalized")).read_text()) + assert receipt == dict(components_only=True, final=final, selected_option_id=selected), receipt + assert poster.reconcile_decision_posts(home=home) == [] + assert len(requests) == 1 +if ledger is not None: + assert settled.read_bytes() == ledger +state_path.write_text(json.dumps(state)) PYTEST - home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key recover-original North \ - >/dev/null 2>&1 || fail "could not settle the completed legacy post" - plugin_py "$home" "$payload" <<'PYTEST' || fail "completed legacy recovery lost original text or retry semantics" +} + +test_19_legacy_buttons_settle_in_every_outcome_state() { + local home state payload out record selected + for state in waiting chosen captain ambiguous closed replaced; do + home="$TMP_ROOT/c19-$state" + setup_home "$home" + open_request "$home" + open_key "$home" legacy + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':legacy","options":[{"label":"North"},{"label":"South"}]}') + deliver "$home" "$POST" + plugin_py "$home" "$payload" "$state" <<'PYTEST' || fail "could not prepare legacy decision" import json, sys from pathlib import Path import outbox_poster as poster -home, path = Path(sys.argv[1]), Path(sys.argv[3]) -stem = path.stem -outbox = path.parent -progress_path = outbox / (stem + ".progress.json") -finalized = outbox / (stem + ".finalized") -messages = json.loads((home / "messages.json").read_text()) -path.unlink(missing_ok=True) -import io -from unittest.mock import patch -sample_id, sample_text = next(iter(messages.items())) -with patch.object(poster, "_bot_token", return_value="test-token"), patch.object( - poster.urllib.request, "urlopen", - return_value=io.BytesIO(json.dumps({"content": sample_text}).encode()), -) as http: - assert poster.discord_message_text("222222222222222222", sample_id) == sample_text - request = http.call_args.args[0] - assert request.get_method() == "GET" - assert request.full_url.endswith("/channels/222222222222222222/messages/" + sample_id) - assert request.get_header("Authorization") == "Bot test-token" -reads, edits = [], [] -def read(channel, message): - assert channel == "222222222222222222" - reads.append(message) - if message.endswith("191"): - raise OSError("temporary read failure") - return messages[message] -def edit(channel, message, content, components): - edits.append((message, content)) - assert message.endswith("191") - assert content.split("\n\n", 1)[1] == messages[message] - assert all(button["disabled"] for row in components for button in row["components"]) - -poster.discord_message_text = read -assert poster.reconcile_decision_posts(home=home, edit=edit) == [] -assert edits == [] -assert "texts" not in json.loads(progress_path.read_text()) -assert not finalized.exists() -poster.discord_message_text = lambda channel, message: messages[message] -def failed_edit(*args): - raise poster.DiscordSendError("transient", "temporary edit failure") -assert poster.reconcile_decision_posts(home=home, edit=failed_edit) == [] -assert json.loads(progress_path.read_text())["texts"] == list(messages.values()) -assert not finalized.exists() -def unexpected_read(*args): - raise AssertionError("persisted originals must not be fetched again") -poster.discord_message_text = unexpected_read -assert poster.reconcile_decision_posts(home=home, edit=edit) == [stem] -assert len(edits) == 1 -assert finalized.read_text() == edits[0][1] -assert poster.reconcile_decision_posts(home=home, edit=edit) == [] -assert len(edits) == 1 +home, path, state = Path(sys.argv[1]), Path(sys.argv[3]), sys.argv[4] +progress_path = path.parent / (path.stem + ".progress.json") progress = json.loads(progress_path.read_text()) -progress.pop("texts") -payload = {"slug": stem.rsplit(".", 2)[0], "kind": "followup", "generation": 1} -poster.write_progress(payload, progress, home) -assert poster.reconcile_decision_posts(home=home, edit=edit) == [] -assert len(edits) == 1 +progress["decision"]["options"][0]["label"] = "North" * 16 +prefix = "Original question?\n1. " + "North" * 16 + "\n2. South\n\n" +original = prefix + "E" * (1900 - len(prefix) - 4) + " \n\t\n" +assert len(original) == 1900 +progress["texts"] = [original] +if state != "captain": + progress["decision"].pop("sections", None) +if state in ("captain", "ambiguous", "closed"): + progress.pop("texts") +slug, kind, generation = path.stem.rsplit(".", 2) +poster.write_progress(dict(slug=slug, kind=kind, generation=int(generation)), progress, home) +path.unlink(missing_ok=True) +(home / "discord.json").write_text(json.dumps(dict(content=original, earlier="Earlier chunk\n"))) PYTEST - pass "19 completed legacy posts recover original chunks before editing and preserve retries and receipts" + selected=none + case "$state" in + waiting|chosen) + out=$(click "$home" "fmd:key:$TASK:legacy:1" "$CAPTAIN" 880000000000000191) + assert_contains "$out" "Sent:" "the legacy click was not filed" + selected=1 + if [ "$state" = chosen ]; then + record=$(button_request "$home" option) + [ "$(record_as_firstmate "$home" "$record")" = open ] || fail "could not record legacy choice" + fi + ;; + captain) + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy \ + "$(printf 'North%.0s' {1..16})" >/dev/null 2>&1 || fail "could not record captain legacy choice" + selected=1 + ;; + ambiguous) + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy \ + "No preference" >/dev/null 2>&1 || fail "could not record ambiguous legacy choice" + ;; + closed) printf 'done: no longer needed\n' >> "$home/state/$TASK.status" ;; + replaced) + emit_decision "$home" 2 '{"key":"key:'"$TASK"':legacy","options":[{"label":"North"},{"label":"South"}]}' >/dev/null + deliver "$home" 700000000000000002 + ;; + esac + if [ "$state" = waiting ]; then + assert_legacy_edit "$home" "$payload" "$selected" false + record=$(button_request "$home" option) + [ "$(record_as_firstmate "$home" "$record")" = open ] || fail "could not finalize waiting legacy choice" + fi + assert_legacy_edit "$home" "$payload" "$selected" true + done + pass "19 every legacy outcome settles buttons with 1900-character text and 80-character options unchanged" } - -test_20_previously_edited_legacy_posts_recover_retained_payloads() { - local home payload +test_20_legacy_waiting_receipts_need_no_original_or_payload() { + local home payload out home="$TMP_ROOT/c20" setup_home "$home" open_request "$home" - open_key "$home" retained-original - payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':retained-original","options":[{"label":"North"},{"label":"South"}]}') - plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare previously edited legacy post" + open_key "$home" legacy-waiting + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':legacy-waiting","options":[{"label":"North"},{"label":"South"}]}') + deliver "$home" "$POST" + out=$(click "$home" "fmd:key:$TASK:legacy-waiting:1" "$CAPTAIN" 880000000000000201) + assert_contains "$out" "Sent: North" "the legacy waiting click was not filed" + plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare legacy waiting receipt" import json, sys from pathlib import Path import outbox_poster as poster home, path = Path(sys.argv[1]), Path(sys.argv[3]) -payload = json.loads(path.read_text()) -payload["text"] = "Earlier explanation. " * 90 + "\n\n" + payload["text"] -payload["decision"].pop("sections", None) -path.write_text(json.dumps(payload)) -(home / "retained.json").write_text(json.dumps(payload)) -plan = poster.split_reply(payload["text"], home=home, limit=1900, cap=25, legacy=True) -assert len(plan["texts"]) == 2 -poster.split_decision_reply = lambda *args, **kwargs: plan -def send(chunk): - return {"discord_message_id": str(700000000000000200 + chunk["chunk_index"]), "buttons": True} -assert poster.deliver_one(path, send=send, home=home) == "sent" -progress = poster.load_progress(payload, home) -assert progress["total"] == progress["posted_count"] == 2 +progress = json.loads((path.parent / (path.stem + ".progress.json")).read_text()) progress.pop("texts") -poster.write_progress(payload, progress, home) +progress["decision"].pop("sections", None) +slug, kind, generation = path.stem.rsplit(".", 2) +poster.write_progress(dict(slug=slug, kind=kind, generation=int(generation)), progress, home) +path.unlink(missing_ok=True) +waiting = "⏳ North sent by Dana - waiting for Firstmate to record it" finalized = path.parent / (path.stem + ".finalized") -finalized.write_text("⏳ North sent by Dana - waiting for Firstmate to record it") +finalized.write_text(waiting) finalized.chmod(0o600) -(home / "plan.json").write_text(json.dumps(plan["texts"])) +(home / "discord.json").write_text(json.dumps(dict(content=waiting, earlier="Earlier chunk\n"))) +(home / "old-receipt.txt").write_text(waiting) PYTEST - home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key retained-original North \ - >/dev/null 2>&1 || fail "could not settle the previously edited legacy post" - plugin_py "$home" "$payload" <<'PYTEST' || fail "retained legacy payload did not restore the final decision display" -import json, sys + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key legacy-waiting South \ + >/dev/null 2>&1 || fail "could not record the legacy waiting answer" + plugin_py "$home" "$payload" <<'PYTEST' || fail "legacy component retry changed its receipt" +import io, json, sys, urllib.error from pathlib import Path +from unittest.mock import patch import outbox_poster as poster home, path = Path(sys.argv[1]), Path(sys.argv[3]) -path.write_text((home / "retained.json").read_text()) -path.chmod(0o600) -plan = json.loads((home / "plan.json").read_text()) finalized = path.parent / (path.stem + ".finalized") -old_receipt = finalized.read_text() -edits = [] -def read(*args): - raise AssertionError("retained payload must recover originals without reading the edited Discord message") -poster.discord_message_text = read -progress_path = path.parent / (path.stem + ".progress.json") -progress = json.loads(progress_path.read_text()) -for texts in (None, [], [""]): - candidate = dict(progress) - if texts is not None: - candidate["texts"] = texts - poster.write_progress(json.loads(path.read_text()), candidate, home) - assert path not in poster.list_pending(home=home) - assert path.is_file() -def failed_edit(*args): - raise poster.DiscordSendError("transient", "temporary edit failure") -assert poster.reconcile_decision_posts(home=home, edit=failed_edit) == [] -assert finalized.read_text() == old_receipt -progress = json.loads((path.parent / (path.stem + ".progress.json")).read_text()) -assert progress["texts"] == plan -assert progress["limit"] == 1900 and progress["cap"] == 25 -assert path not in poster.list_pending(home=home) +before = finalized.read_bytes() +settled = path.parent / (path.stem + ".settled.json") +def http(request, timeout): + body = json.loads(request.data) + assert "content" not in body + raise urllib.error.HTTPError(request.full_url, 429, "retry", {}, None) +with patch.object(poster, "_bot_token", return_value="test-token"), patch.object( + poster.urllib.request, "urlopen", side_effect=http, +): + assert poster.reconcile_decision_posts(home=home) == [] +assert finalized.read_bytes() == before +assert settled.is_file() assert not path.exists() -def edit(channel, message, content, components): - edits.append(dict(message=message, content=content, components=components)) -result = poster.reconcile_decision_posts(home=home, edit=edit) -assert result == [path.stem], dict(result=result, edits=edits) -assert len(edits) == 1 -message = edits[0] -assert message["message"] == "700000000000000201", message -assert "chosen by the captain" in message["content"].split("\n\n", 1)[0], message -assert message["content"].split("\n\n", 1)[1] == plan[-1], message -buttons = [button for row in message["components"] for button in row["components"]] -assert all(button["disabled"] for button in buttons), buttons -assert buttons[0]["style"] == 3 and buttons[0]["label"] == "North ✓", buttons -assert all(button["style"] == 2 for button in buttons[1:]), buttons -assert finalized.read_text() == message["content"] - -assert poster.reconcile_decision_posts(home=home, edit=edit) == [] -assert len(edits) == 1 PYTEST - pass "20 previously edited legacy posts recover retained split plans and finalize the recorded decision" + assert_legacy_edit "$home" "$payload" 2 true + pass "20 legacy waiting receipts retry and re-settle buttons after an answer without text recovery" } @@ -1779,5 +1777,5 @@ test_15_full_labels_keep_digits_and_overlaps_together test_16_recommended_option_renders_green_with_a_single_star test_17_outcome_headers_fit_reserved_and_legacy_posts test_18_decision_plans_preserve_questions_and_legacy_boundaries -test_19_completed_legacy_posts_recover_original_chunks_before_editing -test_20_previously_edited_legacy_posts_recover_retained_payloads +test_19_legacy_buttons_settle_in_every_outcome_state +test_20_legacy_waiting_receipts_need_no_original_or_payload From c72ee8363497a9d50fd2e614515ebfcba4b4db53 Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 18:45:00 +0800 Subject: [PATCH 6/8] no-mistakes(review): Preserve historical choices on settled legacy decisions --- bin/fm-ext-decision.sh | 14 ++++++-- tests/fm-ext-decision.test.sh | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index 4153728bafa..fc7ffee5a0b 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -296,14 +296,22 @@ newer_post_link() { # selected_option_id <stem>: the option chosen for this settled or sent post. selected_option_id() { - local stem=$1 kind outcome option + local stem=$1 kind option if settled "$stem"; then kind=$(jq -r '.kind // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) [ "$kind" = decided ] || return 0 option=$(jq -r '.selected_option_id // empty' "$OUTBOX/$stem.settled.json" 2>/dev/null) if [ -z "$option" ]; then - outcome=$(post_outcome "$stem") || return 0 - option=$(answer_option "$stem" "${outcome#*$'\t'}") + option=$(jq -r --slurpfile progress "$OUTBOX/$stem.progress.json" ' + .text as $outcome + | [$progress[0].decision.options[] + | ("✅ " + .label + " chosen by ") as $prefix + | select($outcome | startswith($prefix))] + | if length == 0 then empty else + (map(.label | length) | max) as $longest + | map(select((.label | length) == $longest)) + | if length == 1 then .[0].id else empty end + end' "$OUTBOX/$stem.settled.json" 2>/dev/null) || return 0 fi elif sent "$stem"; then option=$(sent_field "$stem" '.option_id') diff --git a/tests/fm-ext-decision.test.sh b/tests/fm-ext-decision.test.sh index 7436182e431..4abf411256e 100755 --- a/tests/fm-ext-decision.test.sh +++ b/tests/fm-ext-decision.test.sh @@ -1756,6 +1756,66 @@ PYTEST } +test_21_legacy_settled_choices_survive_reopened_keys() { + local home choice payload selected + for choice in North South unknown; do + home="$TMP_ROOT/c21-$choice" + setup_home "$home" + open_request "$home" + open_key "$home" historical + payload=$(emit_decision "$home" 1 '{"key":"key:'"$TASK"':historical","options":[{"label":"North"},{"label":"South"}]}') + deliver "$home" "$POST" + selected=none + case "$choice" in North) selected=1 ;; South) selected=2 ;; esac + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key historical "$choice" \ + >/dev/null 2>&1 || fail "could not record the historical choice" + home_env "$home" "$ROOT/bin/fm-ext-decision.sh" posts >/dev/null \ + || fail "could not settle the historical post" + plugin_py "$home" "$payload" <<'PYTEST' || fail "could not prepare the immutable legacy settlement" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +stem = path.stem +progress_path = path.parent / (stem + ".progress.json") +progress = json.loads(progress_path.read_text()) +original = progress["texts"][-1] +progress.pop("texts") +progress["decision"].pop("sections", None) +slug, kind, generation = stem.rsplit(".", 2) +poster.write_progress(dict(slug=slug, kind=kind, generation=int(generation)), progress, home) +settled_path = path.parent / (stem + ".settled.json") +settled = json.loads(settled_path.read_text()) +assert settled["kind"] == "decided" +settled.pop("selected_option_id", None) +settled_path.write_text(json.dumps(settled)) +settled_path.chmod(0o600) +(home / "historical-settlement.json").write_bytes(settled_path.read_bytes()) +(home / "discord.json").write_text(json.dumps(dict(content=original, earlier="Earlier chunk\n"))) +PYTEST + open_key "$home" historical + assert_legacy_edit "$home" "$payload" "$selected" true + home_env "$home" "$ROOT/bin/fm-send.sh" "$TASK" --resolve-key historical South \ + >/dev/null 2>&1 || fail "could not answer the reopened key" + plugin_py "$home" "$payload" <<'PYTEST' || fail "a later lifecycle changed the historical settlement" +import json, sys +from pathlib import Path +import outbox_poster as poster + +home, path = Path(sys.argv[1]), Path(sys.argv[3]) +settled_path = path.parent / (path.stem + ".settled.json") +assert settled_path.read_bytes() == (home / "historical-settlement.json").read_bytes() +edits = [] +assert poster.reconcile_decision_posts(home=home, edit=lambda *args: edits.append(args)) == [] +assert edits == [] +assert settled_path.read_bytes() == (home / "historical-settlement.json").read_bytes() +PYTEST + done + pass "21 immutable legacy settlements retain their historical choice or all-grey state after reopening" +} + + test_1_buttons_carry_key_and_option_ids_in_the_thread test_2_standing_click_files_one_request_for_a_worker_key test_3_captain_call_click_is_a_request_the_board_records @@ -1779,3 +1839,4 @@ test_17_outcome_headers_fit_reserved_and_legacy_posts test_18_decision_plans_preserve_questions_and_legacy_boundaries test_19_legacy_buttons_settle_in_every_outcome_state test_20_legacy_waiting_receipts_need_no_original_or_payload +test_21_legacy_settled_choices_survive_reopened_keys From a07b72b01e8f087b347f0c6a28df71338463a39e Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 18:54:52 +0800 Subject: [PATCH 7/8] no-mistakes(document): Clarify decision display and legacy settlement documentation --- .agents/skills/ext-respond/SKILL.md | 4 ++-- bin/fm-ext-decision.sh | 10 +++++++--- .../outbox_poster.py | 14 ++++++-------- docs/configuration.md | 3 ++- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.agents/skills/ext-respond/SKILL.md b/.agents/skills/ext-respond/SKILL.md index 672f62a587a..4a382ec47cc 100644 --- a/.agents/skills/ext-respond/SKILL.md +++ b/.agents/skills/ext-respond/SKILL.md @@ -85,12 +85,12 @@ Its `decision` field carries the key, the post's `occurrence`, the option, any c - `hold:` - one keyed line `<task-id>\t<option>\t<option_id>. <option>\t<mode>` on stdin to `bin/fm-captain-hold.sh answers --source "Discord button: option <option_id> \"<option>\" chosen by <by> (<author>) at <at> for <key>"`. - `key:` - `bin/fm-send.sh <task-id> --resolve-key <slug> "Decision answer: <option> (<the same audit>)"`. When `confirmed` is true, append `, confirmed by clicking "<confirm_prompt>"` to the audit. - That record is what the post reads to show "✅ <option> chosen by <by>"; one naming no single option shows a plain "✅ Decided". + The post reads that record under the [decision display rules](../../../docs/configuration.md#decision-buttons), including the legacy text-preservation exception. A `confirmed` click is the clicker's explicit word for that one decision only, which is the confirmation a merge, prod deploy, deletion, money, or credential option needs; it is never standing authority, and merge authority otherwise stays where AGENTS.md puts it. 2. `choice` `discuss`: nothing is decided; talk it through with the captain in the thread. 3. Finish with an `answer` or `final`, as for any request. -When you settle a decision that has a button post any other way, such as a typed numbered reply, record it through its own channel as usual and name the chosen option in the answer, by its number or label; the post then disables itself and shows that option as the captain's choice. +When you settle a decision that has a button post any other way, such as a typed numbered reply, record it through its own channel as usual and name the chosen option in the answer, by its number or label. ## Name the thread, then mark its state diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index fc7ffee5a0b..2638503cff1 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -30,7 +30,7 @@ # <stem>.settled.json the post's final outcome line, written once # <stem>.finalized the displayed content or legacy button-state receipt # -# What a post shows, where <time> is a Discord timestamp: +# Stored outcome lines, where <time> is a Discord timestamp: # ⏳ <label> sent by <name> <time> - waiting for Firstmate to record it # a click filed a request; buttons off # ✅ <label> chosen by <name> <time> Firstmate recorded the clicked option @@ -47,6 +47,8 @@ # The recorded answer comes from the ledgers through # fm_ext_decision_key_outcome in bin/fm-ext-lib.sh and is never shown itself, # because it may not be public-safe. +# Display formatting and the legacy text-preservation exception are owned by +# docs/configuration.md, "Decision buttons". # # click # The gateway plugin runs this for every button click. --post-id is the @@ -69,7 +71,7 @@ # 0 sent: claims the post once, files one request through # bin/fm-ext-intake.sh --origin button carrying the key, its occurrence, # the option, the decision text, any confirmation, and who and when; -# the post then shows the sent line with its buttons off +# the post then updates under the display rules above # Exit 1 means nothing was filed; the claim is released so the click can be # tried again. If an inbox request was published but notification failed, # exit 0 reports recovery needed; the sent claim remains and retrying uses @@ -79,7 +81,9 @@ # posts # Prints one JSON line per button post whose current content Discord does # not show yet: {post, channel_id, message_id, content, receipt, final, key, -# options}; a listed post always shows with its buttons off. Without +# options, selected_option_id}; content is null for a components-only legacy +# edit, and receipt is the value to pass to finalized. A listed post always +# shows with its buttons off. Without # --settled-only it first settles every live post that a newer post for the # same key replaced, or whose own lifecycle the ledgers report closed. # diff --git a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py index 62f04786bca..120168e6d32 100644 --- a/contrib/hermes-gateway-firstmate-comms/outbox_poster.py +++ b/contrib/hermes-gateway-firstmate-comms/outbox_poster.py @@ -39,10 +39,8 @@ text. Progress keeps the decision, button state, chunk ids, and posting time. Structured sections keep options together on the final chunk and reserve room for a later outcome header. -``reconcile_decision_posts`` edits only that final message, prepending a bold -outcome line and preserving the original last-chunk text whenever it fits. -It disables every button, highlights a unique choice in success style, and -grays the remaining options and Discuss. ``reconcile_thread_states`` then +``reconcile_decision_posts`` edits only that final message under the display +rules in docs/configuration.md, "Decision buttons". ``reconcile_thread_states`` then clears answered needs-you threads and retries failed renames. """ @@ -1536,10 +1534,10 @@ def reconcile_decision_posts( full: bool = True, edit: Callable[[str, str, str | None, list], None] | None = None, ) -> list[str]: - """Show each decision post's current line on Discord with its buttons off: - the sent line once a click filed its request, then the outcome once - Firstmate records the answer. ``full`` also settles posts whose decision - closed or was replaced, which reads Firstmate's ledgers, so the watcher + """Reconcile decision displays through the CLI's content and receipt plan. + A null content preserves legacy message text. + ``full`` also settles posts whose decision closed or was replaced, which + reads Firstmate's ledgers, so the watcher runs it on a slower cadence than the pass after a click. A permanent refusal (the message is gone) counts as shown; anything else is retried on a later pass. Returns the posts it updated. diff --git a/docs/configuration.md b/docs/configuration.md index 3e185dfea99..808e8ce935b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1013,7 +1013,7 @@ A recommended option that also needs confirmation still needs its second click. A confirmed click from a standing-rule author is that author's explicit word for that one decision only, never standing authority, and merge authority stays where `AGENTS.md` puts it. Each post is bound to the lifecycle of its decision that was open when it was posted, so a captain call re-held or a worker key reopened for a new question is a different decision to it. -When Firstmate records an answer on any channel, new posts are disabled and edited with a bold outcome header above the exact original last-chunk text whenever both fit Discord's 2,000-character limit. +When Firstmate records an answer on any channel, only the last chunk of a new post is edited with a bold outcome header above its exact original text whenever both fit Discord's 2,000-character limit; earlier chunks stay unchanged. The uniquely chosen option is success-colored and gains a `✓`; every other option and Discuss are grey, and all buttons are disabled. When no single option matches, every button is grey; a click waiting for Firstmate to record it still highlights the clicked option. The outcome names the clicker when the recorded answer matches that post's click, and otherwise names the captain when the answer names exactly one option by number or label; `✅ Decided <time>` covers answers without one unique option, and `Closed - no longer needed <time>` covers a decision closed without an answer or replaced by a later lifecycle. @@ -1021,6 +1021,7 @@ When a newer post replaces an older one for the same decision, the older outcome Structured posts reserve space for that header, so a normal settlement edit needs no truncation. Legacy posts, identified by missing original chunks or legacy decision records without section metadata, receive only button updates in every outcome state; their message text stays byte for byte unchanged, even if an earlier version replaced it with a waiting line. Legacy posts need no original-text recovery or retained payload, and a recorded answer updates their buttons even when an older waiting receipt exists. +An already-settled legacy post keeps the choice recorded in its own immutable settlement, including when its key is reopened or its task re-held; when that record identifies no unique option, all buttons are grey. For new posts, an edit that cannot fit trims the explanation tail with `(trimmed)` while preserving the header, question, and option list. New decision plans that exceed the configured chunk cap are refused before posting rather than truncating their text. The outcome is read from the same ledgers rather than from any step Firstmate must remember. From 2acbdef6863af3af681a296a9c8375c588827b77 Mon Sep 17 00:00:00 2001 From: dnth <dickson.neoh@gmail.com> Date: Fri, 16 Oct 2026 19:46:15 +0800 Subject: [PATCH 8/8] no-mistakes(lint): Replace ambiguous shell expression with explicit skip guards --- bin/fm-ext-decision.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bin/fm-ext-decision.sh b/bin/fm-ext-decision.sh index 2638503cff1..e2866661622 100755 --- a/bin/fm-ext-decision.sh +++ b/bin/fm-ext-decision.sh @@ -672,7 +672,12 @@ command_posts() { settle_if_closed "$stem" || true fi if ! settled "$stem"; then - sent "$stem" && request_published "$(sent_field "$stem" '.request_id')" || continue + if ! sent "$stem"; then + continue + fi + if ! request_published "$(sent_field "$stem" '.request_id')"; then + continue + fi fi selected=$(selected_option_id "$stem") final=false