diff --git a/.github/workflows/_build.yml b/.github/workflows/_build.yml index c787aebce..46a2c16de 100644 --- a/.github/workflows/_build.yml +++ b/.github/workflows/_build.yml @@ -317,7 +317,7 @@ jobs: persist-credentials: false - name: Download all native candidate pairs - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: pattern: release-candidates-* path: ${{ runner.temp }}/release-candidate-artifacts diff --git a/.github/workflows/_smoke.yml b/.github/workflows/_smoke.yml index ac4ca595d..cfe022a6a 100644 --- a/.github/workflows/_smoke.yml +++ b/.github/workflows/_smoke.yml @@ -103,7 +103,7 @@ jobs: go-version-file: pkg/go/go.mod cache: false - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-${{ matrix.goos }}-${{ matrix.goarch }} @@ -203,7 +203,7 @@ jobs: zip coreutils - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-windows-${{ matrix.arch }} @@ -303,7 +303,7 @@ jobs: go-version-file: pkg/go/go.mod cache: false - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-linux-${{ matrix.arch }}-portable diff --git a/.github/workflows/_soak.yml b/.github/workflows/_soak.yml index 3bbfbd8c7..fe82cfb8a 100644 --- a/.github/workflows/_soak.yml +++ b/.github/workflows/_soak.yml @@ -73,7 +73,7 @@ jobs: run: scripts/build.sh ${{ inputs.version && format('--version {0}', inputs.version) || '' }} CC=${{ matrix.cc }} CXX=${{ matrix.cxx }} - name: Download selected release container if: ${{ inputs.use_release_artifacts }} - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-${{ matrix.goos }}-${{ matrix.goarch }} path: ${{ runner.temp }}/cbm-selected-container @@ -147,7 +147,7 @@ jobs: run: scripts/build.sh ${{ inputs.version && format('--version {0}', inputs.version) || '' }} CC=clang CXX=clang++ - name: Download selected release container if: ${{ inputs.use_release_artifacts }} - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-windows-amd64 path: ${{ runner.temp }}/cbm-selected-container @@ -225,7 +225,7 @@ jobs: run: scripts/build.sh ${{ inputs.version && format('--version {0}', inputs.version) || '' }} CC=clang CXX=clang++ - name: Download selected release container if: ${{ inputs.use_release_artifacts }} - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-windows-arm64 path: ${{ runner.temp }}/cbm-selected-container @@ -287,7 +287,7 @@ jobs: timeout-minutes: 10 run: APT_RETRIES=3; [ -f /etc/apt/apt-mirrors.txt ] && APT_RETRIES=0; sudo apt-get -o Acquire::Retries=$APT_RETRIES -o Acquire::http::Timeout=30 -o Acquire::https::Timeout=30 --error-on=any update && sudo apt-get -o Acquire::Retries=$APT_RETRIES -o Acquire::http::Timeout=30 -o Acquire::https::Timeout=30 install -y python3 git - name: Download selected portable release container - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: binaries-linux-${{ matrix.arch }}-portable path: ${{ runner.temp }}/cbm-selected-container diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 7bf32c277..55e61cbe1 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -231,7 +231,7 @@ jobs: # producer, so arm64, macOS and Windows code under #ifdef has static # evidence only and those legs run every test. if: ${{ inputs.test_selection != '' && runner.os == 'Linux' && runner.arch == 'X64' }} - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: ${{ inputs.test_selection }} path: ${{ runner.temp }}/test-selection @@ -720,7 +720,7 @@ jobs: with: path: repo - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: pattern: shard-manifest-* path: manifests diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 40c80ef63..d7b1f1af0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -185,7 +185,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Download only selected release containers - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: pattern: binaries-* merge-multiple: true @@ -194,7 +194,7 @@ jobs: # download above. Preserve only the three reviewable manifests that bind # candidate bytes, VT results, and the tuple-local decision. - name: Download candidate selection evidence separately - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: release-selection-evidence path: selection-evidence @@ -617,7 +617,7 @@ jobs: # the release-draft job as a same-run artifact (not a draft-release # download — that would need contents beyond read plus gh). - name: Fetch release checksums - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: release-checksums diff --git a/Makefile.cbm b/Makefile.cbm index 8416dc231..162d1324a 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -417,6 +417,7 @@ EXTRACTION_SRCS = \ $(CBM_DIR)/openapi.c \ $(CBM_DIR)/extract_nextjs.c \ $(CBM_DIR)/sql_values.c \ + $(CBM_DIR)/php_inline_html.c \ $(CBM_DIR)/helpers.c \ $(CBM_DIR)/cpp_branch_views.c \ $(CBM_DIR)/result_compact.c \ diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 4238af928..88fa02145 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -3319,6 +3319,18 @@ CBMFileResult *cbm_extract_file(const char *source, int source_len, CBMLanguage return r; } +/* The bytes the grammar parses. PHP that leaves PHP mode (`?> markup ` (#2000). Same length and line breaks as the input; returns +// `source` itself when nothing needs rewriting. Defined in php_inline_html.c. +const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int source_len); void cbm_extract_imports(CBMExtractCtx *ctx); void cbm_extract_usages(CBMExtractCtx *ctx); void cbm_extract_semantic(CBMExtractCtx *ctx); diff --git a/internal/cbm/lsp/php_lsp.c b/internal/cbm/lsp/php_lsp.c index 7aae329e8..85118f671 100644 --- a/internal/cbm/lsp/php_lsp.c +++ b/internal/cbm/lsp/php_lsp.c @@ -20,6 +20,7 @@ #include "php_lsp.h" #include "lsp_node_iter.h" #include "lsp_work.h" +#include "../cbm.h" #include "../helpers.h" #include "../../../src/foundation/hash_table.h" #include @@ -4795,6 +4796,9 @@ void cbm_run_php_lsp_cross_with_bindings(CBMArena *arena, const char *source, in const CBMPHPImportBindings *bindings) { if (!arena || !source || source_len <= 0 || !out) return; + /* The retained source is the file on disk; read it the way the per-file + * extract did, so the cached tree and node text agree (#2000). */ + source = cbm_php_mask_inline_html(arena, source, source_len); TSParser *parser = NULL; TSTree *tree = cached_tree; diff --git a/internal/cbm/php_inline_html.c b/internal/cbm/php_inline_html.c new file mode 100644 index 000000000..e766a1fbd --- /dev/null +++ b/internal/cbm/php_inline_html.c @@ -0,0 +1,348 @@ +// php_inline_html.c — let the php_only grammar read a file that leaves PHP mode. +// +// A PHP file starts in inline-HTML mode and may switch in and out of PHP any +// number of times: `?>
...
` or for the +// text that follows it, so every byte after the first close tag used to land in +// an ERROR region and the declarations there never reached the graph (#2000). +// +// Instead of parsing the markup, blank it. The rewritten buffer has the same +// length as the original and keeps every line break, so byte offsets, rows and +// columns are unchanged and node text inside PHP code is byte-identical: +// +// - markup before the first open tag and between tags becomes spaces +// - the first open tag stays, so the tree keeps its php_tag +// - each later `` becomes `; `, the statement terminator PHP itself treats it as +// +// Only code and `//` / `#` comments can close PHP mode. Strings, heredoc, +// nowdoc and block comments are skipped, `#[` is an attribute, and everything +// after __halt_compiler is left alone, as PHP stops reading there. + +#include "cbm.h" +#include "foundation/arena.h" +#include "foundation/constants.h" +#include +#include +#include + +enum { + PHP_TAG_OPEN_LEN = 5, /* = 0 && i <= len && (size_t)(len - i) >= n && memcmp(s + i, lit, n) == 0; +} + +static bool php_is_ident_char(unsigned char c) { + return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || + c >= PHP_FIRST_NON_ASCII; +} + +static bool php_is_ident_start(unsigned char c) { + return php_is_ident_char(c) && !(c >= '0' && c <= '9'); +} + +static bool php_is_space(unsigned char c) { + return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f' || c == '\v'; +} + +static bool php_is_blank(char c) { + return c == ' ' || c == '\t'; +} + +/* Case-insensitive match of the lowercase `word` at s[i]. */ +static bool php_ieq(const char *s, int len, int i, const char *word) { + int n = (int)strlen(word); + if (i + n > len) { + return false; + } + for (int k = 0; k < n; k++) { + char c = s[i + k]; + if (c >= 'A' && c <= 'Z') { + c = (char)(c - 'A' + 'a'); + } + if (c != word[k]) { + return false; + } + } + return true; +} + +/* Length of the open tag at i, or 0. `` prolog in markup stays markup. */ +static int php_open_tag_len(const char *s, int len, int i) { + if (!php_at(s, len, i, "")) { + return true; + } + } + return false; +} + +/* Blank [from, to) in place, keeping line breaks. */ +static void php_blank(char *out, int from, int to) { + for (int k = from; k < to; k++) { + if (out[k] != '\n' && out[k] != '\r') { + out[k] = ' '; + } + } +} + +/* Index just past the quoted string opened at i. */ +static int php_skip_quoted(const char *s, int len, int i) { + char quote = s[i]; + i += SKIP_ONE; + while (i < len && s[i] != quote) { + i += (s[i] == '\\') ? PAIR_LEN : SKIP_ONE; + } + return i < len ? i + SKIP_ONE : len; +} + +/* Index just past the block comment opened at i. */ +static int php_skip_block_comment(const char *s, int len, int i) { + for (int k = i + PAIR_LEN; k < len; k++) { + if (php_at(s, len, k, "*/")) { + return k + PAIR_LEN; + } + } + return len; +} + +/* Index of the line break or `?>` that ends the one-line comment at i. */ +static int php_skip_line_comment(const char *s, int len, int i) { + while (i < len && s[i] != '\n' && !php_at(s, len, i, "?>")) { + i++; + } + return i; +} + +/* Parse the heredoc/nowdoc opener `<<= len || s[p] != quote) { + return PHP_NOT_FOUND; + } + p++; + } + if (p < len && s[p] == '\r') { + p++; + } + return (p < len && s[p] == '\n') ? p : PHP_NOT_FOUND; +} + +/* True when the line starting at p closes the heredoc. PHP 7.3+ allows the + * closing label to be indented and followed by any non-identifier byte. */ +static bool php_heredoc_closes(const char *s, int len, int p, int label, int label_len, int *end) { + while (p < len && php_is_blank(s[p])) { + p++; + } + if (len - p < label_len || memcmp(s + p, s + label, (size_t)label_len) != 0) { + return false; + } + int after = p + label_len; + if (after < len && php_is_ident_char((unsigned char)s[after])) { + return false; + } + *end = after; + return true; +} + +/* Index just past the heredoc/nowdoc opened by the `<<<` at i. */ +static int php_skip_heredoc(const char *s, int len, int i) { + int label = 0; + int label_len = 0; + int nl = php_heredoc_opener(s, len, i, &label, &label_len); + if (nl < 0) { + return i + PHP_HEREDOC_LEN; + } + while (nl < len) { + int line = nl + SKIP_ONE; + int end = 0; + if (php_heredoc_closes(s, len, line, label, label_len, &end)) { + return end; + } + const char *next = memchr(s + line, '\n', (size_t)(len - line)); + if (!next) { + return len; + } + nl = (int)(next - s); + } + return len; +} + +/* Index just past the identifier at i; sets *halt when it is __halt_compiler + * (a keyword, so not a `$variable` of that name). */ +static int php_skip_ident(const char *s, int len, int i, bool *halt) { + int start = i; + while (i < len && php_is_ident_char((unsigned char)s[i])) { + i++; + } + bool is_var = start > 0 && s[start - SKIP_ONE] == '$'; + *halt = + !is_var && i - start == PHP_HALT_COMPILER_LEN && php_ieq(s, len, start, PHP_HALT_COMPILER); + return i; +} + +/* Scan PHP code from i to the next close tag. Returns the index of that `?>`, + * len when the file ends in PHP mode, or PHP_NOT_FOUND at __halt_compiler. */ +static int php_scan_code(const char *s, int len, int i) { + while (i < len) { + unsigned char c = (unsigned char)s[i]; + if (php_at(s, len, i, "?>")) { + return i; + } + if (c == '\'' || c == '"' || c == '`') { + i = php_skip_quoted(s, len, i); + } else if (php_at(s, len, i, "/*")) { + i = php_skip_block_comment(s, len, i); + } else if (php_at(s, len, i, "//") || (c == '#' && !php_at(s, len, i, "#["))) { + i = php_skip_line_comment(s, len, i); + } else if (php_at(s, len, i, "<<<")) { + i = php_skip_heredoc(s, len, i); + } else if (php_is_ident_start(c)) { + bool halt = false; + i = php_skip_ident(s, len, i, &halt); + if (halt) { + return PHP_NOT_FOUND; + } + } else { + i++; + } + } + return len; +} + +/* The rewritten buffer. It is allocated and filled from the source on the + * first write, so a file whose only `?>` bytes sit in strings or comments + * costs no copy. */ +typedef struct { + CBMArena *arena; + const char *source; + int len; + char *out; + bool failed; +} PHPMask; + +static char *php_mask_writable(PHPMask *m) { + if (!m->out && !m->failed) { + m->out = (char *)cbm_arena_alloc(m->arena, (size_t)m->len + SKIP_ONE); + if (m->out) { + memcpy(m->out, m->source, (size_t)m->len); + m->out[m->len] = '\0'; + } else { + m->failed = true; + } + } + return m->out; +} + +/* Blank [from, to) of the rewritten buffer, keeping line breaks. */ +static void php_mask_blank(PHPMask *m, int from, int to) { + char *out = php_mask_writable(m); + if (out) { + php_blank(out, from, to); + } +} + +/* Turn the `?>` at i into `; `. */ +static void php_mask_close_tag(PHPMask *m, int i) { + char *out = php_mask_writable(m); + if (out) { + out[i] = ';'; + out[i + SKIP_ONE] = ' '; + } +} + +const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int source_len) { + if (!arena || !source || source_len <= 0) { + return source; + } + /* Fast path: a file that opens PHP at its first byte and never writes a + * close tag is exactly what the grammar already accepts. */ + if (php_open_tag_len(source, source_len, 0) > 0 && !php_has_close_tag(source, source_len)) { + return source; + } + + PHPMask m = {.arena = arena, .source = source, .len = source_len}; + bool seen_open = false; + int i = 0; + while (i < source_len && !m.failed) { + /* Inline-HTML mode: blank up to the next open tag. */ + int html_start = i; + int tag = 0; + while (i < source_len && (tag = php_open_tag_len(source, source_len, i)) == 0) { + i++; + } + if (i > html_start) { + php_mask_blank(&m, html_start, i); + } + if (i >= source_len) { + break; + } + if (seen_open) { + php_mask_blank(&m, i, i + tag); + } + seen_open = true; + + /* PHP mode, up to the close tag. */ + int close = php_scan_code(source, source_len, i + tag); + if (close < 0 || close >= source_len) { + break; + } + php_mask_close_tag(&m, close); + i = close + PAIR_LEN; + } + /* On allocation failure parse the file as it is: the old behaviour. */ + return (m.out && !m.failed) ? m.out : source; +} diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index a6f8ced10..09a079212 100644 --- a/src/cli/config_yaml_edit.c +++ b/src/cli/config_yaml_edit.c @@ -2589,6 +2589,10 @@ typedef struct { bool identity_found; size_t identity_start; size_t identity_end; + /* The missing rest of the path begins under a `parent: {}` header. */ + bool parent_inline_empty; + size_t parent_line; + size_t parent_colon; } yaml_mapping_sequence_target_t; static void yaml_sequence_key_vec_free(yaml_sequence_key_vec_t *keys) { @@ -2962,6 +2966,37 @@ static int yaml_sequence_parse_block(const yaml_doc_t *doc, size_t begin_line, s return 0; } +/* `section: {}` is how the mapping-entry editor writes a section it emptied + * (yaml_build_last_mapping_entry_removal), and its upsert accepts it back. On + * a PARENT of the sequence path it is that same empty mapping, so the rest of + * the path is missing beneath it: removal is a no-op and upsert expands the + * header in place. The Hermes remover leaves `hooks: {}` whenever our hook was + * the only entry. The sequence key itself holding `{}`, any other flow value, + * and content nested under the inline mapping stay refused. */ +static int yaml_sequence_target_empty_parent(const yaml_doc_t *doc, size_t key_line, size_t colon, + size_t child_end, size_t next_depth, + size_t sequence_path_len, + yaml_mapping_sequence_target_t *target) { + bool inline_empty = false; + if (next_depth >= sequence_path_len || + yaml_tail_is_explicit_empty_mapping(doc->data, colon, doc->lines[key_line].text_end, + &inline_empty) != 0 || + !inline_empty) { + return YAML_ERROR; + } + for (size_t i = key_line + YAML_UNIT; i < child_end; i++) { + if (!doc->lines[i].blank && !doc->lines[i].comment) { + return YAML_ERROR; + } + } + target->missing_index = next_depth; + target->insert_offset = yaml_sequence_line_offset(doc, child_end); + target->parent_inline_empty = true; + target->parent_line = key_line; + target->parent_colon = colon; + return 0; +} + static int yaml_sequence_analyze(const yaml_doc_t *doc, const char *const *sequence_path, const size_t *path_lengths, size_t sequence_path_len, const char *identity_key, const char *identity_value, @@ -2993,10 +3028,11 @@ static int yaml_sequence_analyze(const yaml_doc_t *doc, const char *const *seque return 0; } const yaml_line_t *header = &doc->lines[key_line]; - if (!yaml_tail_is_empty(doc->data, colon, header->text_end)) { - return YAML_ERROR; - } size_t child_end = yaml_sequence_nested_end(doc, key_line, parent_end); + if (!yaml_tail_is_empty(doc->data, colon, header->text_end)) { + return yaml_sequence_target_empty_parent(doc, key_line, colon, child_end, + depth + YAML_UNIT, sequence_path_len, target); + } if (depth + YAML_UNIT == sequence_path_len) { target->sequence_found = true; target->missing_index = sequence_path_len; @@ -3095,13 +3131,52 @@ static int yaml_sequence_validate_canonical(const char *canonical_item, size_t c return result; } +/* Copies the document up to insert_offset with the `parent: {}` header turned + * into a block header `parent:`, exactly as + * yaml_build_inline_empty_mapping_section_add does for a mapping section: a + * trailing comment and the line ending are kept. */ +static int yaml_sequence_append_expanded_parent(yaml_buf_t *out, const yaml_doc_t *doc, + const yaml_mapping_sequence_target_t *target) { + if (target->parent_line >= doc->line_count) { + return YAML_ERROR; + } + const yaml_line_t *header = &doc->lines[target->parent_line]; + size_t comment = 0U; + if (target->insert_offset < header->end || + yaml_find_comment(doc->data, target->parent_colon + YAML_UNIT, header->text_end, + &comment) != 0 || + yaml_buf_append(out, doc->data, target->parent_colon + YAML_UNIT) != 0) { + return YAML_ERROR; + } + size_t kept_tail = header->text_end; + if (comment < header->text_end) { + if (yaml_buf_append_char(out, ' ') != 0) { + return YAML_ERROR; + } + kept_tail = comment; + } + if (yaml_buf_append(out, doc->data + kept_tail, header->end - kept_tail) != 0 || + yaml_buf_append(out, doc->data + header->end, target->insert_offset - header->end) != 0 || + (out->len > 0U && out->data[out->len - YAML_UNIT] != '\n' && + yaml_buf_append(out, doc->eol, doc->eol_len) != 0)) { + return YAML_ERROR; + } + return 0; +} + static int yaml_sequence_build_insert(yaml_buf_t *out, const yaml_doc_t *doc, const yaml_mapping_sequence_target_t *target, const char *const *sequence_path, const size_t *path_lengths, size_t sequence_path_len, const yaml_buf_t *rendered_item) { - if (target->insert_offset > doc->len || - yaml_buf_append(out, doc->data, target->insert_offset) != 0 || - yaml_append_separator_if_needed(out, doc, target->insert_offset) != 0) { + if (target->insert_offset > doc->len) { + return YAML_ERROR; + } + if (target->parent_inline_empty) { + if (yaml_sequence_append_expanded_parent(out, doc, target) != 0) { + return YAML_ERROR; + } + } else if (yaml_buf_append(out, doc->data, target->insert_offset) != 0 || + yaml_append_separator_if_needed(out, doc, target->insert_offset) != 0) { return YAML_ERROR; } if (!target->sequence_found) { diff --git a/src/daemon/application.c b/src/daemon/application.c index 3b9409f83..780676fe9 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -4026,7 +4026,7 @@ cbm_daemon_runtime_application_callbacks_t cbm_daemon_application_runtime_callba static cbm_daemon_runtime_application_status_t application_client_exchange_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, uint8_t *request, uint32_t request_length, uint8_t **response_out, - uint32_t *response_length_out, uint32_t timeout_ms) { + uint32_t *response_length_out, bool *request_sent_out, uint32_t timeout_ms) { uint8_t *response = NULL; uint32_t response_length = 0; if (response_out) { @@ -4035,13 +4035,16 @@ static cbm_daemon_runtime_application_status_t application_client_exchange_tagge if (response_length_out) { *response_length_out = 0; } + if (request_sent_out) { + *request_sent_out = false; + } cbm_daemon_runtime_application_status_t status = request_token == CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID ? cbm_daemon_runtime_client_application_request(client, request, request_length, &response, &response_length, timeout_ms) - : cbm_daemon_runtime_client_application_request_tagged(client, request_token, request, - request_length, &response, - &response_length, timeout_ms); + : cbm_daemon_runtime_client_application_request_tagged( + client, request_token, request, request_length, &response, &response_length, + request_sent_out, timeout_ms); free(request); if (status != CBM_DAEMON_RUNTIME_APPLICATION_OK) { free(response); @@ -4072,7 +4075,7 @@ static cbm_daemon_runtime_application_status_t application_client_exchange( uint8_t **response_out, uint32_t *response_length_out, uint32_t timeout_ms) { return application_client_exchange_tagged(client, CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID, request, request_length, response_out, - response_length_out, timeout_ms); + response_length_out, NULL, timeout_ms); } cbm_daemon_runtime_application_status_t cbm_daemon_application_client_set_context( @@ -4200,7 +4203,7 @@ cbm_daemon_runtime_application_status_t cbm_daemon_application_client_ui_readine static cbm_daemon_runtime_application_status_t application_client_text_request_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, cbm_daemon_application_request_kind_t kind, const char *text, uint8_t **response_out, - uint32_t *response_length_out, uint32_t timeout_ms) { + uint32_t *response_length_out, bool *request_sent_out, uint32_t timeout_ms) { if (!client || !text || !text[0]) { return CBM_DAEMON_RUNTIME_APPLICATION_REJECTED; } @@ -4216,7 +4219,7 @@ static cbm_daemon_runtime_application_status_t application_client_text_request_t memcpy(request + 1, text, text_length); return application_client_exchange_tagged(client, request_token, request, (uint32_t)text_length + 1U, response_out, - response_length_out, timeout_ms); + response_length_out, request_sent_out, timeout_ms); } static cbm_daemon_runtime_application_status_t application_client_text_request( @@ -4224,7 +4227,7 @@ static cbm_daemon_runtime_application_status_t application_client_text_request( const char *text, uint8_t **response_out, uint32_t *response_length_out, uint32_t timeout_ms) { return application_client_text_request_tagged( client, CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID, kind, text, response_out, - response_length_out, timeout_ms); + response_length_out, NULL, timeout_ms); } cbm_daemon_runtime_application_status_t cbm_daemon_application_client_mcp( @@ -4237,10 +4240,10 @@ cbm_daemon_runtime_application_status_t cbm_daemon_application_client_mcp( cbm_daemon_runtime_application_status_t cbm_daemon_application_client_mcp_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, const char *message, uint8_t **response_out, uint32_t *response_length_out, - uint32_t timeout_ms) { - return application_client_text_request_tagged(client, request_token, - CBM_DAEMON_APPLICATION_REQUEST_MCP, message, - response_out, response_length_out, timeout_ms); + bool *request_sent_out, uint32_t timeout_ms) { + return application_client_text_request_tagged( + client, request_token, CBM_DAEMON_APPLICATION_REQUEST_MCP, message, response_out, + response_length_out, request_sent_out, timeout_ms); } cbm_daemon_runtime_application_status_t cbm_daemon_application_client_tool( diff --git a/src/daemon/application.h b/src/daemon/application.h index 933186aa7..b071ea03c 100644 --- a/src/daemon/application.h +++ b/src/daemon/application.h @@ -149,11 +149,12 @@ cbm_daemon_runtime_application_status_t cbm_daemon_application_client_mcp( cbm_daemon_runtime_client_t *client, const char *message, uint8_t **response_out, uint32_t *response_length_out, uint32_t timeout_ms); -/* Cancellable frontend variant using a token reserved on the runtime client. */ +/* Cancellable frontend variant using a token reserved on the runtime client. + * request_sent_out preserves the runtime's application-frame delivery fact. */ cbm_daemon_runtime_application_status_t cbm_daemon_application_client_mcp_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, const char *message, uint8_t **response_out, uint32_t *response_length_out, - uint32_t timeout_ms); + bool *request_sent_out, uint32_t timeout_ms); cbm_daemon_runtime_application_status_t cbm_daemon_application_client_tool( cbm_daemon_runtime_client_t *client, const char *tool_name, const char *args_json, diff --git a/src/daemon/frontend.c b/src/daemon/frontend.c index cbe4b282e..e76c920b6 100644 --- a/src/daemon/frontend.c +++ b/src/daemon/frontend.c @@ -76,6 +76,7 @@ typedef struct { cbm_mutex_t mutex; cbm_daemon_runtime_client_t *client; cbm_version_cohort_manager_t *cohort_manager; + const cbm_daemon_frontend_session_config_t *session; FILE *out; frontend_item_t queue[FRONTEND_QUEUE_CAPACITY]; size_t head; @@ -89,6 +90,7 @@ typedef struct { int64_t active_id; const char *active_id_str; cbm_daemon_runtime_application_token_t active_request_token; + bool active_cancelled; bool failed; /* Monotonic count of fully processed queue items (responses written or * cancellations acknowledged). The EOF drain below watches it to tell a @@ -117,6 +119,8 @@ static atomic_bool frontend_test_monitor_is_waiting = ATOMIC_VAR_INIT(false); static atomic_uint_fast64_t frontend_test_monitor_observation_count = ATOMIC_VAR_INIT(0); static atomic_uint_fast64_t frontend_test_worker_observation_count = ATOMIC_VAR_INIT(0); static atomic_uint_fast64_t frontend_test_worker_idle_count = ATOMIC_VAR_INIT(0); +static atomic_uint_fast64_t frontend_test_routed_cancellation_count = ATOMIC_VAR_INIT(0); +static atomic_uint_fast32_t frontend_test_replay_budget_ms = ATOMIC_VAR_INIT(0); void cbm_daemon_frontend_test_observer_reset(bool hold_monitor) { atomic_store_explicit(&frontend_test_monitor_observation_count, 0, memory_order_release); @@ -145,6 +149,14 @@ uint64_t cbm_daemon_frontend_test_worker_observations(void) { uint64_t cbm_daemon_frontend_test_worker_idle_cycles(void) { return atomic_load_explicit(&frontend_test_worker_idle_count, memory_order_acquire); } + +uint64_t cbm_daemon_frontend_test_routed_cancellations(void) { + return atomic_load_explicit(&frontend_test_routed_cancellation_count, memory_order_acquire); +} + +uint32_t cbm_daemon_frontend_test_replay_timeout_ms(void) { + return (uint32_t)atomic_load_explicit(&frontend_test_replay_budget_ms, memory_order_acquire); +} #endif static cbm_version_cohort_maintenance_presence_t frontend_observe_maintenance( @@ -352,6 +364,7 @@ static bool frontend_pop_begin(frontend_state_t *state, frontend_item_t *item) { state->active_id_str = item->id_str; item->request_token = request_token; state->active_request_token = request_token; + state->active_cancelled = false; popped = true; } cbm_mutex_unlock(&state->mutex); @@ -365,6 +378,7 @@ static void frontend_end_request(frontend_state_t *state, bool failed) { state->active_id = 0; state->active_id_str = NULL; state->active_request_token = CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; + state->active_cancelled = false; state->failed = state->failed || failed; cbm_mutex_unlock(&state->mutex); } @@ -464,12 +478,8 @@ typedef enum { * request routes its exact runtime token without closing the authenticated * session. A queued request is marked and receives a cancellation error * without ever reaching the daemon. Stale/invalid targets are ignored. */ -static frontend_cancellation_route_t frontend_route_cancellation( - frontend_state_t *state, const char *message, - cbm_daemon_runtime_application_token_t *request_token_out) { - if (request_token_out) { - *request_token_out = CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; - } +static frontend_cancellation_route_t frontend_route_cancellation(frontend_state_t *state, + const char *message) { cbm_jsonrpc_request_t request = {0}; if (!frontend_parse_cancellation(message, &request)) { return FRONTEND_CANCELLATION_NONE; @@ -478,12 +488,9 @@ static frontend_cancellation_route_t frontend_route_cancellation( frontend_cancellation_route_t route = FRONTEND_CANCELLATION_STALE; cbm_mutex_lock(&state->mutex); if (state->in_request && state->active_has_id && - state->active_request_token != CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID && cbm_mcp_cancel_request_matches(request.params_raw, state->active_id, state->active_id_str)) { - if (request_token_out) { - *request_token_out = state->active_request_token; - } + state->active_cancelled = true; route = FRONTEND_CANCELLATION_ACTIVE; } else { for (size_t offset = 0; offset < state->count; offset++) { @@ -499,9 +506,116 @@ static frontend_cancellation_route_t frontend_route_cancellation( } cbm_mutex_unlock(&state->mutex); cbm_jsonrpc_request_free(&request); +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + if (route == FRONTEND_CANCELLATION_ACTIVE || route == FRONTEND_CANCELLATION_QUEUED) { + atomic_fetch_add_explicit(&frontend_test_routed_cancellation_count, 1, + memory_order_release); + } +#endif return route; } +static cbm_daemon_runtime_cancel_result_t frontend_cancel_active(frontend_state_t *state) { + cbm_mutex_lock(&state->mutex); + cbm_daemon_runtime_cancel_result_t result = CBM_DAEMON_RUNTIME_CANCEL_STALE; + if (state->in_request && state->active_cancelled && state->client && + state->active_request_token != CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID) { + result = cbm_daemon_runtime_client_application_cancel(state->client, + state->active_request_token); + } else if (state->in_request && state->active_cancelled) { + /* A reconnect has no daemon request to cancel yet. The worker observes + * active_cancelled before retrying the frame. */ + result = CBM_DAEMON_RUNTIME_CANCEL_ACCEPTED; + } + cbm_mutex_unlock(&state->mutex); + return result; +} + +static bool frontend_configure_client(cbm_daemon_runtime_client_t *client, + const cbm_daemon_frontend_session_config_t *session) { + /* The replay restores what startup set, so it waits within the startup + * budget like the first set_context and set_ui_config in main.c. The + * connect budget only bounds a probe (1 s in production): a replacement + * daemon whose UI save runs longer would end an otherwise good recovery. */ + uint32_t timeout_ms = session->bootstrap.startup_timeout_ms; +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + atomic_store_explicit(&frontend_test_replay_budget_ms, timeout_ms, memory_order_release); +#endif + if (cbm_daemon_application_client_set_context( + client, session->session_root, session->allowed_root, session->tool_profile, NULL, NULL, + timeout_ms) != CBM_DAEMON_RUNTIME_APPLICATION_OK) { + return false; + } + return session->ui_update_mask == 0 || + cbm_daemon_application_client_set_ui_config( + client, session->ui_update_mask, session->ui_enabled, session->ui_port, + timeout_ms) == CBM_DAEMON_RUNTIME_APPLICATION_OK; +} + +static bool frontend_recover_client(frontend_state_t *state, frontend_item_t *item, + bool *cancelled_out) { + if (cancelled_out) { + *cancelled_out = false; + } + + cbm_mutex_lock(&state->mutex); + if (state->stopping || !state->client) { + cbm_mutex_unlock(&state->mutex); + return false; + } + cbm_daemon_runtime_client_t *previous = state->client; + state->client = NULL; + state->active_request_token = CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; + cbm_mutex_unlock(&state->mutex); + if (previous) { + (void)cbm_daemon_runtime_client_close(previous, FRONTEND_CLOSE_TIMEOUT_MS); + } + + cbm_daemon_bootstrap_result_t bootstrap = {0}; + /* Bootstrap can report launch failures on stderr from this worker. + * Keep that diagnostic + * to explain failed recovery. It never writes MCP + * stdout, but blocked stderr can delay + * exit. Do not add more logging here. */ + cbm_daemon_bootstrap_status_t bootstrap_status = + cbm_daemon_bootstrap_execute(&state->session->bootstrap, &bootstrap); + cbm_daemon_runtime_client_t *replacement = + bootstrap_status == CBM_DAEMON_BOOTSTRAP_CONNECTED ? bootstrap.client : NULL; + bool configured = replacement && frontend_configure_client(replacement, state->session); + cbm_daemon_runtime_application_token_t replacement_token = + CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; + + cbm_mutex_lock(&state->mutex); + bool cancelled = state->active_cancelled; + bool usable = configured && !state->stopping && !state->failed; + if (usable && !cancelled) { + usable = + cbm_daemon_runtime_client_application_token_reserve(replacement, &replacement_token); + } + if (usable) { + state->client = replacement; + if (!cancelled) { + state->active_request_token = replacement_token; + item->request_token = replacement_token; + } + if (cancelled_out) { + *cancelled_out = cancelled; + } + } + cbm_mutex_unlock(&state->mutex); + + if (!usable && replacement) { + (void)cbm_daemon_runtime_client_close(replacement, FRONTEND_CLOSE_TIMEOUT_MS); + } + return usable; +} + +static void frontend_response_reset(uint8_t **response, uint32_t *response_length) { + free(*response); + *response = NULL; + *response_length = 0; +} + static void *frontend_worker(void *opaque) { frontend_state_t *state = opaque; for (;;) { @@ -522,10 +636,25 @@ static void *frontend_worker(void *opaque) { if (item.cancelled) { failed = !frontend_write_cancelled_response(state->out, &item); } else { + bool request_sent = false; cbm_daemon_runtime_application_status_t status = - cbm_daemon_application_client_mcp_tagged(state->client, item.request_token, - item.message, &response, &response_length, - FRONTEND_REQUEST_TIMEOUT_MS); + cbm_daemon_application_client_mcp_tagged( + state->client, item.request_token, item.message, &response, &response_length, + &request_sent, FRONTEND_REQUEST_TIMEOUT_MS); + if (status == CBM_DAEMON_RUNTIME_APPLICATION_TRANSPORT_ERROR && !request_sent) { + frontend_response_reset(&response, &response_length); + bool cancelled = false; + if (frontend_recover_client(state, &item, &cancelled)) { + if (cancelled) { + status = CBM_DAEMON_RUNTIME_APPLICATION_CANCELLED; + } else { + bool retry_sent = false; + status = cbm_daemon_application_client_mcp_tagged( + state->client, item.request_token, item.message, &response, + &response_length, &retry_sent, FRONTEND_REQUEST_TIMEOUT_MS); + } + } + } if (status == CBM_DAEMON_RUNTIME_APPLICATION_CANCELLED) { failed = !frontend_write_cancelled_response(state->out, &item); } else { @@ -537,19 +666,18 @@ static void *frontend_worker(void *opaque) { item.content_length_framed); } } - free(response); + frontend_response_reset(&response, &response_length); bool expected_stop = failed && frontend_should_stop(state); frontend_end_request(state, failed && !expected_stop); frontend_item_free(&item); atomic_fetch_add_explicit(&state->completed_items, 1, memory_order_release); if (failed) { if (!expected_stop) { - /* A failed daemon transport cannot wake a thread blocked in - * stdio portably. This frontend owns no state: immediate - * process exit closes the kernel IPC handle, which cancels - * daemon session ownership without a detached reader or an - * unsafe cross-thread fclose. Logging or flushing here could - * itself block on agent-owned stdout/stderr. */ + /* Recovery was unsafe or the one safe retry failed. Immediate + * process exit wakes an agent blocked on stdio and closes the + * kernel IPC handle without a detached reader or cross-thread + * fclose. Logging or flushing here could itself block on + * agent-owned stdout/stderr. */ _Exit(EXIT_FAILURE); } break; @@ -632,11 +760,11 @@ static bool frontend_enqueue(frontend_state_t *state, char *message, bool conten static bool frontend_stop_begin(frontend_state_t *state) { cbm_mutex_lock(&state->mutex); state->stopping = true; + cbm_daemon_runtime_client_t *client = state->client; + bool stop_begun = !client || cbm_daemon_runtime_client_close_begin(client); cbm_mutex_unlock(&state->mutex); - /* Retain the client allocation until the worker is joined. This covers the - * boundary where the worker has claimed an item but has not yet entered the - * runtime exchange: a late call observes closing instead of freed memory. */ - return cbm_daemon_runtime_client_close_begin(state->client); + /* NULL means recovery owns cleanup. Otherwise retain the client until join. */ + return stop_begun; } static bool frontend_cancel_for_maintenance(void *opaque) { @@ -646,24 +774,33 @@ static bool frontend_cancel_for_maintenance(void *opaque) { cbm_mutex_lock(&state->mutex); state->stopping = true; if (state->in_request) { + state->active_cancelled = true; request_token = state->active_request_token; } - cbm_mutex_unlock(&state->mutex); if (request_token == CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID) { + cbm_mutex_unlock(&state->mutex); return false; } - return cbm_daemon_runtime_client_application_cancel(state->client, request_token) == - CBM_DAEMON_RUNTIME_CANCEL_ACCEPTED; + bool cancelled = state->client && + cbm_daemon_runtime_client_application_cancel(state->client, request_token) == + CBM_DAEMON_RUNTIME_CANCEL_ACCEPTED; + cbm_mutex_unlock(&state->mutex); + return cancelled; } int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, - cbm_version_cohort_manager_t *cohort_manager, FILE *in, FILE *out) { - if (!client || !cohort_manager || !in || !out) { + cbm_version_cohort_manager_t *cohort_manager, + const cbm_daemon_frontend_session_config_t *session, FILE *in, + FILE *out) { + if (!client || !cohort_manager || !session || !session->bootstrap.endpoint || + !session->bootstrap.identity || !session->session_root || !session->session_root[0] || + !in || !out) { return -1; } frontend_state_t state = { .client = client, .cohort_manager = cohort_manager, + .session = session, .out = out, }; cbm_mutex_init(&state.mutex); @@ -687,7 +824,7 @@ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, } int result = 0; - bool close_begun = false; + bool stop_begun = false; bool clean_eof = false; for (;;) { char *message = NULL; @@ -699,18 +836,14 @@ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, free(message); break; } - cbm_daemon_runtime_application_token_t cancel_token = - CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; - frontend_cancellation_route_t cancellation = - frontend_route_cancellation(&state, message, &cancel_token); + frontend_cancellation_route_t cancellation = frontend_route_cancellation(&state, message); if (cancellation != FRONTEND_CANCELLATION_NONE) { free(message); if (cancellation == FRONTEND_CANCELLATION_ACTIVE) { - cbm_daemon_runtime_cancel_result_t cancelled = - cbm_daemon_runtime_client_application_cancel(state.client, cancel_token); + cbm_daemon_runtime_cancel_result_t cancelled = frontend_cancel_active(&state); if (cancelled == CBM_DAEMON_RUNTIME_CANCEL_ERROR) { result = -1; - close_begun = frontend_stop_begin(&state); + stop_begun = frontend_stop_begin(&state); break; } } @@ -725,7 +858,7 @@ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, } } - if (clean_eof && !close_begun) { + if (clean_eof && !stop_begun) { frontend_input_closed(&state); /* EOF ends INPUT, not accepted work: as long as queued items keep * completing, every already-enqueued request still receives its @@ -751,8 +884,8 @@ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, } } } - if (!close_begun) { - close_begun = frontend_stop_begin(&state); + if (!stop_begun) { + stop_begun = frontend_stop_begin(&state); } frontend_join_watchdog_t watchdog; cbm_thread_t watchdog_thread; @@ -777,9 +910,9 @@ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, if (!cbm_daemon_maintenance_monitor_stop(&maintenance_monitor)) { _Exit(EXIT_FAILURE); } - if (close_begun) { + if (stop_begun && state.client) { (void)cbm_daemon_runtime_client_close_finish(state.client, FRONTEND_CLOSE_TIMEOUT_MS); - } else { + } else if (!stop_begun) { result = -1; } for (size_t i = 0; i < FRONTEND_QUEUE_CAPACITY; i++) { diff --git a/src/daemon/frontend.h b/src/daemon/frontend.h index 9d172606e..8ee33d276 100644 --- a/src/daemon/frontend.h +++ b/src/daemon/frontend.h @@ -4,14 +4,31 @@ #ifndef CBM_DAEMON_FRONTEND_H #define CBM_DAEMON_FRONTEND_H +#include "daemon/bootstrap.h" #include "daemon/runtime.h" #include "daemon/version_cohort.h" +#include "mcp/mcp.h" #include #include typedef struct cbm_daemon_maintenance_monitor cbm_daemon_maintenance_monitor_t; +/* Connection settings replayed when the authenticated daemon is replaced. + * Background activation + * from initialize is not restored here. + * Pointer fields are borrowed for the complete frontend + * call. */ +typedef struct { + cbm_daemon_bootstrap_config_t bootstrap; + const char *session_root; + const char *allowed_root; + cbm_mcp_tool_profile_t tool_profile; + uint8_t ui_update_mask; + bool ui_enabled; + int ui_port; +} cbm_daemon_frontend_session_config_t; + /* Called once when install/update/uninstall requests an active local command * to stop cooperatively. Returning false does not authorize the command to * outlive the bounded grace period. */ @@ -58,6 +75,14 @@ bool cbm_daemon_frontend_test_monitor_waiting(void); uint64_t cbm_daemon_frontend_test_monitor_observations(void); uint64_t cbm_daemon_frontend_test_worker_observations(void); uint64_t cbm_daemon_frontend_test_worker_idle_cycles(void); +/* Counts matching cancellations marked under the frontend mutex. + * Tests snapshot this monotonic + * count before sending a notification. */ +uint64_t cbm_daemon_frontend_test_routed_cancellations(void); +/* Budget in ms that the most recent session replay gave its context and UI + * settings requests. Recovery tests use distinct connect and startup budgets + * and compare. */ +uint32_t cbm_daemon_frontend_test_replay_timeout_ms(void); #endif /* Takes ownership of client and borrows cohort_manager for the complete call. @@ -66,9 +91,12 @@ uint64_t cbm_daemon_frontend_test_worker_idle_cycles(void); * either thread is blocked in stdio, requests cooperative cancellation for the * exact active request, and then bounds process exit. Kernel IPC close cancels * only this session's daemon work. EOF/parse failure closes the authenticated - * session. An unexpected daemon transport failure likewise terminates the + * session. A transport failure before the application frame is sent replaces + * the daemon client and retries once. Sent or repeated failures terminate the * process so an agent waiting with stdin still open observes server EOF. */ int cbm_daemon_frontend_mcp_run(cbm_daemon_runtime_client_t *client, - cbm_version_cohort_manager_t *cohort_manager, FILE *in, FILE *out); + cbm_version_cohort_manager_t *cohort_manager, + const cbm_daemon_frontend_session_config_t *session, FILE *in, + FILE *out); #endif /* CBM_DAEMON_FRONTEND_H */ diff --git a/src/daemon/runtime.c b/src/daemon/runtime.c index 404251a6c..64f6a05f4 100644 --- a/src/daemon/runtime.c +++ b/src/daemon/runtime.c @@ -3391,13 +3391,16 @@ cbm_daemon_runtime_cancel_result_t cbm_daemon_runtime_client_application_cancel( cbm_daemon_runtime_application_status_t cbm_daemon_runtime_client_application_request_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, const void *request, uint32_t request_length, uint8_t **response_out, - uint32_t *response_length_out, uint32_t timeout_ms) { + uint32_t *response_length_out, bool *request_sent_out, uint32_t timeout_ms) { if (response_out) { *response_out = NULL; } if (response_length_out) { *response_length_out = 0; } + if (request_sent_out) { + *request_sent_out = false; + } if (!client || !response_out || !response_length_out || request_token == CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID || timeout_ms == CBM_DAEMON_IPC_WAIT_FOREVER || @@ -3453,6 +3456,9 @@ cbm_daemon_runtime_application_status_t cbm_daemon_runtime_client_application_re request_can_send && cbm_daemon_ipc_send_frame(connection, CBM_DAEMON_FRAME_REQUEST, CBM_DAEMON_RUNTIME_OP_APPLICATION_REQUEST, wire, (uint32_t)wire_length); + if (request_sent_out) { + *request_sent_out = sent; + } bool send_cancel = false; cbm_mutex_lock(&client->state_mutex); client->application_request_sent = sent; @@ -3551,9 +3557,9 @@ cbm_daemon_runtime_application_status_t cbm_daemon_runtime_client_application_re if (!cbm_daemon_runtime_client_application_token_reserve(client, &request_token)) { return CBM_DAEMON_RUNTIME_APPLICATION_TRANSPORT_ERROR; } - return cbm_daemon_runtime_client_application_request_tagged(client, request_token, request, - request_length, response_out, - response_length_out, timeout_ms); + return cbm_daemon_runtime_client_application_request_tagged( + client, request_token, request, request_length, response_out, response_length_out, NULL, + timeout_ms); } bool cbm_daemon_runtime_client_close_begin(cbm_daemon_runtime_client_t *client) { diff --git a/src/daemon/runtime.h b/src/daemon/runtime.h index 4cf4411aa..76d168c73 100644 --- a/src/daemon/runtime.h +++ b/src/daemon/runtime.h @@ -423,11 +423,12 @@ cbm_daemon_runtime_application_status_t cbm_daemon_runtime_client_application_re /* Execute using the sole outstanding token returned by token_reserve(). This * is the cancellable frontend path; the legacy helper above reserves - * internally. */ + * internally. request_sent_out reports whether the application frame crossed + * the local transport, including failures while awaiting its response. */ cbm_daemon_runtime_application_status_t cbm_daemon_runtime_client_application_request_tagged( cbm_daemon_runtime_client_t *client, cbm_daemon_runtime_application_token_t request_token, const void *request, uint32_t request_length, uint8_t **response_out, - uint32_t *response_length_out, uint32_t timeout_ms); + uint32_t *response_length_out, bool *request_sent_out, uint32_t timeout_ms); /* Begin a two-phase close without freeing the client. This atomically rejects * future exchanges and interrupts an exchange already in flight. Exactly one diff --git a/src/main.c b/src/main.c index 1ad72ed5d..c3fefbf70 100644 --- a/src/main.c +++ b/src/main.c @@ -3395,9 +3395,9 @@ int main(int argc, char **argv) { cbm_daemon_bootstrap_result_t bootstrap_result; cbm_daemon_bootstrap_status_t bootstrap_status = main_client_bootstrap_with_upgrade(&bootstrap_config, &bootstrap_result, true); - cbm_daemon_ipc_endpoint_free(endpoint); if (bootstrap_status != CBM_DAEMON_BOOTSTRAP_CONNECTED || !bootstrap_result.client) { main_report_client_bootstrap_failure(role, &bootstrap_result); + cbm_daemon_ipc_endpoint_free(endpoint); (void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); return EXIT_FAILURE; } @@ -3408,33 +3408,46 @@ int main(int argc, char **argv) { * like the bootstrap before them: a 1 s bound on the reply (a probe budget) * ended the server whenever the daemon's UI save ran long, which the smoke's * `--ui=true --port` run saw as a binary without UI assets. */ - if (role == CBM_DAEMON_PROCESS_MCP_CLIENT && - !main_set_client_context(g_daemon_client, NULL, tool_profile, NULL, NULL, - bootstrap_config.startup_timeout_ms)) { - (void)fprintf(stderr, "codebase-memory-mcp: daemon session context was rejected\n"); - (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS); - g_daemon_client = NULL; - (void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); - return EXIT_FAILURE; + char mcp_session_root[MAIN_PATH_CAP] = {0}; + char mcp_allowed_root[MAIN_PATH_CAP] = {0}; + const char *mcp_allowed_root_ptr = NULL; + if (role == CBM_DAEMON_PROCESS_MCP_CLIENT) { + bool context_resolved = + main_session_context(NULL, mcp_session_root, mcp_allowed_root, &mcp_allowed_root_ptr); + cbm_daemon_runtime_application_status_t context_status = + context_resolved ? cbm_daemon_application_client_set_context( + g_daemon_client, mcp_session_root, mcp_allowed_root_ptr, + tool_profile, NULL, NULL, bootstrap_config.startup_timeout_ms) + : CBM_DAEMON_RUNTIME_APPLICATION_REJECTED; + if (context_status != CBM_DAEMON_RUNTIME_APPLICATION_OK) { + (void)fprintf(stderr, "codebase-memory-mcp: daemon session context was rejected\n"); + (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS); + g_daemon_client = NULL; + cbm_daemon_ipc_endpoint_free(endpoint); + (void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); + return EXIT_FAILURE; + } } /* Persist UI mutations only after the exact-build HELLO succeeds. A * conflicting binary must be observationally read-only: applying its * flags before bootstrap could reconfigure the already-running daemon * even though that client was then rejected. */ + bool mcp_ui_enabled = false; + int mcp_ui_port = 0; + uint8_t mcp_ui_update_mask = 0; if (role == CBM_DAEMON_PROCESS_MCP_CLIENT && cbm_mcp_tool_profile_allows_http(tool_profile)) { - bool ui_enabled = false; - int ui_port = 0; bool explicitly_enabled = false; - uint8_t update_mask = - parse_ui_flags(argc, argv, &ui_enabled, &ui_port, &explicitly_enabled); - if (update_mask != 0 && + mcp_ui_update_mask = + parse_ui_flags(argc, argv, &mcp_ui_enabled, &mcp_ui_port, &explicitly_enabled); + if (mcp_ui_update_mask != 0 && cbm_daemon_application_client_set_ui_config( - g_daemon_client, update_mask, ui_enabled, ui_port, + g_daemon_client, mcp_ui_update_mask, mcp_ui_enabled, mcp_ui_port, bootstrap_config.startup_timeout_ms) != CBM_DAEMON_RUNTIME_APPLICATION_OK) { (void)fprintf(stderr, "codebase-memory-mcp: daemon UI configuration update failed\n"); (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS); g_daemon_client = NULL; + cbm_daemon_ipc_endpoint_free(endpoint); (void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); return EXIT_FAILURE; } @@ -3451,13 +3464,25 @@ int main(int argc, char **argv) { (void)fprintf(stderr, "codebase-memory-mcp: parent-death watchdog could not start\n"); (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS); g_daemon_client = NULL; + cbm_daemon_ipc_endpoint_free(endpoint); (void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); return EXIT_FAILURE; } setup_signal_handlers(); - int result = cbm_daemon_frontend_mcp_run(g_daemon_client, client_cohort_manager, stdin, stdout); + cbm_daemon_frontend_session_config_t frontend_session = { + .bootstrap = bootstrap_config, + .session_root = mcp_session_root, + .allowed_root = mcp_allowed_root_ptr, + .tool_profile = tool_profile, + .ui_update_mask = mcp_ui_update_mask, + .ui_enabled = mcp_ui_enabled, + .ui_port = mcp_ui_port, + }; + int result = cbm_daemon_frontend_mcp_run(g_daemon_client, client_cohort_manager, + &frontend_session, stdin, stdout); g_daemon_client = NULL; /* frontend consumed the handle */ + cbm_daemon_ipc_endpoint_free(endpoint); bool client_cohort_cleanup = main_version_cohort_close(&client_cohort_lease, &client_cohort_manager); atomic_store(&g_shutdown, 1); diff --git a/tests/test_cli.c b/tests/test_cli.c index 3284ef72a..15255ff84 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -9450,6 +9450,143 @@ TEST(cli_hermes_stable_shell_context_contract) { PASS(); } +/* A fake HOME holding only ~/.hermes/config.yaml, so install and uninstall + * reach exactly the Hermes editors. */ +typedef struct { + char tmpdir[256]; + char config_path[640]; + char binary_path[640]; + char *saved_home; + char *saved_path; + char *saved_hermes; +} cli_hermes_cycle_t; + +static bool cli_hermes_cycle_open(cli_hermes_cycle_t *cycle, const char *initial_config) { + memset(cycle, 0, sizeof(*cycle)); + snprintf(cycle->tmpdir, sizeof(cycle->tmpdir), "/tmp/cli-hermes-cycle-XXXXXX"); + if (!cbm_mkdtemp(cycle->tmpdir)) { + cycle->tmpdir[0] = '\0'; + return false; + } + cycle->saved_home = save_test_env("HOME"); + cycle->saved_path = save_test_env("PATH"); + cycle->saved_hermes = save_test_env("HERMES_HOME"); + cbm_setenv("HOME", cycle->tmpdir, 1); + cbm_setenv("PATH", cycle->tmpdir, 1); + cbm_unsetenv("HERMES_HOME"); + + char hermes_dir[512]; + snprintf(hermes_dir, sizeof(hermes_dir), "%s/.hermes", cycle->tmpdir); + snprintf(cycle->config_path, sizeof(cycle->config_path), "%s/config.yaml", hermes_dir); +#ifdef _WIN32 + snprintf(cycle->binary_path, sizeof(cycle->binary_path), + "%s/.local/bin/codebase-memory-mcp.exe", cycle->tmpdir); +#else + snprintf(cycle->binary_path, sizeof(cycle->binary_path), "%s/.local/bin/codebase-memory-mcp", + cycle->tmpdir); +#endif + test_mkdirp(hermes_dir); + return write_test_file(cycle->config_path, initial_config) == 0; +} + +static void cli_hermes_cycle_close(cli_hermes_cycle_t *cycle) { + if (!cycle->tmpdir[0]) { + return; + } + restore_test_env("HOME", cycle->saved_home); + restore_test_env("PATH", cycle->saved_path); + restore_test_env("HERMES_HOME", cycle->saved_hermes); + test_rmdir_r(cycle->tmpdir); + cycle->tmpdir[0] = '\0'; +} + +/* Found while finishing #1862: when our hook was the only entry under + * `hooks:`, uninstall empties that mapping and the mapping editor writes it as + * `hooks: {}` (the same convention as `mcp_servers: {}`). The hook sequence + * editor then refused the file, so install -> uninstall -> install exited 1, + * and so did a repeated uninstall. The explicit empty mapping must be accepted: + * a repeated uninstall changes nothing and the reinstall restores the first + * install byte for byte, with exactly one owned hook. */ +TEST(cli_hermes_hook_reinstalls_after_uninstall_empties_hooks) { + cli_hermes_cycle_t cycle; + if (!cli_hermes_cycle_open(&cycle, "theme: solarized\n")) { + cli_hermes_cycle_close(&cycle); + FAIL("Hermes fake-HOME setup failed"); + } + char *argv[] = {"uninstall", "--yes"}; + int install_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false); + char *installed = read_test_file_alloc(cycle.config_path); + int uninstall_rc = cli_test_cmd_uninstall(2, argv); + char *emptied = read_test_file_alloc(cycle.config_path); + int repeat_rc = cli_test_cmd_uninstall(2, argv); + char *repeated = read_test_file_alloc(cycle.config_path); + int reinstall_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false); + char *reinstalled = read_test_file_alloc(cycle.config_path); + cli_hermes_cycle_close(&cycle); + + bool installed_once = install_rc == 0 && installed && + test_count_substring(installed, "pre_llm_call:") == 1U && + test_count_substring(installed, "hook-augment") == 1U; + bool emptied_clean = uninstall_rc == 0 && emptied && strstr(emptied, "theme: solarized\n") && + test_count_substring(emptied, "hooks: {}\n") == 1U && + !strstr(emptied, "pre_llm_call") && !strstr(emptied, "hook-augment"); + bool repeat_unchanged = repeat_rc == 0 && emptied && repeated && strcmp(repeated, emptied) == 0; + bool reinstalled_once = + reinstall_rc == 0 && installed && reinstalled && strcmp(reinstalled, installed) == 0; + free(installed); + free(emptied); + free(repeated); + free(reinstalled); + if (!installed_once) + FAIL("Hermes install must write exactly one owned pre_llm_call hook"); + if (!emptied_clean) + FAIL("uninstall must remove the owned hook and leave only the empty hooks: {} mapping"); + if (!repeat_unchanged) + FAIL("a repeated uninstall must accept hooks: {} and leave the file unchanged"); + if (!reinstalled_once) + FAIL("reinstall must accept hooks: {} and restore exactly one owned hook"); + PASS(); +} + +/* The same cycle with a user's own hook in the shared pre_llm_call sequence: + * the foreign hook survives untouched and uninstall restores the original + * file byte for byte. */ +TEST(cli_hermes_hook_cycle_keeps_foreign_pre_llm_hook) { + static const char original[] = "theme: solarized\n" + "mcp_servers: {}\n" + "hooks:\n" + " pre_llm_call:\n" + " - command: \"/usr/bin/user-hermes-hook\"\n"; + cli_hermes_cycle_t cycle; + if (!cli_hermes_cycle_open(&cycle, original)) { + cli_hermes_cycle_close(&cycle); + FAIL("Hermes fake-HOME setup failed"); + } + char *argv[] = {"uninstall", "--yes"}; + int install_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false); + char *installed = read_test_file_alloc(cycle.config_path); + int uninstall_rc = cli_test_cmd_uninstall(2, argv); + char *restored = read_test_file_alloc(cycle.config_path); + int reinstall_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false); + char *reinstalled = read_test_file_alloc(cycle.config_path); + cli_hermes_cycle_close(&cycle); + + bool merged = install_rc == 0 && installed && + test_count_substring(installed, "/usr/bin/user-hermes-hook") == 1U && + test_count_substring(installed, "pre_llm_call:") == 1U && + test_count_substring(installed, "hook-augment") == 1U; + bool restored_exactly = uninstall_rc == 0 && restored && strcmp(restored, original) == 0; + bool reinstalled_once = + reinstall_rc == 0 && installed && reinstalled && strcmp(reinstalled, installed) == 0; + free(installed); + free(restored); + free(reinstalled); + if (!merged || !restored_exactly || !reinstalled_once) + FAIL("a user's own pre_llm_call hook must survive install, uninstall and reinstall, and " + "uninstall must restore the original file byte for byte"); + PASS(); +} + #ifndef _WIN32 TEST(cli_detected_agent_summary_includes_registry_clients) { char tmpdir[256]; @@ -20553,6 +20690,8 @@ SUITE(cli) { RUN_TEST(cli_junie_current_durable_context_contract); RUN_TEST(cli_rovo_installs_documented_global_memory); RUN_TEST(cli_hermes_stable_shell_context_contract); + RUN_TEST(cli_hermes_hook_reinstalls_after_uninstall_empties_hooks); + RUN_TEST(cli_hermes_hook_cycle_keeps_foreign_pre_llm_hook); #ifndef _WIN32 RUN_TEST(cli_detected_agent_summary_includes_registry_clients); #endif diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 9efaa2e38..95bc89404 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -1998,6 +1998,129 @@ TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item) { PASS(); } +/* Found while finishing #1862: cbm_yaml_remove_mapping_entry writes a section + * it empties as `section: {}`, and the Hermes remover empties `hooks:` whenever + * our hook was its only entry. The sequence editor refused that explicit empty + * parent mapping, so the next install (and a repeated removal) failed. The + * primitives' own round trip must land back on the first insert. */ +TEST(config_yaml_edit_nested_sequence_reinserts_under_emptied_parent) { + const char *initial = "theme: x\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id", + yaml_hook_identity, yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + char *first = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(first); + ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id", + yaml_hook_identity, yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + ASSERT_EQ(cbm_yaml_remove_mapping_entry(fixture.path, "hooks", "pre_llm_call"), 0); + char *emptied = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(emptied); + ASSERT_STR_EQ(emptied, "theme: x\nhooks: {}\n"); + free(emptied); + ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id", + yaml_hook_identity, yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id", + yaml_hook_identity, yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + char *again = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(again); + ASSERT_STR_EQ(again, first); + free(again); + free(first); + th_cleanup(fixture.dir); + PASS(); +} + +/* An exact empty flow mapping on a PARENT of the sequence path is an empty + * mapping, the same `section: {}` the mapping-entry editor writes and accepts. + * Removal is a byte-identical no-op; upsert expands the header in place, + * keeping its comment, the line endings and every other byte. */ +TEST(config_yaml_edit_nested_sequence_expands_explicit_empty_parent_mapping) { + static const char item_lf[] = " pre_llm_call:\n" + " - id: \"codebase-memory-mcp\"\n" + " type: \"command\"\n" + " command: \"/opt/Codebase Memory/bin/cbm\"\n"; + static const char item_crlf[] = " pre_llm_call:\r\n" + " - id: \"codebase-memory-mcp\"\r\n" + " type: \"command\"\r\n" + " command: \"/opt/Codebase Memory/bin/cbm\"\r\n"; + struct { + const char *initial; + const char *header; + const char *item; + const char *tail; + } cases[] = { + {"theme: x\nhooks: {} # keep me\nafter: 1\n", "theme: x\nhooks: # keep me\n", item_lf, + "after: 1\n"}, + {"theme: x\r\nhooks: {}\r\nafter: 1\r\n", "theme: x\r\nhooks:\r\n", item_crlf, + "after: 1\r\n"}, + {"theme: x\nhooks: {}", "theme: x\nhooks:\n", item_lf, ""}, + {"hooks: {}\n# trailing note\n", "hooks:\n# trailing note\n", item_lf, ""}, + }; + for (size_t i = 0U; i < sizeof(cases) / sizeof(cases[0]); i++) { + char expected[1024]; + ASSERT(snprintf(expected, sizeof(expected), "%s%s%s", cases[i].header, cases[i].item, + cases[i].tail) > 0); + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, cases[i].initial), 0); + ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, + "id", yaml_hook_identity, + yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + char *unchanged = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(unchanged); + ASSERT_STR_EQ(unchanged, cases[i].initial); + free(unchanged); + for (int pass = 0; pass < 2; pass++) { + ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, + 2U, "id", yaml_hook_identity, + yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + char *expanded = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(expanded); + ASSERT_STR_EQ(expanded, expected); + free(expanded); + } + th_cleanup(fixture.dir); + } + PASS(); +} + +/* Only an exact `{}` on a parent with nothing nested beneath it is accepted. + * The sequence key itself holding a mapping, a non-empty flow mapping, a flow + * sequence where a mapping belongs, and content indented under an inline `{}` + * stay refused byte-identically by both upsert and removal. */ +TEST(config_yaml_edit_nested_sequence_still_refuses_non_empty_parent_flow) { + const char *cases[] = { + "hooks:\n pre_llm_call: {}\n", + "hooks: {pre_llm_call: []}\n", + "hooks: []\n", + "hooks: {}\n pre_llm_call:\n - id: \"codebase-memory-mcp\"\n", + }; + for (size_t i = 0U; i < sizeof(cases) / sizeof(cases[0]); i++) { + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, cases[i]), 0); + ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, + "id", yaml_hook_identity, + yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_ERROR); + ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, + "id", yaml_hook_identity, + yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_ERROR); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_STR_EQ(after, cases[i]); + free(after); + th_cleanup(fixture.dir); + } + PASS(); +} + /* Uninstall must be idempotent against a config that was never written. Hermes * is detected by its binary on PATH, so a HOME without `.hermes/` reached the * YAML removers with an absent target — and they failed acquiring the lock @@ -2245,6 +2368,9 @@ SUITE(config_yaml_edit) { RUN_TEST(config_yaml_edit_nested_sequence_preserves_crlf); RUN_TEST(config_yaml_edit_nested_sequence_foreign_identity_is_preserved); RUN_TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item); + RUN_TEST(config_yaml_edit_nested_sequence_reinserts_under_emptied_parent); + RUN_TEST(config_yaml_edit_nested_sequence_expands_explicit_empty_parent_mapping); + RUN_TEST(config_yaml_edit_nested_sequence_still_refuses_non_empty_parent_flow); RUN_TEST(config_yaml_edit_nested_sequence_ignores_foreign_flow_and_block_scalars_issue2209); RUN_TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically); RUN_TEST(config_yaml_edit_remove_on_absent_config_is_a_no_op); diff --git a/tests/test_daemon_frontend.c b/tests/test_daemon_frontend.c index b91705f15..aa49bf4c7 100644 --- a/tests/test_daemon_frontend.c +++ b/tests/test_daemon_frontend.c @@ -4,6 +4,7 @@ #include "test_framework.h" #include "test_helpers.h" +#include "daemon/application.h" #include "daemon/frontend.h" #include "daemon/ipc.h" #include "daemon/service.h" @@ -38,6 +39,8 @@ typedef struct { typedef struct { frontend_maintenance_fixture_t maintenance; char conflict_log[FRONTEND_TEST_PATH_CAP]; + char build_fingerprint[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; + cbm_daemon_build_identity_t identity; cbm_daemon_runtime_service_t *service; cbm_daemon_runtime_client_t *client; } frontend_idle_fixture_t; @@ -45,6 +48,7 @@ typedef struct { typedef struct { cbm_daemon_runtime_client_t *client; cbm_version_cohort_manager_t *manager; + const cbm_daemon_frontend_session_config_t *session; FILE *input; FILE *output; int result; @@ -98,22 +102,22 @@ static cbm_daemon_runtime_application_status_t frontend_idle_request( return CBM_DAEMON_RUNTIME_APPLICATION_OK; } -static void frontend_idle_request_cancel( - void *context, cbm_daemon_runtime_application_session_t *session, - cbm_daemon_runtime_application_token_t request_token) { +static void frontend_idle_request_cancel(void *context, + cbm_daemon_runtime_application_session_t *session, + cbm_daemon_runtime_application_token_t request_token) { (void)context; (void)session; (void)request_token; } -static void frontend_idle_session_cancel( - void *context, cbm_daemon_runtime_application_session_t *session) { +static void frontend_idle_session_cancel(void *context, + cbm_daemon_runtime_application_session_t *session) { (void)context; (void)session; } -static void frontend_idle_session_close( - void *context, cbm_daemon_runtime_application_session_t *session) { +static void frontend_idle_session_close(void *context, + cbm_daemon_runtime_application_session_t *session) { (void)context; (void)session; } @@ -127,22 +131,21 @@ static uint64_t frontend_test_process_id(void) { } static bool frontend_idle_fixture_start(frontend_idle_fixture_t *fixture) { - static const char cache[] = - "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"; + static const char cache[] = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"; memset(fixture, 0, sizeof(*fixture)); if (!frontend_maintenance_fixture_start(&fixture->maintenance, "idle-observer")) { return false; } - char build[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; int log_written = snprintf(fixture->conflict_log, sizeof(fixture->conflict_log), "%s/conflicts.ndjson", fixture->maintenance.parent); if (log_written <= 0 || log_written >= (int)sizeof(fixture->conflict_log) || - !cbm_daemon_runtime_process_build_fingerprint(frontend_test_process_id(), build)) { + !cbm_daemon_runtime_process_build_fingerprint(frontend_test_process_id(), + fixture->build_fingerprint)) { return false; } - cbm_daemon_build_identity_t identity = { + fixture->identity = (cbm_daemon_build_identity_t){ .semantic_version = "2.4.0", - .build_fingerprint = build, + .build_fingerprint = fixture->build_fingerprint, .cache_fingerprint = cache, .protocol_abi = 3, .store_abi = 11, @@ -158,7 +161,7 @@ static bool frontend_idle_fixture_start(frontend_idle_fixture_t *fixture) { }; cbm_daemon_runtime_service_config_t config = { .endpoint = fixture->maintenance.endpoint, - .identity = identity, + .identity = fixture->identity, .conflict_log_path = fixture->conflict_log, .conflict_log_cap_bytes = 64U * 1024U, .max_clients = 2, @@ -169,10 +172,11 @@ static bool frontend_idle_fixture_start(frontend_idle_fixture_t *fixture) { }; fixture->service = cbm_daemon_runtime_service_start(&config); cbm_daemon_runtime_connect_result_t connect_result = {0}; - fixture->client = fixture->service - ? cbm_daemon_runtime_client_connect(fixture->maintenance.endpoint, &identity, - 30000, &connect_result) - : NULL; + fixture->client = + fixture->service + ? cbm_daemon_runtime_client_connect(fixture->maintenance.endpoint, &fixture->identity, + 30000, &connect_result) + : NULL; return fixture->client && connect_result.status == CBM_DAEMON_RUNTIME_CONNECT_ACCEPTED; } @@ -212,8 +216,8 @@ static int frontend_test_close_fd(int fd) { static void *frontend_idle_run(void *opaque) { frontend_idle_run_t *run = opaque; - run->result = - cbm_daemon_frontend_mcp_run(run->client, run->manager, run->input, run->output); + run->result = cbm_daemon_frontend_mcp_run(run->client, run->manager, run->session, run->input, + run->output); return NULL; } #endif @@ -230,6 +234,9 @@ enum { * dev machine (see the index-supervisor calibration). */ FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS = 30000, FRONTEND_EOF_TEST_CATASTROPHIC_TIMEOUT_S = 90, + /* Recovery can spend most of the generic guard restarting an instrumented + * daemon before the clean-EOF case enters its deliberate 15s drain. */ + FRONTEND_RECOVERY_TEST_CATASTROPHIC_TIMEOUT_S = 120, FRONTEND_BACKPRESSURE_MESSAGE_BYTES = 2 * 1024 * 1024, FRONTEND_BACKPRESSURE_FRONTEND_TIMEOUT_S = 90, FRONTEND_BACKPRESSURE_DAEMON_TIMEOUT_S = 90, @@ -239,8 +246,23 @@ enum { * regression black-box: it must remain valid if the exact capacity changes * while still proving that overload cannot hide an already-pending EOF. */ FRONTEND_EOF_TEST_OVERFLOW_MESSAGES = 32, + /* Hold replacement context past the production frontend's 15 second + * no-progress EOF + drain so shutdown deterministically overlaps recovery. */ + FRONTEND_RECOVERY_EOF_RELEASE_MS = 16000, + /* Startup budget of the recovery bootstrap. It differs from the connect + * budget (FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS) so the recovery tests can + * tell which one the session replay waited on, and it is the larger one, + * as in production (1 s connect probe, 30 s startup). */ + FRONTEND_RECOVERY_TEST_STARTUP_TIMEOUT_MS = 45000, }; +typedef enum { + FRONTEND_RECOVERY_RETRY = 0, + FRONTEND_RECOVERY_CANCEL = 1, + FRONTEND_RECOVERY_CLEAN_EOF = 2, +} frontend_recovery_mode_t; + typedef struct { atomic_int requests; atomic_int session_cancels; @@ -268,6 +290,7 @@ typedef struct { typedef struct { int fd; bool overflow; + frontend_recovery_mode_t recovery_mode; frontend_eof_application_context_t *application; atomic_bool finished; atomic_bool succeeded; @@ -275,7 +298,9 @@ typedef struct { typedef struct { char conflict_log[FRONTEND_TEST_PATH_CAP]; + char build_fingerprint[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; cbm_daemon_ipc_endpoint_t *endpoint; + cbm_daemon_build_identity_t identity; cbm_version_cohort_manager_t *manager; cbm_daemon_runtime_service_t *service; cbm_daemon_runtime_client_t *client; @@ -337,16 +362,22 @@ static cbm_daemon_runtime_application_status_t frontend_eof_application_request( } /* Released: fall through and answer normally like every later item. */ } - if (request_length == 0) { + if (request_length == 0 || request[0] == CBM_DAEMON_APPLICATION_REQUEST_SET_CONTEXT) { return CBM_DAEMON_RUNTIME_APPLICATION_OK; } - uint8_t *response = malloc(request_length); + const uint8_t *response_source = request; + uint32_t response_length = request_length; + if (request[0] == CBM_DAEMON_APPLICATION_REQUEST_MCP) { + response_source++; + response_length--; + } + uint8_t *response = malloc(response_length); if (!response) { return CBM_DAEMON_RUNTIME_APPLICATION_HANDLER_ERROR; } - memcpy(response, request, request_length); + memcpy(response, response_source, response_length); *response_out = response; - *response_length_out = request_length; + *response_length_out = response_length; if (context->request_observed_fd >= 0) { const char marker = 'Q'; (void)write(context->request_observed_fd, &marker, 1); @@ -454,6 +485,96 @@ static void *frontend_eof_writer(void *opaque) { return NULL; } +static const char frontend_recovery_request[] = + "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/list\",\"params\":{}}\n"; +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS +static const char frontend_recovery_cancel[] = + "{\"jsonrpc\":\"2.0\",\"method\":\"notifications/cancelled\"," + "\"params\":{\"requestId\":1}}\n"; +static const char frontend_recovery_followup[] = + "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/list\",\"params\":{}}\n"; +#endif + +static void *frontend_recovery_writer(void *opaque) { + frontend_eof_writer_t *writer = opaque; + bool ok = frontend_eof_write_all(writer->fd, frontend_recovery_request, + sizeof(frontend_recovery_request) - 1U); + uint64_t deadline = cbm_now_ms() + FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS; + if (writer->recovery_mode != FRONTEND_RECOVERY_RETRY) { + while (ok && + !atomic_load_explicit(&writer->application->first_request_started, + memory_order_acquire) && + cbm_now_ms() < deadline) { + cbm_usleep(1000); + } + ok = ok && atomic_load_explicit(&writer->application->first_request_started, + memory_order_acquire); + } +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + if (writer->recovery_mode == FRONTEND_RECOVERY_CANCEL) { + uint64_t cancellations = cbm_daemon_frontend_test_routed_cancellations(); + ok = ok && frontend_eof_write_all(writer->fd, frontend_recovery_cancel, + sizeof(frontend_recovery_cancel) - 1U); + /* Keep context blocked until the reader routes the cancellation. */ + while (ok && cbm_daemon_frontend_test_routed_cancellations() == cancellations && + cbm_now_ms() < deadline) { + cbm_usleep(1000); + } + ok = ok && cbm_daemon_frontend_test_routed_cancellations() > cancellations; + atomic_store_explicit(&writer->application->release_first_request, true, + memory_order_release); + ok = ok && frontend_eof_write_all(writer->fd, frontend_recovery_followup, + sizeof(frontend_recovery_followup) - 1U); + } +#endif + if (writer->recovery_mode == FRONTEND_RECOVERY_CLEAN_EOF) { + ok = close(writer->fd) == 0 && ok; + writer->fd = -1; + cbm_usleep(FRONTEND_RECOVERY_EOF_RELEASE_MS * 1000U); + atomic_store_explicit(&writer->application->release_first_request, true, + memory_order_release); + } + int expected_requests = writer->recovery_mode == FRONTEND_RECOVERY_CLEAN_EOF ? 1 : 2; + while (ok && + atomic_load_explicit(&writer->application->requests, memory_order_acquire) < + expected_requests && + cbm_now_ms() < deadline) { + cbm_usleep(1000); + } + ok = ok && atomic_load_explicit(&writer->application->requests, memory_order_acquire) == + expected_requests; + if (writer->fd >= 0) { + ok = close(writer->fd) == 0 && ok; + writer->fd = -1; + } + atomic_store_explicit(&writer->succeeded, ok, memory_order_release); + atomic_store_explicit(&writer->finished, true, memory_order_release); + return NULL; +} + +static cbm_daemon_runtime_service_t *frontend_eof_service_start(frontend_eof_fixture_t *fixture) { + cbm_daemon_runtime_application_callbacks_t callbacks = { + .context = &fixture->application, + .session_open = frontend_eof_application_session_open, + .request = frontend_eof_application_request, + .request_cancel = frontend_eof_application_request_cancel, + .session_cancel = frontend_eof_application_session_cancel, + .session_close = frontend_eof_application_session_close, + }; + cbm_daemon_runtime_service_config_t config = { + .endpoint = fixture->endpoint, + .identity = fixture->identity, + .conflict_log_path = fixture->conflict_log, + .conflict_log_cap_bytes = 64U * 1024U, + .max_clients = 4, + .lease_timeout_ms = 5000, + .request_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .shutdown_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .application = callbacks, + }; + return cbm_daemon_runtime_service_start(&config); +} + static bool frontend_eof_fixture_start(frontend_eof_fixture_t *fixture, const char *parent) { memset(fixture, 0, sizeof(*fixture)); atomic_init(&fixture->application.requests, 0); @@ -467,53 +588,42 @@ static bool frontend_eof_fixture_start(frontend_eof_fixture_t *fixture, const ch fixture->application.request_observed_fd = -1; fixture->application.session_cancel_fd = -1; char key[CBM_DAEMON_KEY_SIZE]; - char build[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; int log_written = snprintf(fixture->conflict_log, sizeof(fixture->conflict_log), "%s/conflicts.ndjson", parent); if (log_written <= 0 || log_written >= (int)sizeof(fixture->conflict_log) || !cbm_daemon_rendezvous_key(key) || - !cbm_daemon_runtime_process_build_fingerprint((uint64_t)getpid(), build)) { + !cbm_daemon_runtime_process_build_fingerprint((uint64_t)getpid(), + fixture->build_fingerprint)) { return false; } fixture->endpoint = cbm_daemon_ipc_endpoint_new(key, parent); fixture->manager = fixture->endpoint ? cbm_version_cohort_manager_new(fixture->endpoint) : NULL; - cbm_daemon_build_identity_t identity = { + fixture->identity = (cbm_daemon_build_identity_t){ .semantic_version = "2.4.0", - .build_fingerprint = build, + .build_fingerprint = fixture->build_fingerprint, .cache_fingerprint = FRONTEND_TEST_CACHE, .protocol_abi = 3, .store_abi = 11, .feature_abi = 7, }; - cbm_daemon_runtime_application_callbacks_t callbacks = { - .context = &fixture->application, - .session_open = frontend_eof_application_session_open, - .request = frontend_eof_application_request, - .request_cancel = frontend_eof_application_request_cancel, - .session_cancel = frontend_eof_application_session_cancel, - .session_close = frontend_eof_application_session_close, - }; - cbm_daemon_runtime_service_config_t config = { - .endpoint = fixture->endpoint, - .identity = identity, - .conflict_log_path = fixture->conflict_log, - .conflict_log_cap_bytes = 64U * 1024U, - .max_clients = 4, - .lease_timeout_ms = 5000, - .request_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, - .shutdown_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, - .application = callbacks, - }; - fixture->service = fixture->manager ? cbm_daemon_runtime_service_start(&config) : NULL; + fixture->service = fixture->manager ? frontend_eof_service_start(fixture) : NULL; cbm_daemon_runtime_connect_result_t connect_result = {0}; fixture->client = fixture->service - ? cbm_daemon_runtime_client_connect(fixture->endpoint, &identity, + ? cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity, FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, &connect_result) : NULL; return fixture->client && connect_result.status == CBM_DAEMON_RUNTIME_CONNECT_ACCEPTED; } +static bool frontend_eof_fixture_restart_service(frontend_eof_fixture_t *fixture) { + bool stopped = fixture->service && cbm_daemon_runtime_service_stop( + fixture->service, FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS); + bool freed = stopped && cbm_daemon_runtime_service_free(fixture->service); + fixture->service = freed ? frontend_eof_service_start(fixture) : NULL; + return stopped && freed && fixture->service; +} + static bool frontend_eof_fixture_finish(frontend_eof_fixture_t *fixture) { bool ok = true; if (fixture->client) { @@ -579,7 +689,21 @@ static int frontend_eof_child_run(const char *parent, bool overflow) { cbm_daemon_runtime_client_t *frontend_client = fixture.client; fixture.client = NULL; /* cbm_daemon_frontend_mcp_run consumes it. */ - int result = cbm_daemon_frontend_mcp_run(frontend_client, fixture.manager, input, output); + cbm_daemon_frontend_session_config_t session = { + .bootstrap = + { + .role = CBM_DAEMON_PROCESS_MCP_CLIENT, + .endpoint = fixture.endpoint, + .identity = &fixture.identity, + .executable_path = "unused", + .connect_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .startup_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + }, + .session_root = parent, + .tool_profile = CBM_MCP_TOOL_PROFILE_ALL, + }; + int result = + cbm_daemon_frontend_mcp_run(frontend_client, fixture.manager, &session, input, output); bool joined = cbm_thread_join(&writer_thread) == 0; bool writer_ok = atomic_load_explicit(&writer.finished, memory_order_acquire) && atomic_load_explicit(&writer.succeeded, memory_order_acquire); @@ -660,6 +784,119 @@ static bool frontend_eof_run_isolated(const char *tag, bool overflow) { return child_ok && cleaned; } +static int frontend_recovery_child_run(const char *parent, frontend_recovery_mode_t recovery_mode) { + (void)alarm(FRONTEND_RECOVERY_TEST_CATASTROPHIC_TIMEOUT_S); + frontend_eof_fixture_t fixture; + if (!frontend_eof_fixture_start(&fixture, parent)) { + (void)frontend_eof_fixture_finish(&fixture); + return 73; + } + atomic_store_explicit(&fixture.application.block_first_request, + recovery_mode != FRONTEND_RECOVERY_RETRY, memory_order_release); + cbm_daemon_runtime_client_t *stale_client = fixture.client; + fixture.client = NULL; + bool replacement_ready = frontend_eof_fixture_restart_service(&fixture); + int input_pipe[2] = {-1, -1}; + bool pipe_ready = replacement_ready && pipe(input_pipe) == 0; + FILE *input = pipe_ready ? fdopen(input_pipe[0], "rb") : NULL; + FILE *output = input ? tmpfile() : NULL; + frontend_eof_writer_t writer = { + .fd = input_pipe[1], + .recovery_mode = recovery_mode, + .application = &fixture.application, + }; + atomic_init(&writer.finished, false); + atomic_init(&writer.succeeded, false); + cbm_thread_t writer_thread; + bool writer_started = + output && cbm_thread_create(&writer_thread, 0, frontend_recovery_writer, &writer) == 0; + if (!writer_started && input_pipe[1] >= 0) { + (void)close(input_pipe[1]); + input_pipe[1] = -1; + } + bool request_ready = writer_started; + cbm_daemon_frontend_session_config_t session = { + .bootstrap = + { + .role = CBM_DAEMON_PROCESS_MCP_CLIENT, + .endpoint = fixture.endpoint, + .identity = &fixture.identity, + .executable_path = "unused", + .connect_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .startup_timeout_ms = FRONTEND_RECOVERY_TEST_STARTUP_TIMEOUT_MS, + }, + .session_root = parent, + .tool_profile = CBM_MCP_TOOL_PROFILE_ALL, + }; + int result = request_ready ? cbm_daemon_frontend_mcp_run(stale_client, fixture.manager, + &session, input, output) + : -1; + stale_client = NULL; /* the frontend consumes the stale or replacement handle */ + bool writer_joined = writer_started && cbm_thread_join(&writer_thread) == 0; + bool writer_ok = writer_joined && + atomic_load_explicit(&writer.finished, memory_order_acquire) && + atomic_load_explicit(&writer.succeeded, memory_order_acquire); + char response[512]; + memset(response, 0, sizeof(response)); + bool response_ready = result == 0 && fflush(output) == 0 && fseek(output, 0, SEEK_SET) == 0; + size_t response_length = response_ready ? fread(response, 1, sizeof(response) - 1U, output) : 0; + bool response_exact = false; + if (recovery_mode == FRONTEND_RECOVERY_CANCEL) { + response_exact = strstr(response, "\"id\":1") && strstr(response, "\"code\":-32800") && + strstr(response, frontend_recovery_followup); + } else if (recovery_mode == FRONTEND_RECOVERY_CLEAN_EOF) { + response_exact = response_length == 0; + } else { + response_exact = response_length == sizeof(frontend_recovery_request) - 1U && + memcmp(response, frontend_recovery_request, + sizeof(frontend_recovery_request) - 1U) == 0; + } + int requests = atomic_load_explicit(&fixture.application.requests, memory_order_acquire); + int expected_requests = recovery_mode == FRONTEND_RECOVERY_CLEAN_EOF ? 1 : 2; + bool input_closed = input && fclose(input) == 0; + bool output_closed = output && fclose(output) == 0; + bool fixture_closed = frontend_eof_fixture_finish(&fixture); +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + /* The replay restores what startup set within the startup budget. On the + * connect budget (a probe bound) a replacement daemon that answers slower + * than a probe would end an otherwise good recovery. Every mode replays. */ + if (cbm_daemon_frontend_test_replay_timeout_ms() != FRONTEND_RECOVERY_TEST_STARTUP_TIMEOUT_MS) { + return 75; + } +#endif + return replacement_ready && request_ready && result == 0 && response_exact && + requests == expected_requests && writer_ok && input_closed && output_closed && + fixture_closed + ? 0 + : 74; +} + +static bool frontend_recovery_run_isolated(frontend_recovery_mode_t recovery_mode) { + char parent[FRONTEND_TEST_PATH_CAP]; + int written = snprintf(parent, sizeof(parent), "%s/cbm-frontend-recovery-XXXXXX", cbm_tmpdir()); + if (written <= 0 || written >= (int)sizeof(parent) || !cbm_mkdtemp(parent)) { + return false; + } + pid_t child = fork(); + if (child == 0) { + (void)signal(SIGALRM, SIG_DFL); + _exit(frontend_recovery_child_run(parent, recovery_mode)); + } + int status = 0; + pid_t waited; + do { + waited = child > 0 ? waitpid(child, &status, 0) : -1; + } while (waited < 0 && errno == EINTR); + bool cleaned = th_rmtree(parent) == 0; + bool child_ok = child > 0 && waited == child && WIFEXITED(status) && WEXITSTATUS(status) == 0; + if (!child_ok && child > 0 && waited == child) { + (void)fprintf(stderr, "frontend recovery fixture failed: status=0x%x exit=%d signal=%d\n", + status, WIFEXITED(status) ? WEXITSTATUS(status) : -1, + WIFSIGNALED(status) ? WTERMSIG(status) : 0); + } + return child_ok && cleaned; +} + static void frontend_test_release_lease(cbm_version_cohort_lease_t **lease) { while (lease && *lease && cbm_version_cohort_lease_release(lease) != CBM_PRIVATE_FILE_LOCK_OK) { cbm_usleep(1000); @@ -920,7 +1157,20 @@ static int frontend_backpressure_frontend_run(const char *parent, int input_fd, return 81; } - int result = cbm_daemon_frontend_mcp_run(client, manager, input, output); + cbm_daemon_frontend_session_config_t session = { + .bootstrap = + { + .role = CBM_DAEMON_PROCESS_MCP_CLIENT, + .endpoint = endpoint, + .identity = &identity, + .executable_path = "unused", + .connect_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .startup_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + }, + .session_root = parent, + .tool_profile = CBM_MCP_TOOL_PROFILE_ALL, + }; + int result = cbm_daemon_frontend_mcp_run(client, manager, &session, input, output); bool joined = cbm_thread_join(&writer_thread) == 0; bool writer_ok = atomic_load_explicit(&writer.finished, memory_order_acquire) && atomic_load_explicit(&writer.succeeded, memory_order_acquire); @@ -1334,9 +1584,20 @@ TEST(daemon_frontend_idle_uses_one_maintenance_observer) { bool pipe_ready = fixture_ready && cbm_pipe(input_pipe) == 0; FILE *input = pipe_ready ? frontend_test_fdopen_read(input_pipe[0]) : NULL; FILE *output = input ? tmpfile() : NULL; + cbm_daemon_frontend_session_config_t session = { + .bootstrap = {.role = CBM_DAEMON_PROCESS_MCP_CLIENT, + .endpoint = fixture.maintenance.endpoint, + .identity = &fixture.identity, + .executable_path = "unused", + .connect_timeout_ms = 30000, + .startup_timeout_ms = 30000}, + .session_root = fixture.maintenance.parent, + .tool_profile = CBM_MCP_TOOL_PROFILE_ALL, + }; frontend_idle_run_t run = { .client = fixture.client, .manager = fixture.maintenance.manager, + .session = &session, .input = input, .output = output, .result = -1, @@ -1418,8 +1679,21 @@ TEST(daemon_frontend_maintenance_exits_while_stdio_reader_is_blocked) { if (!announced) { _exit(70); } + cbm_daemon_frontend_session_config_t session = { + .bootstrap = + { + .role = CBM_DAEMON_PROCESS_MCP_CLIENT, + .endpoint = endpoint, + .identity = &identity, + .executable_path = "unused", + .connect_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + .startup_timeout_ms = FRONTEND_EOF_TEST_REQUEST_TIMEOUT_MS, + }, + .session_root = fixture.parent, + .tool_profile = CBM_MCP_TOOL_PROFILE_ALL, + }; int result = cbm_daemon_frontend_mcp_run((cbm_daemon_runtime_client_t *)(uintptr_t)1, - manager, input, output); + manager, &session, input, output); (void)result; _exit(71); } @@ -1633,6 +1907,29 @@ TEST(daemon_frontend_over_capacity_input_backpressures_without_loss) { PASS(); } +/* A request rejected before its application frame reaches the daemon is safe + * to replay. The frontend must replace its stale authenticated client, restore + * the session, reserve a token from the new connection, and return the one + * response without closing Codex's stdio transport. */ +TEST(daemon_frontend_recovers_one_unsent_request_after_daemon_replacement) { + ASSERT_TRUE(frontend_recovery_run_isolated(FRONTEND_RECOVERY_RETRY)); + PASS(); +} + +/* Context-replay cancellation must not strand a token or break the next request. */ +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS +TEST(daemon_frontend_recovery_cancellation_preserves_next_request) { + ASSERT_TRUE(frontend_recovery_run_isolated(FRONTEND_RECOVERY_CANCEL)); + PASS(); +} +#endif + +/* Clean EOF during context replay must preserve worker-owned client cleanup. */ +TEST(daemon_frontend_clean_eof_during_recovery_preserves_client_ownership) { + ASSERT_TRUE(frontend_recovery_run_isolated(FRONTEND_RECOVERY_CLEAN_EOF)); + PASS(); +} + /* Clean EOF is the normal MCP-session ownership boundary. Accepted work gets a * bounded drain opportunity; work still active at the deadline is cancelled * with that exact session, while the frontend itself reports a clean close. */ @@ -1691,6 +1988,11 @@ SUITE(daemon_frontend) { RUN_TEST(daemon_local_participant_monitor_cancels_then_bounds_active_operation); RUN_TEST(daemon_local_participant_monitor_allows_supervisor_containment_window); RUN_TEST(daemon_frontend_over_capacity_input_backpressures_without_loss); + RUN_TEST(daemon_frontend_recovers_one_unsent_request_after_daemon_replacement); +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + RUN_TEST(daemon_frontend_recovery_cancellation_preserves_next_request); +#endif + RUN_TEST(daemon_frontend_clean_eof_during_recovery_preserves_client_ownership); RUN_TEST(daemon_frontend_eof_drain_timeout_cancels_and_returns_success); RUN_TEST(daemon_frontend_stdout_backpressure_eof_fail_stops_and_cancels_session); RUN_TEST(daemon_frontend_stdout_backpressure_maintenance_stops_and_cancels_session); diff --git a/tests/test_daemon_runtime.c b/tests/test_daemon_runtime.c index 4a9350196..61488358c 100644 --- a/tests/test_daemon_runtime.c +++ b/tests/test_daemon_runtime.c @@ -908,7 +908,8 @@ static void *runtime_application_client_request_thread(void *opaque) { call->status = call->tagged ? cbm_daemon_runtime_client_application_request_tagged( call->client, call->request_token, call->request, call->request_length, - &call->response, &call->response_length, RUNTIME_TEST_TIMEOUT_MS) + &call->response, &call->response_length, NULL, + RUNTIME_TEST_TIMEOUT_MS) : cbm_daemon_runtime_client_application_request( call->client, call->request, call->request_length, &call->response, &call->response_length, RUNTIME_TEST_TIMEOUT_MS); @@ -3586,7 +3587,7 @@ TEST(daemon_runtime_presend_request_cancel_is_sticky_and_nonterminal) { cancel = cbm_daemon_runtime_client_application_cancel(client, request_token); cancelled_status = cbm_daemon_runtime_client_application_request_tagged( client, request_token, blocking_request, (uint32_t)sizeof(blocking_request), - &cancelled_response, &cancelled_response_length, RUNTIME_TEST_TIMEOUT_MS); + &cancelled_response, &cancelled_response_length, NULL, RUNTIME_TEST_TIMEOUT_MS); } if (cancelled_status == CBM_DAEMON_RUNTIME_APPLICATION_CANCELLED) { next_status = cbm_daemon_runtime_client_application_request( @@ -3643,8 +3644,10 @@ TEST(daemon_runtime_allows_only_one_unstarted_application_token) { bool first_reserved = false; bool duplicate_reservation_rejected = false; bool first_exact = false; + bool first_sent = false; bool second_reserved = false; bool second_exact = false; + bool second_sent = false; bool closed = false; bool exited = false; @@ -3664,7 +3667,7 @@ TEST(daemon_runtime_allows_only_one_unstarted_application_token) { cbm_daemon_runtime_application_status_t status = cbm_daemon_runtime_client_application_request_tagged( client, first_token, first_request, (uint32_t)sizeof(first_request), &response, - &response_length, RUNTIME_TEST_TIMEOUT_MS); + &response_length, &first_sent, RUNTIME_TEST_TIMEOUT_MS); first_exact = status == CBM_DAEMON_RUNTIME_APPLICATION_OK && response_length == sizeof(first_request) && response && memcmp(response, first_request, sizeof(first_request)) == 0; @@ -3680,7 +3683,7 @@ TEST(daemon_runtime_allows_only_one_unstarted_application_token) { cbm_daemon_runtime_application_status_t status = cbm_daemon_runtime_client_application_request_tagged( client, second_token, second_request, (uint32_t)sizeof(second_request), &response, - &response_length, RUNTIME_TEST_TIMEOUT_MS); + &response_length, &second_sent, RUNTIME_TEST_TIMEOUT_MS); second_exact = status == CBM_DAEMON_RUNTIME_APPLICATION_OK && response_length == sizeof(second_request) && response && memcmp(response, second_request, sizeof(second_request)) == 0; @@ -3699,9 +3702,11 @@ TEST(daemon_runtime_allows_only_one_unstarted_application_token) { ASSERT_TRUE(first_reserved); ASSERT_TRUE(duplicate_reservation_rejected); ASSERT_TRUE(first_exact); + ASSERT_TRUE(first_sent); ASSERT_TRUE(second_reserved); ASSERT_EQ(second_token, first_token + 1U); ASSERT_TRUE(second_exact); + ASSERT_TRUE(second_sent); ASSERT_TRUE(closed); ASSERT_TRUE(exited); ASSERT_EQ(atomic_load(&context.requests), 2); @@ -3801,6 +3806,10 @@ TEST(daemon_runtime_close_begin_retains_storage_and_rejects_late_exchange) { uint8_t *response = NULL; uint32_t response_length = 0; cbm_daemon_runtime_application_status_t status = CBM_DAEMON_RUNTIME_APPLICATION_OK; + cbm_daemon_runtime_application_token_t request_token = + CBM_DAEMON_RUNTIME_APPLICATION_TOKEN_INVALID; + bool token_reserved = false; + bool request_sent = true; bool close_begun = false; bool duplicate_begin_rejected = false; bool close_acknowledged = false; @@ -3811,6 +3820,8 @@ TEST(daemon_runtime_close_begin_retains_storage_and_rejects_late_exchange) { RUNTIME_TEST_TIMEOUT_MS, &result); } if (client) { + token_reserved = + cbm_daemon_runtime_client_application_token_reserve(client, &request_token); close_begun = cbm_daemon_runtime_client_close_begin(client); duplicate_begin_rejected = close_begun && !cbm_daemon_runtime_client_close_begin(client); } @@ -3818,9 +3829,9 @@ TEST(daemon_runtime_close_begin_retains_storage_and_rejects_late_exchange) { /* Deterministically models the frontend boundary where close begins * after a worker claims an item but before it enters the runtime API. * The retained allocation must reject the call without touching IPC. */ - status = cbm_daemon_runtime_client_application_request( - client, request, (uint32_t)sizeof(request), &response, &response_length, - RUNTIME_TEST_TIMEOUT_MS); + status = cbm_daemon_runtime_client_application_request_tagged( + client, request_token, request, (uint32_t)sizeof(request), &response, &response_length, + &request_sent, RUNTIME_TEST_TIMEOUT_MS); close_acknowledged = cbm_daemon_runtime_client_close_finish(client, RUNTIME_TEST_TIMEOUT_MS); client = NULL; @@ -3838,11 +3849,13 @@ TEST(daemon_runtime_close_begin_retains_storage_and_rejects_late_exchange) { runtime_test_fixture_finish(&fixture); ASSERT_TRUE(started); + ASSERT_TRUE(token_reserved); ASSERT_TRUE(close_begun); ASSERT_TRUE(duplicate_begin_rejected); ASSERT_EQ(status, CBM_DAEMON_RUNTIME_APPLICATION_TRANSPORT_ERROR); ASSERT_NULL(response); ASSERT_EQ(response_length, 0); + ASSERT_FALSE(request_sent); ASSERT_TRUE(close_acknowledged); ASSERT_TRUE(exited); ASSERT_EQ(atomic_load(&context.opened), 1); diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 00547eb59..61dc1a1f4 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -898,6 +898,192 @@ TEST(php_function) { PASS(); } +/* #2000: inline HTML between `?>` and `` fell into an + * ERROR region and never reached the graph. */ +static const CBMCall *find_call_named(CBMFileResult *r, const char *callee) { + for (int i = 0; i < r->calls.count; i++) { + if (r->calls.items[i].callee_name && strcmp(r->calls.items[i].callee_name, callee) == 0) + return &r->calls.items[i]; + } + return NULL; +} + +static int php_def_line(CBMFileResult *r, const char *name) { + for (int i = 0; i < r->defs.count; i++) { + if (strcmp(r->defs.items[i].name, name) == 0) + return (int)r->defs.items[i].start_line; + } + return -1; +} + +TEST(php_inline_html_tail_reaches_graph_issue2000) { + CBMFileResult *r = extract("\n" + "
\n" + "

plain markup after the closing tag

\n" + "
\n" + "has_error); + ASSERT(has_def(r, "Function", "gamma")); + ASSERT(has_def(r, "Function", "delta")); + /* Lines are in original-file coordinates. */ + ASSERT_EQ(php_def_line(r, "gamma"), 2); + ASSERT_EQ(php_def_line(r, "delta"), 8); + const CBMCall *c = find_call_named(r, "gamma"); + ASSERT_NOT_NULL(c); + ASSERT_NOT_NULL(strstr(c->enclosing_func_qn, "delta")); + ASSERT_FALSE(r->parse_incomplete); + cbm_free_result(r); + PASS(); +} + +/* View-file shapes: echo tags keep their calls, and control structures that + * are opened in one PHP block and closed in a later one still parse. */ +TEST(php_inline_html_template_shapes_issue2000) { + CBMFileResult *r = extract("\n" + "<?= page_title($page) ?>\n" + "\n" + "\n" + "

Hello name?>

\n" + "\n" + " log in\n" + "\n" + "\n" + "
  • \n" + "\n" + "\n" + "\n", + CBM_LANG_PHP, "t", "view.php"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT(has_call(r, "page_title")); + ASSERT(has_call(r, "render_item")); + ASSERT(has_def(r, "Function", "footer_links")); + ASSERT_EQ(php_def_line(r, "footer_links"), 12); + ASSERT(has_call(r, "nav_links")); + ASSERT_FALSE(r->parse_incomplete); + cbm_free_result(r); + PASS(); +} + +/* `?>` only leaves PHP mode from code or a one-line comment. Inside strings, + * heredoc, nowdoc and block comments it is text; `#[` is an attribute, not a + * comment; ` quoted';\n" + "$b = \"double ?> quoted \\\" still\";\n" + "$c = << body\n" + " EOT;\n" + "$d = <<<'NOW'\n" + "nowdoc ?> body\n" + "NOW;\n" + "/* block ?> comment */\n" + "#[Pure]\n" + "function one() { return 1; }\n" + "// line comment ends here ?>\n" + "\n" + "has_error); + ASSERT(has_def(r, "Function", "one")); + ASSERT(has_def(r, "Function", "two")); + ASSERT_EQ(php_def_line(r, "two"), 16); + ASSERT(has_call(r, "one")); + ASSERT_FALSE(r->parse_incomplete); + cbm_free_result(r); + PASS(); +} + +/* Markup is blanked, never parsed: code-shaped text outside the PHP tags must + * not produce definitions or calls, wherever it sits. */ +TEST(php_inline_html_markup_never_reaches_graph_issue2000) { + CBMFileResult *r = extract("

    function before() { return leakBefore(); }

    \n" + "\n" + "

    function fake() { return leak(); }

    \n" + "\n" + "function xmlFake() { leakXml(); }\n" + "\n" + "

    class Tail { function tailFake() { leakTail(); } }

    \n", + CBM_LANG_PHP, "t", "markup.php"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT(has_def(r, "Function", "real")); + ASSERT(has_def(r, "Function", "after")); + ASSERT(has_call(r, "real")); + ASSERT_FALSE(has_def_any(r, "before")); + ASSERT_FALSE(has_def_any(r, "fake")); + ASSERT_FALSE(has_def_any(r, "jsFake")); + ASSERT_FALSE(has_def_any(r, "xmlFake")); + ASSERT_FALSE(has_def_any(r, "Tail")); + ASSERT_FALSE(has_def_any(r, "tailFake")); + ASSERT_FALSE(has_call(r, "leak")); + ASSERT_FALSE(r->parse_incomplete); + cbm_free_result(r); + PASS(); +} + +/* The mask hands back the caller's buffer, uncopied, when nothing needs + * rewriting, and a same-length rewrite otherwise. The pointer alone cannot + * tell copy-on-write from an eager copy, so the arena's allocation total is + * sampled around each call: no bytes for `plain` and `quoted`, a copy for + * `mixed`. */ +TEST(php_inline_html_mask_copies_only_on_write_issue2000) { + CBMArena arena; + cbm_arena_init(&arena); + const char *plain = "';\n/* ?> */\n$h = <<\nEOT;\n"; + const char *mixed = "\nx\n"; + + size_t before = cbm_arena_total(&arena); + ASSERT(cbm_php_mask_inline_html(&arena, plain, (int)strlen(plain)) == plain); + ASSERT_EQ(cbm_arena_total(&arena), before); + + before = cbm_arena_total(&arena); + ASSERT(cbm_php_mask_inline_html(&arena, quoted, (int)strlen(quoted)) == quoted); + ASSERT_EQ(cbm_arena_total(&arena), before); + + before = cbm_arena_total(&arena); + const char *masked = cbm_php_mask_inline_html(&arena, mixed, (int)strlen(mixed)); + ASSERT(cbm_arena_total(&arena) > before); + ASSERT(masked != mixed); + ASSERT_EQ((int)strlen(masked), (int)strlen(mixed)); + ASSERT_STR_EQ(masked, "` (common at the end of older class files) is not a partial + * parse, and CRLF line endings keep their line numbers. */ +TEST(php_inline_html_trailing_close_tag_crlf_issue2000) { + CBMFileResult *r = extract("\r\n", + CBM_LANG_PHP, "t", "Repo.php"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT(has_def(r, "Class", "Repo")); + ASSERT(has_def(r, "Method", "find")); + ASSERT_EQ(php_def_line(r, "find"), 3); + ASSERT_FALSE(r->parse_incomplete); + cbm_free_result(r); + PASS(); +} + /* --- Ruby --- */ TEST(ruby_class) { CBMFileResult *r = extract("class Animal\n def initialize(name)\n @name = name\n end\n " @@ -14802,6 +14988,12 @@ SUITE(extraction) { RUN_TEST(python_class_base_extracted_bare); RUN_TEST(php_class); RUN_TEST(php_function); + RUN_TEST(php_inline_html_tail_reaches_graph_issue2000); + RUN_TEST(php_inline_html_template_shapes_issue2000); + RUN_TEST(php_inline_html_close_tag_in_literals_issue2000); + RUN_TEST(php_inline_html_markup_never_reaches_graph_issue2000); + RUN_TEST(php_inline_html_mask_copies_only_on_write_issue2000); + RUN_TEST(php_inline_html_trailing_close_tag_crlf_issue2000); RUN_TEST(ruby_class); RUN_TEST(ruby_module); RUN_TEST(ruby_constructor_callee_is_constant_path); diff --git a/tests/test_php_lsp.c b/tests/test_php_lsp.c index 04ff1001d..7c0314a27 100644 --- a/tests/test_php_lsp.c +++ b/tests/test_php_lsp.c @@ -110,6 +110,65 @@ TEST(phplsp_local_method_via_typed_param) { PASS(); } +/* #2000: with no cached tree (a result spilled under the memory budget comes + * back without one) the cross-file pass re-parses the on-disk source itself. + * It must blank inline HTML exactly as the per-file extract does, or a file's + * tail would lose cross-file resolution only when that file spilled. */ +static int find_resolved_arr(const CBMResolvedCallArray *arr, const char *callerSub, + const char *calleeSub) { + for (int i = 0; i < arr->count; i++) { + const CBMResolvedCall *rc = &arr->items[i]; + if (rc->caller_qn && strstr(rc->caller_qn, callerSub) && rc->callee_qn && + strstr(rc->callee_qn, calleeSub)) + return i; + } + return -1; +} + +TEST(phplsp_cross_reparse_reads_past_inline_html_issue2000) { + const char *src = "\n" + "
    \n" + "

    plain markup after the closing tag

    \n" + "
    \n" + "send(); }\n"; + CBMLSPDef defs[] = { + {.qualified_name = "test.main.gamma", + .short_name = "gamma", + .label = "Function", + .def_module_qn = "test.main"}, + {.qualified_name = "test.main.delta", + .short_name = "delta", + .label = "Function", + .def_module_qn = "test.main"}, + {.qualified_name = "test.mailer.Mailer", + .short_name = "Mailer", + .label = "Class", + .def_module_qn = "test.mailer"}, + {.qualified_name = "test.mailer.Mailer.send", + .short_name = "send", + .label = "Method", + .def_module_qn = "test.mailer", + .receiver_type = "test.mailer.Mailer"}, + }; + const char *imp_names[] = {"Mailer"}; + const char *imp_qns[] = {"test.mailer.Mailer"}; + + CBMArena arena; + cbm_arena_init(&arena); + CBMResolvedCallArray out = {0}; + + cbm_run_php_lsp_cross(&arena, src, (int)strlen(src), "test.main", defs, 4, imp_names, imp_qns, + 1, NULL, &out); + + ASSERT_GTE(find_resolved_arr(&out, "main.delta", "Mailer.send"), 0); + + cbm_arena_destroy(&arena); + PASS(); +} + /* ── 3. Arrow function with typed parameter ───────────────────── */ TEST(phplsp_arrow_function_typed_param) { @@ -5841,6 +5900,7 @@ SUITE(php_lsp) { /* Phase 1 baseline regressions */ RUN_TEST(phplsp_local_method_via_new_assignment); RUN_TEST(phplsp_local_method_via_typed_param); + RUN_TEST(phplsp_cross_reparse_reads_past_inline_html_issue2000); RUN_TEST(phplsp_arrow_function_typed_param); RUN_TEST(phplsp_static_call_resolved); RUN_TEST(phplsp_self_and_parent); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 1958fdb5e..b8bff190a 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -2170,6 +2170,57 @@ TEST(pipeline_nix_scoped_binding_calls_resolve) { PASS(); } +/* #2000, end to end: functions declared after an inline-HTML block reach the + * store with their CALLS edges, in the same file and across files. */ +TEST(pipeline_php_inline_html_calls_resolve_issue2000) { + if (setup_test_repo() != 0) { + FAIL("failed to create temp dir"); + } + + static const struct { + const char *name; + const char *body; + } files[] = { + {"inline_html.php", "\n" + "
    \n" + "

    plain markup after the closing tag

    \n" + "
    \n" + "\n" + "\n" + "

    \n" + "